Bug 6151: Add AllowReturnToBranch system preference
authorIan Walls <ian.walls@bywatersolutions.com>
Thu, 1 Dec 2011 21:15:04 +0000 (16:15 -0500)
committerPaul Poulain <paul.poulain@biblibre.com>
Mon, 17 Sep 2012 17:17:41 +0000 (19:17 +0200)
In order to solve the issue of IndependantBranches being incompatible with HomeOrHoldingBranchReturn,
this patch changes the mechanism by which the question "can I return this material here?" is answered.  Before,
the conditions were "if IndependantBranches is on, and this branch isn't HomeOrHoldingBranchReturn for the item,
then no, otherwise yes".  Now, the question is answered by consulting CanBookBeReturned (new subroutine)

New system preference:  AllowReturnToBranch
Possible values:
  - anywhere (default for new installs, and for existing systems with IndependantBranches turned off)
  - homebranch
  - holdingbranch (which is also the issuing branch in all normal circumstances)
  - homeorholdingbranch (default for existing systems with IndependantBranches turned on)

New subroutine:  CanBookBeReturned
Input:  $item hash (from GetItems), and $branchcode
Output: 0 or 1 to indicate "allowed" or not, and an optional message if not allowed.  Message is the 'correct' branchcode
to return the material to

To Test:
1.  Install patch and new syspref
2.  Check that default value of the preference:
    - if IndependantBranches was OFF at install time, should be 'anywhere'
    - if IndependantBranches was ON at install time, should be 'homeorholdingbranch'

Case:  'anywhere'
1.  Checkout a Library A book at Library A.  Return at Library A should be successful
2.  Repeat step 1, returning to Library B.  Return should be successful
3.  Checkout a Library A book at Library B.  Return to A should be successful
4.  Repeat step 3 with Library B and Library C

Case: 'homebranch'
1.  Checkout a Library A book at Library A.  Return at Library A should be successful
2.  Repeat step 1, returning to Library B.  Return should FAIL (returning message to return at A)
3.  Checkout a Library A book at Library B.  Return to Library A should be successful
4.  Repeat step 3 with Library B and Library C.  Both should FAIL (returning message to return at A)

Case: 'holdingbranch'
1.  Checkout a Library A book at Library A.  Return at Library A should be successful
2.  Repeat step 1, returning to Library B.  Return should FAIL (returning message to return at A)
3.  Checkout a Library A book at Library B.  Return to A should FAIL (returning message to return at B)
4.  Repeat step 3 with Library B. Return should be successful
5.  Repeat step 3 with Library C. Return should FAIL (returning message to return at B)

Case: 'homeorholdingbranch'
1.  Checkout a Library A book at Library A.  Return at Library A should be successful
2.  Repeat step 1, returning to Library B.  Return should FAIL (returning message to return at A)
3.  Checkout a Library A book at Library B.  Return to A should be successful
4.  Repeat step 3 with Library B. Return should be successful
5.  Repeat step 3 with Library C. Return should FAIL (returning message to return at A)

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
C4/Circulation.pm
admin/systempreferences.pl
installer/data/mysql/sysprefs.sql
installer/data/mysql/updatedatabase.pl
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref

index bfd34d8..499fd51 100644 (file)
@@ -1008,6 +1008,53 @@ sub CanBookBeIssued {
     return ( \%issuingimpossible, \%needsconfirmation, \%alerts );
 }
 
+=head2 CanBookBeReturned
+
+  ($returnallowed, $message) = CanBookBeReturned($item, $branch)
+
+Check whether the item can be returned to the provided branch
+
+=over 4
+
+=item C<$item> is a hash of item information as returned from GetItem
+
+=item C<$branch> is the branchcode where the return is taking place
+
+=back
+
+Returns:
+
+=over 4
+
+=item C<$returnallowed> is 0 or 1, corresponding to whether the return is allowed (1) or not (0)
+
+=item C<$message> is the branchcode where the item SHOULD be returned, if the return is not allowed
+
+=cut
+
+sub CanBookBeReturned {
+  my ($item, $branch) = @_;
+  my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
+
+  # assume return is allowed to start
+  my $allowed = 1;
+  my $message;
+
+  # identify all cases where return is forbidden
+  if ($allowreturntobranch eq 'homebranch' && $branch ne $item->{'homebranch'}) {
+     $allowed = 0;
+     $message = $item->{'homebranch'};
+  } elsif ($allowreturntobranch eq 'holdingbranch' && $branch ne $item->{'holdingbranch'}) {
+     $allowed = 0;
+     $message = $item->{'holdingbranch'};
+  } elsif ($allowreturntobranch eq 'homeorholdingbranch' && $branch ne $item->{'homebranch'} && $branch ne $item->{'holdingbranch'}) {
+     $allowed = 0;
+     $message = $item->{'homebranch'}; # FIXME: choice of homebranch is arbitrary
+  }
+
+  return ($allowed, $message);
+}
+
 =head2 AddIssue
 
   &AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate])
