Description
Magnus Enger
2021-04-09 12:00:36 UTC
Certainly caused by commit aa221e0c8bcf0fa0377d6c2154bad0ef61c97a2b Bug 27071: Use GetReservesControlBranch to pick the branch -my $pickup = $input->param('pickup') || C4::Context->userenv->{'branch'}; +my $pickup = $input->param('pickup'); (In reply to Jonathan Druart from comment #1) > Certainly caused by > commit aa221e0c8bcf0fa0377d6c2154bad0ef61c97a2b > Bug 27071: Use GetReservesControlBranch to pick the branch > > -my $pickup = $input->param('pickup') || C4::Context->userenv->{'branch'}; > +my $pickup = $input->param('pickup'); That change slipped in inadvertently! Created attachment 119469 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> BTW: Sorry for this, Magnus :-/ I don't think your patch is correct, Tomas. The main point of the commit I cited is to not pick the logged in branchcode. In the following line we will never pick the expected branchcode: 554 my $reserves_control_branch = $pickup // C4::Reserves::GetReservesControlBranch( $item, $patron_unblessed ); Created attachment 119565 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 119602 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 119603 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk> Signed off. This patch changes the default pickup branch selected when placing a hold on the staff interface. When testing without the patch, the pickup branch for the hold was simply the branch at the top of the list alphabetically. After the patch, the default pickup branch is the library set by the staff interface. after the patch, this branch also has "selected=selected" in the html code. Created attachment 119605 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Works as described and is a solid improvement. Passing QA We need to keep $pickup for $reserves_control_branch, in case it's passed. request.pl can be called with a specific branchcode from this code in request.tt: 1079 [% IF Koha.Preference('UseBranchTransferLimits') %] 1080 $("#pickup").on('change', function(){ 1081 var pickup = $("#pickup").val(); 1082 var url = "?pickup=" + pickup; 1083 url += "&borrowernumber=" + borrowernumber; 1084 url += "&biblionumber=" + biblionumber; 1085 window.location.replace(url); 1086 }); 1087 [% END %] We need to use Branches.GetLoggedInBranchcode to preselect the option, if pickup is not passed. Created attachment 119629 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 119630 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 119673 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 119674 [details] [review] Bug 28118: Default to current branch when placing hold During bug 27071 development, this line got inadvertedly changed. This patch restores the original behaviour. To test: 1. Search for a title 2. Go try place a hold => FAIL: Look at the HTML, there's no pickup location with selected="selected" 3. Switch branch and repeat 2 => FAIL: Still the same 4. Apply this patch 5. Repeat 2 and 3 => SUCCESS: Branch is selected, chosen current branch is picked 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.05 Pushed to 20.05.x for 20.05.11 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |