From 32836f38a97a1391abb6459f4e4149e933424ad3 Mon Sep 17 00:00:00 2001 From: Ian Walls 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) --- C4/Circulation.pm | 59 +++++++++++++++++--- admin/systempreferences.pl | 2 +- installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 10 +++ .../en/modules/admin/preferences/circulation.pref | 9 +++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index debef01..d0c1ace 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -901,6 +901,53 @@ sub CanBookBeIssued { return ( \%issuingimpossible, \%needsconfirmation ); } +=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]) @@ -1556,18 +1603,14 @@ sub AddReturn { $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr; } - # if indy branches and returning to different branch, refuse the return - if ($hbr ne $branch && C4::Context->preference("IndependantBranches")){ + # 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 78d0768..0504634 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 ae2c1cb..6b47300 100755 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -184,6 +184,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 9ae0060..8c0a28f 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -4571,6 +4571,16 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.07.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 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 f4946b5..6b0de39 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 @@ -161,6 +161,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.4.1