Bug 26367

Summary: Warn in HoldsQueue if request itemtype set but request is not item specific
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Hold requestsAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrewfh, caroline.cyr-la-rose, david, fridolin.somers, gmcharlt, kyle, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.02
Attachments: Bug 26367: Unit test
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype
Bug 26367: Unit test
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype
Bug 26367: Unit test
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype

Description Nick Clemens (kidclamp) 2020-09-03 12:11:44 UTC
To recreate:
0 - set AllowHoldItemTypeSelection to 'Allow'
1 - Place a title level hold for a record with an item from Library A for delivery at Library A for a patron from Library A with an itemtype specified
2 - perl misc/cronjobs/holds/build_holds_queue.pl
3 - There are warns:

Use of uninitialized value in hash element at /kohadevbox/koha/C4/HoldsQueue.pm line 523.
Use of uninitialized value in string eq at /kohadevbox/koha/C4/HoldsQueue.pm line 523.
Comment 1 Nick Clemens (kidclamp) 2020-09-03 12:19:32 UTC
Created attachment 109620 [details] [review]
Bug 26367: Unit test

This warn only appears when:
  The borrower and item branches match
  AND
  The hold fulfillment policy is any or the pickupbranch matches the items branch specified by hold_fulfillment_policy
  AND
  The request has an itemtype set

To trigger we simply add some itemtype to existing hold requests in the test

To test:
1 - prove -v t/db_dependent/HoldsQueue.t
Comment 2 Nick Clemens (kidclamp) 2020-09-03 12:19:36 UTC
Created attachment 109621 [details] [review]
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype

To test:
 1 - set AllowHoldItemTypeSelection to 'Allow'
 2 - Find a patron from Library A
 3 - Find a record with an item from Library A
 4 - Place a title level hold with itemtype specified for a delivery at Library A for patron and record above
 5 - perl misc/cronjobs/holds/build_holds_queue.pl
 6 - There are warns:
 Use of uninitialized value in hash element at /kohadevbox/koha/C4/HoldsQueue.pm line 523.
 Use of uninitialized value in string eq at /kohadevbox/koha/C4/HoldsQueue.pm line 523
 7 - Apply this patch
 8 - perl misc/cronjobs/holds/build_holds_queue.pl
 9 - No more warns!
10 - prove -v t/db_dependent/HoldsQueue.t
11 - All tests pass!
Comment 3 David Nind 2020-11-05 06:38:36 UTC
Created attachment 113031 [details] [review]
Bug 26367: Unit test

This warn only appears when:
  The borrower and item branches match
  AND
  The hold fulfillment policy is any or the pickupbranch matches the items branch specified by hold_fulfillment_policy
  AND
  The request has an itemtype set

To trigger we simply add some itemtype to existing hold requests in the test

To test:
1 - prove -v t/db_dependent/HoldsQueue.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2020-11-05 06:38:39 UTC
Created attachment 113032 [details] [review]
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype

To test:
 1 - set AllowHoldItemTypeSelection to 'Allow'
 2 - Find a patron from Library A
 3 - Find a record with an item from Library A
 4 - Place a title level hold with itemtype specified for a delivery at Library A for patron and record above
 5 - perl misc/cronjobs/holds/build_holds_queue.pl
 6 - There are warns:
 Use of uninitialized value in hash element at /kohadevbox/koha/C4/HoldsQueue.pm line 523.
 Use of uninitialized value in string eq at /kohadevbox/koha/C4/HoldsQueue.pm line 523
 7 - Apply this patch
 8 - perl misc/cronjobs/holds/build_holds_queue.pl
 9 - No more warns!
10 - prove -v t/db_dependent/HoldsQueue.t
11 - All tests pass!

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2020-11-05 06:57:48 UTC
Testing notes - to generate the warnings when build_holds_queue.pl is run (using koha-testing-docker):

1. In staff interface change the library to Midway.
2. Search the catalog for Xenophōntos Agēsilaos.
3. Click on 'Place hold'.
4. For the patron choose Henry Acevedo.
5. For hold details for "Request specific item type" select "Books", then Place hold.
Comment 6 Katrin Fischer 2021-01-04 22:30:17 UTC
Created attachment 114823 [details] [review]
Bug 26367: Unit test

This warn only appears when:
  The borrower and item branches match
  AND
  The hold fulfillment policy is any or the pickupbranch matches the items branch specified by hold_fulfillment_policy
  AND
  The request has an itemtype set

To trigger we simply add some itemtype to existing hold requests in the test

To test:
1 - prove -v t/db_dependent/HoldsQueue.t

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2021-01-04 22:30:20 UTC
Created attachment 114824 [details] [review]
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype

To test:
 1 - set AllowHoldItemTypeSelection to 'Allow'
 2 - Find a patron from Library A
 3 - Find a record with an item from Library A
 4 - Place a title level hold with itemtype specified for a delivery at Library A for patron and record above
 5 - perl misc/cronjobs/holds/build_holds_queue.pl
 6 - There are warns:
 Use of uninitialized value in hash element at /kohadevbox/koha/C4/HoldsQueue.pm line 523.
 Use of uninitialized value in string eq at /kohadevbox/koha/C4/HoldsQueue.pm line 523
 7 - Apply this patch
 8 - perl misc/cronjobs/holds/build_holds_queue.pl
 9 - No more warns!
10 - prove -v t/db_dependent/HoldsQueue.t
11 - All tests pass!

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Jonathan Druart 2021-01-07 14:37:46 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 9 Fridolin Somers 2021-01-08 06:49:50 UTC
Pushed to 20.11.x for 20.11.02
Comment 10 Andrew Fuerste-Henry 2021-01-14 17:03:58 UTC
Doesn't apply cleanly to 20.05.x, please rebase
Comment 11 Nick Clemens (kidclamp) 2021-01-15 12:09:24 UTC
(In reply to Andrew Fuerste-Henry from comment #10)
> Doesn't apply cleanly to 20.05.x, please rebase

The tests for this rely on 19889, it reduces warns but does not affect functionality, can be skipped
Comment 12 Didier Gautheron 2021-01-17 07:33:27 UTC
*** Bug 26387 has been marked as a duplicate of this bug. ***