Attachment 141224 Details for Bug 25426 – Bug 25426: Allow return policy to be selected via syspref and not just home library

[patch] Bug 25426: Allow return policy to be selected via syspref and not just home library

Bug-25426-Allow-return-policy-to-be-selected-via-s.patch (text/plain), 13.21 KB, created by Katrin Fischer on 2022-10-03 09:46:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2022-10-03 09:46:23 UTC

Size: 13.21 KB

patch

obsolete

>From 33dd5113d68cc4416002462dc5ec0a15fae791a6 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Thu, 25 Aug 2022 13:05:32 +0000
>Subject: [PATCH] Bug 25426: Allow return policy to be selected via syspref and
> not just home library
>
>1) Apply this patch
>2) Run updatedatabase.pl
>3) Verify CircControlReturnsBranch is set to home library by default
>4) Set a Return policy for Branch A to "Item returns home" ( homebranch )
>5) Set a Return polity for Branch B to "Item returns to issuing library" ( holdingbranch )
>6) Set a Return polity for Branch C to "Item floats" ( noreturn )
>7) Create an item with homebranch of Branch A, holding branch of branch B
>8) Log in as Branch C
>9) Set CircControlReturnsBranch to "the library the item is currently held by"
>10) Check the item in, note it should be returned to the holding library
>11) Set CircControlReturnsBranch to "the library the item is owned by"
>12) Check the item in, note it should be returned to the home library
>13) Set CircControlReturnsBranch to "the library you are logged in at"
>14) Check the item in, note it should float
>
>Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>---
> C4/Circulation.pm                             | 10 +--
> Koha/CirculationRules.pm                      | 47 +++++++++++
> circ/returns.pl                               |  5 +-
> .../data/mysql/atomicupdate/bug_25426.pl      | 17 ++++
> installer/data/mysql/mandatory/sysprefs.sql   |  1 +
> .../admin/preferences/circulation.pref        |  8 ++
> t/db_dependent/Circulation/Branch.t           | 78 +++++++++++++++++--
> 7 files changed, 150 insertions(+), 16 deletions(-)
> create mode 100755 installer/data/mysql/atomicupdate/bug_25426.pl
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 6faf53add9..7d920801cd 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1932,11 +1932,6 @@ holdallowed => Hold policy for this branch and itemtype. Possible values:
>   from_any_library:      Holds allowed from any patron.
>   from_local_hold_group: Holds allowed from libraries in hold group
> 
>-returnbranch => branch to which to return item.  Possible values:
>-  noreturn: do not return, let item remain where checked in (floating collections)
>-  homebranch: return to item's home branch
>-  holdingbranch: return to issuer branch
>-
> This searches branchitemrules in the following order:
> 
>   * Same branchcode and itemtype
>@@ -1955,13 +1950,12 @@ sub GetBranchItemRule {
>     my $rules = Koha::CirculationRules->get_effective_rules({
>         branchcode => $branchcode,
>         itemtype => $itemtype,
>-        rules => ['holdallowed', 'hold_fulfillment_policy', 'returnbranch']
>+        rules => ['holdallowed', 'hold_fulfillment_policy']
>     });
> 
>     # built-in default circulation rule
>     $rules->{holdallowed} //= 'from_any_library';
>     $rules->{hold_fulfillment_policy} //= 'any';
>-    $rules->{returnbranch} //= 'homebranch';
> 
>     return $rules;
> }
>@@ -2094,7 +2088,7 @@ sub AddReturn {
>     }
> 
>         # full item data, but no borrowernumber or checkout info (no issue)
>-    my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
>+    my $hbr = Koha::CirculationRules->get_return_branch_policy($item);
>         # get the proper branch to which to return the item
>     my $returnbranch = $hbr ne 'noreturn' ? $item->$hbr : $branch;
>         # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
>diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
>index b9fc11c941..8b5d4807d9 100644
>--- a/Koha/CirculationRules.pm
>+++ b/Koha/CirculationRules.pm
>@@ -474,6 +474,53 @@ sub clone {
>     }
> }
> 
>+=head2 get_return_branch_policy
>+
>+  my $returnbranch = Koha::CirculationRules->get_return_branch_policy($item);
>+
>+Returns the branch to use for returning the item based on the
>+item type, and a branch selected via CircControlReturnsBrnch.
>+
>+The return value is the branch to which to return item.  Possible values:
>+  noreturn: do not return, let item remain where checked in (floating collections)
>+  homebranch: return to item's home branch
>+  holdingbranch: return to issuer branch
>+
>+This searches branchitemrules in the following order:
>+  * Same branchcode and itemtype
>+  * Same branchcode, itemtype '*'
>+  * branchcode '*', same itemtype
>+  * branchcode and itemtype '*'
>+
>+=cut
>+
>+sub get_return_branch_policy {
>+    my ( $self, $item ) = @_;
>+
>+    my $pref = C4::Context->preference('CircControlReturnsBranch');
>+
>+    my $branchcode =
>+        $pref eq 'ItemHomeLibrary'     ? $item->homebranch
>+      : $pref eq 'ItemHoldingLibrary' ? $item->holdingbranch
>+      : $pref eq 'CheckInLibrary'      ? C4::Context->userenv
>+          ? C4::Context->userenv->{branch}
>+          : $item->homebranch
>+      : $item->homebranch;
>+
>+    my $itemtype = $item->effective_itemtype;
>+
>+    my $rule = Koha::CirculationRules->get_effective_rule(
>+        {
>+            rule_name  => 'returnbranch',
>+            itemtype   => $itemtype,
>+            branchcode => $branchcode,
>+        }
>+    );
>+
>+    return $rule ? $rule->rule_value : 'homebranch';
>+}
>+
>+
> =head3 get_opacitemholds_policy
> 
> my $can_place_a_hold_at_item_level = Koha::CirculationRules->get_opacitemholds_policy( { patron => $patron, item => $item } );
>diff --git a/circ/returns.pl b/circ/returns.pl
>index 12e4d70b05..4493afb297 100755
>--- a/circ/returns.pl
>+++ b/circ/returns.pl
>@@ -47,10 +47,11 @@ use Koha::AuthorisedValues;
> use Koha::BiblioFrameworks;
> use Koha::Calendar;
> use Koha::Checkouts;
>+use Koha::CirculationRules;
> use Koha::DateUtils qw( dt_from_string );
> use Koha::Holds;
>-use Koha::Items;
> use Koha::Item::Transfers;
>+use Koha::Items;
> use Koha::Patrons;
> use Koha::Recalls;
> 
>@@ -296,7 +297,7 @@ if ($barcode) {
>         }
> 
>         # make sure return branch respects home branch circulation rules, default to homebranch
>-        my $hbr = GetBranchItemRule($item->homebranch, $itemtype ? $itemtype->itemtype : undef )->{'returnbranch'} || "homebranch";
>+        my $hbr = Koha::CirculationRules->get_return_branch_policy($item);
>         $returnbranch = $hbr ne 'noreturn' ? $item->$hbr : $userenv_branch; # can be noreturn, homebranch or holdingbranch
> 
>         my $materials = $item->materials;
>diff --git a/installer/data/mysql/atomicupdate/bug_25426.pl b/installer/data/mysql/atomicupdate/bug_25426.pl
>new file mode 100755
>index 0000000000..aa7d513ac4
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_25426.pl
>@@ -0,0 +1,17 @@
>+use Modern::Perl;
>+
>+return {
>+    bug_number => "25426",
>+    description => "Add new syspref CircControlReturnsBranch",
>+    up => sub {
>+        my ($args) = @_;
>+        my ($dbh, $out) = @$args{qw(dbh out)};
>+
>+        $dbh->do(q{
>+            INSERT INTO systempreferences VALUES (
>+              'CircControlReturnsBranch','ItemHomeLibrary','ItemHomeLibrary|ItemHoldingLibrary|CheckInLibrary',
>+              'Specify the agency that controls the return policy','Choice'
>+            )
>+        });
>+    },
>+};
>diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql
>index db99d68f81..d07f68772b 100644
>--- a/installer/data/mysql/mandatory/sysprefs.sql
>+++ b/installer/data/mysql/mandatory/sysprefs.sql
>@@ -136,6 +136,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('CircAutoPrintQuickSlip','qslip',NULL,'Choose what should happen when an empty barcode field is submitted in circulation: Display a print quick slip window, Display a print slip window or Clear the screen.','Choice'),
> ('CircConfirmItemParts', '0', NULL, 'Require staff to confirm that all parts of an item are present at checkin/checkout.', 'Yes/No'),
> ('CircControl','ItemHomeLibrary','PickupLibrary|PatronLibrary|ItemHomeLibrary','Specify the agency that controls the circulation and fines policy','Choice'),
>+('CircControlReturnsBranch','ItemHomeLibrary','ItemHomeLibrary|ItemHoldingLibrary|CheckInLibrary','Specify the agency that controls the return policy','Choice'),
> ('CircSidebar','1',NULL,'Activate or deactivate the navigation sidebar on all Circulation pages','YesNo'),
> ('CirculateILL','0','','If enabled, it is possible to circulate ILL items from within ILL','YesNo'),
> ('ClaimsBccCopy','0','','Bcc the ClaimAcquisition and ClaimIssues alerts','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 bcdb4b2b88..73f4128255 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
>@@ -270,6 +270,14 @@ Circulation:
>                   PickupLibrary: the library you are logged in at.
>                   PatronLibrary: the library the patron is from.
>                   ItemHomeLibrary: the library the item is from.
>+        -
>+            - For check in, use the circulation rules of
>+            - pref: CircControlReturnsBranch
>+              type: choice
>+              choices:
>+                  ItemHoldingLibrary: the library the item is currently held by.
>+                  CheckInLibrary: the library you are logged in at.
>+                  ItemHomeLibrary: the library the item is owned by.
>         -
>             - Use the checkout and fines rules of
>             - pref: HomeOrHoldingBranch
>diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t
>index 68a029d133..ef6fc31408 100755
>--- a/t/db_dependent/Circulation/Branch.t
>+++ b/t/db_dependent/Circulation/Branch.t
>@@ -25,7 +25,7 @@ use Koha::CirculationRules;
> 
> use Koha::Patrons;
> 
>-use Test::More tests => 17;
>+use Test::More tests => 18;
> use t::lib::Mocks;
> use t::lib::TestBuilder;
> 
>@@ -264,22 +264,22 @@ is_deeply(
>         $samplebranch1->{branchcode},
>         $sampleitemtype1->{itemtype},
>     ),
>-    { returnbranch => 'homebranch', holdallowed => 'invalid_value', @lazy_any },
>+    { holdallowed => 'invalid_value', @lazy_any },
>     "GetBranchitem returns holdallowed and return branch"
> );
> is_deeply(
>     GetBranchItemRule(),
>-    { returnbranch => 'homebranch', holdallowed => 'from_local_hold_group', @lazy_any },
>+    { holdallowed => 'from_local_hold_group', @lazy_any },
> "Without parameters GetBranchItemRule returns the values in default_circ_rules"
> );
> is_deeply(
>     GetBranchItemRule( $samplebranch2->{branchcode} ),
>-    { returnbranch => 'holdingbranch', holdallowed => 'from_home_library', @lazy_any },
>+    { holdallowed => 'from_home_library', @lazy_any },
> "With only a branchcode GetBranchItemRule returns values in default_branch_circ_rules"
> );
> is_deeply(
>     GetBranchItemRule( -1, -1 ),
>-    { returnbranch => 'homebranch', holdallowed => 'from_local_hold_group', @lazy_any },
>+    { holdallowed => 'from_local_hold_group', @lazy_any },
>     "With only one parametern GetBranchItemRule returns default values"
> );
> 
>@@ -329,5 +329,71 @@ t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
> is($messages->{NeedsTransfer},undef,"AddReturn respects branch item return policy - noreturn");
> t::lib::Mocks::mock_preference( 'item-level_itypes', 0 );
> 
>-$schema->storage->txn_rollback;
>+subtest "Test GetBranchItemRule" => sub {
>+    plan tests => 3;
>+
>+    $schema->storage->txn_begin;
>+
>+    $dbh->do('DELETE FROM circulation_rules');
>+
>+    my $homebranch    = $builder->build( { source => 'Branch' } )->{branchcode};
>+    my $holdingbranch = $builder->build( { source => 'Branch' } )->{branchcode};
>+    my $checkinbranch = $builder->build( { source => 'Branch' } )->{branchcode};
> 
>+    my $manager = $builder->build_object( { class => "Koha::Patrons" } );
>+    t::lib::Mocks::mock_userenv(
>+        {
>+            patron     => $manager,
>+            branchcode => $checkinbranch,
>+        }
>+    );
>+
>+    my $item = Koha::Item->new(
>+        {
>+            biblionumber  => $biblionumber,
>+            homebranch    => $homebranch,
>+            holdingbranch => $holdingbranch,
>+            itype         => $sampleitemtype1->{itemtype}
>+        }
>+    )->store;
>+
>+    Koha::CirculationRules->set_rule(
>+        {
>+            branchcode   => $homebranch,
>+            itemtype     => undef,
>+            rule_name    => 'returnbranch',
>+            rule_value   => 'homebranch',
>+        }
>+    );
>+
>+    Koha::CirculationRules->set_rule(
>+        {
>+            branchcode   => $holdingbranch,
>+            itemtype     => undef,
>+            rule_name    => 'returnbranch',
>+            rule_value   => 'holdingbranch',
>+        }
>+    );
>+
>+    Koha::CirculationRules->set_rule(
>+        {
>+            branchcode   => $checkinbranch,
>+            itemtype     => undef,
>+            rule_name    => 'returnbranch',
>+            rule_value   => 'noreturn',
>+        }
>+    );
>+
>+    t::lib::Mocks::mock_preference( 'CircControlReturnsBranch', 'ItemHomeLibrary' );
>+    is( Koha::CirculationRules->get_return_branch_policy($item), 'homebranch' );
>+
>+    t::lib::Mocks::mock_preference( 'CircControlReturnsBranch', 'ItemHoldingLibrary' );
>+    is( Koha::CirculationRules->get_return_branch_policy($item), 'holdingbranch' );
>+
>+    t::lib::Mocks::mock_preference( 'CircControlReturnsBranch', 'CheckInLibrary' );
>+    is( Koha::CirculationRules->get_return_branch_policy($item), 'noreturn' );
>+
>+    $schema->storage->txn_rollback;
>+};
>+
>+$schema->storage->txn_rollback;
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 25426: 139798 | 139802 | 139964 | 141224 | 141225 | 141226 | 141227