Bug 30846 - "If any unavailable" doesn't consider negative notforloan values as unavailable
Summary: "If any unavailable" doesn't consider negative notforloan values as unavailable
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Hammat wele
QA Contact: Emily Lamancusa
URL:
Keywords: release-notes-needed
Depends on:
Blocks: 30845 34694 34836
  Show dependency treegraph
 
Reported: 2022-05-25 12:14 UTC by Nick Clemens
Modified: 2023-10-09 15:25 UTC (History)
10 users (show)

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


Attachments
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable (9.42 KB, patch)
2023-01-23 15:43 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable (9.47 KB, patch)
2023-01-25 15:41 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable (9.45 KB, patch)
2023-02-27 14:37 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 30846: (follow-up) Unit tests (1.11 KB, patch)
2023-02-27 14:37 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable (9.50 KB, patch)
2023-05-24 18:12 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 30846: (follow-up) Unit tests (1.15 KB, patch)
2023-05-24 18:12 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable (9.38 KB, patch)
2023-07-21 14:41 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 30846: (follow-up) Unit tests (1.15 KB, patch)
2023-07-21 14:41 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 30846: (Fix) Unit tests (1.36 KB, patch)
2023-08-14 15:20 UTC, Hammat wele
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-25 12:14:59 UTC

    
Comment 1 Caroline Cyr La Rose 2023-01-02 20:21:36 UTC
I'm not sure if this is what you meant Nick, but I came across this situation and thought it was maybe the same. Let me know if it's completely different and I'll create a new bug for this.

I'm trying to set up Koha so that only unavailable items can be put on hold, including those with negative not for loan values. (The exact use case is a library that wants to prevent on shelf holds, but want people to be able to put on hold the most recent issue of a serial even if it is for reference only while the other issue hasn't yet arrived. When the next issue arrives, the previous issue will be lent out, in order of the holds... Not sure if that makes sense, but it's the same principle as placing holds on "ordered" items.)

I was not able to test this on master because of bug 32445, but I tried it on 22.05.05

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
   1.1. Go to Administration > Circulation and fines rules
   1.2. In the matrix, add a circulation like this
        - Patron category: All
        - Item type: Books
        - Current checkouts allowed: 10
        - Current on-site checkouts allowed: 10
        - Loan period: 21
        - Holds allowed (total): 10
        - Holds allowed (daily): 10
        - Holds per record (count): 10
        - On shelf holds allowed: If any unavailable
        - OPAC item level holds: Force
   1.3. Click Save
2. Create a record with one "Ordered" item (or any negative value not for loan status)
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040$c, 245$a, and 942$c (942$c should be set to Books))
   2.4. Click Save
   2.5. Fill out the following item fields
        - Not for loan: Ordered
        - Koha item type: Books
   2.6. Click Add item
   2.7. Click Normal to go to the detailed record
3. Try to place a hold on the "Ordered" item
   3.1. From the detailed record, click OPAC view: Open in new window.
   --> Note that the "Place hold" option is not present
4. Add a second "Available" item
   4.1. Back in the staff interface tab with the detailed record, click New > New item
   4.2. Make sure the item type is set to Books
   4.3. Add a barcode in p
   4.4. Click Add item
5. Try again to place a hold on the "Ordered" item
   5.1. Go back to the OPAC tab and refresh the page
   --> Note that the "Place hold" option is still not present
6. Check out the available item to a patron
   6.1. In the staff interface tab, copy the barcode from the available item
   6.2. Go to Patrons
   6.3. Click on Search
   6.4. Click Check out next to one of the patrons
   6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the "Ordered" item
   7.1. Go back to the OPAC tab and refresh the page
   --> Note that the "Place hold" option is now present
   7.2. Click Place hold
   --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
Comment 2 Caroline Cyr La Rose 2023-01-11 19:47:30 UTC
Now that bug 32445 is fixed, I tried in master and it still doesn't allow holds when an item has a negative not for loan value.
Comment 3 Caroline Cyr La Rose 2023-01-19 20:48:18 UTC
Nick said he ceded this bug and will create an new one for the CanBookBeReserved/CanItemBeReserved thing :)
Comment 4 Hammat wele 2023-01-23 15:43:12 UTC
Created attachment 145586 [details] [review]
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable

