Bug 34545 - AutoSelfCheckID fails silently if userid is invalid
Summary: AutoSelfCheckID fails silently if userid is invalid
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-16 18:28 UTC by Kyle M Hall
Modified: 2023-08-25 09:47 UTC (History)
4 users (show)

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


Attachments
Bug 34545: Koha should die with an error if AutoSelfCheckID is invalid (1.43 KB, patch)
2023-08-16 18:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34545: Koha should die with an error if AutoSelfCheckID is invalid (1.49 KB, patch)
2023-08-16 23:18 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 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 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.