|
Description
Aleisha Amohia
2022-05-05 04:19:21 UTC
Created attachment 134625 [details] [review] Bug 30687: Override hold policy when no valid pickup location This patch allows hold policy to be overridden when there is no valid pickup location and AllowHoldPolicyOverride is enabled. This is re-implementing old behaviour that has been lost. To test: 1. Go to Administration -> Circulation and fines rules. Ensure there is a default circulation rule and default reserves policy. 2. Change the library using the dropdown and scroll to the bottom of the page to set a reserves policy by item type for Library A, which the following settings: - Item type: BK - Reserve policy: From home library - Reserve pickup library match: any library - Return policy: Item returns home 3. Go to Administration -> system preferences. Set the AllowHoldPolicyOverride setting to Allow. 4. Create a biblio with an item where the item's home library is Library A and the itemtype is BK. 5. Log in to the staff client as a Patron that belongs to Library B. 6. Search for the biblio you just created and try to request a hold for yourself. Notice there is a blocking notice "Cannot place reserve", the "Place reserve" button is disabled, and the item has a "No valid pickup location" error. 7. Apply the patch and refresh the page or re-request the hold. 8. Confirm you can now place the hold, acknowledging that you must override hold policy to do so (note the exclamation point icon). 9. Set the AllowHoldPolicyOverride system preference to "Don't allow". 10. Repeat Step 6. Confirm you are blocked again from requesting the hold and cannot override it. Sponsored-by: Chartered Accountants Australia and New Zealand Created attachment 137643 [details] [review] Bug 30687: Override hold policy when no valid pickup location This patch allows hold policy to be overridden when there is no valid pickup location and AllowHoldPolicyOverride is enabled. This is re-implementing old behaviour that has been lost. To test: 1. Go to Administration -> Circulation and fines rules. Ensure there is a default circulation rule and default reserves policy. 2. Change the library using the dropdown and scroll to the bottom of the page to set a reserves policy by item type for Library A, which the following settings: - Item type: BK - Reserve policy: From home library - Reserve pickup library match: any library - Return policy: Item returns home 3. Go to Administration -> system preferences. Set the AllowHoldPolicyOverride setting to Allow. 4. Create a biblio with an item where the item's home library is Library A and the itemtype is BK. 5. Log in to the staff client as a Patron that belongs to Library B. 6. Search for the biblio you just created and try to request a hold for yourself. Notice there is a blocking notice "Cannot place reserve", the "Place reserve" button is disabled, and the item has a "No valid pickup location" error. 7. Apply the patch and refresh the page or re-request the hold. 8. Confirm you can now place the hold, acknowledging that you must override hold policy to do so (note the exclamation point icon). 9. Set the AllowHoldPolicyOverride system preference to "Don't allow". 10. Repeat Step 6. Confirm you are blocked again from requesting the hold and cannot override it. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Daniel <daniel.jones@cheshiresharedservices.gov.uk> Signing off as test plan works as described. One thing I noticed was that I couldn't complete the hold by checking in the item. Is this expected behaviour? (In reply to Daniel Jones from comment #3) > Signing off as test plan works as described. > > One thing I noticed was that I couldn't complete the hold by checking in the > item. Is this expected behaviour? Do you mean you couldn't set the hold as waiting? Or fill the hold? What do you mean by complete the hold? (In reply to Aleisha Amohia from comment #4) > (In reply to Daniel Jones from comment #3) > > Signing off as test plan works as described. > > > > One thing I noticed was that I couldn't complete the hold by checking in the > > item. Is this expected behaviour? > > Do you mean you couldn't set the hold as waiting? Or fill the hold? > > What do you mean by complete the hold? The hold cannot be filled. Once it is checked in, the book just asks to be returned to its home library, and not sent to the patron's library. Hi Aleisha, could you please look into Daniel's comment? If the hold cannot be filled at the pickup-location this might not work as intended. (In reply to Katrin Fischer from comment #6) > Hi Aleisha, could you please look into Daniel's comment? If the hold cannot > be filled at the pickup-location this might not work as intended. The library community has spoken loud and clearly in the past and present, and the consensus is that librarians should be able to force holds that are unfillable. In the past I attempted to limit forced holds to those that would be filled, and what we found out is that libraries utilize those unfillable holds for a variety of reasons. We ended up reverting those changes in community. We also have partners with this very same issue Aleisha is describing. I think it's safe to say that the questions of "can this hold be placed" and "can this hold be filled" are very much separate questions. In that spirit, I think the question of "should this item be trapped instead of transferred back" should be dealt with on a separate bug report. Created attachment 140683 [details] [review] Bug 30687: Override hold policy when no valid pickup location This patch allows hold policy to be overridden when there is no valid pickup location and AllowHoldPolicyOverride is enabled. This is re-implementing old behaviour that has been lost. To test: 1. Go to Administration -> Circulation and fines rules. Ensure there is a default circulation rule and default reserves policy. 2. Change the library using the dropdown and scroll to the bottom of the page to set a reserves policy by item type for Library A, which the following settings: - Item type: BK - Reserve policy: From home library - Reserve pickup library match: any library - Return policy: Item returns home 3. Go to Administration -> system preferences. Set the AllowHoldPolicyOverride setting to Allow. 4. Create a biblio with an item where the item's home library is Library A and the itemtype is BK. 5. Log in to the staff client as a Patron that belongs to Library B. 6. Search for the biblio you just created and try to request a hold for yourself. Notice there is a blocking notice "Cannot place reserve", the "Place reserve" button is disabled, and the item has a "No valid pickup location" error. 7. Apply the patch and refresh the page or re-request the hold. 8. Confirm you can now place the hold, acknowledging that you must override hold policy to do so (note the exclamation point icon). 9. Set the AllowHoldPolicyOverride system preference to "Don't allow". 10. Repeat Step 6. Confirm you are blocked again from requesting the hold and cannot override it. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Daniel <daniel.jones@cheshiresharedservices.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to Kyle M Hall from comment #7) > (In reply to Katrin Fischer from comment #6) > > Hi Aleisha, could you please look into Daniel's comment? If the hold cannot > > be filled at the pickup-location this might not work as intended. > > The library community has spoken loud and clearly in the past and present, > and the consensus is that librarians should be able to force holds that are > unfillable. In the past I attempted to limit forced holds to those that > would be filled, and what we found out is that libraries utilize those > unfillable holds for a variety of reasons. We ended up reverting those > changes in community. We also have partners with this very same issue > Aleisha is describing. I think it's safe to say that the questions of "can > this hold be placed" and "can this hold be filled" are very much separate > questions. In that spirit, I think the question of "should this item be > trapped instead of transferred back" should be dealt with on a separate bug > report. I am actually confused about this one and feel like I have been missing out on the discussion. How do libraries utilize holds that can never be filled? I'd be interested in examples to understand better. At the moment there is no way to utilize those unless you change the item values or you cancel the hold as it can otherwise not move to filled. I don't remember from testing, but does this unlock all pickup locations or will a hold with no pickup location be placed? (see bug 31086). I think nick could answer that question better than me. I just know they need to ;) Additionally, I think Aleisha's report does vary a bit from my own reports. What we are finding in 21.11 if that if some items should be holdable with an override, but at least one item is never holdable because of a lack of pickup location, it becomes impossible to place an overridden hold. (In reply to Kyle M Hall from comment #10) > I think nick could answer that question better than me. I just know they > need to ;) I do believe a hold that does not trigger will still resolve when item is checked out to the patron - so libraries can utilize these with reports Additionally - due to floating/transfers/limits etc. there are times when a hold is unfillable currently, but can be filled in the future when the item is at another location. Created attachment 140712 [details] [review] Bug 30687: Override hold policy when no valid pickup location This patch allows hold policy to be overridden when there is no valid pickup location and AllowHoldPolicyOverride is enabled. This is re-implementing old behaviour that has been lost. To test: 1. Go to Administration -> Circulation and fines rules. Ensure there is a default circulation rule and default reserves policy. 2. Change the library using the dropdown and scroll to the bottom of the page to set a reserves policy by item type for Library A, which the following settings: - Item type: BK - Reserve policy: From home library - Reserve pickup library match: any library - Return policy: Item returns home 3. Go to Administration -> system preferences. Set the AllowHoldPolicyOverride setting to Allow. 4. Create a biblio with an item where the item's home library is Library A and the itemtype is BK. 5. Log in to the staff client as a Patron that belongs to Library B. 6. Search for the biblio you just created and try to request a hold for yourself. Notice there is a blocking notice "Cannot place reserve", the "Place reserve" button is disabled, and the item has a "No valid pickup location" error. 7. Apply the patch and refresh the page or re-request the hold. 8. Confirm you can now place the hold, acknowledging that you must override hold policy to do so (note the exclamation point icon). 9. Set the AllowHoldPolicyOverride system preference to "Don't allow". 10. Repeat Step 6. Confirm you are blocked again from requesting the hold and cannot override it. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Daniel <daniel.jones@cheshiresharedservices.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 140713 [details] [review] Bug 30687: (QA follow-up) Allow pickup location choice when overridden The original test plan did not recreate behaviour for me, but I simply added: Default checkout, hold and return policy Hold policy: from home library Hold pickup library match: Item hold group (with no groups defined) Then I couldn't place hold before, but could place hold after patch, however, I couldn't choose a pickup location for the item level hold and the hold failed To test: 1 - Set rules as above 2 - Confirm you can choose a library for item specific hold 3 - Confirm all locations are marked with exclamation point 4 - Confirm hold can successfully be placed Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Katrin Fischer from comment #9) > I don't remember from testing, but does this unlock all pickup locations or > will a hold with no pickup location be placed? (see bug 31086). It will "unlock" all pickup locations, however, they are displayed with a note/exclamation point to indicate that they are violating policy This was done previously as it is believed override should allow you to override holdability and pickup rules I tried to solve the same issue but fixed it differently (I didn't know about this bug report until now) I tested this bug's patches and it doesn't work for me. I used the following test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold At step 8, I still see the error message "No items are available to be placed on hold." and the message " No valid pickup location " in the items table. "Place hold" buttons are disabled. Here's my patch: https://git.biblibre.com/biblibre/kohac/commit/a1fe134264a855a13b2586545475b5256deef10d Should I submit my patch in another bug report (as this one is already Passed QA) ? (In reply to Julian Maurice from comment #15) > I tried to solve the same issue but fixed it differently (I didn't know > about this bug report until now) > I tested this bug's patches and it doesn't work for me. Please FQA in such cases. > Should I submit my patch in another bug report (as this one is already > Passed QA) ? I'd say you should attach your patch here for other devs review in context (test plan, etc). Thanks! Created attachment 143457 [details] [review] Bug 30687 (alternative patch): Always allow to force hold pickup location If AllowHoldPolicyOverride is enabled and only some pickup locations are available, you still have the possibility to force one of the others pickup locations. But when there are zero pickup locations available, that is not possible. This patch change that by always displaying the list of pickup locations when AllowHoldPolicyOverride is enabled. Test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold Patch didn't apply in sandbox Created attachment 144427 [details] [review] Bug 30687 (alternative patch): Always allow to force hold pickup location If AllowHoldPolicyOverride is enabled and only some pickup locations are available, you still have the possibility to force one of the others pickup locations. But when there are zero pickup locations available, that is not possible. This patch change that by always displaying the list of pickup locations when AllowHoldPolicyOverride is enabled. Test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold Rebased the alternative patch (In reply to Catrina Berka from comment #18) > Patch didn't apply in sandbox I'm not sure if a sandbox can be used to test this bug, because it has two sets of patches that try to fix the bug in different ways. It doesn't make sense to apply all patches in this case. I rise to major because a lot of librarians asked about this regression Created attachment 144825 [details] [review] Bug 30687: [alternate-alternate] Allow pickup locatoin to be forced when override is allowed This is Julian's patch with some extra cleanup to reduce repeated code If AllowHoldPolicyOverride is enabled and only some pickup locations are available, you still have the possibility to force one of the others pickup locations. But when there are zero pickup locations available, that is not possible. This patch change that by always displaying the list of pickup locations when AllowHoldPolicyOverride is enabled. Test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold Current status: Needs Signoff Created attachment 144896 [details] [review] Bug 30687: [alternate-alternate] Allow pickup locatoin to be forced when override is allowed This is Julian's patch with some extra cleanup to reduce repeated code If AllowHoldPolicyOverride is enabled and only some pickup locations are available, you still have the possibility to force one of the others pickup locations. But when there are zero pickup locations available, that is not possible. This patch change that by always displaying the list of pickup locations when AllowHoldPolicyOverride is enabled. Test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold Signed-off-by: David Nind <david@davidnind.com> I've tested and signed off the alternate-alternate patch. I've drafted a release note - I don't feel confident enough that I've got this right. Could someone check this, edit where appropriate, and add? Thanks! I'm akso not sure if this was introduced in 21.11. Draft release note: This fixes a regression introduced in Koha 21.11(???). Previously, if hold overrides were enabled (the AllowHoldPolicyOverride system preference) and there was no pickup location available (for example, if a library was not a pickup location), then it was still possible to select an alternative pickup location. This restores this ability - by default no pickup location is selected, but any location can now be selected from the dropdown list (each option now has an exclamation mark at the end with the message "This pickup location is not allowed according to circulation rules"). Note: the test plan says the exclamation mark is at the start of the option, but for me it displayed at the end. Also, does this need a note about how some libraries want to be able to place unfillable holds for a variety of reasons? (see comment #7 and following discussions). Julian: can you please review Nick's re-versioning of your patch? Aleisha: do you agree with what's been said and this last version of a solution for the bug? Created attachment 148502 [details] [review] Bug 30687: [alternate-alternate] Allow pickup locatoin to be forced when override is allowed This is Julian's patch with some extra cleanup to reduce repeated code If AllowHoldPolicyOverride is enabled and only some pickup locations are available, you still have the possibility to force one of the others pickup locations. But when there are zero pickup locations available, that is not possible. This patch change that by always displaying the list of pickup locations when AllowHoldPolicyOverride is enabled. Test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> (In reply to Tomás Cohen Arazi from comment #26) > Julian: can you please review Nick's re-versioning of your patch? I can say it works at least as well as mine. I signed it off (In reply to Tomás Cohen Arazi from comment #26) > Aleisha: do you agree with what's been said and this last version of a > solution for the bug? Aleisha, ping? Yes alternate-alternate looks good thanks! Created attachment 150260 [details] [review] Bug 30687: Allow pickup location to be forced when override is allowed This is Julian's patch with some extra cleanup to reduce repeated code If AllowHoldPolicyOverride is enabled and only some pickup locations are available, you still have the possibility to force one of the others pickup locations. But when there are zero pickup locations available, that is not possible. This patch change that by always displaying the list of pickup locations when AllowHoldPolicyOverride is enabled. Test plan: 1. Apply patch 2. Disable AllowHoldPolicyOverride 3. Create a biblio B with an item I at library A. 4. Configure this library A to not be a pickup location 5. Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6. Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7. Enable AllowHoldPolicyOverride 8. Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9. Verify you can place a title-level hold and an item-level hold Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x Backported to 22.05.x for upcoming 22.05.14 Not backporting to 21.11.x |