When we set up a circulation rule where 'On shelf holds allowed' is 'If any unavailable' and we have a record with one 'Ordered' item, we cannot place this item on hold.

This patch allows placing hold on item with negative not for loan values, when using rule with 'On shelf holds allowed' set to 'If any unavailable'

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
    1.1. Go to Administration > Circulation and fines rules
    1.2. In the matrix, add a circulation like this
          - Patron category: All
          - Item type: Books
          - Current checkouts allowed: 10
          - Current on-site checkouts allowed: 10
          - Loan period: 21
          - Holds allowed (total): 10
          - Holds allowed (daily): 10
          - Holds per record (count): 10
          - On shelf holds allowed: If any unavailable
          - OPAC item level holds: Force
    1.3. Click Save
2. Create a record with one 'Ordered' item (or any negative value not for loan status)
    2.1. Go to Cataloging
    2.2. Click New record
    2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040, 245, and 942 (942 should be set to Books))
    2.4. Click Save
    2.5. Fill out the following item fields
          - Not for loan: Ordered
          - Koha item type: Books
    2.6. Click Add item
    2.7. Click Normal to go to the detailed record
3. Try to place a hold on the 'Ordered' item
    3.1. From the detailed record, click OPAC view: Open in new window.
    --> Note that the 'Place hold' option is not present
4. Add a second 'Available' item
    4.1. Back in the staff interface tab with the detailed record, click New > New item
    4.2. Make sure the item type is set to Books
    4.3. Add a barcode in p
    4.4. Click Add item
5. Try again to place a hold on the 'Ordered' item
    5.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is still not present
6. Check out the available item to a patron
    6.1. In the staff interface tab, copy the barcode from the available item
    6.2. Go to Patrons
    6.3. Click on Search
    6.4. Click Check out next to one of the patrons
    6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the 'Ordered' item
    7.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is now present
    7.2. Click Place hold
    --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
8. Apply the patch
9. Go to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is still present
        9.1. Click Place hold
        --> Note that you can now place a hold on the 'Checked out' or the 'Ordered' item.
10. Check in the item to make it available again
    10.1. In the staff interface tab, click on 'Show checkouts' button
    10.2. Select the Checked out item and click on 'Renew or check in selected items' button.
11. Try again to place a hold on the 'Ordered' item
    11.1. Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
    --> Note that the 'Place hold' option is still present
    11.2. Click Place hold
    --> Note that only the 'Ordered' item is available to place on hold, if you click Show unholdable items, it will show the Available item and you can't place a hold on it.
12. Delete the available item to keep only the Ordered item
    12.1 in the staff interface tab, click on 'Search catalog' and search for the record
    12.2 click on 'Edit' then 'Edit items'
    12.3 Delete the available item
13. Try to place a hold on the remain 'Ordered' item
    13.1 Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is present
    13.2. Click Place hold
    --> Note that you can place a hold on the Ordered item.
Comment 5 Biblibre Sandboxes 2023-01-25 15:41:44 UTC
Created attachment 145660 [details] [review]
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable

When we set up a circulation rule where 'On shelf holds allowed' is 'If any unavailable' and we have a record with one 'Ordered' item, we cannot place this item on hold.

This patch allows placing hold on item with negative not for loan values, when using rule with 'On shelf holds allowed' set to 'If any unavailable'

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
    1.1. Go to Administration > Circulation and fines rules
    1.2. In the matrix, add a circulation like this
          - Patron category: All
          - Item type: Books
          - Current checkouts allowed: 10
          - Current on-site checkouts allowed: 10
          - Loan period: 21
          - Holds allowed (total): 10
          - Holds allowed (daily): 10
          - Holds per record (count): 10
          - On shelf holds allowed: If any unavailable
          - OPAC item level holds: Force
    1.3. Click Save
2. Create a record with one 'Ordered' item (or any negative value not for loan status)
    2.1. Go to Cataloging
    2.2. Click New record
    2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040, 245, and 942 (942 should be set to Books))
    2.4. Click Save
    2.5. Fill out the following item fields
          - Not for loan: Ordered
          - Koha item type: Books
    2.6. Click Add item
    2.7. Click Normal to go to the detailed record
