Attachment 94649 Details for Bug 23051 – Bug 23051: (follow-up) Renewing in OPAC

[patch] Bug 23051: (follow-up) Renewing in OPAC

Bug-23051-follow-up-Renewing-in-OPAC.patch (text/plain), 8.88 KB, created by Andrew Isherwood on 2019-10-24 10:12:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2019-10-24 10:12:21 UTC

Size: 8.88 KB

patch

obsolete

>From 70cb04e20d8c0d08c06e5a41b77197c020165bff Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 23051: 90559 | 90560 | 90561 | 91058 | 91059 | 91060 | 91792 | 91793 | 91794 | 91795 | 92684 | 92685 | 92686 | 92687 | 92688 | 92689 | 93922 | 93923 | 93924 | 93925 | 94020 | 94021 | 94022 | 94023 | 94024 | 94402 | 94434 | 94453 | 94525 | 94604 | 94605 | 94606 | 94607 | 94608 | 94609 | 94610 | 94611 | 94629 | 94649 | 95557 | 95558 | 95559 | 95560 | 100127 | 100128 | 100129 | 100130 | 100131 | 100132 | 100133 | 100134 | 100135 | 100138 | 100172 | 100182 | 100183 | 100184 | 100185 | 100186 | 100187 | 100188 | 100271