[Bug 42188] New: Fix display regressions in action logs for biblio changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Bug ID: 42188 Summary: Fix display regressions in action logs for biblio changes Initiative type: --- Sponsorship --- status: 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: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org Depends on: 40135 When we introduced the diff for biblio changes into the action logs, we migrated from MARC::Record->as_formatted to Biblio->unblessed plus some special handling for the attached marc record. Whilst this was following the pattern from elsewhere it actually has some unintended consequences for display (character encoding, field ordering etc) and it is no longer possible to automatically reconstruct the record because a hash does not guarantee fields order and the dollar sign we introduce may occur within the field content. Let's reconsider this and perhaps use marc_in_json. We will need to also take into account non-ascii characters and encoding. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40135 [Bug 40135] Record diff in action logs when modifying a bibliographic record -- 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=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42032 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42032 [Bug 42032] Add diff support to AUTHORITIES action logs -- 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=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |januszop@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- 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=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch Sponsorship status|--- |Unsponsored -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196219&action=edit Bug 42188: Use MARC-in-JSON for biblio action log diffs Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure method, which produces standard MARC-in-JSON. This fixes three problems with the old approach: - Dollar signs used as subfield delimiters in the formatted string were ambiguous when subfield content itself contained " $x " sequences. - Grouping fields by tag into a hash lost inter-field ordering, making lossless reconstruction of the MARC record impossible. - Non-ASCII characters were subject to Perl string context encoding issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly. _unblessed_for_log in Koha::Biblio is updated to return only the three biblio columns that are genuinely independent of the MARC record (frameworkcode, opac_suppressed, serial). The remaining columns (author, title, subtitle, notes, etc.) are denormalised copies of MARC data already captured in the _marc key, so including them produced redundant and noisy diffs. The fmt() helper in viewlog.js is updated to render JavaScript arrays as ordered lists rather than objects with numeric keys, improving the display of MARC subfield arrays in the diff table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196220&action=edit Bug 42188: Add/update unit tests for MARC-in-JSON action log diffs Update the existing _unblessed_for_log() subtest to reflect the new behaviour: MARC-derived columns (title, author, copyrightdate, etc.) are now excluded, while frameworkcode is always included even when it is an empty string (the Default framework). Add a new subtest 'CataloguingLog MARC-in-JSON diff tests' covering: - ADD: diff _marc key is MARC-in-JSON (has leader + fields array), indicators and subfield values are correct, and MARC-derived biblio columns are absent from the diff. - MODIFY: diff _marc key present, MARC-derived columns absent. - DELETE: diff _marc key present with correct MARC-in-JSON structure. Use from_json() instead of decode_json() when reading diffs back from the database, as the DBD layer returns decoded Perl Unicode strings which decode_json (which expects UTF-8 bytes) would reject for any non-ASCII content. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #3 from Janusz Kaczmarek <januszop@gmail.com> --- Martin, and how about modifying (in C4::Log.pm line 99): $infos = "biblio " . Dumper( ref $original eq 'HASH' ? $original : {} ); into: $infos = "biblio " . ( ref $original eq 'HASH' ? encode_json($original) : qw{} ); or even better: $infos = "biblio " . ( ref $original eq 'HASH' ? to_json($original, {utf8 => 1, pretty => 1}) : qw{} ); This would solve the problem of non-ASCII characters and make the Info column more readable, IMO. And would not change the diff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=42188 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196444&action=edit Bug 42188: Use to_json for biblio action log info column Replace Dumper with to_json for the biblio info column in logaction, fixing UTF-8 encoding and extending coverage to ADD and DELETE actions which previously logged only the literal string "biblio". All three actions (ADD, MODIFY, DELETE) now log "biblio " followed by a pretty-printed JSON representation of the relevant state: pre-change for MODIFY, new state for ADD, and deleted state for DELETE. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196219|0 |1 is obsolete| | Attachment #196220|0 |1 is obsolete| | Attachment #196444|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=42188 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196445&action=edit Bug 42188: Use MARC-in-JSON for biblio action log diffs Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure method, which produces standard MARC-in-JSON. This fixes three problems with the old approach: - Dollar signs used as subfield delimiters in the formatted string were ambiguous when subfield content itself contained " $x " sequences. - Grouping fields by tag into a hash lost inter-field ordering, making lossless reconstruction of the MARC record impossible. - Non-ASCII characters were subject to Perl string context encoding issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly. _unblessed_for_log in Koha::Biblio is updated to return only the three biblio columns that are genuinely independent of the MARC record (frameworkcode, opac_suppressed, serial). The remaining columns (author, title, subtitle, notes, etc.) are denormalised copies of MARC data already captured in the _marc key, so including them produced redundant and noisy diffs. The fmt() helper in viewlog.js is updated to render JavaScript arrays as ordered lists rather than objects with numeric keys, improving the display of MARC subfield arrays in the diff table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196446 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196446&action=edit Bug 42188: Add/update unit tests for MARC-in-JSON action log diffs Update the existing _unblessed_for_log() subtest to reflect the new behaviour: MARC-derived columns (title, author, copyrightdate, etc.) are now excluded, while frameworkcode is always included even when it is an empty string (the Default framework). Add a new subtest 'CataloguingLog MARC-in-JSON diff tests' covering: - ADD: diff _marc key is MARC-in-JSON (has leader + fields array), indicators and subfield values are correct, and MARC-derived biblio columns are absent from the diff. - MODIFY: diff _marc key present, MARC-derived columns absent. - DELETE: diff _marc key present with correct MARC-in-JSON structure. Use from_json() instead of decode_json() when reading diffs back from the database, as the DBD layer returns decoded Perl Unicode strings which decode_json (which expects UTF-8 bytes) would reject for any non-ASCII content. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196447 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196447&action=edit Bug 42188: Use to_json for biblio action log info column Replace Dumper with to_json for the biblio info column in logaction, fixing UTF-8 encoding and extending coverage to ADD and DELETE actions which previously logged only the literal string "biblio". All three actions (ADD, MODIFY, DELETE) now log "biblio " followed by a pretty-printed JSON representation of the relevant state: pre-change for MODIFY, new state for ADD, and deleted state for DELETE. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #8 from Janusz Kaczmarek <januszop@gmail.com> --- It seems to me that storing data in the Info field on ADD is inconsistent with the general principle that, for biblio, authorities, and items, this column contains the record state *before* modification. I think that, as before, in the case of ADD this content should be empty. Otherwise, after the first modification, we end up with exactly the same record state in Info. On the other hand, adding Info on DELETE is valuable -- an ordinary librarian no longer needs to refer to deletedbiblio_metadata. What do we think about continuing to store the data here, as before, using as_formatted, given that the overhead is small? Or perhaps we could transform the Info data on the fly, just for display, into a more human-readable format? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196445|0 |1 is obsolete| | Attachment #196446|0 |1 is obsolete| | Attachment #196447|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=42188 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197922 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197922&action=edit Bug 42188: Use MARC-in-JSON for biblio action log diffs Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure method, which produces standard MARC-in-JSON. This fixes three problems with the old approach: - Dollar signs used as subfield delimiters in the formatted string were ambiguous when subfield content itself contained " $x " sequences. - Grouping fields by tag into a hash lost inter-field ordering, making lossless reconstruction of the MARC record impossible. - Non-ASCII characters were subject to Perl string context encoding issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly. _unblessed_for_log in Koha::Biblio is updated to return only the three biblio columns that are genuinely independent of the MARC record (frameworkcode, opac_suppressed, serial). The remaining columns (author, title, subtitle, notes, etc.) are denormalised copies of MARC data already captured in the _marc key, so including them produced redundant and noisy diffs. The fmt() helper in viewlog.js is updated to render JavaScript arrays as ordered lists rather than objects with numeric keys, improving the display of MARC subfield arrays in the diff table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197923 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197923&action=edit Bug 42188: Add/update unit tests for MARC-in-JSON action log diffs Update the existing _unblessed_for_log() subtest to reflect the new behaviour: MARC-derived columns (title, author, copyrightdate, etc.) are now excluded, while frameworkcode is always included even when it is an empty string (the Default framework). Add a new subtest 'CataloguingLog MARC-in-JSON diff tests' covering: - ADD: diff _marc key is MARC-in-JSON (has leader + fields array), indicators and subfield values are correct, and MARC-derived biblio columns are absent from the diff. - MODIFY: diff _marc key present, MARC-derived columns absent. - DELETE: diff _marc key present with correct MARC-in-JSON structure. Use from_json() instead of decode_json() when reading diffs back from the database, as the DBD layer returns decoded Perl Unicode strings which decode_json (which expects UTF-8 bytes) would reject for any non-ASCII content. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197924&action=edit Bug 42188: Use to_json for biblio action log info column Replace Dumper with to_json for the biblio info column in logaction, fixing UTF-8 encoding and extending coverage to ADD and DELETE actions which previously logged only the literal string "biblio". All three actions (ADD, MODIFY, DELETE) now log "biblio " followed by a pretty-printed JSON representation of the relevant state: pre-change for MODIFY, new state for ADD, and deleted state for DELETE. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197925&action=edit Bug 42188: (follow-up) Skip JSON payload in info column on ADD The info column for CATALOGUING action logs is, by convention, the record state before modification -- MODIFY writes the pre-change JSON payload, DELETE writes the state at time of deletion, but ADD has no pre-change state. Writing the new record state into info on ADD duplicates the state that the subsequent MODIFY (if any) captures as its "before" snapshot, and conflicts with that convention. Skip the JSON payload on ADD so info degrades to the bare "biblio" prefix. The diff column already captures the full creation (every field "added" against an empty hashref), so no information is lost. Test plan: 1. prove t/db_dependent/Koha/Biblio.t 2. Observe that the "CataloguingLog MARC-in-JSON diff tests" subtest now also asserts: - ADD info == "biblio" - MODIFY info matches /^biblio \{/ - DELETE info matches /^biblio \{/ 3. Create / modify / delete a record via the staff UI with CataloguingLog on, then check action_logs: the ADD row's info column should read just "biblio". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197926&action=edit Bug 42188: (follow-up) Pretty-render biblio info column in log viewer The CATALOGUING info column for MODIFY/DELETE logs stores "biblio " followed by a pretty-printed JSON dump of the biblio hashref, which includes a MARC-in-JSON _marc structure. Raw, this is noisy and hard for librarians to read in the log viewer. Extract the nested-object formatter already used by the struct-diff renderer into a shared fmtJsonValue() helper, and use it from a new renderCatalogInfo() that runs over each div.loginfo on page load. If the cell text matches "biblio {JSON}" it is replaced with the pretty rendering; otherwise the cell is left untouched so non-catalog info payloads (and the bare "biblio" logged on ADD) are unaffected. Falling back silently on a parse failure means the old non-JSON Dumper-format item payloads continue to display as-is. Test plan: 1. Enable CataloguingLog. 2. Create, edit, and delete a biblio record via the staff UI. 3. Go to Tools -> Log viewer, filter by module=Cataloguing. 4. Verify: - ADD row's info cell shows just "biblio". - MODIFY and DELETE rows' info cells render as a nested list (leader, fields array of tag/indicator/subfield objects, plus frameworkcode/opac_suppressed/serial). - Diff column still renders the before/after struct-diff table. 5. Check another module's log entries (e.g. MEMBERS) are untouched. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- (In reply to Janusz Kaczmarek from comment #8)
It seems to me that storing data in the Info field on ADD is inconsistent with the general principle that, for biblio, authorities, and items, this column contains the record state *before* modification. I think that, as before, in the case of ADD this content should be empty.
Hmm, yes, this was an unintentional side effect.. I've dropped it again in the followup.
On the other hand, adding Info on DELETE is valuable -- an ordinary librarian no longer needs to refer to deletedbiblio_metadata.
This one was intentional, and I've left it in :)
What do we think about continuing to store the data here, as before, using as_formatted, given that the overhead is small? Or perhaps we could transform the Info data on the fly, just for display, into a more human-readable format?
I need to keep the structure format for generating the diff so the easiest approach here was to improve the rendering in viewlog.. see follow-up for that :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_26_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 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=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197922|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=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197923|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=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197924|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=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197925|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=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197926|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=42188 --- Comment #15 from David Nind <david@davidnind.com> --- Created attachment 198041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198041&action=edit Bug 42188: Use MARC-in-JSON for biblio action log diffs Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure method, which produces standard MARC-in-JSON. This fixes three problems with the old approach: - Dollar signs used as subfield delimiters in the formatted string were ambiguous when subfield content itself contained " $x " sequences. - Grouping fields by tag into a hash lost inter-field ordering, making lossless reconstruction of the MARC record impossible. - Non-ASCII characters were subject to Perl string context encoding issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly. _unblessed_for_log in Koha::Biblio is updated to return only the three biblio columns that are genuinely independent of the MARC record (frameworkcode, opac_suppressed, serial). The remaining columns (author, title, subtitle, notes, etc.) are denormalised copies of MARC data already captured in the _marc key, so including them produced redundant and noisy diffs. The fmt() helper in viewlog.js is updated to render JavaScript arrays as ordered lists rather than objects with numeric keys, improving the display of MARC subfield arrays in the diff table. 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=42188 --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 198042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198042&action=edit Bug 42188: Add/update unit tests for MARC-in-JSON action log diffs Update the existing _unblessed_for_log() subtest to reflect the new behaviour: MARC-derived columns (title, author, copyrightdate, etc.) are now excluded, while frameworkcode is always included even when it is an empty string (the Default framework). Add a new subtest 'CataloguingLog MARC-in-JSON diff tests' covering: - ADD: diff _marc key is MARC-in-JSON (has leader + fields array), indicators and subfield values are correct, and MARC-derived biblio columns are absent from the diff. - MODIFY: diff _marc key present, MARC-derived columns absent. - DELETE: diff _marc key present with correct MARC-in-JSON structure. Use from_json() instead of decode_json() when reading diffs back from the database, as the DBD layer returns decoded Perl Unicode strings which decode_json (which expects UTF-8 bytes) would reject for any non-ASCII content. 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=42188 --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 198043 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198043&action=edit Bug 42188: Use to_json for biblio action log info column Replace Dumper with to_json for the biblio info column in logaction, fixing UTF-8 encoding and extending coverage to ADD and DELETE actions which previously logged only the literal string "biblio". All three actions (ADD, MODIFY, DELETE) now log "biblio " followed by a pretty-printed JSON representation of the relevant state: pre-change for MODIFY, new state for ADD, and deleted state for DELETE. 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=42188 --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 198044 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198044&action=edit Bug 42188: (follow-up) Skip JSON payload in info column on ADD The info column for CATALOGUING action logs is, by convention, the record state before modification -- MODIFY writes the pre-change JSON payload, DELETE writes the state at time of deletion, but ADD has no pre-change state. Writing the new record state into info on ADD duplicates the state that the subsequent MODIFY (if any) captures as its "before" snapshot, and conflicts with that convention. Skip the JSON payload on ADD so info degrades to the bare "biblio" prefix. The diff column already captures the full creation (every field "added" against an empty hashref), so no information is lost. Test plan: 1. prove t/db_dependent/Koha/Biblio.t 2. Observe that the "CataloguingLog MARC-in-JSON diff tests" subtest now also asserts: - ADD info == "biblio" - MODIFY info matches /^biblio \{/ - DELETE info matches /^biblio \{/ 3. Create / modify / delete a record via the staff UI with CataloguingLog on, then check action_logs: the ADD row's info column should read just "biblio". 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=42188 --- Comment #19 from David Nind <david@davidnind.com> --- Created attachment 198045 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198045&action=edit Bug 42188: (follow-up) Pretty-render biblio info column in log viewer The CATALOGUING info column for MODIFY/DELETE logs stores "biblio " followed by a pretty-printed JSON dump of the biblio hashref, which includes a MARC-in-JSON _marc structure. Raw, this is noisy and hard for librarians to read in the log viewer. Extract the nested-object formatter already used by the struct-diff renderer into a shared fmtJsonValue() helper, and use it from a new renderCatalogInfo() that runs over each div.loginfo on page load. If the cell text matches "biblio {JSON}" it is replaced with the pretty rendering; otherwise the cell is left untouched so non-catalog info payloads (and the bare "biblio" logged on ADD) are unaffected. Falling back silently on a parse failure means the old non-JSON Dumper-format item payloads continue to display as-is. Test plan: 1. Enable CataloguingLog. 2. Create, edit, and delete a biblio record via the staff UI. 3. Go to Tools -> Log viewer, filter by module=Cataloguing. 4. Verify: - ADD row's info cell shows just "biblio". - MODIFY and DELETE rows' info cells render as a nested list (leader, fields array of tag/indicator/subfield objects, plus frameworkcode/opac_suppressed/serial). - Diff column still renders the before/after struct-diff table. 5. Check another module's log entries (e.g. MEMBERS) are untouched. 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=42188 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |To be added. release notes| | | |(This enhancement relates | |to improvements made to the | |log viewer added in Koha | |26.05, Bug 40135 - Record | |diff in action logs when | |modifying a bibliographic | |record.) Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |lisette@bywatersolutions.co |y.org |m --- Comment #20 from Lisette Scheer <lisette@bywatersolutions.com> --- Works great and is now working as expected again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198041|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=42188 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198042|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=42188 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198043|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=42188 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198044|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=42188 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198045|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=42188 --- Comment #21 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 198054 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198054&action=edit Bug 42188: Use MARC-in-JSON for biblio action log diffs Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure method, which produces standard MARC-in-JSON. This fixes three problems with the old approach: - Dollar signs used as subfield delimiters in the formatted string were ambiguous when subfield content itself contained " $x " sequences. - Grouping fields by tag into a hash lost inter-field ordering, making lossless reconstruction of the MARC record impossible. - Non-ASCII characters were subject to Perl string context encoding issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly. _unblessed_for_log in Koha::Biblio is updated to return only the three biblio columns that are genuinely independent of the MARC record (frameworkcode, opac_suppressed, serial). The remaining columns (author, title, subtitle, notes, etc.) are denormalised copies of MARC data already captured in the _marc key, so including them produced redundant and noisy diffs. The fmt() helper in viewlog.js is updated to render JavaScript arrays as ordered lists rather than objects with numeric keys, improving the display of MARC subfield arrays in the diff table. 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=42188 --- Comment #22 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 198055 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198055&action=edit Bug 42188: Add/update unit tests for MARC-in-JSON action log diffs Update the existing _unblessed_for_log() subtest to reflect the new behaviour: MARC-derived columns (title, author, copyrightdate, etc.) are now excluded, while frameworkcode is always included even when it is an empty string (the Default framework). Add a new subtest 'CataloguingLog MARC-in-JSON diff tests' covering: - ADD: diff _marc key is MARC-in-JSON (has leader + fields array), indicators and subfield values are correct, and MARC-derived biblio columns are absent from the diff. - MODIFY: diff _marc key present, MARC-derived columns absent. - DELETE: diff _marc key present with correct MARC-in-JSON structure. Use from_json() instead of decode_json() when reading diffs back from the database, as the DBD layer returns decoded Perl Unicode strings which decode_json (which expects UTF-8 bytes) would reject for any non-ASCII content. 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=42188 --- Comment #23 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 198056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198056&action=edit Bug 42188: Use to_json for biblio action log info column Replace Dumper with to_json for the biblio info column in logaction, fixing UTF-8 encoding and extending coverage to ADD and DELETE actions which previously logged only the literal string "biblio". All three actions (ADD, MODIFY, DELETE) now log "biblio " followed by a pretty-printed JSON representation of the relevant state: pre-change for MODIFY, new state for ADD, and deleted state for DELETE. 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=42188 --- Comment #24 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 198057 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198057&action=edit Bug 42188: (follow-up) Skip JSON payload in info column on ADD The info column for CATALOGUING action logs is, by convention, the record state before modification -- MODIFY writes the pre-change JSON payload, DELETE writes the state at time of deletion, but ADD has no pre-change state. Writing the new record state into info on ADD duplicates the state that the subsequent MODIFY (if any) captures as its "before" snapshot, and conflicts with that convention. Skip the JSON payload on ADD so info degrades to the bare "biblio" prefix. The diff column already captures the full creation (every field "added" against an empty hashref), so no information is lost. Test plan: 1. prove t/db_dependent/Koha/Biblio.t 2. Observe that the "CataloguingLog MARC-in-JSON diff tests" subtest now also asserts: - ADD info == "biblio" - MODIFY info matches /^biblio \{/ - DELETE info matches /^biblio \{/ 3. Create / modify / delete a record via the staff UI with CataloguingLog on, then check action_logs: the ADD row's info column should read just "biblio". 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=42188 --- Comment #25 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 198058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198058&action=edit Bug 42188: (follow-up) Pretty-render biblio info column in log viewer The CATALOGUING info column for MODIFY/DELETE logs stores "biblio " followed by a pretty-printed JSON dump of the biblio hashref, which includes a MARC-in-JSON _marc structure. Raw, this is noisy and hard for librarians to read in the log viewer. Extract the nested-object formatter already used by the struct-diff renderer into a shared fmtJsonValue() helper, and use it from a new renderCatalogInfo() that runs over each div.loginfo on page load. If the cell text matches "biblio {JSON}" it is replaced with the pretty rendering; otherwise the cell is left untouched so non-catalog info payloads (and the bare "biblio" logged on ADD) are unaffected. Falling back silently on a parse failure means the old non-JSON Dumper-format item payloads continue to display as-is. Test plan: 1. Enable CataloguingLog. 2. Create, edit, and delete a biblio record via the staff UI. 3. Go to Tools -> Log viewer, filter by module=Cataloguing. 4. Verify: - ADD row's info cell shows just "biblio". - MODIFY and DELETE rows' info cells render as a nested list (leader, fields array of tag/indicator/subfield objects, plus frameworkcode/opac_suppressed/serial). - Diff column still renders the before/after struct-diff table. 5. Check another module's log entries (e.g. MEMBERS) are untouched. 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=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | Text to go in the|To be added. |Restore and improve the release notes| |action logs displays for |(This enhancement relates |biblio modifications. |to improvements made to the | |log viewer added in Koha |We use MARC-in-JSON format |26.05, Bug 40135 - Record |to give a structure, |diff in action logs when |properly encoded, output in |modifying a bibliographic |the details field. |record.) | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #26 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #27 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- prove -v /kohadevbox/koha/t/db_dependent/Biblio.t # Subtest: ModBiblio on invalid record 1..4 ok 1 - Modification of biblio was successful and recorded not ok 2 - Biblio data logged in action log info # Failed test 'Biblio data logged in action log info' # at /kohadevbox/koha/t/db_dependent/Biblio.t line 1228. # 'biblio { # "frameworkcode" : "", # "_marc" : { # "fields" : [ # { # "001" : "1180" # }, # { # "005" : "20260506142821.0" # }, # { # "650" : { # "subfields" : [ # { # "a" : "00aD000015937" # } # ], # "ind2" : " ", # "ind1" : " " # } # }, # { # "999" : { # "ind1" : " ", # "ind2" : " ", # "subfields" : [ # { # "c" : "1180" # }, # { # "9" : "1022" # } # ] # } # }, # { # "998" : { # "ind1" : " ", # "ind2" : " ", # "subfields" : [ # { # "a" : "321" # }, # { # "b" : "Jane Doe" # }, # { # "c" : "321" # }, # { # "d" : "Jane Doe" # } # ] # } # } # ], # "leader" : "00130 a2200073 4500" # }, # "opac_suppressed" : 0 # } # ' # doesn't match '(?^u:biblionumber)' ok 3 - Diff column is populated for MODIFY ok 4 - MARC field changes are captured in the diff column # Looks like you failed 1 test of 4. not ok 21 - ModBiblio on invalid record -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 --- Comment #28 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198673 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198673&action=edit Bug 42188: (follow-up) Update ModBiblio test for new info format The 'ModBiblio on invalid record' subtest asserted the action log info column contained 'biblionumber', but Koha::Biblio::_unblessed_for_log restricts the logged hash to frameworkcode, opac_suppressed, and serial. The biblionumber lives in action_logs.object, not in the info JSON. Update the regex to match the new "biblio { ... }" prefix format set by C4::Log::logaction so the assertion confirms biblio data is logged without depending on a field that is intentionally excluded. Test plan: 1. prove t/db_dependent/Biblio.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main --- Comment #29 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- follow-up pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42188 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #30 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