From 70cb04e20d8c0d08c06e5a41b77197c020165bff Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Thu, 24 Oct 2019 11:09:22 +0100 Subject: [PATCH] Bug 23051: (follow-up) Renewing in OPAC This follow up patch addresses the following parts of Nick's feedback in comment #35: - I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. We've introduced the syspref RenewAccruingItemInOpac (default is off) which, when enabled, will cause items attached to fines that are paid off in the OPAC (via payment plugins), to be automatically renewed. --- Koha/Account.pm | 7 +- Koha/Account/Line.pm | 94 +++++++++++++--------- ..._23051_add_RenewAccruingItemInOpac_syspref.perl | 8 ++ installer/data/mysql/sysprefs.sql | 1 + .../en/modules/admin/preferences/circulation.pref | 7 ++ 5 files changed, 76 insertions(+), 41 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_23051_add_RenewAccruingItemInOpac_syspref.perl diff --git a/Koha/Account.pm b/Koha/Account.pm index 81b92b0ed1..b006becf59 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -120,8 +120,11 @@ sub pay { # Attempt to renew the item associated with this debit if # appropriate if ($fine->renewable) { - my $outcome = $fine->renew_item; - push @{$renew_outcomes}, $outcome; + # We're ignoring the definition of $interface above, by all + # accounts we can't rely on C4::Context::interface, so here + # we're only using what we've been explicitly passed + my $outcome = $fine->renew_item($params->{interface}); + push @{$renew_outcomes}, $outcome if $outcome; } # Same logic exists in Koha::Account::Line::apply diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index a62601f7fb..d26e489f37 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -239,7 +239,7 @@ sub apply { # Attempt to renew the item associated with this debit if # appropriate if ($debit->renewable) { - $debit->renew_item; + $debit->renew_item($params->{interface}); } # Same logic exists in Koha::Account::pay @@ -433,51 +433,67 @@ as a consequence of the fine on an item being fully paid off =cut sub renew_item { - my ($self) = @_; + my ($self, $params) = @_; my $outcome = {}; + # We want to reject the call to renew if any of these apply: + # - The RenewAccruingItemWhenPaid syspref is off + # - The line item doesn't have an item attached to it + # - The line item doesn't have a patron attached to it + # + # - The RenewAccruingItemInOpac syspref is off + # AND + # - There is an interface param passed and it's value is 'opac' + if ( - C4::Context->preference('RenewAccruingItemWhenPaid') && - $self->item && - $self->patron + !C4::Context->preference('RenewAccruingItemWhenPaid') || + !$self->item || + !$self->patron || + ( + !C4::Context->preference('RenewAccruingItemInOpac') && + $params->{interface} && + $params->{interface} eq 'opac' + ) ) { - my $itemnumber = $self->item->itemnumber; - my $borrowernumber = $self->patron->borrowernumber; - # Only do something if this item has no fines left on it - my $fine = C4::Overdues::GetFine($itemnumber, $borrowernumber); - if ($fine && $fine > 0) { - return { - itemnumber => $itemnumber, - error => 'has_fine', - success => 0 - }; - } - my ( $can_renew, $error ) = C4::Circulation::CanBookBeRenewed( + return; + } + + my $itemnumber = $self->item->itemnumber; + my $borrowernumber = $self->patron->borrowernumber; + # Only do something if this item has no fines left on it + my $fine = C4::Overdues::GetFine($itemnumber, $borrowernumber); + if ($fine && $fine > 0) { + return { + itemnumber => $itemnumber, + error => 'has_fine', + success => 0 + }; + } + my ( $can_renew, $error ) = C4::Circulation::CanBookBeRenewed( + $borrowernumber, + $itemnumber + ); + if ( $can_renew ) { + my $due_date = C4::Circulation::AddRenewal( $borrowernumber, - $itemnumber + $itemnumber, + $self->{branchcode}, + undef, + undef, + 1 ); - if ( $can_renew ) { - my $due_date = C4::Circulation::AddRenewal( - $borrowernumber, - $itemnumber, - $self->{branchcode}, - undef, - undef, - 1 - ); - return { - itemnumber => $itemnumber, - due_date => $due_date, - success => 1 - }; - } else { - return { - itemnumber => $itemnumber, - error => $error, - success => 0 - }; - } + return { + itemnumber => $itemnumber, + due_date => $due_date, + success => 1 + }; + } else { + return { + itemnumber => $itemnumber, + error => $error, + success => 0 + }; } } diff --git a/installer/data/mysql/atomicupdate/bug_23051_add_RenewAccruingItemInOpac_syspref.perl b/installer/data/mysql/atomicupdate/bug_23051_add_RenewAccruingItemInOpac_syspref.perl new file mode 100644 index 0000000000..c118180474 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_23051_add_RenewAccruingItemInOpac_syspref.perl @@ -0,0 +1,8 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + + $dbh->do( q| INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('RenewAccruingItemInOpac', '0', 'If enabled, when the fines on an item accruing is paid off in the OPAC via a payment plugin, attempt to renew that item. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue', '', 'YesNo'); | ); + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 23051 - Add RenewAccruingItemInOpac syspref)\n"; +} diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index d7ba79b313..e5c840a954 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -505,6 +505,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('RecordLocalUseOnReturn','0',NULL,'If ON, statistically record returns of unissued items as local use, instead of return','YesNo'), ('RefundLostOnReturnControl','CheckinLibrary','CheckinLibrary|ItemHomeBranch|ItemHoldingBranch','If a lost item is returned, choose which branch to pick rules for refunding.','Choice'), ('RenewAccruingItemWhenPaid','0','','If enabled, when the fines on an item accruing is paid off, attempt to renew that item. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue','YesNo'), +('RenewAccruingItemInOpac','0','','If enabled, when the fines on an item accruing is paid off in the OPAC via a payment plugin, attempt to renew that item. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue','YesNo'), ('RenewalLog','0','','If ON, log information about renewals','YesNo'), ('RenewalPeriodBase','date_due','date_due|now','Set whether the renewal date should be counted from the date_due or from the moment the Patron asks for renewal ','Choice'), ('RenewalSendNotice','0','',NULL,'YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 889ca997ce..aab5599387 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -492,6 +492,13 @@ Circulation: no: "Don't renew" - the item automatically. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue. - + - If a patron pays off all fines on an overdue item that is accruing fines in the OPAC via a payment plugin + - pref: RenewAccruingItemInOpac + choices: + yes: Renew + no: "Don't renew" + - the item automatically. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue. + - - pref: ItemsDeniedRenewal type: textarea syntax: text/x-yaml -- 2.11.0