Bug 35573 - Koha is not correctly warning of overridden items when placing a hold if AllowHoldPolicyOverride
Summary: Koha is not correctly warning of overridden items when placing a hold if Allo...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Thibaud Guillot
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-14 15:48 UTC by Nick Clemens
Modified: 2024-04-05 09:52 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 35573: Revert "Bug 30687: Allow pickup location to be forced when override is allowed" (6.46 KB, patch)
2024-02-05 12:18 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 35573: Correctly display warning when placing a hold (4.98 KB, patch)
2024-02-05 12:18 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 35573: Revert "Bug 30687: Allow pickup location to be forced when override is allowed" (6.51 KB, patch)
2024-02-20 17:06 UTC, David Nind
Details | Diff | Splinter Review
Bug 35573: Correctly display warning when placing a hold (5.03 KB, patch)
2024-02-20 17:06 UTC, David Nind
Details | Diff | Splinter Review
Bug 35573: Revert "Bug 30687: Allow pickup location to be forced when override is allowed" (6.57 KB, patch)
2024-04-02 11:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35573: Correctly display warning when placing a hold (5.08 KB, patch)
2024-04-02 11:09 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2023-12-14 15:48:29 UTC
If AllowHoldPolicyOverride is enabled we are not warning that items require override if there are valid pickup locations.


Regressions caused by (me on) bug 30687
Comment 1 Thibaud Guillot 2024-02-05 12:18:03 UTC
Created attachment 161758 [details] [review]
Bug 35573: Revert "Bug 30687: Allow pickup location to be forced when override is allowed"

This reverts commit ab93008da7eb61b697e4586c679b88c2eebaacd0.
Comment 2 Thibaud Guillot 2024-02-05 12:18:06 UTC
Created attachment 161759 [details] [review]
Bug 35573: Correctly display warning when placing a hold

Test plan:

1) Add a basic circulation rule like : 0 hold allowed on patron category
   'A' and set AllowHoldPolicyOverride to 'Allow'
2) Make a hold on a patron belongs to this category
3) Normally you should see a warning on item (override needed) but it's
   not the case
4) Apply this patch
5) Refresh and repeat step 2

This patch comes from an alternative patch on the original bug (I'm including
the original test plan for your information) :

(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
Comment 3 David Nind 2024-02-20 17:06:20 UTC
Created attachment 162304 [details] [review]
Bug 35573: Revert "Bug 30687: Allow pickup location to be forced when override is allowed"

This reverts commit ab93008da7eb61b697e4586c679b88c2eebaacd0.

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-02-20 17:06:22 UTC
Created attachment 162305 [details] [review]
Bug 35573: Correctly display warning when placing a hold

Test plan:

1) Add a basic circulation rule like : 0 hold allowed on patron category
   'A' and set AllowHoldPolicyOverride to 'Allow'
2) Make a hold on a patron belongs to this category
3) Normally you should see a warning on item (override needed) but it's
   not the case
4) Apply this patch
5) Refresh and repeat step 2

This patch comes from an alternative patch on the original bug (I'm including
the original test plan for your information) :

(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

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Nick Clemens 2024-04-02 11:09:55 UTC
Created attachment 164251 [details] [review]
Bug 35573: Revert "Bug 30687: Allow pickup location to be forced when override is allowed"

This reverts commit ab93008da7eb61b697e4586c679b88c2eebaacd0.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens 2024-04-02 11:09:57 UTC
Created attachment 164252 [details] [review]
Bug 35573: Correctly display warning when placing a hold

Test plan:

1) Add a basic circulation rule like : 0 hold allowed on patron category
   'A' and set AllowHoldPolicyOverride to 'Allow'
2) Make a hold on a patron belongs to this category
3) Normally you should see a warning on item (override needed) but it's
   not the case
4) Apply this patch
5) Refresh and repeat step 2

This patch comes from an alternative patch on the original bug (I'm including
the original test plan for your information) :

(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

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens 2024-04-02 11:10:33 UTC
I think the reversion means there is some cleanup to do for the script, however, this is a simple fix and works well
Comment 8 Katrin Fischer 2024-04-05 09:52:15 UTC
Pushed for 24.05!

Well done everyone, thank you!