Bug 34545

Summary: AutoSelfCheckID fails silently if userid is invalid
Product: Koha Reporter: Kyle M Hall (khall) <kyle>
Component: Self checkoutAssignee: Kyle M Hall (khall) <kyle>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: dcook, kyle, liz, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 34545: Koha should die with an error if AutoSelfCheckID is invalid
Bug 34545: Koha should die with an error if AutoSelfCheckID is invalid

Description Kyle M Hall (khall) 2023-08-16 18:28:13 UTC
If AutoSelfCheckID is an not valid ( either by deleting the patron or by changing the patron's userid ), self-checkout will display the login screen and no error will be generated in any logs.
Comment 1 Kyle M Hall (khall) 2023-08-16 18:37:20 UTC
Created attachment 154469 [details] [review]
Bug 34545: Koha should die with an error if AutoSelfCheckID is invalid

If AutoSelfCheckID is an not valid ( either by deleting the patron or by changing the patron's userid ), self-checkout will display the login screen and no error will be generated in any logs.

Test Plan:
1) Apply this patch
2) Verify AutoSelfCheckID is working as expected
3) Change AutoSelfCheckID to an invalid userid
4) Restart all the things!
5) Verify visiting sco-main.pl generates an error!
Comment 2 David Cook 2023-08-16 23:18:31 UTC
Created attachment 154473 [details] [review]
Bug 34545: Koha should die with an error if AutoSelfCheckID is invalid

If AutoSelfCheckID is an not valid ( either by deleting the patron or by changing the patron's userid ), self-checkout will display the login screen and no error will be generated in any logs.

Test Plan:
1) Apply this patch
2) Verify AutoSelfCheckID is working as expected
3) Change AutoSelfCheckID to an invalid userid
4) Restart all the things!
5) Verify visiting sco-main.pl generates an error!

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 3 David Cook 2023-08-16 23:20:17 UTC
I'm signing off on this one, since it works as described.

However, I'm not sure about the approach.

I think adding a warning in the logs is a very good idea, but generating an internal server error seems suboptimal. It might be better to generate a more graceful error message on the screen. That said, an ISE is probably better than what we have at the moment. Failing early and noisy would get the situation fixed ASAP I suspect.
Comment 4 Marcel de Rooy 2023-08-25 09:47:31 UTC
Since this is just two lines, we should be able to meet:

PERL26: Koha::Exceptions (Approved on Apr 26, 2017)
Instead of die or croak when meeting unfavorable conditions in our code, we should raise exceptions via Koha::Exceptions.