To test: 1. Change syspref: RecordStaffUserOnCheckout to "record" 2. Choose a customer who has borrowed something before=has circulation history (e.g. customer 209 in koha-kktest https://koha-kktest.lib.helsinki.fi/cgi-bin/koha/mainpage.pl) and see that circulation history is can be seen. 3. Borrow something and see circulation history now -> got internal error (example customer id 232). 4. Change RecorsStaffUserOnCheckout to "don't record". 5. See the previous customer's circulation history now, should work as supposed.
Created attachment 159053 [details] [review] Bug 35335: Fix 500 error on circulation history tab The 500 error was caused by this problem: Koha::Patron->issuer is not covered by tests! To reproduce: 1. Change syspref RecordStaffUserOnCheckout to "record" 2. Choose a customer who has borrowed something before (has circulation history) and go to that customers Circulation History tab to check if it can be seen. 3. Borrow something and go check that customer again, but this time you will see 500 error. 4. Apply the patch. 5. Check customer's Circulation History tab again, now it should work as intended.
Are you sure? IMO the correct fix is s/issuer.issuer_id/issuer.borrowernumber, next line.
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.
Created attachment 159138 [details] [review] Bug 35335: Fix 500 error on circulation history tab The 500 error was caused by this problem: Koha::Patron->issuer is not covered by tests! To reproduce: 1. Change syspref RecordStaffUserOnCheckout to "record" 2. Choose a customer who has borrowed something before (has circulation history) and go to that customers Circulation History tab to check if it can be seen. 3. Borrow something and go check that customer again, but this time you will see 500 error. 4. Apply the patch. 5. Check customer's Circulation History tab again, now it should work as intended.
Created attachment 159139 [details] [review] Bug 35335: Fix 500 error on circulation history tab The 500 error was caused by this problem: Koha::Patron->issuer is not covered by tests! To reproduce: 1. Change syspref RecordStaffUserOnCheckout to "record" 2. Choose a customer who has borrowed something before (has circulation history) and go to that customers Circulation History tab to check if it can be seen. 3. Borrow something and go check that customer again, but this time you will see 500 error. 4. Apply the patch. 5. Check customer's Circulation History tab again, now it should work as intended.
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
Peter, not sure why you don't want to apply the fix I am suggesting, it's working perfectly for me.
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>
Thanks Jonathan! This fixed the error for me, and the 'Checked out by' column now shows who checked it out.
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>
> 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 :)
Pushed to master for 23.11. Nice work everyone, thanks!