|
Description
Katrin Fischer
2019-09-30 09:49:06 UTC
Created attachment 93231 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer To test: 1 - Go to Circulation->Transfer 2 - Note your signed in branch 3 - Find an item from your branch and create a transfer to branch B 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to brnach B 7 - Confirm the item is held at your current branch and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Comment on attachment 93231 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer Review of attachment 93231 [details] [review]: ----------------------------------------------------------------- ::: circ/branchtransfers.pl @@ +127,5 @@ > if ($barcode) { > > ( $transferred, $messages ) = > + transferbook({ > + from_branch => C4::Context->userenv->{'branch'}, Hmm, not sure this is right.. you might be transferring from the branch you're logged in at.. but you may also be transferring from elsewhere to you logged in branch... I'd say we probably need to add a field to the form and default to the items current holdingbranch if the user doesn't explicitly state the items in their branch? Created attachment 98942 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch To test: 1 - Go to Circulation->Transfer 2 - Find an item from your branch and create a transfer to branch B 3 - Note you can specify the origin and default is 'Item's holding library' 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to branch B from branch D 7 - Confirm the item is held at branch D and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Applying: Bug 23695: Set holding branch to transferring branch when manually initiating a transfer Using index info to reconstruct a base tree... M C4/Circulation.pm M circ/branchtransfers.pl M t/db_dependent/Circulation.t M t/db_dependent/Koha/Items.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Koha/Items.t Auto-merging t/db_dependent/Circulation.t CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t Auto-merging circ/branchtransfers.pl Auto-merging C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 23695: Set holding branch to transferring branch when manually initiating a transfer The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort" Created attachment 99955 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer To test: 1 - Go to Circulation->Transfer 2 - Note your signed in branch 3 - Find an item from your branch and create a transfer to branch B 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to brnach B 7 - Confirm the item is held at your current branch and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Created attachment 99956 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch To test: 1 - Go to Circulation->Transfer 2 - Find an item from your branch and create a transfer to branch B 3 - Note you can specify the origin and default is 'Item's holding library' 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to branch B from branch D 7 - Confirm the item is held at branch D and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Created attachment 100567 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer To test: 1 - Go to Circulation->Transfer 2 - Note your signed in branch 3 - Find an item from your branch and create a transfer to branch B 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to brnach B 7 - Confirm the item is held at your current branch and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 100568 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch To test: 1 - Go to Circulation->Transfer 2 - Find an item from your branch and create a transfer to branch B 3 - Note you can specify the origin and default is 'Item's holding library' 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to branch B from branch D 7 - Confirm the item is held at branch D and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 101441 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer To test: 1 - Go to Circulation->Transfer 2 - Note your signed in branch 3 - Find an item from your branch and create a transfer to branch B 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to brnach B 7 - Confirm the item is held at your current branch and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 101442 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch To test: 1 - Go to Circulation->Transfer 2 - Find an item from your branch and create a transfer to branch B 3 - Note you can specify the origin and default is 'Item's holding library' 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to branch B from branch D 7 - Confirm the item is held at branch D and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 101443 [details] [review] Bug 23695: (follow-up) Adjust for addition of transfer trigger Hi Nick, I've switched this to be a bug, as bug 23129 was also considered one. Sadly it does on longer apply, can you please rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 23695: Set holding branch to transferring branch when manually initiating a transfer Applying: Bug 23695: Alter transferbook to take a hash of params and specify from_branch error: sha1 information is lacking or useless (circ/branchtransfers.pl). error: could not build fake ancestor Created attachment 107463 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer To test: 1 - Go to Circulation->Transfer 2 - Note your signed in branch 3 - Find an item from your branch and create a transfer to branch B 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to brnach B 7 - Confirm the item is held at your current branch and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107464 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch To test: 1 - Go to Circulation->Transfer 2 - Find an item from your branch and create a transfer to branch B 3 - Note you can specify the origin and default is 'Item's holding library' 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to branch B from branch D 7 - Confirm the item is held at branch D and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 107465 [details] [review] Bug 23695: (follow-up) Adjust for addition of transfer trigger Comment on attachment 107464 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch Review of attachment 107464 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt @@ +72,4 @@ > <input type="hidden" name="biblionumber" value="[% biblionumber | html %]" /> > <input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]" /> > <input type="hidden" name="tobranchcd" value="[% tobranchcd | html %]" /> > + <input type="hidden" name="frombranchcd" value="[% tobranchcd | html %]" /> At first glance, the 'tobranch' value here seems incorrect.. Created attachment 108183 [details] [review] Bug 23695: Set holding branch to transferring branch when manually initiating a transfer To test: 1 - Go to Circulation->Transfer 2 - Note your signed in branch 3 - Find an item from your branch and create a transfer to branch B 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to brnach B 7 - Confirm the item is held at your current branch and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108184 [details] [review] Bug 23695: Alter transferbook to take a hash of params and specify from_branch To test: 1 - Go to Circulation->Transfer 2 - Find an item from your branch and create a transfer to branch B 3 - Note you can specify the origin and default is 'Item's holding library' 4 - Confirm the item is marked as held at your current branch and is being transferred to B 5 - Find an item from a third branch, branch C 6 - Transfer that item to branch B from branch D 7 - Confirm the item is held at branch D and is being transferred to B 8 - prove -v t/db_dependent/Circulation.t 9 - prove -v t/db_dependent/Koha/Items.t 10 - prove -v t/db_dependent/RotatingCollections.t Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108185 [details] [review] Bug 23695: (follow-up) Adjust for addition of transfer trigger Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 108186 [details] [review] Bug 23695: (QA follow-up) Fix Copy/Paste The value remained as 'tobranchcd' and needed to be updated to 'frombranchcd' to allow reservered transfers to be correctly set. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> After the QA fix, this all works well, is well tested and passes the QA scripts. Passing QA What about raising an exception if to_branch or from_branch are missing? Created attachment 108273 [details] [review] Bug 23695: (follow-up) Add exceptions for missing branch parameters Created attachment 108495 [details] [review] Bug 23695: (follow-up) Add exceptions for missing branch parameters JD: amended patch Remove QA issues: FAIL t/db_dependent/Circulation.t FAIL valid "my" variable $doreturn masks earlier declaration in same scope "my" variable $messages masks earlier declaration in same scope Pushed to master for 20.11, thanks to everybody involved! patchset does not apply cleanly to 20.05.x, no backport please rebase if needed Hmmm.. I think there's something wrong here re-reading the code.. We appear to expose the option in the UI to pick the 'Originating branch', but we don't pass that through to transferbook and instead rely on the current users branch. Created attachment 111391 [details] [review] Bug 23695: (QA follow-up) Revert 'Orginating library' option At my request, we added an 'Originating library' option to the manual transfers form. However, we did not properly pass this through in the controller and I have since discovered that my request was ill conceived/a misunderstanding of requirements. This patch removes the option from the template and drops the references to it in the controller. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Followup patch added as discussed.. I've asked Nick to take a quick look as a final sanity check before pushing. Created attachment 111394 [details] [review] Bug 23695: (QA follow-up) Revert 'Orginating library' option At my request, we added an 'Originating library' option to the manual transfers form. However, we did not properly pass this through in the controller and I have since discovered that my request was ill conceived/a misunderstanding of requirements. This patch removes the option from the template and drops the references to it in the controller. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Shifted back to the PQA queue for the final followup.. Thanks for verifying Nick. Pushed to master for 20.11, thanks to everybody involved! |