|
Description
Nick Clemens (kidclamp)
2021-03-04 15:31:16 UTC
Created attachment 117941 [details] [review] Bug 27865: Pass the x-koha-override header to PUT /holds/:hold_id from the patron's page This patch makes the AJAX call to update an existing hold, pass the 'x-koha-override' header with the value of 'any', so any pickup location policy override that is needed, is applied. As the override is taken into account only if AllowHoldPolicyOverride is enabled, this in fact restores the previous behavior. To test: 1. Have a patron with a hold 2. Have AllowHoldPolicyOverride set to 'yes' 3. Try changing the pickup location to an invalid one => FAIL: The options are the same as before 27205, yet the AJAX call fails because of invalid pickup location. 4. Apply this patch 5. Repeat 3 => SUCCESS: The API call succeeds, and the hold is updated (it is actually refetch, so just verify it has the value you selected 6. Sign off :-D Created attachment 118119 [details] [review] Bug 27865: Pass the x-koha-override header to PUT /holds/:hold_id from the patron's page This patch makes the AJAX call to update an existing hold, pass the 'x-koha-override' header with the value of 'any', so any pickup location policy override that is needed, is applied. As the override is taken into account only if AllowHoldPolicyOverride is enabled, this in fact restores the previous behavior. To test: 1. Have a patron with a hold 2. Have AllowHoldPolicyOverride set to 'yes' 3. Try changing the pickup location to an invalid one => FAIL: The options are the same as before 27205, yet the AJAX call fails because of invalid pickup location. 4. Apply this patch 5. Repeat 3 => SUCCESS: The API call succeeds, and the hold is updated (it is actually refetch, so just verify it has the value you selected 6. Sign off :-D Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> There is a lot of bugs depending on this I feel would be in better hands with other QA team members, but I wanted to test the end result a bit: I am not sure if this works correctly. I am testing this right after bug 27894 with the same holds. For my holds, I only allow the pick-up location = item's home library. Also Fairview is set to be no pickup location on library level. In the patron account, on details and checkouts tabs: - Fairview doesn't show up - OK - All other libraries show independent of the AllowHoldPolicyOverride setting - NOT OK. With Don't allow only the home library of the item should be available. - In comparison: for the same holds, the pull down on detail page > holds only shows the home library as expected with the stricter setting. Should we not also establish a visual warning here like 27894 does? (In reply to Katrin Fischer from comment #4) > There is a lot of bugs depending on this I feel would be in better hands > with other QA team members, but I wanted to test the end result a bit: > > I am not sure if this works correctly. I am testing this right after bug > 27894 with the same holds. > > For my holds, I only allow the pick-up location = item's home library. > Also Fairview is set to be no pickup location on library level. > > In the patron account, on details and checkouts tabs: > - Fairview doesn't show up - OK > - All other libraries show independent of the AllowHoldPolicyOverride > setting - NOT OK. With Don't allow only the home library of the item should > be available. > - In comparison: for the same holds, the pull down on detail page > holds > only shows the home library as expected with the stricter setting. > > Should we not also establish a visual warning here like 27894 does? Yeah, adding the visual feedback here would be helpful. The idea here was to provide a backportable fix to a regression, though. Will fix it to make it show all pickup locations only if AllowHoldPolicyOverride is set. I now recall it. This bug wasn't about fixing the dropdown, but fixing the PUT, which was failing even though the dropdown presents the libraries to override. (In reply to Tomás Cohen Arazi from comment #6) > I now recall it. This bug wasn't about fixing the dropdown, but fixing the > PUT, which was failing even though the dropdown presents the libraries to > override. So I should be able to change the pickup location to an invalid one or only when the override is possible? (In reply to Katrin Fischer from comment #7) > (In reply to Tomás Cohen Arazi from comment #6) > > I now recall it. This bug wasn't about fixing the dropdown, but fixing the > > PUT, which was failing even though the dropdown presents the libraries to > > override. > > So I should be able to change the pickup location to an invalid one or only > when the override is possible? Only when the override is possible. The PUT route should reject the request. But that's not in the scope of this bug. That is taken care of on bug 27898, and it is correctly tested in my opinion. What you might be facing here, is wrong dropdown rendering. Which could be solved the way we did in bug 27894 and friends. As I said, the idea was to make it easy to backport this, and move the dropdown rendering into using Select2 + API + visual feedback on overrides, on a separate bug. Created attachment 120291 [details] [review] Bug 27865: Pass the x-koha-override header to PUT /holds/:hold_id from the patron's page This patch makes the AJAX call to update an existing hold, pass the 'x-koha-override' header with the value of 'any', so any pickup location policy override that is needed, is applied. As the override is taken into account only if AllowHoldPolicyOverride is enabled, this in fact restores the previous behavior. To test: 1. Have a patron with a hold 2. Have AllowHoldPolicyOverride set to 'yes' 3. Try changing the pickup location to an invalid one => FAIL: The options are the same as before 27205, yet the AJAX call fails because of invalid pickup location. 4. Apply this patch 5. Repeat 3 => SUCCESS: The API call succeeds, and the hold is updated (it is actually refetch, so just verify it has the value you selected 6. Sign off :-D Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Created attachment 120308 [details] [review] Bug 27865: Pass the x-koha-override header to PUT /holds/:hold_id from the patron's page This patch makes the AJAX call to update an existing hold, pass the 'x-koha-override' header with the value of 'any', so any pickup location policy override that is needed, is applied. As the override is taken into account only if AllowHoldPolicyOverride is enabled, this in fact restores the previous behavior. To test: 1. Have a patron with a hold 2. Have AllowHoldPolicyOverride set to 'yes' 3. Try changing the pickup location to an invalid one => FAIL: The options are the same as before 27205, yet the AJAX call fails because of invalid pickup location. 4. Apply this patch 5. Repeat 3 => SUCCESS: The API call succeeds, and the hold is updated (it is actually refetch, so just verify it has the value you selected 6. Sign off :-D Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Dependencies not in 20.11.x |