http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8236

--- Comment #67 from Katrin Fischer <[email protected]> ---
Comment on attachment 22280
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22280
Bug 8236 Block renewing for overdue items

Review of attachment 22280:
 --> 
(http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8236&attachment=22280)
-----------------------------------------------------------------

Patch doesn't apply with a conflict in C4/Circulation.pm, that I cannot fix as
part of the QA.
I did a code review and found some issues with the code, that should also be
taken care of.
Without testing I am a bit worried that the change in restriction handling
might have side effects on this patch. Fridolin, could you check that when
working on this again?

::: C4/Circulation.pm
@@ +75,4 @@
>       push @EXPORT, qw(
>               &CanBookBeIssued
>               &CanBookBeRenewed
> +        &HasWhateverRenewalToBeBlocked

I am not sure about the name for the new sub - I can't really tell what it's
going to do. Maybe try to make it a bit more clear or check with a native
speaker for a better wording?

We will also need some unit tests for it.

@@ +2481,5 @@
>  
>      my $issuingrule = GetIssuingRule($borrower->{categorycode}, 
> $item->{itype}, $branchcode);
>  
>      if ( ( $issuingrule->{renewalsallowed} > $itemissue->{renewals} ) || 
> $override_limit ) {
> +            $renewokay =  ( $overduesblockrenew eq 'blockitem' and $overdue 
> and !$override_limit ) ? 0 : 1;

These changes should be backed up by some unit tests as well.

::: installer/data/mysql/updatedatabase.pl
@@ +7331,5 @@
>  }
>  
> +$DBversion = "3.13.00.XXX";
> +if ( CheckVersion($DBversion) ) {
> +    $dbh->do("INSERT INTO systempreferences 
> (variable,value,explanation,options,type) VALUES 
> ('OverduesBlockRenew','allow','If any of a patron checked out documents is 
> late, should renewal be allowed, blocked only on overdue items or blocked on 
> whatever checked out document','allow|blockitem|blockall','Choice')");

Maye the options would be more clear as:
block, bockoverdue, allow ?

::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ +72,4 @@
>          $( '#override_limit' ).click( function () {
>              if ( this.checked ) {
>                  $( '.renewals-allowed' ).show(); $( '.renewals-disabled' 
> ).hide();
> +                $("input[name='renew_checked']").val('Renew or Return 
> checked items');

This looks like a translation problem. There are a few more below I marked with
'T'.

@@ +107,5 @@
> +            return;
> +        }
> +    });
> +    if(!valid){
> +        $("input[name='renew_checked']").val('Return checked items');

T

::: koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
@@ +86,4 @@
>      $( '#override_limit' ).click( function () {
>          if ( this.checked ) {
>             $( '.renewals-allowed' ).show(); $( '.renewals-disabled' ).hide();
> +           $("input[name='renew_checked']").val('Renew or Return checked 
> items');

T

@@ +154,5 @@
> +            return;
> +        }
> +    });
> +    if(!valid){
> +        $("input[name='renew_checked']").val('Return checked items');

T

::: koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt
@@ +191,5 @@
>                  [% IF ( OpacRenewalAllowed ) %]
>                      <td class="renew">[% IF ( ISSUE.status ) %][% IF ( 
> canrenew ) %]<input type="checkbox" name="item" value="[% ISSUE.itemnumber 
> %]"/> <a href="/cgi-bin/koha/opac-renew.pl?from=opac_user&amp;item=[% 
> ISSUE.itemnumber %]&amp;borrowernumber=[% ISSUE.borrowernumber 
> %]">Renew</a>[% END %] <span class="renewals">([% ISSUE.renewsleft %] of [% 
> ISSUE.renewsallowed %] renewals remaining)</span>
>                          [% ELSE %]
> +                        [% IF ( ISSUE.norenew_overdue ) %]
> +                           Renewal not allowed <span 
> class="renewals">(overdue on a document)</span>

I think instead of using 'document' better use 'item' to be consistent with the
language in Koha.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to