[Bug 40136] New: Record diff in action logs when modifying a patron
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Bug ID: 40136 Summary: Record diff in action logs when modifying a patron Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Blocks: 37940 Patron edits do not yet record a diff in the action logs. They should. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37940 [Bug 37940] [OMNIBUS] All modification logged via action logs should include original to create JSON diff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |lisette@bywatersolutions.co |ity.org |m CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 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=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20956 CC| |martin.renvoize@openfifth.c | |o.uk Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 [Bug 20956] BorrowersLog is not logging permission changes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|lisette@bywatersolutions.co |martin.renvoize@openfifth.c |m |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 194872 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194872&action=edit Bug 40136: Populate diff column for patron CREATE, MODIFY and DELETE logs Previously MEMBERS/CREATE and MEMBERS/DELETE logged empty info with no diff, and MEMBERS/MODIFY stored a custom {field:{before,after}} JSON in the info column with no diff column entry. This updates all three to use the standard logaction pattern: - CREATE: diff({}, patron_data) — all fields shown as added - MODIFY: diff(old_state, new_state) via Struct::Diff; info column now holds the full updated patron state (consistent with other modules) - DELETE: diff(patron_data, {}) — all fields shown as removed - MODIFY_CARDNUMBER: likewise uses standard pattern with diff column Password, lastseen and updated_on are excluded from all log payloads. DateTime objects (inflated date/datetime columns) are stringified to their ISO representation before JSON serialisation. Tests updated to verify changes via the diff column rather than the former custom info format. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=40136 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194872|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 194940 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194940&action=edit Bug 40136: Populate diff column for patron CREATE, MODIFY and DELETE logs Previously MEMBERS/CREATE and MEMBERS/DELETE logged empty info with no diff, and MEMBERS/MODIFY stored a custom {field:{before,after}} JSON in the info column with no diff column entry. This updates all three to use the standard logaction pattern: - CREATE: diff({}, patron_data) — all fields shown as added - MODIFY: diff(old_state, new_state) via Struct::Diff; info column now holds the full updated patron state (consistent with other modules) - DELETE: diff(patron_data, {}) — all fields shown as removed - MODIFY_CARDNUMBER: likewise uses standard pattern with diff column Password, lastseen and updated_on are excluded from all log payloads. DateTime objects (inflated date/datetime columns) are stringified to their ISO representation before JSON serialisation. Tests updated to verify changes via the diff column rather than the former custom info format. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |This enhancement adds a release notes| |'diff' to the action logs | |when modifying a patron | |(creating, modifying, | |changing a card number, and | |deleting). | | | |Previously, no diff was | |recorded (although some | |information was recorded in | |the info column). --- Comment #3 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Before the patch: - Add a new patron account with basic details - Edit the patron account (add an email, change messaging preferences, and so on) - Edit the patron's card number - Delete the patron account 2. Go the log viewer (Tools > Additional tools > Log viewer) and click submit. 3. In the log viewer, show the 'Diff' column (Columns > Diff). 4. Note that when: - Creating a patron: recorded, but no details in the Info and Diff columns - Modifying a patron: details of the change recorded in the Info column but no details in the Diff column - Modifying a card number: details of the change recorded in Info column but no details in the Diff column - Deleting a patron: recorded, but no details in the Info and Diff columns 5. Apply the patch and restart everything (restart_all). 6. Repeat steps 1 and 2. 7. Note that there are now details for each action in the Info and Diff columns. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |lisette@bywatersolutions.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #4 from Lisette Scheer <lisette@bywatersolutions.com> --- Mostly worked great, really happy to see this, but I'm failing it because it records every field in the diff, even when there's no change. With CREATE that makes sense, since there were no fields there before, with the DELETE I could also see having them all, but its unhelpful for MODIFY, especially with how long each entry becomes, as well as if you use the pretty diff it highlights everything, not just what changed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 195036 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195036&action=edit Bug 40136: (follow-up) Refactor MODIFY log to use _unblessed_for_log Replace the manual unblessed + skip_fields + blessed-stringify logic in the MODIFY actionlog section with calls to _unblessed_for_log, which already handles all three concerns. This removes the need for @skip_fields, the dateexpiry normalisation workaround, and the inline blessed checks. Also restrict the log_from/log_to hashrefs to only the changed keys, so the diff column contains only fields that actually changed rather than the full patron state. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=40136 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 195037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195037&action=edit Bug 40136: (follow-up) Fix extended_attributes logaction to populate diff column The extended_attributes setter and add_extended_attribute both logged attribute changes using the old custom {before, after} format passed as the info string, with no $original argument, so the diff column was never populated. Split the existing $change->{before}/$change->{after} into separate $log_from/$log_to hashrefs and call logaction with the standard signature, so Struct::Diff generates the diff column as with other MEMBERS/MODIFY entries. Update tests to reflect the new info column format (after-state only) and remove a stray debug Data::Dumper/print left in the test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 195046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195046&action=edit Bug 40136: (follow-up) Strip undef and empty fields from patron log data Exclude fields with undef or empty string values from _unblessed_for_log so that CREATE and DELETE log diffs only show fields with meaningful values. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36698 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36698 [Bug 36698] Display 'diff' nicely in action logs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|36698 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36698 [Bug 36698] Display 'diff' nicely in action logs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #8 from Lisette Scheer <lisette@bywatersolutions.com> --- Worked great! Thanks Martin. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194940|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195036|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195037|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195046|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #9 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195083 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195083&action=edit Bug 40136: Populate diff column for patron CREATE, MODIFY and DELETE logs Previously MEMBERS/CREATE and MEMBERS/DELETE logged empty info with no diff, and MEMBERS/MODIFY stored a custom {field:{before,after}} JSON in the info column with no diff column entry. This updates all three to use the standard logaction pattern: - CREATE: diff({}, patron_data) — all fields shown as added - MODIFY: diff(old_state, new_state) via Struct::Diff; info column now holds the full updated patron state (consistent with other modules) - DELETE: diff(patron_data, {}) — all fields shown as removed - MODIFY_CARDNUMBER: likewise uses standard pattern with diff column Password, lastseen and updated_on are excluded from all log payloads. DateTime objects (inflated date/datetime columns) are stringified to their ISO representation before JSON serialisation. Tests updated to verify changes via the diff column rather than the former custom info format. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #10 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195084 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195084&action=edit Bug 40136: (follow-up) Refactor MODIFY log to use _unblessed_for_log Replace the manual unblessed + skip_fields + blessed-stringify logic in the MODIFY actionlog section with calls to _unblessed_for_log, which already handles all three concerns. This removes the need for @skip_fields, the dateexpiry normalisation workaround, and the inline blessed checks. Also restrict the log_from/log_to hashrefs to only the changed keys, so the diff column contains only fields that actually changed rather than the full patron state. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #11 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195085 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195085&action=edit Bug 40136: (follow-up) Fix extended_attributes logaction to populate diff column The extended_attributes setter and add_extended_attribute both logged attribute changes using the old custom {before, after} format passed as the info string, with no $original argument, so the diff column was never populated. Split the existing $change->{before}/$change->{after} into separate $log_from/$log_to hashrefs and call logaction with the standard signature, so Struct::Diff generates the diff column as with other MEMBERS/MODIFY entries. Update tests to reflect the new info column format (after-state only) and remove a stray debug Data::Dumper/print left in the test. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #12 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195086&action=edit Bug 40136: (follow-up) Strip undef and empty fields from patron log data Exclude fields with undef or empty string values from _unblessed_for_log so that CREATE and DELETE log diffs only show fields with meaningful values. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #13 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195087 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195087&action=edit Bug 40136: (QA follow-up) Fix tests Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #14 from Lisette Scheer <lisette@bywatersolutions.com> --- Actually I went back and tested attributes more and there's some inconsistency. If you change both regular fields and a patron attribute, only the attribute change is logged. CREATE logs creation + modify of the attribute. MODIFY logs work fine when not editing a patron with an attribute, but if they have an attribute, it only logs that. DELETE only logs the patron data, not the attributes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 195126 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195126&action=edit Bug 40136: (follow-up) Unify patron log entries when attributes change with patron data Three issues were reported with patron action logging: 1. CREATE followed by extended_attributes() produced two log entries (CREATE for patron fields + MODIFY for attributes) instead of one. 2. MODIFY that changed both regular fields and patron attributes produced two separate MODIFY entries instead of one unified entry. 3. DELETE log did not include the patron's extended attribute values. Fix: - logaction() now explicitly returns the stored Koha::ActionLog object so callers can capture the action_id. - Koha::Patron::store() saves the action_id of the CREATE or MODIFY log it writes into $self->{_patron_log_id}. - Koha::Patron::extended_attributes() setter checks for _patron_log_id. If set, it calls _merge_attribute_log() to fold the attribute changes into the existing log entry (updating its info and diff JSON) instead of creating a new MODIFY entry. This covers both the CREATE and MODIFY cases. - Koha::Patron::delete() now iterates extended_attributes before deletion and adds attribute.CODE keys to the patron_data snapshot, so the DELETE diff includes the removed attributes. - New private helper _merge_attribute_log() decodes an existing action log entry's info/diff JSON, merges in the attribute change data using Struct::Diff, and saves the updated entry. - Tests added to t/db_dependent/Koha/Patrons.t covering all three scenarios. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #16 from Lisette Scheer <lisette@bywatersolutions.com> --- Attributes work better now and log properly in all instances. When editing a patron and adding fields that were blank, they are not logged. It does properly show them on creation, but not on editing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #17 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 195159 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195159&action=edit Bug 40136: (follow-up) Log newly-filled fields when editing a patron Previously, _unblessed_for_log strips undef/empty fields, so fields that were blank before an edit were absent from $from_storage. Because @keys was built only from keys %{$from_storage}, any field that transitioned from empty to a value was never iterated and the change went unlogged. Fix by unioning keys from both old and new state so that added, changed, and cleared fields are all detected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #18 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Wow.. this bug just keeps giving aye ;P -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36698 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36698 [Bug 36698] Display 'diff' nicely in action logs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #19 from Lisette Scheer <lisette@bywatersolutions.com> --- This is working great. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195083|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195084|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195085|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195086|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195087|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195126|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195159|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #20 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195218&action=edit Bug 40136: Populate diff column for patron CREATE, MODIFY and DELETE logs Previously MEMBERS/CREATE and MEMBERS/DELETE logged empty info with no diff, and MEMBERS/MODIFY stored a custom {field:{before,after}} JSON in the info column with no diff column entry. This updates all three to use the standard logaction pattern: - CREATE: diff({}, patron_data) — all fields shown as added - MODIFY: diff(old_state, new_state) via Struct::Diff; info column now holds the full updated patron state (consistent with other modules) - DELETE: diff(patron_data, {}) — all fields shown as removed - MODIFY_CARDNUMBER: likewise uses standard pattern with diff column Password, lastseen and updated_on are excluded from all log payloads. DateTime objects (inflated date/datetime columns) are stringified to their ISO representation before JSON serialisation. Tests updated to verify changes via the diff column rather than the former custom info format. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #21 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195219&action=edit Bug 40136: (follow-up) Refactor MODIFY log to use _unblessed_for_log Replace the manual unblessed + skip_fields + blessed-stringify logic in the MODIFY actionlog section with calls to _unblessed_for_log, which already handles all three concerns. This removes the need for @skip_fields, the dateexpiry normalisation workaround, and the inline blessed checks. Also restrict the log_from/log_to hashrefs to only the changed keys, so the diff column contains only fields that actually changed rather than the full patron state. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #22 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195220&action=edit Bug 40136: (follow-up) Fix extended_attributes logaction to populate diff column The extended_attributes setter and add_extended_attribute both logged attribute changes using the old custom {before, after} format passed as the info string, with no $original argument, so the diff column was never populated. Split the existing $change->{before}/$change->{after} into separate $log_from/$log_to hashrefs and call logaction with the standard signature, so Struct::Diff generates the diff column as with other MEMBERS/MODIFY entries. Update tests to reflect the new info column format (after-state only) and remove a stray debug Data::Dumper/print left in the test. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #23 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195221 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195221&action=edit Bug 40136: (follow-up) Strip undef and empty fields from patron log data Exclude fields with undef or empty string values from _unblessed_for_log so that CREATE and DELETE log diffs only show fields with meaningful values. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #24 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195222 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195222&action=edit Bug 40136: (QA follow-up) Fix tests Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #25 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195223 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195223&action=edit Bug 40136: (follow-up) Unify patron log entries when attributes change with patron data Three issues were reported with patron action logging: 1. CREATE followed by extended_attributes() produced two log entries (CREATE for patron fields + MODIFY for attributes) instead of one. 2. MODIFY that changed both regular fields and patron attributes produced two separate MODIFY entries instead of one unified entry. 3. DELETE log did not include the patron's extended attribute values. Fix: - logaction() now explicitly returns the stored Koha::ActionLog object so callers can capture the action_id. - Koha::Patron::store() saves the action_id of the CREATE or MODIFY log it writes into $self->{_patron_log_id}. - Koha::Patron::extended_attributes() setter checks for _patron_log_id. If set, it calls _merge_attribute_log() to fold the attribute changes into the existing log entry (updating its info and diff JSON) instead of creating a new MODIFY entry. This covers both the CREATE and MODIFY cases. - Koha::Patron::delete() now iterates extended_attributes before deletion and adds attribute.CODE keys to the patron_data snapshot, so the DELETE diff includes the removed attributes. - New private helper _merge_attribute_log() decodes an existing action log entry's info/diff JSON, merges in the attribute change data using Struct::Diff, and saves the updated entry. - Tests added to t/db_dependent/Koha/Patrons.t covering all three scenarios. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #26 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 195224 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195224&action=edit Bug 40136: (follow-up) Log newly-filled fields when editing a patron Previously, _unblessed_for_log strips undef/empty fields, so fields that were blank before an edit were absent from $from_storage. Because @keys was built only from keys %{$from_storage}, any field that transitioned from empty to a value was never iterated and the change went unlogged. Fix by unioning keys from both old and new state so that added, changed, and cleared fields are all detected. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42085 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 [Bug 42085] Permissions should be logged when a patron is deleted -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|37940 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37940 [Bug 37940] [OMNIBUS] All modification logged via action logs should include original to create JSON diff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #27 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Thanks for all the hard work! Pushed to main for the next 26.05.00 release as RM Assistant -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #28 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196069 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196069&action=edit Bug 40136: (follow-up) Avoid DateTime inflation in _unblessed_for_log Switch from unblessed() to get_column() so that date/datetime column values are returned as raw ISO strings directly from DBIC's internal storage, without constructing DateTime objects that would only be immediately stringified. This makes _unblessed_for_log cheaper on every call, which is particularly noticeable in the test suite where BorrowersLog defaults to enabled and patron stores are frequent. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #29 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196096 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196096&action=edit Bug 40136: (follow-up) Fix wrong `_result_source` call Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 --- Comment #30 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196118 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196118&action=edit Bug 40136: (follow-up) Fix IsItemIssued.t mock_userenv mock_userenv was called before the test patron was created and used the default borrowernumber (51), which doesn't necessarily exist in the test database. This caused safe_to_delete to crash on the not_same_branch check when Koha::Patrons->find returned undef. Move mock_userenv after patron creation and pass the actual borrowernumber. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #31 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- This will not be backported to 25.11.x due to it being deemed an enhancement or a dependancy not being met -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org