Bug 28108

Summary: Move action logs 'SERIAL CLAIM' and 'ACQUISITION CLAIM' to a new 'CLAIMS' module
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: ToolsAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: david, fridolin.somers, jonathan.druart, kelly, severine.queune
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement changes the logging system preferences and the log viewer tool so that: - ClaimsLog: logs when an acquisitions claim or a serials claim notice is sent (Claims module in the log viewer) - AcquisitionLog: logs all other changes for acquisition actions (Acquisitions module in the log viewer). Previously ClaimsLog was called LettersLog, but the name was considered confusing after support for logging changes to notices and slips was added in Koha 21.05 (bug 14233).
Version(s) released in:
21.05.00
Bug Depends on: 14233    
Bug Blocks: 36389    
Attachments: Bug 28108: Fix filtering
Bug 28108: Fix filtering
Bug 28108: Fix filtering
Bug 28108: (QA follow-up) Make the system preference description more precise
Bug 28108: (QA follow-up) Add actions to log viewer
Bug 28108: Replace one remaining occurrence
Bug 28108: (QA follow-up) Move 'ORDER' back into 'ACQUITISIONS'
Bug 28108: (QA follow-up) Move 'ORDER' back into 'ACQUITISIONS'
Bug 28108: (QA follow-up) Fix syspref text

Description Martin Renvoize 2021-04-07 13:38:21 UTC
We stripped out the fix for claims/acquisitions/letter logging filtering from bug 14233.

I'll attach a copy of the code here for prosperity.
Comment 1 Martin Renvoize 2021-04-07 13:39:14 UTC
Created attachment 119295 [details] [review]
Bug 28108: Fix filtering
Comment 2 David Nind 2021-04-08 18:51:20 UTC
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!
Comment 3 David Nind 2021-04-08 18:52:01 UTC
Hope I got this mostly right!Feel free to correct....
Comment 4 Séverine Queune 2021-04-14 06:38:27 UTC
Created attachment 119548 [details] [review]
Bug 28108: Fix filtering

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 5 Séverine Queune 2021-04-14 06:41:22 UTC
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.
Comment 6 Jonathan Druart 2021-04-14 09:55:12 UTC
Do we have something better to suggest or do we go with "Claims" as a "module" for action logs?
Comment 7 David Nind 2021-04-19 19:05:35 UTC
If claims is a subset of the acquisitions module, then how about "Acquisitions - Claims"?
Comment 8 Jonathan Druart 2021-04-21 12:27:13 UTC
(bug 23971 is adding AcquisitionLog)
Comment 9 Katrin Fischer 2021-05-09 13:53:33 UTC
(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 :)
Comment 10 Katrin Fischer 2021-05-09 14:49:52 UTC
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>
Comment 11 Katrin Fischer 2021-05-09 14:49:58 UTC
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>
Comment 12 Katrin Fischer 2021-05-09 14:50:03 UTC
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
Comment 13 Jonathan Druart 2021-05-10 10:13:18 UTC
Created attachment 120763 [details] [review]
Bug 28108: Replace one remaining occurrence
Comment 14 Jonathan Druart 2021-05-10 12:34:00 UTC
Why do we move 'ACQUISITION ORDER' to claims?
Comment 15 Katrin Fischer 2021-05-10 13:06:22 UTC
(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.
Comment 16 Martin Renvoize 2021-05-10 13:23:23 UTC
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.
Comment 17 Martin Renvoize 2021-05-10 13:24:41 UTC
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>
Comment 18 Martin Renvoize 2021-05-10 13:35:33 UTC
Created attachment 120791 [details] [review]
Bug 28108: (QA follow-up) Fix syspref text
Comment 19 Jonathan Druart 2021-05-10 14:18:33 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 20 Fridolin Somers 2021-05-12 15:09:20 UTC
Enhancement not pushed to 20.11.x