3. Try to place a hold on the 'Ordered' item
    3.1. From the detailed record, click OPAC view: Open in new window.
    --> Note that the 'Place hold' option is not present
4. Add a second 'Available' item
    4.1. Back in the staff interface tab with the detailed record, click New > New item
    4.2. Make sure the item type is set to Books
    4.3. Add a barcode in p
    4.4. Click Add item
5. Try again to place a hold on the 'Ordered' item
    5.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is still not present
6. Check out the available item to a patron
    6.1. In the staff interface tab, copy the barcode from the available item
    6.2. Go to Patrons
    6.3. Click on Search
    6.4. Click Check out next to one of the patrons
    6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the 'Ordered' item
    7.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is now present
    7.2. Click Place hold
    --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
8. Apply the patch
9. Go to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is still present
        9.1. Click Place hold
        --> Note that you can now place a hold on the 'Checked out' or the 'Ordered' item.
10. Check in the item to make it available again
    10.1. In the staff interface tab, click on 'Show checkouts' button
    10.2. Select the Checked out item and click on 'Renew or check in selected items' button.
11. Try again to place a hold on the 'Ordered' item
    11.1. Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
    --> Note that the 'Place hold' option is still present
    11.2. Click Place hold
    --> Note that only the 'Ordered' item is available to place on hold, if you click Show unholdable items, it will show the Available item and you can't place a hold on it.
12. Delete the available item to keep only the Ordered item
    12.1 in the staff interface tab, click on 'Search catalog' and search for the record
    12.2 click on 'Edit' then 'Edit items'
    12.3 Delete the available item
13. Try to place a hold on the remain 'Ordered' item
    13.1 Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is present
    13.2. Click Place hold
    --> Note that you can place a hold on the Ordered item.

Signed-off-by: Amaury GAU <amaury.gau@bulac.fr>
Comment 6 Jonathan Druart 2023-02-23 09:12:11 UTC
Please provide tests for the changes in IsAvailableForItemLevelRequest.
Comment 7 Hammat wele 2023-02-27 14:37:38 UTC
Created attachment 147443 [details] [review]
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable

When we set up a circulation rule where 'On shelf holds allowed' is 'If any unavailable' and we have a record with one 'Ordered' item, we cannot place this item on hold.

This patch allows placing hold on item with negative not for loan values, when using rule with 'On shelf holds allowed' set to 'If any unavailable'

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
    1.1. Go to Administration > Circulation and fines rules
    1.2. In the matrix, add a circulation like this
          - Patron category: All
          - Item type: Books
          - Current checkouts allowed: 10
          - Current on-site checkouts allowed: 10
          - Loan period: 21
          - Holds allowed (total): 10
          - Holds allowed (daily): 10
          - Holds per record (count): 10
          - On shelf holds allowed: If any unavailable
          - OPAC item level holds: Force
    1.3. Click Save
2. Create a record with one 'Ordered' item (or any negative value not for loan status)
    2.1. Go to Cataloging
    2.2. Click New record
    2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040, 245, and 942 (942 should be set to Books))
    2.4. Click Save
    2.5. Fill out the following item fields
          - Not for loan: Ordered
          - Koha item type: Books
    2.6. Click Add item
    2.7. Click Normal to go to the detailed record
3. Try to place a hold on the 'Ordered' item
    3.1. From the detailed record, click OPAC view: Open in new window.
    --> Note that the 'Place hold' option is not present
4. Add a second 'Available' item
    4.1. Back in the staff interface tab with the detailed record, click New > New item
    4.2. Make sure the item type is set to Books
    4.3. Add a barcode in p
    4.4. Click Add item
5. Try again to place a hold on the 'Ordered' item
    5.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is still not present
6. Check out the available item to a patron
    6.1. In the staff interface tab, copy the barcode from the available item
    6.2. Go to Patrons
    6.3. Click on Search
    6.4. Click Check out next to one of the patrons
    6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the 'Ordered' item
    7.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is now present
    7.2. Click Place hold
    --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
