Bug 35335 - Circulation history tab in patron information causes 500 error
Summary: Circulation history tab in patron information causes 500 error
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords: rel_23_11_candidate
Depends on: 33948
Blocks:
  Show dependency treegraph
 
Reported: 2023-11-14 18:33 UTC by Peter Vashchuk
Modified: 2024-04-17 08:43 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 35335: Fix 500 error on circulation history tab (1.82 KB, patch)
2023-11-16 14:59 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 35335: Fix 500 error on circulation history tab (1.82 KB, patch)
2023-11-20 13:25 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 35335: Fix 500 error on circulation history tab (1.82 KB, patch)
2023-11-20 13:37 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 35335: Fix 500 on circulation history (1.83 KB, patch)
2023-11-21 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35335: Fix 500 on circulation history (1.88 KB, patch)
2023-11-21 13:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 35335: Fix 500 on circulation history (1.94 KB, patch)
2023-11-23 22:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 2023-11-24 00:13:28 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!