Bugzilla – Attachment 33455 Details for
Bug 7981
Remove HomeOrHoldingBranchReturn syspref
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 7981 - Transfer message when returning doesn't properly respect HomeOrHoldingBranchReturn syspref
Bug 7981 - Remove HomeOrHoldingBranchReturn syspref (text/plain), 6.42 KB, created by
Benjamin Rokseth
on 2014-11-11 12:38:04 UTC
(
hide
)
Description:
Bug 7981 - Transfer message when returning doesn't properly respect HomeOrHoldingBranchReturn syspref
Filename:
MIME Type:
Creator:
Benjamin Rokseth
Created:
2014-11-11 12:38:04 UTC
Size:
6.42 KB
patch
obsolete
>From 18502b84de565e0a5bd5ce57bea5b8ada711643b Mon Sep 17 00:00:00 2001 >From: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> >Date: Tue, 6 May 2014 19:12:18 +0200 >Subject: [PATCH] Bug 7981 - Transfer message when returning doesn't properly > respect HomeOrHoldingBranchReturn syspref > >This patch removes HomeOrHoldingBranchReturn syspref and makes circ/returns.pl respect branch >circulation rules from C4::Circulation::GetBranchItemRule. Also transfer slip notice should reflect this. > >Default should always be to return item to home branch. > >Test plan: >- make sure syspref 'AutomaticItemReturn' is set to 'false' >- unset 'Default checkout, hold and return policy' or set 'Return policy' to 'Item returns home' >- checkout an item and do a checkin from different branch than items homebranch >- verify that you're prompted with a transfer message to item's home branch and that print slip matches > >- set 'Return policy' to 'Item returns to issuing library' >- do a checkout and a checkin from branch different than item's home branch >- verify that you're not prompted with a transfer message and that holding library is your current branch > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> > >Follow-up: >- Added 3 tests in t/db_dependent/Circulation_Branches.t to test AddReturn > policies >- Removed HomeOrHoldingBranchReturn from sysprefs.sql >- Added notice on removing syspref in updatedatabase > >QA edits: >- removed trailing whitespace in tests >- moved branchname lookup from returns.pl to template >--- > C4/Circulation.pm | 36 +++++++++++++++++++++--------------- > 1 file changed, 21 insertions(+), 15 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index fb5b82f..ff5ef67 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1620,6 +1620,7 @@ holdallowed => Hold policy for this branch and itemtype. Possible values: > 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: > >@@ -1738,6 +1739,14 @@ fields from the reserves table of the Koha database, and > C<biblioitemnumber>. It also has the key C<ResFound>, whose value is > either C<Waiting>, C<Reserved>, or 0. > >+=item C<WasReturned> >+ >+Value 1 if return is successful. >+ >+=item C<NeedsTransfer> >+ >+If AutomaticItemReturn is disabled, return branch is given as value of NeedsTransfer. >+ > =back > > C<$iteminformation> is a reference-to-hash, giving information about the >@@ -1769,7 +1778,7 @@ sub AddReturn { > return (0, { BadBarcode => $barcode }); # no barcode means no item or borrower. bail out. > } > my $issue = GetItemIssue($itemnumber); >-# warn Dumper($iteminformation); >+ > if ($issue and $issue->{borrowernumber}) { > $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber}) > or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existant borrowernumber '$issue->{borrowernumber}'\n" >@@ -1789,9 +1798,9 @@ sub AddReturn { > my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed"; > # full item data, but no borrowernumber or checkout info (no issue) > # we know GetItem should work because GetItemnumberFromBarcode worked >- my $hbr = GetBranchItemRule($item->{'homebranch'}, $item->{'itype'})->{'returnbranch'} || "homebranch"; >+ my $hbr = GetBranchItemRule($item->{'homebranch'}, $item->{'itype'})->{'returnbranch'} || "homebranch"; > # get the proper branch to which to return the item >- $hbr = $item->{$hbr} || $branch ; >+ my $returnbranch = $item->{$hbr} || $branch ; > # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) > > my $borrowernumber = $borrower->{'borrowernumber'} || undef; # we don't know if we had a borrower or not >@@ -1818,9 +1827,9 @@ sub AddReturn { > > # check if the book is in a permanent collection.... > # FIXME -- This 'PE' attribute is largely undocumented. afaict, there's no user interface that reflects this functionality. >- if ( $hbr ) { >+ if ( $returnbranch ) { > my $branches = GetBranches(); # a potentially expensive call for a non-feature. >- $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr; >+ $branches->{$returnbranch}->{PE} and $messages->{'IsPermanent'} = $returnbranch; > } > > # check if the return is allowed at this branch >@@ -2019,21 +2028,18 @@ sub AddReturn { > DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); > } > >- # FIXME: make this comment intelligible. >- #adding message if holdingbranch is non equal a userenv branch to return the document to homebranch >- #we check, if we don't have reserv or transfert for this document, if not, return it to homebranch . >- >- if ( !$is_in_rotating_collection && ($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $hbr) and not $messages->{'WrongTransfer'}){ >- if ( C4::Context->preference("AutomaticItemReturn" ) or >+ # Transfer to returnbranch if Automatic transfer set or append message NeedsTransfer >+ if (($doreturn or $messages->{'NotIssued'}) and !$resfound and ($branch ne $returnbranch) and not $messages->{'WrongTransfer'}){ >+ if (C4::Context->preference("AutomaticItemReturn" ) or > (C4::Context->preference("UseBranchTransferLimits") and >- ! IsBranchTransferAllowed($branch, $hbr, $item->{C4::Context->preference("BranchTransferLimitsType")} ) >+ ! IsBranchTransferAllowed($branch, $returnbranch, $item->{C4::Context->preference("BranchTransferLimitsType")} ) > )) { >- $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $hbr; >+ $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $returnbranch; > $debug and warn "item: " . Dumper($item); >- ModItemTransfer($item->{'itemnumber'}, $branch, $hbr); >+ ModItemTransfer($item->{'itemnumber'}, $branch, $returnbranch); > $messages->{'WasTransfered'} = 1; > } else { >- $messages->{'NeedsTransfer'} = 1; # TODO: instead of 1, specify branchcode that the transfer SHOULD go to, $item->{homebranch} >+ $messages->{'NeedsTransfer'} = $returnbranch; > } > } > >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 7981
:
26723
|
27487
|
27950
|
27992
|
28199
|
33455
|
36615
|
37724
|
37725
|
37726
|
38151
|
38174
|
38176
|
38177
|
38178
|
38179
|
38180
|
38237
|
38238
|
38241
|
38242
|
38243
|
38244
|
38245
|
39504
|
39848
|
39849
|
39850
|
39851
|
39852
|
39853