[Koha-bugs] [Bug 8236] Prevent renewing if overdue or restriction

[bugzilla-daemon at bugs.koha-community.org]

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

--- Comment #67 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
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&item=[% ISSUE.itemnumber %]&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.