Bug 28108 - Move action logs 'SERIAL CLAIM' and 'ACQUISITION CLAIM' to a new 'CLAIMS' module
Summary: Move action logs 'SERIAL CLAIM' and 'ACQUISITION CLAIM' to a new 'CLAIMS' module
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 14233
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-07 13:38 UTC by Martin Renvoize
Modified: 2021-05-12 15:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
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


Attachments
Bug 28108: Fix filtering (7.92 KB, patch)
2021-04-07 13:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28108: Fix filtering (8.13 KB, patch)
2021-04-14 06:38 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 28108: Fix filtering (8.19 KB, patch)
2021-05-09 14:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 28108: (QA follow-up) Make the system preference description more precise (1.20 KB, patch)
2021-05-09 14:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 28108: (QA follow-up) Add actions to log viewer (1.14 KB, patch)
2021-05-09 14:50 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 28108: Replace one remaining occurrence (1.31 KB, patch)
2021-05-10 10:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28108: (QA follow-up) Move 'ORDER' back into 'ACQUITISIONS' (1.73 KB, patch)
2021-05-10 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28108: (QA follow-up) Move 'ORDER' back into 'ACQUITISIONS' (1.80 KB, patch)
2021-05-10 13:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28108: (QA follow-up) Fix syspref text (1.04 KB, patch)
2021-05-10 13:35 UTC, Martin Renvoize
Details | Diff | Splinter Review

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