Bug 28748 - When hold is overridden cannot select a pickup location
Summary: When hold is overridden cannot select a pickup location
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 28338
Blocks:
  Show dependency treegraph
 
Reported: 2021-07-23 13:25 UTC by Nick Clemens
Modified: 2021-10-23 01:35 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.05


Attachments
Bug 27895: Prepare pickup locations for overridden holds (3.27 KB, patch)
2021-10-06 12:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27895: Prepare pickup locations for overridden holds (3.33 KB, patch)
2021-10-06 13:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28748: Prepare pickup locations for overridden holds (3.27 KB, patch)
2021-10-07 13:23 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28748: Prepare pickup locations for overridden holds (3.37 KB, patch)
2021-10-07 13:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-07-23 13:25:33 UTC
Bug 28338 added a check of hold pickup locations for each hold item on a record

When selecting an item that normally is not for hold there is no pickup dropdown, the hold is placed with no location


To recreate:
1 - Enable  AllowHoldPolicyOverride 
2 - Set 'Default checkout, hold and return policy' - Hold policy 'From home library' Hold pickup library  match 'Item's home library'
3 - Find a record with items from multiple libraries
4 - Attempt to place a hold on an item that normally wouldn't be allowed
5 - Notice these items have no holds dropdown
6 - Place the hold
7 - Note the pickup location is blank
Comment 1 Nick Clemens 2021-10-06 12:09:31 UTC
Created attachment 125782 [details] [review]
Bug 27895: Prepare pickup locations for overridden holds

The logic in request.pl only passes pickup locations if a hold is allowed,
they are not calculated when the hold is overridden.

This patch copies the pickup locations code into the override conditional,
adjusting to mark the item overridden

This also highlights an condition of override:
If a hold is allowed, but there are no valid pickup locations the override
is not allowed.

To test:
 1 - Set a holds rule to allow on-shelf holds only if all unavailable
 2 - Set AllowHoldPolicyOverride to 'Allow'
 3 - Find a record with several items available
 4 - Attempt to place a hold
 5 - The items have a yellow triangle, but no pickup locations
 6 - Place an item level hold
 7 - Note the hold has no pickup location
 8 - Cancel the hold
 9 - Apply patch and restart all
10 - Attempt hold again
11 - Items still have yellow triangle, but there are dropdowns for pickup location
12 - Place an item level hold, verify a pickup location is set
13 - Cancel the hold
14 - Alter circ rule to allow on shelf holds
15 - Change 'Default checkout, hold and return policy' - 'Hold pickup library match' to "item's hold group"
16 - Ensure there is no group for the item
17 - Attempt to place hold
18 - Override is not allowed 'No valid pickup locations'
Comment 2 Tomás Cohen Arazi 2021-10-06 13:40:42 UTC
Created attachment 125814 [details] [review]
Bug 27895: Prepare pickup locations for overridden holds

The logic in request.pl only passes pickup locations if a hold is allowed,
they are not calculated when the hold is overridden.

This patch copies the pickup locations code into the override conditional,
adjusting to mark the item overridden

This also highlights an condition of override:
If a hold is allowed, but there are no valid pickup locations the override
is not allowed.

To test:
 1 - Set a holds rule to allow on-shelf holds only if all unavailable
 2 - Set AllowHoldPolicyOverride to 'Allow'
 3 - Find a record with several items available
 4 - Attempt to place a hold
 5 - The items have a yellow triangle, but no pickup locations
 6 - Place an item level hold
 7 - Note the hold has no pickup location
 8 - Cancel the hold
 9 - Apply patch and restart all
10 - Attempt hold again
11 - Items still have yellow triangle, but there are dropdowns for pickup location
12 - Place an item level hold, verify a pickup location is set
13 - Cancel the hold
14 - Alter circ rule to allow on shelf holds
15 - Change 'Default checkout, hold and return policy' - 'Hold pickup library match' to "item's hold group"
16 - Ensure there is no group for the item
17 - Attempt to place hold
18 - Override is not allowed 'No valid pickup locations'

https://bugs.koha-community.org/show_bug.cgi?id=28748
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Marcel de Rooy 2021-10-07 07:09:44 UTC
(In reply to Tomás Cohen Arazi from comment #2)

> https://bugs.koha-community.org/show_bug.cgi?id=28748
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

What about 28748 ?
Comment 4 Marcel de Rooy 2021-10-07 07:12:32 UTC
Correct bug numbers on patches etc
Confusing
Comment 5 Nick Clemens 2021-10-07 13:23:45 UTC
Created attachment 125880 [details] [review]
Bug 28748: Prepare pickup locations for overridden holds

The logic in request.pl only passes pickup locations if a hold is allowed,
they are not calculated when the hold is overridden.

This patch copies the pickup locations code into the override conditional,
adjusting to mark the item overridden

This also highlights an condition of override:
If a hold is allowed, but there are no valid pickup locations the override
is not allowed.

To test:
 1 - Set a holds rule to allow on-shelf holds only if all unavailable
 2 - Set AllowHoldPolicyOverride to 'Allow'
 3 - Find a record with several items available
 4 - Attempt to place a hold
 5 - The items have a yellow triangle, but no pickup locations
 6 - Place an item level hold
 7 - Note the hold has no pickup location
 8 - Cancel the hold
 9 - Apply patch and restart all
10 - Attempt hold again
11 - Items still have yellow triangle, but there are dropdowns for pickup location
12 - Place an item level hold, verify a pickup location is set
13 - Cancel the hold
14 - Alter circ rule to allow on shelf holds
15 - Change 'Default checkout, hold and return policy' - 'Hold pickup library match' to "item's hold group"
16 - Ensure there is no group for the item
17 - Attempt to place hold
18 - Override is not allowed 'No valid pickup locations'

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Marcel de Rooy 2021-10-07 13:34:54 UTC
Created attachment 125881 [details] [review]
Bug 28748: Prepare pickup locations for overridden holds

The logic in request.pl only passes pickup locations if a hold is allowed,
they are not calculated when the hold is overridden.

This patch copies the pickup locations code into the override conditional,
adjusting to mark the item overridden

This also highlights an condition of override:
If a hold is allowed, but there are no valid pickup locations the override
is not allowed.

To test:
 1 - Set a holds rule to allow on-shelf holds only if all unavailable
 2 - Set AllowHoldPolicyOverride to 'Allow'
 3 - Find a record with several items available
 4 - Attempt to place a hold
 5 - The items have a yellow triangle, but no pickup locations
 6 - Place an item level hold
 7 - Note the hold has no pickup location
 8 - Cancel the hold
 9 - Apply patch and restart all
10 - Attempt hold again
11 - Items still have yellow triangle, but there are dropdowns for pickup location
12 - Place an item level hold, verify a pickup location is set
13 - Cancel the hold
14 - Alter circ rule to allow on shelf holds
15 - Change 'Default checkout, hold and return policy' - 'Hold pickup library match' to "item's hold group"
16 - Ensure there is no group for the item
17 - Attempt to place hold
18 - Override is not allowed 'No valid pickup locations'

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2021-10-08 15:00:53 UTC
+                            my $default_pickup_location = $pickup_locations->search({ branchcode => $item->{holdingbranch} })->next;

Cannot we use ->find here?
Comment 8 Jonathan Druart 2021-10-12 09:18:29 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 9 Kyle M Hall 2021-10-22 13:43:36 UTC
Pushed to 21.05.x for 21.05.05
Comment 10 Fridolin Somers 2021-10-23 01:35:26 UTC
Depends on Bug 28338 not in 20.11.x