Bug 30742 - Confusion when placing hold on record with no items available because of not for loan
Summary: Confusion when placing hold on record with no items available because of not ...
Status: CLOSED FIXED
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: Joonas Kylmälä
URL:
Keywords:
Depends on:
Blocks: 30892
  Show dependency treegraph
 
Reported: 2022-05-12 16:05 UTC by Nick Clemens
Modified: 2023-06-08 22:26 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.01, 21.11.07


Attachments
Bug 30742: Prevent placing holds on items/records where all items notforloan (11.76 KB, patch)
2022-05-13 15:27 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs (6.29 KB, patch)
2022-05-13 15:27 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30742: Prevent placing holds on items/records where all items notforloan (11.80 KB, patch)
2022-05-14 23:07 UTC, David Nind
Details | Diff | Splinter Review
Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs (6.33 KB, patch)
2022-05-14 23:07 UTC, David Nind
Details | Diff | Splinter Review
Bug 30742: Prevent placing holds on items/records where all items notforloan (11.95 KB, patch)
2022-05-21 18:39 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs (6.48 KB, patch)
2022-05-21 18:40 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 30742: [21.11.x] Prevent placing holds on items/records where all items notforloan Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs Bug 30892: (bug 30742 follow-up) Send single bib as a holdable bib (16.45 KB, patch)
2022-06-09 15:30 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 2022-05-12 16:05:10 UTC
In default ktd data record 272 has 3 items, all of an itemtype that is not for loan.

If you go to this record and place a hold for an individual all items have an 'X' but no explanation

The higher 'Place hold' button is disabled

Beneath the items table, however, there is a 'Place holds' button that is enabled and allows a title level hold.

If you select this record and others from search results (search for "a") there is a message:
 Cannot place hold on some items
And there is no pickup location, however, clicking 'PLace holds' after setting locations for other books does indeed place a hold on this record, with no pickup location, and the record appears twice on the page after holds are placed
Comment 1 David Cook 2022-05-13 00:29:51 UTC
Fun times 😬
Comment 2 Nick Clemens 2022-05-13 15:27:15 UTC
Created attachment 134982 [details] [review]
Bug 30742: Prevent placing holds on items/records where all items notforloan

This patch does a few things:
1 - Adds itemtype not for loan status to display
2 - Adds a conditional to display notforlaon status as the reason a hold cannot be placed
3 - Seperates the lower 'Place hold(s)' buttons for single and multi holds into two template sections
4 - Handles the case where all bibs in a multi hold have no items available
5 - Disables the button for single hlds when all items are unavailable

To test:
 1 - Find or create a record with all items of itemtype marked 'notforloan'
 2 - Attempt to place single hold on this record from staff client
 3 - See one disab;ed button, one enabled 'Place holds' button
 4 - Click 'Place holds' - hold placed
 5 - Cancel hold
 6 - Place multiple holds with some bibs that can be held, and this one that cannot
 7 - Notice message that 'Cannot place hold on some items'
 8 - Click 'Place holds' - hold is generated for the notforloan bib
 9 - Apply patch
10 - Place single hold
11 - Note you now see not for loan status on items
12 - Note the red x also includes message abnout not for loan status
13 - Note the 'Place hold' button is disabled
14 - Attempt multi hold
15 - Message now includes "No items available: One or more records have no items that can be held"
16 - Click 'Place holds'
17 - Above still places the hold - this is for a followup patch
Comment 3 Nick Clemens 2022-05-13 15:27:19 UTC
Created attachment 134983 [details] [review]
Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs

Currently place request gets a list of bad_bibs that is created via javascript on the
template. It ignores this list

Ths patch instead doesn't add info for bad bibs, and provides a list of the bibs that
can be held

To test:
1 - Attempt multi hold with some items that can be held, and one that cannot due to notforloan
2 - Fill in pickup locations and place hold
3 - Note hold is place on bib with no avilable items and hsows twice in results
4 - Apply patch
5 - repeat with another patron
6 - Note no aidditonal hold on record with notforloan items
7 - Note with with not for loan items appears only once in results
Comment 4 David Nind 2022-05-14 23:07:35 UTC
Created attachment 135002 [details] [review]
Bug 30742: Prevent placing holds on items/records where all items notforloan

This patch does a few things:
1 - Adds itemtype not for loan status to display
2 - Adds a conditional to display notforlaon status as the reason a hold cannot be placed
3 - Seperates the lower 'Place hold(s)' buttons for single and multi holds into two template sections
4 - Handles the case where all bibs in a multi hold have no items available
5 - Disables the button for single hlds when all items are unavailable

To test:
 1 - Find or create a record with all items of itemtype marked 'notforloan'
 2 - Attempt to place single hold on this record from staff client
 3 - See one disab;ed button, one enabled 'Place holds' button
 4 - Click 'Place holds' - hold placed
 5 - Cancel hold
 6 - Place multiple holds with some bibs that can be held, and this one that cannot
 7 - Notice message that 'Cannot place hold on some items'
 8 - Click 'Place holds' - hold is generated for the notforloan bib
 9 - Apply patch
10 - Place single hold
11 - Note you now see not for loan status on items
12 - Note the red x also includes message abnout not for loan status
13 - Note the 'Place hold' button is disabled
14 - Attempt multi hold
15 - Message now includes "No items available: One or more records have no items that can be held"
16 - Click 'Place holds'
17 - Above still places the hold - this is for a followup patch

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2022-05-14 23:07:41 UTC
Created attachment 135003 [details] [review]
Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs

Currently place request gets a list of bad_bibs that is created via javascript on the
template. It ignores this list

Ths patch instead doesn't add info for bad bibs, and provides a list of the bibs that
can be held

