Bug 37522 - Logging item modification should record the original version of the item
Summary: Logging item modification should record the original version of the item
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial
Assignee: Janusz Kaczmarek
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 37943
  Show dependency treegraph
 
Reported: 2024-07-30 11:02 UTC by Janusz Kaczmarek
Modified: 2024-11-07 16:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Actions logs for changes to items now include the pre and post change item data in the database.
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37522: Logging item modification should record the original version of the item (2.77 KB, patch)
2024-07-30 11:20 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 37522: Logging item modification should record the original version of the item (2.82 KB, patch)
2024-07-30 13:42 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37522: Unit tests (2.39 KB, patch)
2024-09-16 09:35 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 37522: Logging item modification should record the original version of the item (2.90 KB, patch)
2024-09-16 16:25 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37522: Unit tests (2.47 KB, patch)
2024-09-16 16:25 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37522: Logging item modification should record the original version of the item (2.94 KB, patch)
2024-10-28 15:00 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37522: Unit tests (2.51 KB, patch)
2024-10-28 15:00 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2024-07-30 11:02:29 UTC
When modifying bibliographic record, Koha records in the log the original version of the record.  This allows one to see what has been changed in the record.

However, in case of item modification Koha records in the log the the current version of the item (i.e. the version after the edit).  This is inconsistent with the biblio modification behavior and makes it difficult or even impossible to trace changes (especially in case of the first modif since the initial version is not being recorded). 

The ability to track changes is quite important for library staff and the inconsistency here is not understandable.
Comment 1 Janusz Kaczmarek 2024-07-30 11:20:19 UTC
Created attachment 169855 [details] [review]
Bug 37522: Logging item modification should record the original version of the item

When modifying bibliographic record, Koha records in the log the
original version of the record.  This allows one to see what has been
changed in the record.

However, in case of item modification Koha records in the log the the
current version of the item (i.e. the version after the edit).  This is
inconsistent with the biblio modification behavior and makes it
difficult or even impossible to trace changes (especially in case of the
first modif since the initial version is not being recorded).

This patch makes the item modification logging feature work similarly to
logging modifications in bibliographic records.

Test plan:
==========
1. With CataloguingLog enabled (default in ktd) make a modification of
   an item.
2. Check the Modification log.  You should see as the recorded varsion
   the version of the item after the modification.
3. Apply the patch; restart_all.
4. Repeat p. 1.
5. In the Modification log you should see now the version of the item
   that existed before the recent modification.

Sponsored-by: Ignatianum University in Cracow
Comment 2 Roman Dolny 2024-07-30 13:42:29 UTC
Created attachment 169862 [details] [review]
Bug 37522: Logging item modification should record the original version of the item

When modifying bibliographic record, Koha records in the log the
original version of the record.  This allows one to see what has been
changed in the record.

However, in case of item modification Koha records in the log the the
current version of the item (i.e. the version after the edit).  This is
inconsistent with the biblio modification behavior and makes it
difficult or even impossible to trace changes (especially in case of the
first modif since the initial version is not being recorded).

This patch makes the item modification logging feature work similarly to
logging modifications in bibliographic records.

Test plan:
==========
1. With CataloguingLog enabled (default in ktd) make a modification of
   an item.
2. Check the Modification log.  You should see as the recorded version
   the version of the item after the modification.
3. Apply the patch; restart_all.
4. Repeat p. 1.
5. In the Modification log you should see now the version of the item
   that existed before the recent modification.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 3 Katrin Fischer 2024-07-31 11:30:25 UTC
Hi, can you please detail how it works/should work (before/after)?

If I understand correctly it changes from logging the new version to logging the old version.

I think this would be a behavior change for libraries working with the logs and it would not be "visible" when the change occurred in the logs.

We discussed at one of the last meetings a new coding guideline that new log entries should be using the new JSON diff. Could you have a look please?

https://wiki.koha-community.org/wiki/Coding_Guidelines#ACTN1:_New_additions_to_actions_logs_should_use_the_JSON_Diff_format
Comment 4 Janusz Kaczmarek 2024-07-31 13:06:17 UTC
(In reply to Katrin Fischer from comment #3)
> Hi, can you please detail how it works/should work (before/after)?

Now: 
1) log records previous version of modified biblio record (very useful);
2) log records current version of modified item record (which is inconsistent with biblio behavior; not useful, since we have the current version in the database; leading to data loss in case of the first modif: the initial values go to /dev/null--which is most frustrating when working with the logs).

Also note that the librarian sees the changes both of the biblio record and item record in one single table, but the meaning of the rows is opposite.

It is my strong conviction that is should be consistent (the inconsistency here is not understandable for the librarians) and that it is probably buggy, maybe from the very beginning (the initial value of the item is never recorded in the log). 

> If I understand correctly it changes from logging the new version to logging
> the old version.

Yes--for items.

> I think this would be a behavior change for libraries working with the logs
> and it would not be "visible" when the change occurred in the logs.

Yes, it changes the behavior for better, IMHO, by eliminating the (most probably buggy) inconsistency.

> We discussed at one of the last meetings a new coding guideline that new log
> entries should be using the new JSON diff. Could you have a look please?
> 
> https://wiki.koha-community.org/wiki/Coding_Guidelines#ACTN1:
> _New_additions_to_actions_logs_should_use_the_JSON_Diff_format

