If IndependantBranches is enabled, all holds for an item display the currently logged in library no matter what, and modifying the holds will set all pickup locations for all the holds to that logged in library!
Created attachment 15172 [details] [review] Bug 9575 - Serious Holds Problem with IndependantBranches Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled.
Created attachment 16058 [details] [review] Bug 9575 - Serious Holds Problem with IndependantBranches Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled. Signed-off-by: Liz Rea <liz@catalyst.net.nz> works as advertised, passes tests
QA comment: Why do you modify 2 routines when you can just modify the pl file? To me, Getbranches($foo, $branch) is not consistent. I think you can do something like if ( C4::Context->preference('IndependantBranches') ) { $reserve{'branchloop'} = GetBranchDetail($res->{'branchcode'}); } else { $reserve{'branchloop'} = GetBranchesLoop($res->{'branchcode'}); } Feel free to discuss if you disagree :)
Created attachment 16073 [details] [review] Bug 9575 - Serious Holds Problem with IndependantBranches [Alternative] Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled.
(In reply to comment #3) > QA comment: > > Why do you modify 2 routines when you can just modify the pl file? Only because I hadn't thought of that! Here's an alternative with that implementation.
Thanks Kyle. I switch back to Needs Signed Off, because this alternative patch entirely differs from your first one.
Hi Kyle, I with the alternative patch I seem to get empty pick up location drop downs without independant branches on. I didn't go any further than that. Liz
Created attachment 16860 [details] [review] Bug 9575 - Serious Holds Problem with IndependantBranches Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 16861 [details] [review] Bug 9575 - Serious Holds Problem with IndependantBranches Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 18048 [details] [review] [SIGNED-OFF] Bug 9575 - Serious Holds Problem with IndependantBranches If IndependantBranches is enabled, all holds for an item display the currently logged in library no matter what, and modifying the holds will set all pickup locations for all the holds to that logged in library! Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Kyle, I am sorry, but this doesn't work for me. I created 3 different holds with 3 different pickup libraries away from my logged in library. I turned on independentbranches and like you described, suddenly all holds showed my current branch as pickup library. But after applying the patch and refreshing the page several times, this is still the case.
(In reply to comment #11) > But after applying the patch and > refreshing the page several times, this is still the case. After you applied the patch did you start with a new set of three holds with different pickup locations?
Hi Kyle and Owen, I skipped 5) - modifying the holds, because the pull downs were wrong and it seemed obvious that modifying would ruing the pickup location. But now I have problems reproducing the original problem with the wrong pull downs, I will try to investigate more later.
Ok, I think I have resolved it - the problem occurs not if your user is a superlibrarian. It does occur, when you don't have superlibrarian permissions.
Created attachment 18123 [details] [review] [PASSED QA] Bug 9575 - Serious Holds Problem with IndependantBranches If IndependantBranches is enabled, all holds for an item display the currently logged in library no matter what, and modifying the holds will set all pickup locations for all the holds to that logged in library! Test plan: 1) Disable IndependantBranches 2) Place 3 holds on a record with 3 different pickup locations 3) Enable IndependantBranches 4) Refresh the holds page for that record, all the holds show now show a pickup location of the logged in branch 5) Modify the order of the holds 6) Disable IndependantBranches 7) Refresh the holds, verify the holds all now have a pickup location of the logged in branch 8) Apply the patch 9) Repeat steps 1-3 10) Refresh the holds page for that record, all the holds should now show the correct pickup branch, but it cannot be changed as IndependantBranches is enabled. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> In my tests the above test plan only worked out if my user had no superlibrarian permission. But then behaviour for independentbranches off/on was all correct. All tests and QA script pass.
This patch has been pushed to master and 3.12.x.