@@ -1623,18 +1670,14 @@ sub AddReturn {
         $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
     }
 
-    # if indy branches and returning to different branch, refuse the return unless canreservefromotherbranches is turned on
-    if ($hbr ne $branch && C4::Context->preference("IndependantBranches") && !(C4::Context->preference("canreservefromotherbranches"))){
+    # check if the return is allowed at this branch
+    my ($returnallowed, $message) = CanBookBeReturned($item, $branch);
+    unless ($returnallowed){
         $messages->{'Wrongbranch'} = {
             Wrongbranch => $branch,
-            Rightbranch => $hbr,
+            Rightbranch => $message
         };
         $doreturn = 0;
-        # bailing out here - in this case, current desired behavior
-        # is to act as if no return ever happened at all.
-        # FIXME - even in an indy branches situation, there should
-        # still be an option for the library to accept the item
-        # and transfer it to its owning library.
         return ( $doreturn, $messages, $issue, $borrower );
     }
 
index 8b37919..175f6a0 100755 (executable)
@@ -204,7 +204,7 @@ $tabsysprefs{AllowAllMessageDeletion}        = "Circulation";
 $tabsysprefs{OverdueNoticeBcc}               = "Circulation";
 $tabsysprefs{OverduesBlockCirc}              = "Circulation";
 $tabsysprefs{UseTransportCostMatrix}         = "Circulation";
-
+$tabsysprefs{AllowReturnToBranch}            = "Circulation";
 
 # Staff Client
 $tabsysprefs{template}                = "StaffClient";
index 33efb78..d1d2ab5 100644 (file)
@@ -180,6 +180,7 @@ INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('PatronsPerPage','20','Number of Patrons Per Page displayed by default','20','Integer');
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('HomeOrHoldingBranch','holdingbranch','Used by Circulation to determine which branch of an item to check with independent branches on, and by search to determine which branch to choose for availability ','holdingbranch|homebranch','Choice');
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('HomeOrHoldingBranchReturn','homebranch','Used by Circulation to determine which branch of an item to check checking-in items','holdingbranch|homebranch','Choice');
+INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('AllowReturnToBranch', 'anywhere', 'Where an item may be returned', 'anywhere|homebranch|holdingbranch|homeorholdingbranch', 'Choice');
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OpacHighlightedWords','1','If Set, then queried words are higlighted in OPAC','','YesNo');
 
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OAI-PMH','0','if ON, OAI-PMH server is enabled',NULL,'YesNo');
index b26c769..4953023 100755 (executable)
@@ -5795,6 +5795,18 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     SetVersion($DBversion);
 }
 
+
+$DBversion = "3.09.00.047";
+if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
+    # to preserve default behaviour as best as possible, set this new preference differently depending on whether IndependantBranches is set or not
+    my $prefvalue = 'anywhere';
+    if (C4::Context->preference("IndependantBranches")) { $prefvalue = 'homeorholdingbranch';}
+    $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('AllowReturnToBranch', '$prefvalue', 'Where an item may be returned', 'anywhere|homebranch|holdingbranch|homeorholdingbranch', 'Choice');");
+
+    print "Upgrade to $DBversion done: adding AllowReturnToBranch syspref (bug 6151)";
+    SetVersion($DBversion);
+}
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)
index 15f6794..9ff3056 100644 (file)
@@ -197,6 +197,15 @@ Circulation:
               choices:
                   homebranch: the library the item is from.
                   holdingbranch: the library the item was checked out from.
+        -
+            - Allow materials to be returned to
+            - pref: AllowReturnToBranch
+              type: choice
+              choices:
+                  anywhere: to any library.
+                  homebranch: only the library the item is from.
+                  holdingbranch: only the library the item was checked out from.
+                  homeorholdingbranch: either the library the item is from or the library it was checked out from.
         -
             - Calculate the due date using 
             - pref: useDaysMode