Bug 37943 - Object creation should be logged with a JSON diff of changes, implement for items
Summary: Object creation should be logged with a JSON diff of changes, implement for i...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Kyle M Hall (khall)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 37522
Blocks: 37940 37944
  Show dependency treegraph
 
Reported: 2024-09-17 13:26 UTC by Kyle M Hall (khall)
Modified: 2024-11-22 21:43 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement will store modification diffs, if the action is set to ADD or CREATE and an "original" object is passed in.
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37943: Log object create as JSON diff, implement for items (2.96 KB, patch)
2024-09-17 13:30 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37943: Log object create as JSON diff, implement for items (3.01 KB, patch)
2024-09-26 15:59 UTC, Sukhmandeep
Details | Diff | Splinter Review
Bug 37943: Log object create as JSON diff, implement for items (3.08 KB, patch)
2024-10-28 15:01 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 Kyle M Hall (khall) 2024-09-17 13:26:00 UTC
We should store newly created objects as diffs just as we do for modifications.
This enhancement will store modification diffs if the action is set to ADD or CREATE, and an "original" object is passed in.
Comment 1 Kyle M Hall (khall) 2024-09-17 13:30:28 UTC
Created attachment 171624 [details] [review]
Bug 37943: Log object create as JSON diff, implement for items

We should store newly created objects as diffs just as we do for modifications.
This enhancement will store modification diffs if the action is set to ADD or CREATE and an "original" object is passed in.

Test Plan:
1) Enable CataloguingLog
2) Create an item
3) Query the database for the newest action log:
   select * from action_logs order by action_id desc limit 1\G
4) Note the diff column is NULL
5) Apply this patch
6) Restart all the things!
7) Create another item
8) Query the database again
9) Note the diff has been created!
Comment 2 Sukhmandeep 2024-09-26 15:59:59 UTC
Created attachment 171996 [details] [review]
Bug 37943: Log object create as JSON diff, implement for items

We should store newly created objects as diffs just as we do for modifications.
This enhancement will store modification diffs if the action is set to ADD or CREATE and an "original" object is passed in.

Test Plan:
1) Enable CataloguingLog
2) Create an item
3) Query the database for the newest action log:
   select * from action_logs order by action_id desc limit 1\G
4) Note the diff column is NULL
5) Apply this patch
6) Restart all the things!
7) Create another item
8) Query the database again
9) Note the diff has been created!

Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com>
Comment 3 Martin Renvoize (ashimema) 2024-10-28 15:01:52 UTC
Created attachment 173549 [details] [review]
Bug 37943: Log object create as JSON diff, implement for items

We should store newly created objects as diffs just as we do for modifications.
This enhancement will store modification diffs if the action is set to ADD or CREATE and an "original" object is passed in.

Test Plan:
1) Enable CataloguingLog
2) Create an item
3) Query the database for the newest action log:
   select * from action_logs order by action_id desc limit 1\G
4) Note the diff column is NULL
5) Apply this patch
6) Restart all the things!
7) Create another item
8) Query the database again
9) Note the diff has been created!

Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Katrin Fischer 2024-10-30 08:02:25 UTC
Can we please get some unit tests for the change to Log.pm?
Comment 5 Katrin Fischer 2024-10-30 10:59:57 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 6 Jonathan Druart 2024-10-30 15:15:37 UTC
I don't think ->store should call discard_changes. If you need an additional fetch then do it before logging, don't you think?
Comment 7 Jonathan Druart 2024-10-30 15:16:23 UTC
(In reply to Jonathan Druart from comment #6)
> I don't think ->store should call discard_changes. If you need an additional
> fetch then do it before logging, don't you think?

Nevermind, forget that. There was one additional fetch already

-    $self->get_from_storage->_after_item_action_hooks({ action => $action });
+    $self->_after_item_action_hooks({ action => $action });
Comment 8 Jonathan Druart 2024-10-30 15:37:21 UTC
t/db_dependent/Circulation/CalcFine.t .. 1/7 
    #   Failed test 'Amount is calculated correctly'
    #   at t/db_dependent/Circulation/CalcFine.t line 271.
    #          got: '0'
    #     expected: '6'
    # Looks like you failed 1 test of 2.


See bug 37943
Comment 9 Kyle M Hall (khall) 2024-10-31 13:46:13 UTC
(In reply to Jonathan Druart from comment #8)
> t/db_dependent/Circulation/CalcFine.t .. 1/7 
>     #   Failed test 'Amount is calculated correctly'
>     #   at t/db_dependent/Circulation/CalcFine.t line 271.
>     #          got: '0'
>     #     expected: '6'
>     # Looks like you failed 1 test of 2.
> 
> 
> See bug 37943

Did this get fixed? Main is passing this test for me.
Comment 10 Jonathan Druart 2024-10-31 13:56:40 UTC
Sorry wrong bug, it was bug 38303.
Comment 11 Lucas Gass (lukeg) 2024-11-22 21:43:45 UTC
Enhancement, no 24.05.x backport.