28108 – Move action logs 'SERIAL CLAIM' and 'ACQUISITION CLAIM' to a new 'CLAIMS' module

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:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Tools (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Martin Renvoize (ashimema)
QA Contact:	Katrin Fischer

URL:
Keywords:

Depends on:	14233
Blocks:	36389
	Show dependency tree / graph

Reported:	2021-04-07 13:38 UTC by Martin Renvoize (ashimema)
Modified:	2024-03-21 16:00 UTC (History)
CC List:	5 users (show)

See Also:
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
Circulation function:

Attachments
Bug 28108: Fix filtering (7.92 KB, patch) 2021-04-07 13:39 UTC, Martin Renvoize (ashimema)	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 (ashimema)	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 (ashimema)	Details \| Diff \| Splinter Review
Bug 28108: (QA follow-up) Fix syspref text (1.04 KB, patch) 2021-05-10 13:35 UTC, Martin Renvoize (ashimema)	Details \| Diff \| Splinter Review
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Martin Renvoize (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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