[Bug 35099] New: Cannot load records with invalid marcxml
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Bug ID: 35099 Summary: Cannot load records with invalid marcxml Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: MARC Bibliographic data support Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org After bug 26314 we cannot load records with marcxml issues - the get_volumes_query calls Koha::Biblio->metadata->record and does not catch the exception -- 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=35099 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34014 Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com Depends on| |26314 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26314 [Bug 26314] "Volumes: show volumes" showing regardless of whether there are volumes linked to the record https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34014 [Bug 34014] There is no way to fix records with broken MARCXML -- 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=35099 Ron Houk <ron.houk@leblibrary.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ron.houk@leblibrary.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- There is a test to catch that t/db_dependent/selenium/regressions.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157394 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157394&action=edit Bug 35099: Defensive coding for bad MARC21 records This patch modernises the get_marc_volumes method to match the form of get_marc_components. The original code was submitted around a similar time but didn't get the modernisations introduced since. We remove the original localised caching as well as putting an eval around the search_simple_compat call to prevent crashes on bad marc records found in the database. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157394|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157398 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157398&action=edit Bug 35099: Defensive coding for bad MARC21 records This patch modernises the get_marc_volumes method to match the form of get_marc_components. The original code was submitted around a similar time but didn't get the modernisations introduced since. We remove the original localised caching as well as putting an eval around the search_simple_compat call to prevent crashes on bad marc records found in the database. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23846 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23846 [Bug 23846] Handle records with broken MARCXML on the bibliographic detail view -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157423 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157423&action=edit Bug 35099: Prevent crash from invalid marc in the parent record When getting the query for components, we need to parse the MARC::Record. If that fails, we simply should not attempt the component search as the user should see a warning about the degraded MARC To test: prove -v t/db_dependent/selenium/regressions.t It fails apply patch, restart_all prove -v t/db_dependent/selenium/regressions.t It passes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157398|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157430 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157430&action=edit Bug 35099: Defensive coding for bad MARC21 records This patch modernises the get_marc_volumes method to match the form of get_marc_components. The original code was submitted around a similar time but didn't get the modernisations introduced since. We remove the original localised caching as well as putting an eval around the search_simple_compat call to prevent crashes on bad marc records found in the database. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157423|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157431 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157431&action=edit Bug 35099: Prevent crash from invalid marc in the parent record When getting the query for components, we need to parse the MARC::Record. If that fails, we simply should not attempt the component search as the user should see a warning about the degraded MARC To test: prove -v t/db_dependent/selenium/regressions.t It fails apply patch, restart_all prove -v t/db_dependent/selenium/regressions.t It passes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157432 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157432&action=edit Bug 35099: Improve consistency This patch inproves the consistency between get_components_query and get_get_volumes_query methods so they both check for MARC21 and check that the record is valid prior to building the query. We also add a check for !$invalid_marc_record in the staff client details view to match the check for the equivilent compenents check. We need both as the OPAC doesn't do an early check for invalid records in the controller. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. tests are missing 2. 739 return [] if ( C4::Context->preference('marcflavour') ne 'MARC21' ); Why that? Where is it advertised? 3. 781 my $marc; 782 eval { $marc = $self->metadata->record; }; 783 return unless $marc; I disagree with that, and I thought we agreed on it. We must assume the MARC is good, and deal with invalid record on the detail page (and edition now) only. The comment is also wrong, you could call this method from somewhere else, and we just ignore the error, which we should (almost) never do. 4. Finally, isn't this change enough? -my $show_volumes = @{ $biblio->get_marc_volumes(1) } ? 1 : 0; +my $show_volumes = ( !$invalid_marc_record && @{ $biblio->get_marc_volumes(1) } ) ? 1 : 0; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I just tried to make it all more consistent with the existing methods that accomplish the same. The OPAC doesn't do early checks and thus currently explodes I suspect without this patchset.. looks likely only staff was covered to me. I worked in a fix quickly for release.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I confirm that the following change makes the test pass and 369 is accessible with a nice red warning. -my $show_volumes = @{ $biblio->get_marc_volumes(1) } ? 1 : 0; +my $show_volumes = ( !$invalid_marc_record && @{ $biblio->get_marc_volumes(1) } ) ? 1 : 0; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Jonathan Druart <jonathan.druart+koha@gmail.com> 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=35099 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157430|0 |1 is obsolete| | Attachment #157431|0 |1 is obsolete| | Attachment #157432|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 157510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157510&action=edit Bug 35099: (bug 26314 follow-up) Fix detail view for records with invalid MARCXML -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|martin.renvoize@ptfs-europe |jonathan.druart+koha@gmail. |.com |com --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This patch fixes the original issue. If you have other concerns you should open new bug reports and provide tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Nick Clemens <nick@bywatersolutions.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=35099 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157510|0 |1 is obsolete| | --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 157513 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157513&action=edit Bug 35099: (bug 26314 follow-up) Fix detail view for records with invalid MARCXML Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Trivial fix, passing QA - there are larger issues around this, but this gets things working -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #157513|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 157515 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157515&action=edit Bug 35099: (bug 26314 follow-up) Fix detail view for records with invalid MARCXML Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- lol We clashed.. but were doing the same thing :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35117 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35117 [Bug 35117] get_marc_components and get_marc_volumes should be more consistent -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35119 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35119 [Bug 35119] Make bibliographic errors more prominent and match current styling -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 26314 not in 23.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35099 Bug 35099 depends on bug 26314, which changed state. Bug 26314 Summary: "Volumes: show volumes" showing regardless of whether there are volumes linked to the record https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26314 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org