Bug 32203

Summary: Refactor checkauth() auth rejected to improve maintainability
Product: Koha Reporter: David Cook <dcook>
Component: AuthenticationAssignee: Bugs List <koha-bugs>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: dpavlin
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32205
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 32203: Add render_auth_page() function

Description David Cook 2022-11-14 23:51:14 UTC
checkauth() appears to be 706 lines at the time of this writing.

The "AUTH rejected" section is 154 lines or over 20% of the function. 

It's a section devoted mainly to setting up the authentication template, so it's a very low risk part to change, and for the most part easy to change, since you just need to check the display on a failed auth.
Comment 1 David Cook 2022-11-15 01:13:18 UTC
Created attachment 143888 [details] [review]
Bug 32203: Add render_auth_page() function

This change adds a "render_auth_page()" function to C4::Auth for
rendering the login page for a failed auth/login.

Test plan:
0. Don't apply patch yet
1. In new tab, go to http://localhost:8081/ and do a failed login
2. In new tab, go to http://localhost:8080/ and do a failed login
3. Apply patch
4. In new tab, go to http://localhost:8081/ and do a failed login
5. In new tab, go to http://localhost:8080/ and do a failed login
6. Note how the failed login screens look the same after the patch
Comment 2 David Cook 2022-11-15 01:16:25 UTC
I think I let my excitement to refactor C4::Auth get the better of me. 

Resetting this one to default assignee as I've run out of steam. This patch probably needs to be split out into 30-50 smaller patches to make the amount of testing actually workable.
Comment 3 David Cook 2022-11-15 01:29:14 UTC
Actually, I think I made a mistake taking out "suggestion" as it doesn't use Koha.Preference() in the masthead, so the purchase suggestions wouldn't show up correctly.
Comment 4 David Cook 2023-09-20 02:42:26 UTC
This one is still worthwhile but I think it would be a good idea to break it up into multiple more easily testable bug reports.