8. Apply the patch
9. Go to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is still present
        9.1. Click Place hold
        --> Note that you can now place a hold on the 'Checked out' or the 'Ordered' item.
10. Check in the item to make it available again
    10.1. In the staff interface tab, click on 'Show checkouts' button
    10.2. Select the Checked out item and click on 'Renew or check in selected items' button.
11. Try again to place a hold on the 'Ordered' item
    11.1. Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
    --> Note that the 'Place hold' option is still present
    11.2. Click Place hold
    --> Note that only the 'Ordered' item is available to place on hold, if you click Show unholdable items, it will show the Available item and you can't place a hold on it.
12. Delete the available item to keep only the Ordered item
    12.1 in the staff interface tab, click on 'Search catalog' and search for the record
    12.2 click on 'Edit' then 'Edit items'
    12.3 Delete the available item
13. Try to place a hold on the remain 'Ordered' item
    13.1 Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is present
    13.2. Click Place hold
    --> Note that you can place a hold on the Ordered item.

Signed-off-by: Amaury GAU <amaury.gau@bulac.fr>
Comment 8 Hammat wele 2023-02-27 14:37:41 UTC
Created attachment 147444 [details] [review]
Bug 30846: (follow-up) Unit tests
Comment 9 Sam Lau 2023-05-24 18:12:10 UTC
Created attachment 151648 [details] [review]
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable

When we set up a circulation rule where 'On shelf holds allowed' is 'If any unavailable' and we have a record with one 'Ordered' item, we cannot place this item on hold.

This patch allows placing hold on item with negative not for loan values, when using rule with 'On shelf holds allowed' set to 'If any unavailable'

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
    1.1. Go to Administration > Circulation and fines rules
    1.2. In the matrix, add a circulation like this
          - Patron category: All
          - Item type: Books
          - Current checkouts allowed: 10
          - Current on-site checkouts allowed: 10
          - Loan period: 21
          - Holds allowed (total): 10
          - Holds allowed (daily): 10
          - Holds per record (count): 10
          - On shelf holds allowed: If any unavailable
          - OPAC item level holds: Force
    1.3. Click Save
2. Create a record with one 'Ordered' item (or any negative value not for loan status)
    2.1. Go to Cataloging
    2.2. Click New record
    2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040, 245, and 942 (942 should be set to Books))
    2.4. Click Save
    2.5. Fill out the following item fields
          - Not for loan: Ordered
          - Koha item type: Books
    2.6. Click Add item
    2.7. Click Normal to go to the detailed record
3. Try to place a hold on the 'Ordered' item
    3.1. From the detailed record, click OPAC view: Open in new window.
    --> Note that the 'Place hold' option is not present
4. Add a second 'Available' item
    4.1. Back in the staff interface tab with the detailed record, click New > New item
    4.2. Make sure the item type is set to Books
    4.3. Add a barcode in p
    4.4. Click Add item
5. Try again to place a hold on the 'Ordered' item
    5.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is still not present
6. Check out the available item to a patron
    6.1. In the staff interface tab, copy the barcode from the available item
    6.2. Go to Patrons
    6.3. Click on Search
    6.4. Click Check out next to one of the patrons
    6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the 'Ordered' item
    7.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is now present
    7.2. Click Place hold
    --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
8. Apply the patch
9. Go to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is still present
        9.1. Click Place hold
        --> Note that you can now place a hold on the 'Checked out' or the 'Ordered' item.
10. Check in the item to make it available again
    10.1. In the staff interface tab, click on 'Show checkouts' button
    10.2. Select the Checked out item and click on 'Renew or check in selected items' button.
11. Try again to place a hold on the 'Ordered' item
    11.1. Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
    --> Note that the 'Place hold' option is still present
    11.2. Click Place hold
    --> Note that only the 'Ordered' item is available to place on hold, if you click Show unholdable items, it will show the Available item and you can't place a hold on it.
12. Delete the available item to keep only the Ordered item
    12.1 in the staff interface tab, click on 'Search catalog' and search for the record
    12.2 click on 'Edit' then 'Edit items'
    12.3 Delete the available item
