Bug 31395

Summary: Checking in non-existent barcodes makes Koha explode
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: CirculationAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: CLOSED FIXED QA Contact: Kyle M Hall (khall) <kyle>
Severity: major    
Priority: P5 - low CC: andrew, gmcharlt, kyle.m.hall, kyle
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:
22.11.00
Circulation function:
Bug Depends on: 22456    
Bug Blocks:    
Attachments: Bug 31395: Only try to cancel holds if item found
Bug 31395: Only try to cancel holds if item found
Bug 31395: Only try to cancel holds if item found

Description Lucas Gass (lukeg) 2022-08-18 15:06:26 UTC
To recreate:

-Checkin a barcode that doesn't exist. 
-Boom, explosion. 


Can't call method "id" on an undefined value at /kohadevbox/koha/circ/returns.pl line 351
Comment 1 Tomás Cohen Arazi (tcohen) 2022-08-18 17:19:39 UTC
Created attachment 139407 [details] [review]
Bug 31395: Only try to cancel holds if item found

This patch makes the code dealing with waiting holds with cancellation
requests be dependent on the fact an item has been found.

The returns.pl controller is a bit messy as the real return takes place
outside the main `if ($item)` block. This should be refactored and
probably run inside a transaction...

In the meantime this patch will make the job.

To test:
1. Try to return an invalid barcode (e.g. ASDQWE)
=> FAIL: Things explode
2. Apply this patch
3. Repeat 1
=> SUCCESS: Doesn't explode!
4. Verify that returning an item with a waiting hold with cancellation
   requests still cancells the hold.
=> SUCCESS: It does!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 ByWater Sandboxes 2022-08-18 19:55:29 UTC
Created attachment 139417 [details] [review]
Bug 31395: Only try to cancel holds if item found

This patch makes the code dealing with waiting holds with cancellation
requests be dependent on the fact an item has been found.

The returns.pl controller is a bit messy as the real return takes place
outside the main `if ($item)` block. This should be refactored and
probably run inside a transaction...

In the meantime this patch will make the job.

To test:
1. Try to return an invalid barcode (e.g. ASDQWE)
=> FAIL: Things explode
2. Apply this patch
3. Repeat 1
=> SUCCESS: Doesn't explode!
4. Verify that returning an item with a waiting hold with cancellation
   requests still cancells the hold.
=> SUCCESS: It does!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 3 Kyle M Hall (khall) 2022-08-19 11:56:46 UTC
Created attachment 139485 [details] [review]
Bug 31395: Only try to cancel holds if item found

This patch makes the code dealing with waiting holds with cancellation
requests be dependent on the fact an item has been found.

The returns.pl controller is a bit messy as the real return takes place
outside the main `if ($item)` block. This should be refactored and
probably run inside a transaction...

In the meantime this patch will make the job.

To test:
1. Try to return an invalid barcode (e.g. ASDQWE)
=> FAIL: Things explode
2. Apply this patch
3. Repeat 1
=> SUCCESS: Doesn't explode!
4. Verify that returning an item with a waiting hold with cancellation
   requests still cancells the hold.
=> SUCCESS: It does!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Tomás Cohen Arazi (tcohen) 2022-08-19 12:58:16 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 5 Lucas Gass (lukeg) 2022-10-14 19:20:13 UTC
Missing the dependency for 22.05.x, no backport