[Bug 32306] New: Hold modifications don't log correctly
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Bug ID: 32306 Summary: Hold modifications don't log correctly Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Transaction logs Assignee: koha-bugs@lists.koha-community.org Reporter: emmi.takkinen@koha-suomi.fi QA Contact: testopia@bugs.koha-community.org When one modifies holds from records holds page, these changes don't log into action_logs table correctly. Instead holds data before modifications is stored to action_logs. To test: 1. Find hold to modify. 2. If possible, locate this hold from action_logs table: SELECT * FROM action_logs WHERE module = "HOLDS" AND object = <reserve_id>; 3. Modify hold, change its pickup library from A to B. 4. Save changes. 5. Take a look at action_logs table. => Modification is saved, but holds pickup library is still A. 6. Modify hold again, e.g add expiration date. 7. Save. => Expiration date is empty, but pickup library is now B. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |emmi.takkinen@koha-suomi.fi |ity.org | Status|NEW |Needs Signoff --- Comment #1 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 144215 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144215&action=edit Bug 32306: Add unit tests In ModReserves logging changes actually happens before we store any of made changes. To test: 1. Run prove t/db_dependent/Reserves.t => Tests should fail. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 --- Comment #2 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 144216 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144216&action=edit Bug 32306: Log after storing changes in ModReserve When one modifies holds from records holds page, these changes don't log into action_logs table correctly. Instead holds data before modifications is stored to action_logs. This patch moves call for logaction after changes have been stored in ModReserve. To test: 1. Find hold to modify. 2. If possible, locate this hold from action_logs table: SELECT * FROM action_logs WHERE module = "HOLDS" AND object = <reserve_id>; 3. Modify hold, change its pickup library from A to B. 4. Save changes. 5. Take a look at action_logs table. => Modification is saved, but holds pickup library is still A. 6. Modify hold again, e.g add expiration date. 7. Save. 8. Find this new modification from action_logs. => In action_logs table expiration date is empty, but pickup library is now B. 9. Apply this patch. 10. Modify hold, change its pickup library back from B to A. 11. Save changes. => In action_logs table holds modification has now a correct pickup library A. Also prove t/db_dependent/Reserves.t. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144215|0 |1 is obsolete| | --- Comment #3 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 145587 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145587&action=edit Bug 32306: Add unit tests In ModReserves logging changes actually happens before we store any of made changes. To test: 1. Run prove t/db_dependent/Reserves.t => Tests should fail. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144216|0 |1 is obsolete| | --- Comment #4 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 145588 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145588&action=edit Bug 32306: Log after storing changes in ModReserve When one modifies holds from records holds page, these changes don't log into action_logs table correctly. Instead holds data before modifications is stored to action_logs. This patch moves call for logaction after changes have been stored in ModReserve. To test: 1. Find hold to modify. 2. If possible, locate this hold from action_logs table: SELECT * FROM action_logs WHERE module = "HOLDS" AND object = <reserve_id>; 3. Modify hold, change its pickup library from A to B. 4. Save changes. 5. Take a look at action_logs table. => Modification is saved, but holds pickup library is still A. 6. Modify hold again, e.g add expiration date. 7. Save. 8. Find this new modification from action_logs. => In action_logs table expiration date is empty, but pickup library is now B. 9. Apply this patch. 10. Modify hold, change its pickup library back from B to A. 11. Save changes. => In action_logs table holds modification has now a correct pickup library A. Also prove t/db_dependent/Reserves.t. Sponsored-by: Koha-Suomi Oy Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |nick@bywatersolutions.com --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Nick, do you see a good reason to log before the exp date is modified? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #5)
Nick, do you see a good reason to log before the exp date is modified?
I think we are inconsistent throughout the code of logging before or after actions. On one hand - storing the before should provide info on what was changed by comparing to the current state On the other, storing after means that the log preserves what was done My preference would be to store both the before and after, or the diff to see what changed (storing both would allow front end diffing) I think at a minimum adding a "BEFORE" or "AFTER" param to the info would at least make it obvious which we stored -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 --- Comment #7 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Is it worth reviving Bug 25159 for standardizing action logs as JSON diffs? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 --- Comment #8 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Nick Clemens from comment #6)
My preference would be to store both the before and after, or the diff to see what changed (storing both would allow front end diffing)
+1 for this. IMO trying to decide which logs should log after and which should log before changes would be almost impossible. Bug 25159 indeed looks like worth reviving as Emily suggested. And don't we already do this with some logs? Yep, MEMBERS MODIFY looks like this:
UPDATED FIELD(S): branchcode: A => B, dateexpiry: 2023-01-01 => 2024-01-01
This bug actually sparked a conversation at our end about saving only diff to the logs and we decided to work on this at some point. But we are currently in the middle of version change, we probably are able to work on this earliest in the summer. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + found => "W", + priority => 0, + itemnumber => undef, Waiting hold without an item number ?? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Not clear if we finished discussing the questions raised here. => In Discussion Intuitively, this unit test feels too long (maintenance). Changing from BEFORE to AFTER for one specific case feels wrong too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
And don't we already do this with some logs? Yep, MEMBERS MODIFY looks like this:
UPDATED FIELD(S): branchcode: A => B, dateexpiry: 2023-01-01 => 2024-01-01
I think changing to the way we log patron changes would be really nice and work well for holds changes. It logs before and after and is quite easy to read. But it looks a little differently for me? { "B_address" : { "after" : "", "before" : null }, "B_address2" : { "after" : "", "before" : null }, "B_city" : { "after" : "", "before" : null }, "B_country" : { "after" : "", "before" : null }, "B_email" : { "after" : "", "before" : null }, "B_phone" : { "after" : "", "before" : null }, "B_state" : { "after" : "", "before" : null }, "B_streetnumber" : { "after" : "", "before" : null }, ... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 --- Comment #12 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- I think it's save to say that this bug is a duplicate of bug 25159? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Katie Bliss <kebliss@dmpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kebliss@dmpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32306 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|In Discussion |RESOLVED --- Comment #13 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- *** This bug has been marked as a duplicate of bug 25159 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org