To test:
1 - Attempt multi hold with some items that can be held, and one that cannot due to notforloan
2 - Fill in pickup locations and place hold
3 - Note hold is place on bib with no avilable items and hsows twice in results
4 - Apply patch
5 - repeat with another patron
6 - Note no aidditonal hold on record with notforloan items
7 - Note with with not for loan items appears only once in results

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Tomás Cohen Arazi 2022-05-16 18:01:49 UTC
Such bad_bibs

\o/
Comment 7 Joonas Kylmälä 2022-05-21 18:39:58 UTC
Created attachment 135255 [details] [review]
Bug 30742: Prevent placing holds on items/records where all items notforloan

This patch does a few things:
1 - Adds itemtype not for loan status to display
2 - Adds a conditional to display notforlaon status as the reason a hold cannot be placed
3 - Seperates the lower 'Place hold(s)' buttons for single and multi holds into two template sections
4 - Handles the case where all bibs in a multi hold have no items available
5 - Disables the button for single hlds when all items are unavailable

To test:
 1 - Find or create a record with all items of itemtype marked 'notforloan'
 2 - Attempt to place single hold on this record from staff client
 3 - See one disab;ed button, one enabled 'Place holds' button
 4 - Click 'Place holds' - hold placed
 5 - Cancel hold
 6 - Place multiple holds with some bibs that can be held, and this one that cannot
 7 - Notice message that 'Cannot place hold on some items'
 8 - Click 'Place holds' - hold is generated for the notforloan bib
 9 - Apply patch
10 - Place single hold
11 - Note you now see not for loan status on items
12 - Note the red x also includes message abnout not for loan status
13 - Note the 'Place hold' button is disabled
14 - Attempt multi hold
15 - Message now includes "No items available: One or more records have no items that can be held"
16 - Click 'Place holds'
17 - Above still places the hold - this is for a followup patch

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 8 Joonas Kylmälä 2022-05-21 18:40:02 UTC
Created attachment 135256 [details] [review]
Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs

Currently place request gets a list of bad_bibs that is created via javascript on the
template. It ignores this list

Ths patch instead doesn't add info for bad bibs, and provides a list of the bibs that
can be held

To test:
1 - Attempt multi hold with some items that can be held, and one that cannot due to notforloan
2 - Fill in pickup locations and place hold
3 - Note hold is place on bib with no avilable items and hsows twice in results
4 - Apply patch
5 - repeat with another patron
6 - Note no aidditonal hold on record with notforloan items
7 - Note with with not for loan items appears only once in results

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 9 Joonas Kylmälä 2022-05-21 19:16:20 UTC
This fixes UI side of things, not for just notforloan as advertised but other checks that happen in IsAvailableForItemLevelRequest() as well. This puts things in the right direction so passing QA on this. Further work would be to prevent placing the hold actually (placing the hold happens in placerequest.pl which this doesn't add any of the checks to).
Comment 10 Tomás Cohen Arazi 2022-06-01 16:34:23 UTC
Pushed for 22.11. Thanks everyone!
Comment 11 Lucas Gass 2022-06-06 15:17:59 UTC
Pushed to 22.05.x for 22.05.01
Comment 12 Arthur Suzuki 2022-06-07 13:21:33 UTC
conflicts while trying to apply on 21.11.x...
Comment 13 Nick Clemens 2022-06-09 15:30:18 UTC
Created attachment 135906 [details] [review]
Bug 30742: [21.11.x] Prevent placing holds on items/records where all items notforloan Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs Bug 30892: (bug 30742 follow-up) Send single bib as a holdable bib

Rebeased/rewritten patch - includes fix from 30892

This patch does a few things:
1 - Adds itemtype not for loan status to display
2 - Adds a conditional to display notforlaon status as the reason a hold cannot be placed
3 - Seperates the lower 'Place hold(s)' buttons for single and multi holds into two template sections
4 - Handles the case where all bibs in a multi hold have no items available
5 - Disables the button for single hlds when all items are unavailable

To test:
 1 - Find or create a record with all items of itemtype marked 'notforloan'
 2 - Attempt to place single hold on this record from staff client
 3 - See one disab;ed button, one enabled 'Place holds' button
 4 - Click 'Place holds' - hold placed
 5 - Cancel hold
 6 - Place multiple holds with some bibs that can be held, and this one that cannot
 7 - Notice message that 'Cannot place hold on some items'
 8 - Click 'Place holds' - hold is generated for the notforloan bib
 9 - Apply patch
10 - Place single hold
11 - Note you now see not for loan status on items
12 - Note the red x also includes message abnout not for loan status
13 - Note the 'Place hold' button is disabled
14 - Attempt multi hold
15 - Message now includes "No items available: One or more records have no items that can be held"
16 - Click 'Place holds'
17 - Above still places the hold - this is for a followup patch

Currently place request gets a list of bad_bibs that is created via javascript on the
template. It ignores this list

Ths patch instead doesn't add info for bad bibs, and provides a list of the bibs that
can be held

To test:
1 - Attempt multi hold with some items that can be held, and one that cannot due to notforloan
2 - Fill in pickup locations and place hold
3 - Note hold is place on bib with no avilable items and hsows twice in results
4 - Apply patch
5 - repeat with another patron
6 - Note no aidditonal hold on record with notforloan items
7 - Note with with not for loan items appears only once in results

Bug 30892
To test:
1. Try placing hold
2. Everything seems to work but no hold gets placed.
3. Apply patch
4. Verify holds are no placed correctly.
Comment 14 Arthur Suzuki 2022-06-20 09:17:13 UTC
pushed to 21.11.x for 21.11.07
Comment 15 Victor Grousset/tuxayo 2022-06-25 23:44:25 UTC
Not backported for now to 21.05.x because bug 30892 will be needed.