Several fines-related system preferences (OPACFineNoRenewals, noissuescharge, etc) can be disabled by leaving them blank. But maxoutstanding treats a blank as a zero, blocking OPAC holds for *any* fine balance. I'd like this to change to match the functionality of the other sysprefs. If nothing else, we should add some language to say a blank is treated as a zero.
To Test: 1) Set maxoutstanding to blank 2) give yourself $0.50 in fines 3) go to opac, confirm you're blocked from placing holds 4) set maxoutstanding=1 5) confirm you are no longer blocked from placing holds
Created attachment 103377 [details] [review] Bug 25220: maxoutstanding ignored if null in opac-reserve.pl To Test: 1) set maxoutstanding to empty 2) give yourself a fine of $1 3) try to place a hold on the opac 4) confirm you cannot place hold 5) apply patch, restart_all 6) reload opac 7) confirm hold is allowed 8) set maxoutstanding to $2 9) confirm hold is now blocked
Created attachment 103378 [details] [review] Bug 25220: make maxoutstanding set to null not give warning on opac-user.pl To test: 1) set maxoutstanding to empty 2) give yourself $1 in fine 3) go to your account on OPAC 4) confirm you have message that holds are blocked 5) apply patch, restart_all 6) reload OPAC 7) confirm message is gone 8) set maxoutstanding to $2 9) reload opac, confirm message is back
Correcting / consolidating test plan: To test: 1) set maxoutstanding to empty 2) give yourself $1 in fine 3) go to your account on OPAC 4) confirm you have message that holds are blocked 5) try to place a hold on opac 6) confirm hold is blocked 7) apply patch, restart_all 8) reload OPAC 9) confirm you can now place a hold 10) confirm message on your account is gone 11) set maxoutstanding to $.5 12) reload opac 13) confirm you are blocked from hold 14) confirm message on account is back
Created attachment 103388 [details] [review] Bug 25220: atomicupdate to maintain behavior of existing installs To test: 1) on an existing install, set maxoutstanding to null 2) apply patch 3) confirm maxoutstanding is now set to zero
Created attachment 104506 [details] [review] Bug 25220: maxoutstanding ignored if null in opac-reserve.pl To Test: 1) set maxoutstanding to empty 2) give yourself a fine of $1 3) try to place a hold on the opac 4) confirm you cannot place hold 5) apply patch, restart_all 6) reload opac 7) confirm hold is allowed 8) set maxoutstanding to $2 9) confirm hold is now blocked Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 104507 [details] [review] Bug 25220: make maxoutstanding set to null not give warning on opac-user.pl To test: 1) set maxoutstanding to empty 2) give yourself $1 in fine 3) go to your account on OPAC 4) confirm you have message that holds are blocked 5) apply patch, restart_all 6) reload OPAC 7) confirm message is gone 8) set maxoutstanding to $2 9) reload opac, confirm message is back Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 104508 [details] [review] Bug 25220: atomicupdate to maintain behavior of existing installs To test: 1) on an existing install, set maxoutstanding to null 2) apply patch 3) confirm maxoutstanding is now set to zero Amended during signoff: Corrected atomicupdate file extension from .pl to .perl. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
C4/SIP/ILS/Patron.pm: && $self->fee_amount > C4::Context->preference("maxoutstanding") ); circ/offline.pl:$template->{'VARS'}->{'maxoutstanding'} = circ/offline.pl: C4::Context->preference('maxoutstanding') || 0; installer/data/mysql/atomicupdate/bug_25220.perl: $dbh->do( "UPDATE systempreferences set value=0 where variable='maxoutstanding' and (value='' or value is null)" ); installer/data/mysql/atomicupdate/bug_25220.perl: NewVersion( $DBversion, 25220, "maxoutstanding should be disabled by a null value"); installer/data/mysql/sysprefs.sql:('maxoutstanding','5','','maximum amount withstanding to be able make holds','Integer'), koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref: - pref: maxoutstanding koha-tmpl/intranet-tmpl/prog/en/modules/circ/offline.tt: if (parseInt(patron.fine) > [% maxoutstanding | html %]) { koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt: [% IF amount_outstanding && Koha.Preference('maxoutstanding') && amount_outstanding > Koha.Preference('maxoutstanding') %] koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt: [% IF ( expiry || diffbranch || patron.is_debarred || ( amount_outstanding && Koha.Preference('maxoutstanding') && amount_outstanding > Koha.Preference('maxoutstanding') ) ) %] koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt: [% IF amount_outstanding && Koha.Preference('maxoutstanding') && amount_outstanding > Koha.Preference('maxoutstanding') %] opac/opac-reserve.pl:my $maxoutstanding = C4::Context->preference("maxoutstanding"); opac/opac-reserve.pl:$maxoutstanding = undef unless looks_like_number($maxoutstanding); opac/opac-reserve.pl:$template->param( noreserve => 1 ) unless $maxoutstanding; opac/opac-reserve.pl:if ( $amountoutstanding && defined($maxoutstanding) && ($amountoutstanding > $maxoutstanding) ) { opac/opac-user.pl:my $maxoutstanding = C4::Context->preference('maxoutstanding'); opac/opac-user.pl:$maxoutstanding = undef unless looks_like_number ( $maxoutstanding ); opac/opac-user.pl:if ( $amountoutstanding && defined($maxoutstanding) && ( $amountoutstanding > $maxoutstanding ) ){ Are you sure that you only needed to change opac-reserve and opac-user ? If I am looking at this git grep, I wouldnt. Also note that the test Koha.Pref( ) && a > b, looks a bit more elegant, although it could trigger warnings on non-numerical comparison if you wrote "abc" in maxoutstanding. But who would? + $dbh->do( "UPDATE systempreferences set value=0 where variable='maxoutstanding' and (value='' or value is null)" ); This does not look good. Value is a text field. You mean '0'? :) I think we dont need this change. And also you dont correct 'abc'?