Bug 24331 - Internal server error when placing hold on item if itemtype undefined
Summary: Internal server error when placing hold on item if itemtype undefined
Status: RESOLVED DUPLICATE of bug 25449
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-02 16:05 UTC by Nick Clemens
Modified: 2020-05-20 15:10 UTC (History)
6 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:


Attachments
Bug 24331: Don't die on items with no type (2.68 KB, patch)
2020-01-03 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24331: Don't die on items with no type (2.75 KB, patch)
2020-01-18 23:53 UTC, Myka Kennedy Stephens
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-01-02 16:05:28 UTC
Error is:
Can't call method "notforloan" on an undefined value at /usr/share/koha/lib/C4/Reserves.pm line 1199.


C4::Reserves::IsAvailableForItemLevelRequest:

1198     my $itemtype = $item->effective_itemtype;
1199     my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan;

We should check that we found an itemtype before calling notforloan
Comment 1 Nick Clemens 2020-01-03 13:30:49 UTC
Created attachment 96793 [details] [review]
Bug 24331: Don't die on items with no type

To test:
1 - Find or create an item with no itemtype set
2 - Attempt to place a hold
3 - INternal server error
4 - Apply patch
5 - Repeat
6 - No error
7 - Set the items itemtype to an itemtype marked not for loan
8 - Set circ rules to allow holds on this itemtype
9 - Attempt to place hold, item is not holdable
10 - Remove not for loan from itemtype
11 - Item is holdable
Comment 2 Myka Kennedy Stephens 2020-01-18 23:53:10 UTC
Created attachment 97584 [details] [review]
Bug 24331: Don't die on items with no type

To test:
1 - Find or create an item with no itemtype set
2 - Attempt to place a hold
3 - INternal server error
4 - Apply patch
5 - Repeat
6 - No error
7 - Set the items itemtype to an itemtype marked not for loan
8 - Set circ rules to allow holds on this itemtype
9 - Attempt to place hold, item is not holdable
10 - Remove not for loan from itemtype
11 - Item is holdable

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 3 Myka Kennedy Stephens 2020-01-18 23:53:45 UTC
Patch works as described in test plan. Thank you!
Comment 4 Jonathan Druart 2020-01-21 15:24:31 UTC
Nick, did not we (more or less) agree that we should not test if items had valid item type?
See bug 21842.
Comment 5 Marcel de Rooy 2020-01-24 06:52:37 UTC
(In reply to Jonathan Druart from comment #4)
> Nick, did not we (more or less) agree that we should not test if items had
> valid item type?
> See bug 21842.

Although I still intuitively feel that such tests are appropriate, I also had the impression that we agreed not to add such tests everywhere.
If this still needs discussion, please change status accordingly. It would be wise to have a specific coding guideline for this, Jonathan.
Comment 6 Katrin Fischer 2020-05-10 10:20:11 UTC
I've added an item about this to the next dev meeting agenda:
https://wiki.koha-community.org/wiki/Development_IRC_meeting_20_May_2020#Agenda

Please feel free to add any arguments to it! Goal would be to move towards a general guideline here.
Comment 7 Jonathan Druart 2020-05-20 15:10:54 UTC
https://meetings.koha-community.org/2020/development_irc_meeting_20_may_2020.2020-05-20-14.00.log.html

Search for "bug 24331".

We agreed on making it mandatory, see bug 25449.

*** This bug has been marked as a duplicate of bug 25449 ***