The proposed patch make use of the diff feature: after applying it action_logs.diff column is filled as well, according to the proposal (but not visible from the interface).
Comment 5 Katrin Fischer 2024-08-15 09:46:34 UTC
Thanks for the replies and sorry for not coming back to this earlier.
I am resetting to SO and I think another set of eyes would be good.
Also adding Kyle and Nick.
Comment 6 Marcel de Rooy 2024-08-16 06:57:18 UTC
Module changes need tests. Please add.
Comment 7 Janusz Kaczmarek 2024-09-16 09:35:00 UTC
Created attachment 171520 [details] [review]
Bug 37522: Unit tests

NB, since the 6th argument to logaction is now mandatory for logging item
modifications, one had to modify one line from the previous tests by
adding this additional parameter.
Comment 8 Kyle M Hall (khall) 2024-09-16 16:25:33 UTC
Created attachment 171566 [details] [review]
Bug 37522: Logging item modification should record the original version of the item

When modifying bibliographic record, Koha records in the log the
original version of the record.  This allows one to see what has been
changed in the record.

However, in case of item modification Koha records in the log the the
current version of the item (i.e. the version after the edit).  This is
inconsistent with the biblio modification behavior and makes it
difficult or even impossible to trace changes (especially in case of the
first modif since the initial version is not being recorded).

This patch makes the item modification logging feature work similarly to
logging modifications in bibliographic records.

Test plan:
==========
1. With CataloguingLog enabled (default in ktd) make a modification of
   an item.
2. Check the Modification log.  You should see as the recorded version
   the version of the item after the modification.
3. Apply the patch; restart_all.
4. Repeat p. 1.
5. In the Modification log you should see now the version of the item
   that existed before the recent modification.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall (khall) 2024-09-16 16:25:40 UTC
Created attachment 171567 [details] [review]
Bug 37522: Unit tests

NB, since the 6th argument to logaction is now mandatory for logging item
modifications, one had to modify one line from the previous tests by
adding this additional parameter.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall (khall) 2024-09-16 16:26:33 UTC
Looks good! It does make me wonder if we should be storing a JSON diff for object creation as well. I'm not sure if that is within the scope of this bug though.
Comment 11 Janusz Kaczmarek 2024-09-17 10:11:00 UTC
(In reply to Kyle M Hall from comment #10)
> Looks good! It does make me wonder if we should be storing a JSON diff for
> object creation as well. I'm not sure if that is within the scope of this
> bug though.

Thank you, Kyle, for the remark.  I think, this is beyond the idea of this patch.  In fact, we probably should make a general review od all logaction calls and add the 6th argument to consistently create the content ot the diff column, as you made it in bug 25159 for holds.  (In case of item creation and also changes made to bibliographic records this does not seem to be straightforward...)
Comment 12 Kyle M Hall (khall) 2024-09-17 11:48:06 UTC
(In reply to Janusz Kaczmarek from comment #11)
> (In reply to Kyle M Hall from comment #10)
> > Looks good! It does make me wonder if we should be storing a JSON diff for
> > object creation as well. I'm not sure if that is within the scope of this
> > bug though.
> 
> Thank you, Kyle, for the remark.  I think, this is beyond the idea of this
> patch.  In fact, we probably should make a general review od all logaction
> calls and add the 6th argument to consistently create the content ot the
> diff column, as you made it in bug 25159 for holds.  (In case of item
> creation and also changes made to bibliographic records this does not seem
> to be straightforward...)

Agreed. I will file a followup bug report!
Comment 13 Martin Renvoize (ashimema) 2024-10-28 15:00:23 UTC
Created attachment 173547 [details] [review]
Bug 37522: Logging item modification should record the original version of the item

When modifying bibliographic record, Koha records in the log the
original version of the record.  This allows one to see what has been
changed in the record.

However, in case of item modification Koha records in the log the the
current version of the item (i.e. the version after the edit).  This is
inconsistent with the biblio modification behavior and makes it
difficult or even impossible to trace changes (especially in case of the
first modif since the initial version is not being recorded).

This patch makes the item modification logging feature work similarly to
logging modifications in bibliographic records.

Test plan:
==========
1. With CataloguingLog enabled (default in ktd) make a modification of
   an item.
2. Check the Modification log.  You should see as the recorded version
   the version of the item after the modification.
3. Apply the patch; restart_all.
4. Repeat p. 1.
5. In the Modification log you should see now the version of the item
   that existed before the recent modification.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2024-10-28 15:00:26 UTC
Created attachment 173548 [details] [review]
Bug 37522: Unit tests

NB, since the 6th argument to logaction is now mandatory for logging item
modifications, one had to modify one line from the previous tests by
adding this additional parameter.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Katrin Fischer 2024-10-29 15:04:49 UTC
(In reply to Kyle M Hall (khall) from comment #10)
> Looks good! It does make me wonder if we should be storing a JSON diff for
> object creation as well. I'm not sure if that is within the scope of this
> bug though.

We shoudld we have a coding guideline now enforcing this for new logs too :)
Comment 16 Katrin Fischer 2024-10-29 15:05:26 UTC
Please explain the consequence of this change for the release notes, thanks!
Comment 17 Katrin Fischer 2024-10-29 15:17:53 UTC
Pushed for 24.11!

Well done everyone, thank you!