Bug 29553

Summary: Holds: Can't call method "notforloan" on an undefined value when placing a hold
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Hold requestsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: minor    
Priority: P5 - low CC: andrewfh, gmcharlt, kyle, tomascohen, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29554
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.02.21.05.09
Attachments: Bug 29553: Fix crash on undefined notforloan value
Bug 29553: Fix crash on undefined notforloan value
Bug 29553: Fix crash on undefined notforloan value
Bug 29553: Regression tests
Bug 29553: (QA follow-up) Check defined instead of evaluating as boolean

Description Marcel de Rooy 2021-11-22 10:52:15 UTC
C4::Reserves::IsAvailableForItemLevelRequest
    my $itemtype = $item->effective_itemtype;
    my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan;

If you have set item level itypes to biblioitems, but your biblioitem record has itemtype NULL, placing a hold will crash. (You must have an item too.)
Comment 1 Marcel de Rooy 2021-11-22 10:55:58 UTC
Created attachment 127925 [details] [review]
Bug 29553: Fix crash on undefined notforloan value

Test plan:
Set item level itypes to biblioitems.
Find a record with itemtype NULL, having an item.
Place a hold. Without this patch, it crashes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Thibaud Guillot 2021-12-01 11:06:04 UTC
Created attachment 128122 [details] [review]
Bug 29553: Fix crash on undefined notforloan value

Test plan:
Set item level itypes to biblioitems.
Find a record with itemtype NULL, having an item.
Place a hold. Without this patch, it crashes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: ThibaudGLT <thibaud.guillot@biblibre.com>
Comment 3 Marcel de Rooy 2021-12-01 12:18:17 UTC
(In reply to Thibaud Guillot from comment #2)
> Created attachment 128122 [details] [review] [review]
> Bug 29553: Fix crash on undefined notforloan value
> 
> Test plan:
> Set item level itypes to biblioitems.
> Find a record with itemtype NULL, having an item.
> Place a hold. Without this patch, it crashes.
> 
> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> Signed-off-by: ThibaudGLT <thibaud.guillot@biblibre.com>

Thx Thibaud. I will adjust the patch status accordingly.
Comment 4 Tomás Cohen Arazi 2021-12-17 15:54:39 UTC
Created attachment 128681 [details] [review]
Bug 29553: Fix crash on undefined notforloan value

Test plan:
Set item level itypes to biblioitems.
Find a record with itemtype NULL, having an item.
Place a hold. Without this patch, it crashes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: ThibaudGLT <thibaud.guillot@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Tomás Cohen Arazi 2021-12-17 15:54:43 UTC
Created attachment 128682 [details] [review]
Bug 29553: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2021-12-17 15:54:49 UTC
Created attachment 128683 [details] [review]
Bug 29553: (QA follow-up) Check defined instead of evaluating as boolean

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Fridolin Somers 2022-01-10 07:04:59 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 8 Kyle M Hall 2022-01-14 13:55:33 UTC
Pushed to 21.11.x for 21.11.02
Comment 9 Andrew Fuerste-Henry 2022-01-21 17:42:09 UTC
Pushed to 21.05.x for 21.05.09
Comment 10 Victor Grousset/tuxayo 2022-02-07 18:54:07 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.