Bug 40034 - CheckReserves dies if itype doesn't exist
Summary: CheckReserves dies if itype doesn't exist
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-05-30 12:30 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-06-22 21:47 UTC (History)
2 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.01
Circulation function:


Attachments
Bug 40034: Regression tests (1.38 KB, patch)
2025-06-02 13:16 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40034: Only check notforloan on itemtype if it exists (1.45 KB, patch)
2025-06-02 13:16 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40034: Regression tests (1.42 KB, patch)
2025-06-02 19:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 40034: Only check notforloan on itemtype if it exists (1.50 KB, patch)
2025-06-02 19:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 40034: Regression tests (1.52 KB, patch)
2025-06-06 06:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 40034: Only check notforloan on itemtype if it exists (1.59 KB, patch)
2025-06-06 06:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2025-05-30 12:30:40 UTC
Filing like this becauseI'm not sure how tackle this. I feel tempted to just propose making item types a real FK.

In the meantime, this line explodes if the item type doesn't exist on the DB:

```perl
        if ($item_type) {
            return if Koha::ItemTypes->find($item_type)->notforloan;
        }
```
Comment 1 Tomás Cohen Arazi (tcohen) 2025-06-02 13:16:20 UTC
Created attachment 182908 [details] [review]
Bug 40034: Regression tests
Comment 2 Tomás Cohen Arazi (tcohen) 2025-06-02 13:16:23 UTC
Created attachment 182909 [details] [review]
Bug 40034: Only check notforloan on itemtype if it exists

This patch makes the code in `CheckReserves` survive the non-existence
of the referenced item type in the `items` table for the passed item.

To test:
1. Apply the regression tests
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests explode like this:

```
Can't call method "notforloan" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 870.
```

3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Comment 3 David Nind 2025-06-02 19:23:02 UTC
Created attachment 182913 [details] [review]
Bug 40034: Regression tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2025-06-02 19:23:05 UTC
Created attachment 182914 [details] [review]
Bug 40034: Only check notforloan on itemtype if it exists

This patch makes the code in `CheckReserves` survive the non-existence
of the referenced item type in the `items` table for the passed item.

To test:
1. Apply the regression tests
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests explode like this:

```
Can't call method "notforloan" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 870.
```

3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Marcel de Rooy 2025-06-06 06:56:09 UTC
Created attachment 183042 [details] [review]
Bug 40034: Regression tests

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2025-06-06 06:56:12 UTC
Created attachment 183043 [details] [review]
Bug 40034: Only check notforloan on itemtype if it exists

This patch makes the code in `CheckReserves` survive the non-existence
of the referenced item type in the `items` table for the passed item.

To test:
1. Apply the regression tests
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests explode like this:

```
Can't call method "notforloan" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 870.
```

3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Lucas Gass (lukeg) 2025-06-06 15:45:37 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 8 Paul Derscheid 2025-06-22 21:47:19 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03