13. Try to place a hold on the remain 'Ordered' item
    13.1 Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is present
    13.2. Click Place hold
    --> Note that you can place a hold on the Ordered item.

Signed-off-by: Amaury GAU <amaury.gau@bulac.fr>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 10 Sam Lau 2023-05-24 18:12:13 UTC
Created attachment 151649 [details] [review]
Bug 30846: (follow-up) Unit tests

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 11 Sam Lau 2023-05-24 18:13:26 UTC
The follow up unit test failed, however, it also failed before the patch was even applied. It left this message:

 Failed test 'IsAvailableForItemLevelRequest() tests'
#   at t/db_dependent/Reserves.t line 1493.
t/db_dependent/Reserves.t .. 74/77 # Looks like you failed 1 test of 77.
t/db_dependent/Reserves.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/77 subtests

Test Summary Report
-------------------
t/db_dependent/Reserves.t (Wstat: 256 Tests: 77 Failed: 1)
  Failed test:  72
  Non-zero exit status: 1
Files=1, Tests=77,  6 wallclock secs ( 0.02 usr  0.01 sys +  3.85 cusr  0.94 csys =  4.82 CPU)
Result: FAIL
Comment 12 Hammat wele 2023-05-29 21:40:58 UTC
(In reply to Sam Lau from comment #11)
> The follow up unit test failed, however, it also failed before the patch was
> even applied. It left this message:
> 
>  Failed test 'IsAvailableForItemLevelRequest() tests'
> #   at t/db_dependent/Reserves.t line 1493.
> t/db_dependent/Reserves.t .. 74/77 # Looks like you failed 1 test of 77.
> t/db_dependent/Reserves.t .. Dubious, test returned 1 (wstat 256, 0x100)
> Failed 1/77 subtests
> 
> Test Summary Report
> -------------------
> t/db_dependent/Reserves.t (Wstat: 256 Tests: 77 Failed: 1)
>   Failed test:  72
>   Non-zero exit status: 1
> Files=1, Tests=77,  6 wallclock secs ( 0.02 usr  0.01 sys +  3.85 cusr  0.94
> csys =  4.82 CPU)
> Result: FAIL

Hi Sam, I'm sorry, I'm not able to reproduce the failed test. it may have something to do with your configuration? if it also failed before applying the patch it's probably not related to this patch
Comment 13 Katrin Fischer 2023-06-10 19:35:13 UTC
Tests pass for me, I will continue testing.
Comment 14 Hammat wele 2023-07-21 14:41:50 UTC
Created attachment 153797 [details] [review]
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable

When we set up a circulation rule where 'On shelf holds allowed' is 'If any unavailable' and we have a record with one 'Ordered' item, we cannot place this item on hold.

This patch allows placing hold on item with negative not for loan values, when using rule with 'On shelf holds allowed' set to 'If any unavailable'

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
    1.1. Go to Administration > Circulation and fines rules
    1.2. In the matrix, add a circulation like this
          - Patron category: All
          - Item type: Books
          - Current checkouts allowed: 10
          - Current on-site checkouts allowed: 10
          - Loan period: 21
          - Holds allowed (total): 10
          - Holds allowed (daily): 10
          - Holds per record (count): 10
          - On shelf holds allowed: If any unavailable
          - OPAC item level holds: Force
    1.3. Click Save
2. Create a record with one 'Ordered' item (or any negative value not for loan status)
    2.1. Go to Cataloging
    2.2. Click New record
    2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040, 245, and 942 (942 should be set to Books))
    2.4. Click Save
    2.5. Fill out the following item fields
          - Not for loan: Ordered
          - Koha item type: Books
    2.6. Click Add item
    2.7. Click Normal to go to the detailed record
3. Try to place a hold on the 'Ordered' item
    3.1. From the detailed record, click OPAC view: Open in new window.
    --> Note that the 'Place hold' option is not present
4. Add a second 'Available' item
    4.1. Back in the staff interface tab with the detailed record, click New > New item
    4.2. Make sure the item type is set to Books
    4.3. Add a barcode in p
    4.4. Click Add item
5. Try again to place a hold on the 'Ordered' item
    5.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is still not present
