Bug 38771 - Typo 'AuthScuccessLog' system preference
Summary: Typo 'AuthScuccessLog' system preference
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Roman Dolny
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-12-22 17:48 UTC by Roman Dolny
Modified: 2025-04-17 06:53 UTC (History)
2 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the log viewer authentication module "Log not enabled" warning message for the log viewer. If either AuthFailureLog or AuthSuccessLog system preferences are set to "Don't log", the "Log not enabled" warning icon is now shown. Previously, if one of the system preferences was set to "Log", no warning icon was shown.
Version(s) released in:
25.05.00,24.11.03
Circulation function:


Attachments
Bug 38771: Typo 'AuthScuccessLog' system preference (1.91 KB, patch)
2024-12-22 18:01 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 38771: Typo 'AuthScuccessLog' system preference (1.96 KB, patch)
2024-12-22 20:17 UTC, David Nind
Details | Diff | Splinter Review
Bug 38771: Typo 'AuthScuccessLog' system preference (1.96 KB, patch)
2024-12-22 21:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 38771: (follow-up) Fix the authentication log not enabled message (2.03 KB, patch)
2024-12-22 21:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 38771: Add "Auth" prefix to system preference search query (1.70 KB, patch)
2024-12-27 15:17 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Roman Dolny 2024-12-22 17:48:31 UTC
Typo in system preference 'AuthScuccessLog' -> 'AuthSuccessLog'.
Comment 1 Roman Dolny 2024-12-22 18:01:08 UTC
Created attachment 175899 [details] [review]
Bug 38771: Typo 'AuthScuccessLog' system preference

Typo in system preference 'AuthScuccessLog' -> 'AuthSuccessLog'.
Comment 2 David Nind 2024-12-22 20:17:18 UTC Comment hidden (obsolete)
Comment 3 David Nind 2024-12-22 21:27:54 UTC
Created attachment 175901 [details] [review]
Bug 38771: Typo 'AuthScuccessLog' system preference

Typo in system preference 'AuthScuccessLog' -> 'AuthSuccessLog'.

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-12-22 21:27:56 UTC
Created attachment 175902 [details] [review]
Bug 38771: (follow-up) Fix the authentication log not enabled message

This corrects the display of the "Log not enabled" message for the
authentication module in the log viewer.

If either AuthFailureLog or AuthSuccessLog system preferences are
set to "Don't log":
- the "Log not enabled" warning icon is now shown
- clicking the triangle icon now links to the two system
  preferences

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2024-12-22 21:30:42 UTC
I've signed off, as this does fix a typo in the code.

However, I have a question for QA, as I think the existing logic is wrong.

I've added a follow-up to correct the logic. Please remove or amend the follow-up patch if you think this is incorrect.

Question for QA
===============

1. I think the warning icon with "Log not enabled" should show if either AuthFailureLog or AuthSuccessLog are not enabled.

2. After the patch, the warning icon with "Log not enabled" is only shown if both the authentication log system preferences are not enabled. [1]

3. It is the only module that I can see that has the case with two system preferences, and it uses an ||, but I think it should be an &&. That is, if either of the system preference are not enabled, it should show the warning.


[1] Settings and the warning triangle in the log viewer, after the patch
    ====================================================================

1. If both AuthFailureLog and AuthSuccessLog enabled:
   => No warning triangle in the log viewer tool with "Log not enabled"
      (as expected)

2. If both AuthFailureLog and AuthSuccessLog not enabled:
   => Warning triangle in the log viewer tool with "Log not enabled"
      (as expected)

3. If AuthFailureLog enabled and AuthSuccessLog not enabled:
   => No warning triangle in the log viewer tool with "Log not enabled"
      (not as expected)

4. If AuthFailureLog not enabled and AuthSuccessLog enabled:
   => No warning triangle in the log viewer tool with "Log not enabled"
      (not as expected)
Comment 6 Katrin Fischer 2024-12-27 15:16:18 UTC
I have updated this bit to match the system preferences exactly:

data-log="FailureLog|SuccessLog">

To

data-log="AuthFailureLog|AuthSuccessLog">

Verified the link still works as expected.
Comment 7 Katrin Fischer 2024-12-27 15:17:32 UTC
Created attachment 175957 [details] [review]
Bug 38771: Add "Auth" prefix to system preference search query
Comment 8 Katrin Fischer 2024-12-27 16:37:02 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 9 David Nind 2024-12-27 17:39:38 UTC
(In reply to Katrin Fischer from comment #6)
> I have updated this bit to match the system preferences exactly:
> 
> data-log="FailureLog|SuccessLog">
> 
> To
> 
> data-log="AuthFailureLog|AuthSuccessLog">
> 
> Verified the link still works as expected.

Thanks Katrin!
Comment 10 Paul Derscheid 2025-03-19 23:01:54 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.03
Comment 11 Wainui Witika-Park 2025-04-17 06:53:09 UTC
Not backporting to 24.05.x unless requested