Bug 35335

Summary: Circulation history tab in patron information causes 500 error
Product: Koha Reporter: Peter Vashchuk <stalkernoid>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: david, fridolin.somers, gmcharlt, jonathan.druart, kyle.m.hall, victor
Version: unspecifiedKeywords: rel_23_11_candidate
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00
Circulation function:
Bug Depends on: 33948    
Bug Blocks:    
Attachments: Bug 35335: Fix 500 error on circulation history tab
Bug 35335: Fix 500 error on circulation history tab
Bug 35335: Fix 500 error on circulation history tab
Bug 35335: Fix 500 on circulation history
Bug 35335: Fix 500 on circulation history
Bug 35335: Fix 500 on circulation history

Description Peter Vashchuk 2023-11-14 18:33:44 UTC Comment hidden (obsolete)
Comment 1 Peter Vashchuk 2023-11-16 14:59:17 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2023-11-16 15:11:21 UTC
Are you sure? IMO the correct fix is s/issuer.issuer_id/issuer.borrowernumber, next line.
Comment 3 David Nind 2023-11-19 20:01:47 UTC
I have tested, and it removes the error: Template process failed: undef error - The method Koha::Patron->issuer_id is not covered by tests!

However, I assume that RecordStaffUserOnCheckout is supposed to show the staff member who checked things out and show this in the 'Checked out by' column. This is empty for me for items checked out after the patch is applied.

Testing notes (using koha-testing-docker):

1. Check out and check in a couple of items for a patron so that the patron has some entries in their circulation history tab, before starting the test plan.
Comment 4 Peter Vashchuk 2023-11-20 13:25:44 UTC Comment hidden (obsolete)
Comment 5 Peter Vashchuk 2023-11-20 13:37:14 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2023-11-21 12:48:42 UTC
Created attachment 159156 [details] [review]
Bug 35335: Fix 500 on circulation history

Koha::Patron->issuer is not covered by tests!

Test plan:
1. Set RecordStaffUserOnCheckout to "record"
2. Check an item out and in to a patron
3. Go to their circulation history
=> Without the patch you get an error
=> With this patch applied the table displayed correct. The "checkout by" column contains the name and the link to the librarian
Comment 7 Jonathan Druart 2023-11-21 12:49:26 UTC
Peter, not sure why you don't want to apply the fix I am suggesting, it's working perfectly for me.
Comment 8 David Nind 2023-11-21 13:05:44 UTC
Created attachment 159157 [details] [review]
Bug 35335: Fix 500 on circulation history

Koha::Patron->issuer is not covered by tests!

Test plan:
1. Set RecordStaffUserOnCheckout to "record"
2. Check an item out and in to a patron
3. Go to their circulation history
=> Without the patch you get an error
=> With this patch applied the table displayed correct. The "checkout by" column contains the name and the link to the librarian

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2023-11-21 13:07:50 UTC
Thanks Jonathan!

This fixed the error for me, and the 'Checked out by' column now shows who checked it out.
Comment 10 Victor Grousset/tuxayo 2023-11-23 22:50:09 UTC
Created attachment 159219 [details] [review]
Bug 35335: Fix 500 on circulation history

Koha::Patron->issuer is not covered by tests!

Test plan:
1. Set RecordStaffUserOnCheckout to "record"
2. Check an item out and in to a patron
3. Go to their circulation history
=> Without the patch you get an error
=> With this patch applied the table displayed correct. The "checkout by" column contains the name and the link to the librarian

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 11 Victor Grousset/tuxayo 2023-11-23 22:54:47 UTC
> 2. Check an item out and in to a patron

"and in" doesn't seem necessary. So we can circumvent Bug 35366, David had to do the same.

Works, makes sense, QA script happy, code looks good, passing QA :)
Comment 12 Tomás Cohen Arazi (tcohen) 2023-11-24 00:13:28 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!