Attachment 11294 Details for Bug 6151 – Bug 6151: Add AllowReturnToBranch system preference

[patch] Bug 6151: Add AllowReturnToBranch system preference

Bug-6151-Add-AllowReturnToBranch-system-preference.patch (text/plain), 10.59 KB, created by Kyle M Hall (khall) on 2012-08-02 12:27:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2012-08-02 12:27:01 UTC

Size: 10.59 KB

patch

obsolete

>From c2869ec64967988e1701bd1c08c49f740a50deb3 Mon Sep 17 00:00:00 2001
>From: Ian Walls <ian.walls@bywatersolutions.com>
>Date: Thu, 1 Dec 2011 16:15:04 -0500
>Subject: [PATCH] Bug 6151: Add AllowReturnToBranch system preference
>
>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                                  |   59 +++++++++++++++++---
> admin/systempreferences.pl                         |    2 +-
> installer/data/mysql/sysprefs.sql                  |    1 +
> installer/data/mysql/updatedatabase.pl             |   11 ++++
> .../en/modules/admin/preferences/circulation.pref  |    9 +++
> 5 files changed, 73 insertions(+), 9 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 1a13e7a..f6fdf7d 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -934,6 +934,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])
>@@ -1585,18 +1632,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 );
>     }
> 
>diff --git a/admin/systempreferences.pl b/admin/systempreferences.pl
>index e12bd5f..deff2e5 100755
>--- a/admin/systempreferences.pl
>+++ b/admin/systempreferences.pl
>@@ -203,7 +203,7 @@ $tabsysprefs{DisplayClearScreenButton}       = "Circulation";
> $tabsysprefs{AllowAllMessageDeletion}        = "Circulation";
> $tabsysprefs{OverdueNoticeBcc}               = "Circulation";
> $tabsysprefs{OverduesBlockCirc}              = "Circulation";
>-
>+$tabsysprefs{AllowReturnToBranch}            = "Circulation";
> 
> # Staff Client
> $tabsysprefs{template}                = "StaffClient";
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index 6a53379..b0f934b 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -187,6 +187,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');
>diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
>index 9977f63..2ff47cd 100755
>--- a/installer/data/mysql/updatedatabase.pl
>+++ b/installer/data/mysql/updatedatabase.pl
>@@ -5610,6 +5610,17 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
>     SetVersion($DBversion);
> }
> 
>+$DBversion = "3.09.00.XXX";
>+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)
>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 e07fc0d..100c547 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
>@@ -185,6 +185,15 @@ Circulation:
>                   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
>               choices:
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 6151: 6497 | 6701 | 11250 | 11294 | 11295 | 11410 | 11585 | 11595 | 12208 | 12209 | 12210