Bug 26367 - Warn in HoldsQueue if request itemtype set but request is not item specific
Summary: Warn in HoldsQueue if request itemtype set but request is not item specific
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
: 26387 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-09-03 12:11 UTC by Nick Clemens
Modified: 2021-12-13 21:09 UTC (History)
7 users (show)

See Also:
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 (2.02 KB, patch)
2020-09-03 12:19 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype (1.84 KB, patch)
2020-09-03 12:19 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26367: Unit test (2.07 KB, patch)
2020-11-05 06:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype (1.89 KB, patch)
2020-11-05 06:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 26367: Unit test (2.12 KB, patch)
2021-01-04 22:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26367: Prevent warn about undefined values when record level hold has an itemtype (1.95 KB, patch)
2021-01-04 22:30 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 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 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 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 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. ***