6. Check out the available item to a patron
    6.1. In the staff interface tab, copy the barcode from the available item
    6.2. Go to Patrons
    6.3. Click on Search
    6.4. Click Check out next to one of the patrons
    6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the 'Ordered' item
    7.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is now present
    7.2. Click Place hold
    --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
8. Apply the patch
9. Go to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is still present
        9.1. Click Place hold
        --> Note that you can now place a hold on the 'Checked out' or the 'Ordered' item.
10. Check in the item to make it available again
    10.1. In the staff interface tab, click on 'Show checkouts' button
    10.2. Select the Checked out item and click on 'Renew or check in selected items' button.
11. Try again to place a hold on the 'Ordered' item
    11.1. Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
    --> Note that the 'Place hold' option is still present
    11.2. Click Place hold
    --> Note that only the 'Ordered' item is available to place on hold, if you click Show unholdable items, it will show the Available item and you can't place a hold on it.
12. Delete the available item to keep only the Ordered item
    12.1 in the staff interface tab, click on 'Search catalog' and search for the record
    12.2 click on 'Edit' then 'Edit items'
    12.3 Delete the available item
13. Try to place a hold on the remain 'Ordered' item
    13.1 Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is present
    13.2. Click Place hold
    --> Note that you can place a hold on the Ordered item.

Signed-off-by: Amaury GAU <amaury.gau@bulac.fr>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 15 Hammat wele 2023-07-21 14:41:53 UTC
Created attachment 153798 [details] [review]
Bug 30846: (follow-up) Unit tests

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 16 Hammat wele 2023-07-21 14:42:41 UTC
Rebased
Comment 17 Emily Lamancusa 2023-08-09 15:13:39 UTC
The patch itself looks good and passes the qa script, but the unit doesn't quite test what it's supposed to. Please make the following adjustments to the test:

 - This bug addresses the use case where an item has a negative notforloan value 
   *and* the circulation rule for on-shelf holds is set to "if any unavailable",
   so that circ rule should be set before running the test. As written, the test
   passes even without the patch because it's using the default setting of "yes".
   Please add code to set the circ rule to "if any unavailable" for the test
   (if needed, see t/db_dependent/Koha/CirculationRules.t for example)

- Please change the failure message to describe what's being tested

- Don't forget to perltidy the test file :)
Comment 18 Hammat wele 2023-08-14 15:20:23 UTC
Created attachment 154404 [details] [review]
Bug 30846: (Fix) Unit tests
Comment 19 Emily Lamancusa 2023-08-14 16:07:04 UTC
Thanks for the follow-up, Hammat! Code and tests look good, and qa tool is happy. Passing QA.
Comment 20 Tomás Cohen Arazi 2023-08-15 12:19:59 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 21 Fridolin Somers 2023-08-17 20:31:58 UTC
Pushed to 23.05.x for 23.05.03
Comment 22 Fridolin Somers 2023-08-17 20:32:16 UTC
This behavior change needs a good release note
Comment 23 Pedro Amorim 2023-08-28 10:22:47 UTC
Nice work everyone!

Pushed to 22.11.x for next release
Comment 24 Jonathan Druart 2023-09-26 09:38:20 UTC
Hammat, do you agree with the solution suggested on bug 34886?
Comment 25 Hammat wele 2023-09-26 16:00:51 UTC
(In reply to Jonathan Druart from comment #24)
> Hammat, do you agree with the solution suggested on bug 34886?

Yes, I agree with this solution, the patch looks good and shows the Place Hold button without being logged in to start the hold process.
Comment 26 Fridolin Somers 2023-10-02 22:50:39 UTC
In my opinion, we should revert from oldstable in order for stability and to avoid behavior changes.

Note that Bug 34694 is not in oldstabe.
Comment 27 Katrin Fischer 2023-10-09 15:25:45 UTC
(In reply to Hammat wele from comment #25)
> (In reply to Jonathan Druart from comment #24)
> > Hammat, do you agree with the solution suggested on bug 34886?
> 
> Yes, I agree with this solution, the patch looks good and shows the Place
> Hold button without being logged in to start the hold process.

This is not what we are seeing right now (now requires login) - I am trying to work through the comments, but was this changed?