Bug 14233 - Add logging support to notices and slips management
Summary: Add logging support to notices and slips management
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P4 enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Kyle M Hall
URL:
Keywords:
: 27602 (view as bug list)
Depends on: 14206
Blocks: 22794 26734 28108
  Show dependency treegraph
 
Reported: 2015-05-20 07:07 UTC by Indranil Das Gupta
Modified: 2021-04-14 08:05 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
This enhancement adds support for logging changes to notices and slips. Features include: - a system preference (NoticesLog) which allows you to enable logging changes for notices and slips - logging new notices, changes to notices, and deletion of notices - filtering in the log viewer so you can show all changes for a specific notice - a comparison feature, so you can see what changes were made to a notice.
Version(s) released in:
21.05.00


Attachments
Bug 14233: Add 'id' field to `letter` table to allow linking (3.26 KB, patch)
2021-02-10 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.05 KB, patch)
2021-02-10 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging to notice creation/update actions (2.06 KB, patch)
2021-02-10 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.41 KB, patch)
2021-02-10 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging to notice creation/update actions (2.33 KB, patch)
2021-02-10 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.36 KB, patch)
2021-02-10 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.57 KB, patch)
2021-02-10 14:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.41 KB, patch)
2021-02-10 14:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging to notice create/update/delete (3.34 KB, patch)
2021-02-10 14:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.36 KB, patch)
2021-02-10 14:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.57 KB, patch)
2021-02-10 14:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (1.93 KB, patch)
2021-02-11 12:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Fix for delete entire notice (1.64 KB, patch)
2021-02-11 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (1.93 KB, patch)
2021-02-11 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.41 KB, patch)
2021-02-24 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging to notice create/update/delete (3.34 KB, patch)
2021-02-24 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.36 KB, patch)
2021-02-24 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.57 KB, patch)
2021-02-24 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Fix for delete entire notice (1.64 KB, patch)
2021-02-24 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix typo in viewlog.tt (1.85 KB, patch)
2021-02-24 13:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Record updated content (1.01 KB, patch)
2021-02-24 13:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add notice code to log viewer (1.81 KB, patch)
2021-02-24 13:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (1.93 KB, patch)
2021-02-24 13:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.41 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging to notice create/update/delete (3.34 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.36 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.57 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Fix for delete entire notice (1.64 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix typo in viewlog.tt (1.85 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Record updated content (1.01 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add notice code to log viewer (1.81 KB, patch)
2021-02-25 10:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Clarify 'LetterLog' (8.26 KB, patch)
2021-02-25 10:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add NoticesLog preference (4.28 KB, patch)
2021-02-25 10:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (1.93 KB, patch)
2021-02-25 10:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.47 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: Add logging to notice create/update/delete (3.40 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.44 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.63 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (follow-up) Fix for delete entire notice (1.70 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix typo in viewlog.tt (1.91 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Record updated content (1.07 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add notice code to log viewer (1.87 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (follow-up) Clarify 'LetterLog' (8.38 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add NoticesLog preference (4.38 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (1.99 KB, patch)
2021-02-25 13:38 UTC, Kelly McElligott
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix filtering issue (2.32 KB, patch)
2021-02-25 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.52 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging to notice create/update/delete (3.45 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.47 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.68 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Fix for delete entire notice (1.75 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix typo in viewlog.tt (1.96 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Record updated content (1.11 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add notice code to log viewer (1.92 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Clarify 'LetterLog' (8.36 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add NoticesLog preference (4.39 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (2.03 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix filtering issue (2.36 KB, patch)
2021-03-11 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add 'id' field to `letter` table to allow linking (3.59 KB, patch)
2021-03-11 17:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: Add logging to notice create/update/delete (3.52 KB, patch)
2021-03-11 17:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: Add filtering of NOTICES type action logs to the logviewer (8.54 KB, patch)
2021-03-11 17:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: Allow comparing notice changes (2.75 KB, patch)
2021-03-11 17:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (follow-up) Fix for delete entire notice (1.82 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix typo in viewlog.tt (2.03 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Record updated content (1.18 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add notice code to log viewer (1.99 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (follow-up) Clarify 'LetterLog' (8.43 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (follow-up) Add NoticesLog preference (4.46 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: [DO NOT PUSH] DBIC Schema File Update (2.10 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix filtering issue (2.43 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Fix template filters (3.14 KB, patch)
2021-03-11 17:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14233: (QA follow-up) Claims -> Acquisitions (8.32 KB, patch)
2021-03-16 13:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: DBIC schema changes (1.91 KB, patch)
2021-04-07 13:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14233: DB changes - NoticesLog pref + letter.id PK (4.98 KB, patch)
2021-04-07 13:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14233: Add logging support to notices and slips management (13.42 KB, patch)
2021-04-07 13:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14233: Select the correct tab (1.85 KB, patch)
2021-04-07 13:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14233: DBIC schema changes (1.96 KB, patch)
2021-04-07 19:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 14233: DB changes - NoticesLog pref + letter.id PK (5.06 KB, patch)
2021-04-07 19:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 14233: Add logging support to notices and slips management (13.48 KB, patch)
2021-04-07 19:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 14233: Select the correct tab (1.89 KB, patch)
2021-04-07 19:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 14233: DBIC schema changes (1.97 KB, patch)
2021-04-09 08:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: DB changes - NoticesLog pref + letter.id PK (5.04 KB, patch)
2021-04-09 08:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Add logging support to notices and slips management (13.48 KB, patch)
2021-04-09 08:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Select the correct tab (1.91 KB, patch)
2021-04-09 08:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14233: Set default for NoticesLog to 0 (2.20 KB, patch)
2021-04-13 08:46 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Indranil Das Gupta 2015-05-20 07:07:29 UTC
Notices are a key feature that provides notifications to Koha users and patrons. The present code does not provide logging when notices are added or deleted. This bug aims to fix that.
Comment 1 Katrin Fischer 2020-08-30 21:18:41 UTC
This would also be a great use case for the compare prefs patch that Owen has worked on: 
Bug 26207 - Compare values of system preference log entries
Comment 2 Barbara Johnson 2020-10-29 15:14:38 UTC
With COVID-19 and going fines free we had to make multiple changes to notices text as well as stopping notices for awhile so it would be very helpful to have logging for notices.  Also agree that the compare prefs patch would be helpful with notices.
Comment 3 Jonathan Druart 2021-02-03 14:24:38 UTC
*** Bug 27602 has been marked as a duplicate of this bug. ***
Comment 4 David Cook 2021-02-04 05:49:28 UTC
At a bare minimum, we could add it to the action_logs, no?
Comment 5 Martin Renvoize 2021-02-04 07:02:21 UTC
(In reply to David Cook from comment #4)
> At a bare minimum, we could add it to the action_logs, no?

Indeed, that's what I though to do.  However, it will need a little imaginative thinking was the table structure doesn't lend itself to the code + category approach in the letter table.

I actually do use action logs in the database update for bug 26734.
Comment 6 Martin Renvoize 2021-02-10 12:10:02 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize 2021-02-10 12:10:05 UTC Comment hidden (obsolete)
Comment 8 Martin Renvoize 2021-02-10 12:10:08 UTC Comment hidden (obsolete)
Comment 9 Martin Renvoize 2021-02-10 12:10:45 UTC
This is a proof of concept right now... just looking for some feedback.
Comment 10 Martin Renvoize 2021-02-10 12:55:22 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize 2021-02-10 12:55:26 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize 2021-02-10 12:55:29 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize 2021-02-10 14:04:34 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize 2021-02-10 14:51:47 UTC
Created attachment 116671 [details] [review]
Bug 14233: Add 'id' field to `letter` table to allow linking

To allow for 'object' linking from the action_logs table we need an
int(11) identifier. This patch updates the letter table to add such a
field as a replacement for the multi-field primary key and moves that
key to a unique constraint.

Test plan
1/ Run the database update
2/ Check that the 'letter' table now includes an 'id' primary key and
'letter_uniq_1' multi-field unique key
3/ Signoff
Comment 15 Martin Renvoize 2021-02-10 14:51:51 UTC
Created attachment 116672 [details] [review]
Bug 14233: Add logging to notice create/update/delete

This patch adds the call to logaction to record when a notice is added
or changed.

Test plan
1/ Add a new notice and check a corresponding line appears in the
action_logs table.
2/ Update a notice and check that a corresponding line appears in the
action_logs table.
3/ Signoff
Comment 16 Martin Renvoize 2021-02-10 14:51:54 UTC
Created attachment 116673 [details] [review]
Bug 14233: Add filtering of NOTICES type action logs to the logviewer

This patch adds handling for the new 'NOTICES' type action_logs to the
logviewer page.

Test plan
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.
5/ Signoff
Comment 17 Martin Renvoize 2021-02-10 14:51:57 UTC
Created attachment 116674 [details] [review]
Bug 14233: Allow comparing notice changes

This patch adds the 'compare' option to the Notices logs.

Test plan
1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
7/ Signoff
Comment 18 Martin Renvoize 2021-02-10 14:53:23 UTC
Note, to test this you will need to run a dbic schema update
Comment 19 Owen Leonard 2021-02-11 12:16:59 UTC
(In reply to Martin Renvoize from comment #15)
> Created attachment 116672 [details] [review] [review]
> Bug 14233: Add logging to notice create/update/delete
> 
> This patch adds the call to logaction to record when a notice is added
> or changed.
> 
> Test plan
> 1/ Add a new notice and check a corresponding line appears in the
> action_logs table.

At this stage I get an error:

C4::Log::logaction(): DBI Exception: DBD::mysql::st execute failed: Incorrect integer value: 'reserves' for column 'object' at row 1 [for Statement "Insert into action_logs (timestamp,user,module,action,object,info,interface) values (now(),?,?,?,?,?,?)" with ParamValues: 0="20375", 1='NOTICES', 2='CREATE', 3="reserves", 4="Lorem ipsum", 5='Intranet'] at /kohadevbox/koha/tools/letter.pl line 346
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

The notice gets added but I don't see anything in the log.
Comment 20 Martin Renvoize 2021-02-11 12:51:38 UTC
Created attachment 116713 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update
Comment 21 Martin Renvoize 2021-02-11 12:53:06 UTC
Hmm, I can't seem to replicate that here...  I've uploaded a DO NOT PUSH schema patch to make testing easier.. just in case it was a missing dbic schema dump that caused the problem.?
Comment 22 Martin Renvoize 2021-02-11 15:25:14 UTC
Created attachment 116760 [details] [review]
Bug 14233: (follow-up) Fix for delete entire notice

This fixes the overall notice delete option.
Comment 23 Martin Renvoize 2021-02-11 15:25:18 UTC
Created attachment 116761 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update
Comment 24 David Nind 2021-02-23 21:54:53 UTC
I had a go at testing this and a few things I noted:

1. The link in the log viewer 'Object' column doesn't go to the actual notice - it just lists the notices. This is because there are two ?? after letter.pl (for example: /cgi-bin/koha/tools/letter.pl??op=add_form&branchcode=&module=members&code=A_NOTICE_2&lang=default)

2. I'm not sure whether changes to the notice name should be logged or not - if I change the notice name it doesn't show in the log. I'm therefore assuming that only actual changes to the notice itself are logged.

3. The changes logged - I'm not sure whether this is the correct behaviour, but assume it is:
   . If I make a change to a notice, in the log viewer 'Info' column it shows what it was before the change was made.
   . If that is what is expected, then it is working - you can only compare previous changes, but not to the current version of the notice.

4. I could add new notices, the change was logged, and I didn't get the error the Owen did in Comment 19, presumably because of the follow up patch.

5. Is it possible to add the notice code somewhere? Maybe in front of the notice name in the log viewer's 'Object' column. Maybe a future enhancement?

6. I was confused that there is already a 'Letter' module listed (before and after the patch is applied). Is the 'Letter' module something different from the 'Notices' module, or the same - I prefer notices as it matches up with the name in the tools section.

Sorry about all the questions!

Testing notes (using koha-testing-docker):
- Apply patches and then updatedatabase, dbic, flush_memcached, restart_all
- MySQL commands for database table checks in test plans:
  . koha-mysql kohadev
  . show columns from letter;
  . show index from letter;
Comment 25 Martin Renvoize 2021-02-24 13:01:44 UTC
Created attachment 117251 [details] [review]
Bug 14233: Add 'id' field to `letter` table to allow linking

To allow for 'object' linking from the action_logs table we need an
int(11) identifier. This patch updates the letter table to add such a
field as a replacement for the multi-field primary key and moves that
key to a unique constraint.

Test plan
1/ Run the database update
2/ Check that the 'letter' table now includes an 'id' primary key and
'letter_uniq_1' multi-field unique key
3/ Signoff
Comment 26 Martin Renvoize 2021-02-24 13:01:48 UTC
Created attachment 117252 [details] [review]
Bug 14233: Add logging to notice create/update/delete

This patch adds the call to logaction to record when a notice is added
or changed.

Test plan
1/ Add a new notice and check a corresponding line appears in the
action_logs table.
2/ Update a notice and check that a corresponding line appears in the
action_logs table.
3/ Signoff
Comment 27 Martin Renvoize 2021-02-24 13:01:51 UTC
Created attachment 117253 [details] [review]
Bug 14233: Add filtering of NOTICES type action logs to the logviewer

This patch adds handling for the new 'NOTICES' type action_logs to the
logviewer page.

Test plan
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.
5/ Signoff
Comment 28 Martin Renvoize 2021-02-24 13:01:55 UTC
Created attachment 117254 [details] [review]
Bug 14233: Allow comparing notice changes

This patch adds the 'compare' option to the Notices logs.

Test plan
1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
7/ Signoff
Comment 29 Martin Renvoize 2021-02-24 13:01:58 UTC
Created attachment 117255 [details] [review]
Bug 14233: (follow-up) Fix for delete entire notice

This fixes the overall notice delete option.
Comment 30 Martin Renvoize 2021-02-24 13:02:01 UTC
Created attachment 117256 [details] [review]
Bug 14233: (QA follow-up) Fix typo in viewlog.tt

We had a double ?? in the object link.. Thanks for spotting that David
:)
Comment 31 Martin Renvoize 2021-02-24 13:02:05 UTC
Created attachment 117257 [details] [review]
Bug 14233: (QA follow-up) Record updated content

We should record the updated template, not the content prior to the
update.. That way we can properly compare between versions to see the
changes made.
Comment 32 Martin Renvoize 2021-02-24 13:02:08 UTC
Created attachment 117258 [details] [review]
Bug 14233: (follow-up) Add notice code to log viewer
Comment 33 Martin Renvoize 2021-02-24 13:02:11 UTC
Created attachment 117259 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update
Comment 34 Martin Renvoize 2021-02-24 13:07:28 UTC
(In reply to David Nind from comment #24)

Great feedback David, thanks for testing :)

> 1. The link in the log viewer 'Object' column doesn't go to the actual
> notice - it just lists the notices. This is because there are two ?? after
> letter.pl (for example:
> /cgi-bin/koha/tools/letter.
> pl??op=add_form&branchcode=&module=members&code=A_NOTICE_2&lang=default)

Good catch, fixed in the first QA follow-up.

> 2. I'm not sure whether changes to the notice name should be logged or not -
> if I change the notice name it doesn't show in the log. I'm therefore
> assuming that only actual changes to the notice itself are logged.

I didn't add logging for title changes at this time, but perhaps that could be added too.. I wasn't sure of the best format to take though and felt at the moment it was more important to expose the template changes diff view than record the title changes and lose the diff view. Perhaps a later followup bug :)

> 3. The changes logged - I'm not sure whether this is the correct behaviour,
> but assume it is:
>    . If I make a change to a notice, in the log viewer 'Info' column it
> shows what it was before the change was made.
>    . If that is what is expected, then it is working - you can only compare
> previous changes, but not to the current version of the notice.

Oops, this is a mistake on my part, it should be the 'after' so we can do the comparison with current and prior. I've corrected this in the second QA follow-up.

> 4. I could add new notices, the change was logged, and I didn't get the
> error the Owen did in Comment 19, presumably because of the follow-up patch.

Correct, that was the earlier follow-up :)

> 5. Is it possible to add the notice code somewhere? Maybe in front of the
> notice name in the log viewer's 'Object' column. Maybe a future enhancement?

Done, added in the 'follow-up'.

> 6. I was confused that there is already a 'Letter' module listed (before and
> after the patch is applied). Is the 'Letter' module something different from
> the 'Notices' module, or the same - I prefer notices as it matches up with
> the name in the tools section.

I hadn't even noticed the existing 'Letter' filtering.. when I dug into it, it seems that it's linked to the acquisitions claims emails.  I wonder if we could rename it to 'Claims' in the UI to clarify it?
Comment 35 Kelly McElligott 2021-02-24 17:43:43 UTC
This is working as expected however, I think a System Preference for Logging of Notices is important.  From what I can see, these logs do not have an option of being turned off since they are not in the Logging System preferences.

Can this be added?
Comment 36 Martin Renvoize 2021-02-24 21:55:18 UTC
(In reply to Kelly McElligott from comment #35)
> This is working as expected however, I think a System Preference for Logging
> of Notices is important.  From what I can see, these logs do not have an
> option of being turned off since they are not in the Logging System
> preferences.
> 
> Can this be added?

I considered this at the time but then took president from the preferences log.. changing preferences is always logged as far as I can tell without any ability to turn that off... I felt notices should fall into the same category.. they are unlikely to be changed often, but when they are changed logging is rather important..

Do you believe they should be preference driven then.. and if so what should we default to.. logging or not logging?
Comment 37 Kelly McElligott 2021-02-24 23:51:02 UTC
Hi,
Thanks for the quick response.  Talking to the other educators, we would like to keep it consistent with a system preference to allow the ability to turn it on/off. I don't know what logging is set to ON by default?
Comment 38 Martin Renvoize 2021-02-25 10:11:31 UTC
Created attachment 117294 [details] [review]
Bug 14233: Add 'id' field to `letter` table to allow linking

To allow for 'object' linking from the action_logs table we need an
int(11) identifier. This patch updates the letter table to add such a
field as a replacement for the multi-field primary key and moves that
key to a unique constraint.

Test plan
1/ Run the database update
2/ Check that the 'letter' table now includes an 'id' primary key and
'letter_uniq_1' multi-field unique key
3/ Signoff
Comment 39 Martin Renvoize 2021-02-25 10:11:35 UTC
Created attachment 117295 [details] [review]
Bug 14233: Add logging to notice create/update/delete

This patch adds the call to logaction to record when a notice is added
or changed.

Test plan
1/ Add a new notice and check a corresponding line appears in the
action_logs table.
2/ Update a notice and check that a corresponding line appears in the
action_logs table.
3/ Signoff
Comment 40 Martin Renvoize 2021-02-25 10:11:38 UTC
Created attachment 117296 [details] [review]
Bug 14233: Add filtering of NOTICES type action logs to the logviewer

This patch adds handling for the new 'NOTICES' type action_logs to the
logviewer page.

Test plan
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.
5/ Signoff
Comment 41 Martin Renvoize 2021-02-25 10:11:42 UTC
Created attachment 117297 [details] [review]
Bug 14233: Allow comparing notice changes

This patch adds the 'compare' option to the Notices logs.

Test plan
1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
7/ Signoff
Comment 42 Martin Renvoize 2021-02-25 10:11:45 UTC
Created attachment 117298 [details] [review]
Bug 14233: (follow-up) Fix for delete entire notice

This fixes the overall notice delete option.
Comment 43 Martin Renvoize 2021-02-25 10:11:49 UTC
Created attachment 117299 [details] [review]
Bug 14233: (QA follow-up) Fix typo in viewlog.tt

We had a double ?? in the object link.. Thanks for spotting that David
:)
Comment 44 Martin Renvoize 2021-02-25 10:11:53 UTC
Created attachment 117300 [details] [review]
Bug 14233: (QA follow-up) Record updated content

We should record the updated template, not the content prior to the
update.. That way we can properly compare between versions to see the
changes made.
Comment 45 Martin Renvoize 2021-02-25 10:11:56 UTC
Created attachment 117301 [details] [review]
Bug 14233: (follow-up) Add notice code to log viewer
Comment 46 Martin Renvoize 2021-02-25 10:12:00 UTC
Created attachment 117302 [details] [review]
Bug 14233: (follow-up) Clarify 'LetterLog'

The LetterLog logging wasn't clear as to what it was logging, it
actually logs aquisitions related notices being sent.. mostly 'Claims'.

This patch updates LetterLog to ClaimsLog to more clearly reflect what
it means. (I also discovered that there was a bug in the MODULE these
were being logged as.. they were in fact being logged as 'ACQUISITION'
and not 'LETTER' in the first place.. and as such not filtering
correctly on the viewlog page).
Comment 47 Martin Renvoize 2021-02-25 10:12:04 UTC
Created attachment 117303 [details] [review]
Bug 14233: (follow-up) Add NoticesLog preference

This patch adds a preference to enable/disable the notice template
logging.
Comment 48 Martin Renvoize 2021-02-25 10:12:07 UTC
Created attachment 117304 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update
Comment 49 Kelly McElligott 2021-02-25 13:38:00 UTC
Created attachment 117311 [details] [review]
Bug 14233: Add 'id' field to `letter` table to allow linking

To allow for 'object' linking from the action_logs table we need an
int(11) identifier. This patch updates the letter table to add such a
field as a replacement for the multi-field primary key and moves that
key to a unique constraint.

Test plan
1/ Run the database update
2/ Check that the 'letter' table now includes an 'id' primary key and
'letter_uniq_1' multi-field unique key
3/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 50 Kelly McElligott 2021-02-25 13:38:04 UTC
Created attachment 117312 [details] [review]
Bug 14233: Add logging to notice create/update/delete

This patch adds the call to logaction to record when a notice is added
or changed.

Test plan
1/ Add a new notice and check a corresponding line appears in the
action_logs table.
2/ Update a notice and check that a corresponding line appears in the
action_logs table.
3/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 51 Kelly McElligott 2021-02-25 13:38:08 UTC
Created attachment 117313 [details] [review]
Bug 14233: Add filtering of NOTICES type action logs to the logviewer

This patch adds handling for the new 'NOTICES' type action_logs to the
logviewer page.

Test plan
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.
5/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 52 Kelly McElligott 2021-02-25 13:38:11 UTC
Created attachment 117314 [details] [review]
Bug 14233: Allow comparing notice changes

This patch adds the 'compare' option to the Notices logs.

Test plan
1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
7/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 53 Kelly McElligott 2021-02-25 13:38:15 UTC
Created attachment 117315 [details] [review]
Bug 14233: (follow-up) Fix for delete entire notice

This fixes the overall notice delete option.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 54 Kelly McElligott 2021-02-25 13:38:19 UTC
Created attachment 117316 [details] [review]
Bug 14233: (QA follow-up) Fix typo in viewlog.tt

We had a double ?? in the object link.. Thanks for spotting that David
:)

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 55 Kelly McElligott 2021-02-25 13:38:22 UTC
Created attachment 117317 [details] [review]
Bug 14233: (QA follow-up) Record updated content

We should record the updated template, not the content prior to the
update.. That way we can properly compare between versions to see the
changes made.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 56 Kelly McElligott 2021-02-25 13:38:26 UTC
Created attachment 117318 [details] [review]
Bug 14233: (follow-up) Add notice code to log viewer

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 57 Kelly McElligott 2021-02-25 13:38:30 UTC
Created attachment 117319 [details] [review]
Bug 14233: (follow-up) Clarify 'LetterLog'

The LetterLog logging wasn't clear as to what it was logging, it
actually logs aquisitions related notices being sent.. mostly 'Claims'.

This patch updates LetterLog to ClaimsLog to more clearly reflect what
it means. (I also discovered that there was a bug in the MODULE these
were being logged as.. they were in fact being logged as 'ACQUISITION'
and not 'LETTER' in the first place.. and as such not filtering
correctly on the viewlog page).

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 58 Kelly McElligott 2021-02-25 13:38:34 UTC
Created attachment 117320 [details] [review]
Bug 14233: (follow-up) Add NoticesLog preference

This patch adds a preference to enable/disable the notice template
logging.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 59 Kelly McElligott 2021-02-25 13:38:38 UTC
Created attachment 117321 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 60 David Nind 2021-02-25 13:41:40 UTC
Thanks Martin for all the follow ups and the responses to questions!

I've retested (and Kelly has just signed off!), there is one more thing I noticed:

- I added a notice, made some changes, then deleted.

- These all show up in the log.

- After deleting a notice, the Object column 'name' is changed to the ID (which makes sense, as it is deleted and doesn't know the name).

- If I click on compare for a deleted item it doesn't filter down to all the items with the same ID. The 'Message Log' in the browser shows 'Showing results for' and after that it is blank, with a non-deleted item it shows as 'Showing results for <object name>' with the name of the object showing up in the search box.

- I can still compare by put the ID in the data table search box, then the compare works as expected.

David
Comment 61 Martin Renvoize 2021-02-25 15:50:14 UTC
Created attachment 117345 [details] [review]
Bug 14233: (QA follow-up) Fix filtering issue

As highlighted by David, the filtering was broken for the case where a
notice had been deleted. This patch resolves that issue and also adds in
the 'Log not enabled' display when the logs are not enabled.
Comment 62 Martin Renvoize 2021-02-25 15:50:50 UTC
Thanks for testing again guys, and nice work spotting that David... I've added another followup that should hopefully resolve that final issue :)
Comment 63 David Nind 2021-02-25 20:36:26 UTC
Thanks Martin! Can confirm that that fixes things. Like the little icon saying 'Log not enabled'.
Comment 64 Martin Renvoize 2021-03-11 14:50:13 UTC
Created attachment 118132 [details] [review]
Bug 14233: Add 'id' field to `letter` table to allow linking

To allow for 'object' linking from the action_logs table we need an
int(11) identifier. This patch updates the letter table to add such a
field as a replacement for the multi-field primary key and moves that
key to a unique constraint.

Test plan
1/ Run the database update
2/ Check that the 'letter' table now includes an 'id' primary key and
'letter_uniq_1' multi-field unique key
3/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 65 Martin Renvoize 2021-03-11 14:50:16 UTC
Created attachment 118133 [details] [review]
Bug 14233: Add logging to notice create/update/delete

This patch adds the call to logaction to record when a notice is added
or changed.

Test plan
1/ Add a new notice and check a corresponding line appears in the
action_logs table.
2/ Update a notice and check that a corresponding line appears in the
action_logs table.
3/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 66 Martin Renvoize 2021-03-11 14:50:20 UTC
Created attachment 118134 [details] [review]
Bug 14233: Add filtering of NOTICES type action logs to the logviewer

This patch adds handling for the new 'NOTICES' type action_logs to the
logviewer page.

Test plan
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.
5/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 67 Martin Renvoize 2021-03-11 14:50:24 UTC
Created attachment 118135 [details] [review]
Bug 14233: Allow comparing notice changes

This patch adds the 'compare' option to the Notices logs.

Test plan
1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
7/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 68 Martin Renvoize 2021-03-11 14:50:28 UTC
Created attachment 118136 [details] [review]
Bug 14233: (follow-up) Fix for delete entire notice

This fixes the overall notice delete option.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 69 Martin Renvoize 2021-03-11 14:50:31 UTC
Created attachment 118137 [details] [review]
Bug 14233: (QA follow-up) Fix typo in viewlog.tt

We had a double ?? in the object link.. Thanks for spotting that David
:)

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 70 Martin Renvoize 2021-03-11 14:50:35 UTC
Created attachment 118138 [details] [review]
Bug 14233: (QA follow-up) Record updated content

We should record the updated template, not the content prior to the
update.. That way we can properly compare between versions to see the
changes made.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 71 Martin Renvoize 2021-03-11 14:50:39 UTC
Created attachment 118139 [details] [review]
Bug 14233: (follow-up) Add notice code to log viewer

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 72 Martin Renvoize 2021-03-11 14:50:43 UTC
Created attachment 118140 [details] [review]
Bug 14233: (follow-up) Clarify 'LetterLog'

The LetterLog logging wasn't clear as to what it was logging, it
actually logs aquisitions related notices being sent.. mostly 'Claims'.

This patch updates LetterLog to ClaimsLog to more clearly reflect what
it means. (I also discovered that there was a bug in the MODULE these
were being logged as.. they were in fact being logged as 'ACQUISITION'
and not 'LETTER' in the first place.. and as such not filtering
correctly on the viewlog page).

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 73 Martin Renvoize 2021-03-11 14:50:47 UTC
Created attachment 118141 [details] [review]
Bug 14233: (follow-up) Add NoticesLog preference

This patch adds a preference to enable/disable the notice template
logging.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 74 Martin Renvoize 2021-03-11 14:50:51 UTC
Created attachment 118142 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 75 Martin Renvoize 2021-03-11 14:50:55 UTC
Created attachment 118143 [details] [review]
Bug 14233: (QA follow-up) Fix filtering issue

As highlighted by David, the filtering was broken for the case where a
notice had been deleted. This patch resolves that issue and also adds in
the 'Log not enabled' display when the logs are not enabled.

Signed-off-by: David Nind <david@davidnind.com>
Comment 76 Kyle M Hall 2021-03-11 17:16:40 UTC
Created attachment 118155 [details] [review]
Bug 14233: Add 'id' field to `letter` table to allow linking

To allow for 'object' linking from the action_logs table we need an
int(11) identifier. This patch updates the letter table to add such a
field as a replacement for the multi-field primary key and moves that
key to a unique constraint.

Test plan
1/ Run the database update
2/ Check that the 'letter' table now includes an 'id' primary key and
'letter_uniq_1' multi-field unique key
3/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 77 Kyle M Hall 2021-03-11 17:16:51 UTC
Created attachment 118156 [details] [review]
Bug 14233: Add logging to notice create/update/delete

This patch adds the call to logaction to record when a notice is added
or changed.

Test plan
1/ Add a new notice and check a corresponding line appears in the
action_logs table.
2/ Update a notice and check that a corresponding line appears in the
action_logs table.
3/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 78 Kyle M Hall 2021-03-11 17:16:55 UTC
Created attachment 118157 [details] [review]
Bug 14233: Add filtering of NOTICES type action logs to the logviewer

This patch adds handling for the new 'NOTICES' type action_logs to the
logviewer page.

Test plan
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.
5/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 79 Kyle M Hall 2021-03-11 17:16:59 UTC
Created attachment 118158 [details] [review]
Bug 14233: Allow comparing notice changes

This patch adds the 'compare' option to the Notices logs.

Test plan
1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
7/ Signoff

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 80 Kyle M Hall 2021-03-11 17:17:02 UTC
Created attachment 118159 [details] [review]
Bug 14233: (follow-up) Fix for delete entire notice

This fixes the overall notice delete option.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 81 Kyle M Hall 2021-03-11 17:17:06 UTC
Created attachment 118160 [details] [review]
Bug 14233: (QA follow-up) Fix typo in viewlog.tt

We had a double ?? in the object link.. Thanks for spotting that David
:)

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 82 Kyle M Hall 2021-03-11 17:17:10 UTC
Created attachment 118161 [details] [review]
Bug 14233: (QA follow-up) Record updated content

We should record the updated template, not the content prior to the
update.. That way we can properly compare between versions to see the
changes made.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 83 Kyle M Hall 2021-03-11 17:17:14 UTC
Created attachment 118162 [details] [review]
Bug 14233: (follow-up) Add notice code to log viewer

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 84 Kyle M Hall 2021-03-11 17:17:18 UTC
Created attachment 118163 [details] [review]
Bug 14233: (follow-up) Clarify 'LetterLog'

The LetterLog logging wasn't clear as to what it was logging, it
actually logs aquisitions related notices being sent.. mostly 'Claims'.

This patch updates LetterLog to ClaimsLog to more clearly reflect what
it means. (I also discovered that there was a bug in the MODULE these
were being logged as.. they were in fact being logged as 'ACQUISITION'
and not 'LETTER' in the first place.. and as such not filtering
correctly on the viewlog page).

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 85 Kyle M Hall 2021-03-11 17:17:22 UTC
Created attachment 118164 [details] [review]
Bug 14233: (follow-up) Add NoticesLog preference

This patch adds a preference to enable/disable the notice template
logging.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 86 Kyle M Hall 2021-03-11 17:17:25 UTC
Created attachment 118165 [details] [review]
Bug 14233: [DO NOT PUSH] DBIC Schema File Update

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 87 Kyle M Hall 2021-03-11 17:17:29 UTC
Created attachment 118166 [details] [review]
Bug 14233: (QA follow-up) Fix filtering issue

As highlighted by David, the filtering was broken for the case where a
notice had been deleted. This patch resolves that issue and also adds in
the 'Log not enabled' display when the logs are not enabled.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 88 Kyle M Hall 2021-03-11 17:17:33 UTC
Created attachment 118167 [details] [review]
Bug 14233: (QA follow-up) Fix template filters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 89 Jonathan Druart 2021-03-16 11:14:21 UTC
I think we should move the "clarify 'LetterLog'" patch to a separate bug report.

I don't think "claims" should be a "module" and it will delay these patches if we have the discussion now.
Comment 90 Jonathan Druart 2021-03-16 11:16:32 UTC
(In reply to Jonathan Druart from comment #89)
> I don't think "claims" should be a "module" and it will delay these patches
> if we have the discussion now.

Katrin, Kelly, what's your opinion on this?
Comment 91 Kelly McElligott 2021-03-16 11:26:24 UTC
I agree Jonathan.
Comment 92 Martin Renvoize 2021-03-16 13:36:54 UTC
I can provide a followup to change from 'Claims' to 'Acquisitions' which is the module claims sits in.. I think that would be the cleanest approach.  Currently, without this patch here the functionality is broken.
Comment 93 Martin Renvoize 2021-03-16 13:49:16 UTC
Created attachment 118323 [details] [review]
Bug 14233: (QA follow-up) Claims -> Acquisitions

Change the CLAIMS and ClaimsLog strings to ACQUSITIONS and
AcquisitionsLog to clarify it's module level.
Comment 94 Katrin Fischer 2021-03-16 22:05:12 UTC
The code is not nice to read, but I believe AcquisitionsLog is wrong here, because we also send serial claims in this part of the code (claimissues). So I think we should really leave the clarification to a separate patch as suggested.

NoticesLog makes sense for all changes done in Tools > Notices and slips. We should only deal with that here, the other thing is a separate issue and looks like it requires careful untangling.

It's also something we should not "hide" here so it's easy to spot in release notes. One issue at a time!
Comment 95 Katrin Fischer 2021-03-16 22:07:30 UTC
(In reply to Martin Renvoize from comment #92)
> I can provide a followup to change from 'Claims' to 'Acquisitions' which is
> the module claims sits in.. I think that would be the cleanest approach. 
> Currently, without this patch here the functionality is broken.

Why is the funtionality broken without the patch? (probably missing something from previous discussion as I am not sure how we ended up with wanting to rename it here in the first place)
Comment 96 David Nind 2021-03-17 10:52:36 UTC
(In reply to Katrin Fischer from comment #95)
> (In reply to Martin Renvoize from comment #92)
> > I can provide a followup to change from 'Claims' to 'Acquisitions' which is
> > the module claims sits in.. I think that would be the cleanest approach. 
> > Currently, without this patch here the functionality is broken.
> 
> Why is the funtionality broken without the patch? (probably missing
> something from previous discussion as I am not sure how we ended up with
> wanting to rename it here in the first place)

See comment #34 - you can blame me! (And Martin's follow up patch) 8-)
Comment 97 Martin Renvoize 2021-04-06 18:13:08 UTC
What needs doing to get this moving again.. :( ?
Comment 98 Jonathan Druart 2021-04-07 10:46:54 UTC
(In reply to Martin Renvoize from comment #97)
> What needs doing to get this moving again.. :( ?

Please answer comment 94.

Can you also clarify why we cannot stick to the NoticesLog changes only on this bug report?
Comment 99 Jonathan Druart 2021-04-07 13:06:51 UTC
Created attachment 119291 [details] [review]
Bug 14233: DBIC schema changes
Comment 100 Jonathan Druart 2021-04-07 13:07:00 UTC
Created attachment 119292 [details] [review]
Bug 14233: DB changes - NoticesLog pref + letter.id PK
Comment 101 Jonathan Druart 2021-04-07 13:07:07 UTC
Created attachment 119293 [details] [review]
Bug 14233: Add logging support to notices and slips management

This enhancements adds support for logging changes to notices and slips.
Features include:
- a system preference (NoticesLog) which allows you to enable logging changes for notices and slips
- logging new notices, changes to notices, and deletion of notices
- filtering in the log viewer so you can show all changes for a specific notice
- a comparison feature, so you can see what changes were made to a notice.

Test plan:
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.

1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.
Comment 102 Jonathan Druart 2021-04-07 13:07:16 UTC
Created attachment 119294 [details] [review]
Bug 14233: Select the correct tab
Comment 103 Jonathan Druart 2021-04-07 13:08:43 UTC
Trying to unblock the situation. I squashed the patches and remove the changes related to LetterLog.

So basically we will now have the same behaviour we have in master for claims/LetterLog and a new NoticesLog pref will drive the CRUD for notice templates.

Back to Needs Signoff.
Comment 104 David Nind 2021-04-07 19:20:30 UTC
Created attachment 119312 [details] [review]
Bug 14233: DBIC schema changes

Signed-off-by: David Nind <david@davidnind.com>
Comment 105 David Nind 2021-04-07 19:20:36 UTC
Created attachment 119313 [details] [review]
Bug 14233: DB changes - NoticesLog pref + letter.id PK

Signed-off-by: David Nind <david@davidnind.com>
Comment 106 David Nind 2021-04-07 19:20:42 UTC
Created attachment 119314 [details] [review]
Bug 14233: Add logging support to notices and slips management

This enhancements adds support for logging changes to notices and slips.
Features include:
- a system preference (NoticesLog) which allows you to enable logging changes for notices and slips
- logging new notices, changes to notices, and deletion of notices
- filtering in the log viewer so you can show all changes for a specific notice
- a comparison feature, so you can see what changes were made to a notice.

Test plan:
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.

1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.

Signed-off-by: David Nind <david@davidnind.com>
Comment 107 David Nind 2021-04-07 19:20:48 UTC
Created attachment 119315 [details] [review]
Bug 14233: Select the correct tab

Signed-off-by: David Nind <david@davidnind.com>
Comment 108 Martin Renvoize 2021-04-09 08:13:40 UTC
Comment on attachment 119315 [details] [review]
Bug 14233: Select the correct tab

Review of attachment 119315 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
@@ +340,4 @@
>                                              <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% loopro.object | uri %]" title="Display detail for this authority">Authority [% loopro.object | html %]</a>
>                                          [% ELSIF ( loopro.module == 'NOTICES' ) %]
>                                              [% IF ( loopro.notice ) %]
> +                                                <a href="/cgi-bin/koha/tools/letter.pl?op=add_form&branchcode=[% loopro.notice.branchcode | uri %]&module=[% loopro.notice.module | uri %]&code=[% loopro.notice.code | uri %]#lang_[% loopro.notice.lang | uri %]">Notice [% loopro.notice.title | html %] ([% loopro.notice.code | html %])</a>

I don't understand this change. & => #
Comment 109 Jonathan Druart 2021-04-09 08:21:27 UTC
(In reply to Martin Renvoize from comment #108)
> Comment on attachment 119315 [details] [review] [review]
> Bug 14233: Select the correct tab
> 
> Review of attachment 119315 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
> @@ +340,4 @@
> >                                              <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% loopro.object | uri %]" title="Display detail for this authority">Authority [% loopro.object | html %]</a>
> >                                          [% ELSIF ( loopro.module == 'NOTICES' ) %]
> >                                              [% IF ( loopro.notice ) %]
> > +                                                <a href="/cgi-bin/koha/tools/letter.pl?op=add_form&branchcode=[% loopro.notice.branchcode | uri %]&module=[% loopro.notice.module | uri %]&code=[% loopro.notice.code | uri %]#lang_[% loopro.notice.lang | uri %]">Notice [% loopro.notice.title | html %] ([% loopro.notice.code | html %])</a>
> 
> I don't understand this change. & => #

if you are editing es-ES, on the log viewer the link was not redirecting you the the letter.pl es-ES tab, but the default one instead
Comment 110 Martin Renvoize 2021-04-09 08:28:36 UTC
Created attachment 119359 [details] [review]
Bug 14233: DBIC schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 111 Martin Renvoize 2021-04-09 08:28:41 UTC
Created attachment 119360 [details] [review]
Bug 14233: DB changes - NoticesLog pref + letter.id PK

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 112 Martin Renvoize 2021-04-09 08:28:45 UTC
Created attachment 119361 [details] [review]
Bug 14233: Add logging support to notices and slips management

This enhancements adds support for logging changes to notices and slips.
Features include:
- a system preference (NoticesLog) which allows you to enable logging changes for notices and slips
- logging new notices, changes to notices, and deletion of notices
- filtering in the log viewer so you can show all changes for a specific notice
- a comparison feature, so you can see what changes were made to a notice.

Test plan:
1/ Add some 'Notice' action logs by adding a new notice or updateing an
existing notice
2/ Go to the 'Log viewer' under 'Tools'
3/ Note you can filter by 'Notices'
4/ Confirm the changes you made appear in the logs and that the 'Object'
field links back to the updated notice ready for editing.

1/ Make some changes to at least two different notices to trigger logs
2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
3/ Note, you can now see the 'Compare' checkbox option for these logs
4/ Select the checkbox on one of the notices and note that the results
are now filtered by that specific notice.
5/ Select a second checkbox to expose the option to 'View comparison'
6/ Click 'View comparison' to see the 'Diff view' of the two notices.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 113 Martin Renvoize 2021-04-09 08:28:50 UTC
Created attachment 119362 [details] [review]
Bug 14233: Select the correct tab

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Jonathan Druart 2021-04-09 14:40:46 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 115 Jonathan Druart 2021-04-13 08:46:36 UTC
Created attachment 119510 [details] [review]
Bug 14233: Set default for NoticesLog to 0

To prevent a warning on the about page:
 System preference 'NoticesLog' must be '0' or '1', but is ''.
Comment 116 Jonathan Druart 2021-04-14 08:05:55 UTC
Follow-up pushed to master.