Bug 28108

Created attachment 119295 [details] [review]
Bug 28108: Fix filtering

For the history of this bug see comments 34 (item 6), 46, 89, 90, 91, 92, 93 and 94 from bug 14233 for the details.

My notes from the comments:

- The module name Letters in Tools > Logs was confusing after logging of notices was added in bug 14233.
- The name of the module in Tools > Logs was changed from 'Letter' to 'Claims', as this reflects what is being logged: it was intended to log acquisition claims emails.
- The preference name for enabling logging was changed from LetterLog to ClaimsLog.
- The filters were updated so it shows log entries for 'CLAIMS', as it was discovered that there was a bug and it was actually showing logs for the Acquisitions module, so was not really showing what was intended "..actually logs aquisitions related notices being sent.. mostly 'Claims'".

Draft test plan:
1. Before applying the patch
   . Enable LetterLog system preference 
   . Generate log entries for claims letters in acquisitions [not sure how to do this - https://koha-community.org/manual/20.11/en/html/acquisitions.html#claims-and-late-orders]
   . Notice that log entries are not showing for letters
2. Apply the patch
3. Update the database: databaseupdate
4. With just the Claims module selected in the log viewer it should now show the expected log entries [may need to perform actions to generate the required log entries]
5. Run tests, these should pass:
   . prove t/db_dependent/Letters.t
   . prove t/db_dependent/UsageStats.t
6. Sign off!

Hope I got this mostly right!Feel free to correct....

Created attachment 119548 [details] [review]
Bug 28108: Fix filtering

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Thanks David !
The patch works fine for both superadmin and staff user with 'view_system_logs' permission.
As a tiny precision for the test plan, the patch renames the 'LetterLog' syspref to 'ClaimsLog', so you have to activate it before claiming orders and then search the logs.

Do we have something better to suggest or do we go with "Claims" as a "module" for action logs?

If claims is a subset of the acquisitions module, then how about "Acquisitions - Claims"?

(bug 23971 is adding AcquisitionLog)

(In reply to David Nind from comment #7)
> If claims is a subset of the acquisitions module, then how about
> "Acquisitions - Claims"?

This is actually not only about acq claims - it also covers serial issue claims and acq orders:

WHERE module = 'ACQUISITIONS' AND ( action = 'SERIAL CLAIM' OR action = 'ACQUISITION ORDER' OR action = 'ACQUISITION CLAIM')

Ultimately, that's a bit of a weird mix, especially since orders are not claims... we might want to split them up at some point. But let's fix this bug first :)

Created attachment 120747 [details] [review]
Bug 28108: Fix filtering

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 120748 [details] [review]
Bug 28108: (QA follow-up) Make the system preference description more precise

There has been some confusion on what the pref really covers, so
I hope this helps to resolve.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 120749 [details] [review]
Bug 28108: (QA follow-up) Add actions to log viewer

The following actions were missing from the template,
so the code showed instead of a translatable description:

* ACQUISITION CLAIM
* ACQUISITION ORDER
* SERIAL ORDER

Created attachment 120763 [details] [review]
Bug 28108: Replace one remaining occurrence

Why do we move 'ACQUISITION ORDER' to claims?

(In reply to Jonathan Druart from comment #14)
> Why do we move 'ACQUISITION ORDER' to claims?

I think it's already been there since we introduced the message to SendAlerts - so it's not actually a change in behaviour. I noted it in my comments - I think moving it out again would be good for a separate enhancement bug.

Created attachment 120781 [details] [review]
Bug 28108: (QA follow-up) Move 'ORDER' back into 'ACQUITISIONS'

With the re-introduction of 'AQUISITIONS' as a logging module in Koha,
it makes sence to move the ORDER notice logging back into that module
whilst leaving the other 'CLAIMS' notices under their own logging module
code.

Created attachment 120782 [details] [review]
Bug 28108: (QA follow-up) Move 'ORDER' back into 'ACQUITISIONS'

With the re-introduction of 'AQUISITIONS' as a logging module in Koha,
it makes sence to move the ORDER notice logging back into that module
whilst leaving the other 'CLAIMS' notices under their own logging module
code.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 120791 [details] [review]
Bug 28108: (QA follow-up) Fix syspref text

Pushed to master for 21.05, thanks to everybody involved!

Enhancement not pushed to 20.11.x