[Bug 29690] New: Software error in details.pl when invalid MARCXML
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Bug ID: 29690 Summary: Software error in details.pl when invalid MARCXML Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Cataloging Assignee: koha-bugs@lists.koha-community.org Reporter: fridolin.somers@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. -- 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=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |fridolin.somers@biblibre.co |ity.org |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=29690 Fridolin Somers <fridolin.somers@biblibre.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=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial 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=29690 --- Comment #1 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 128484 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128484&action=edit Bug 29690: Add eval on metadata record fetch in details.pl when invalid MARCXML Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. Test plan : 1) Create a biblio record with invalid MARCXML (see Bug 29690) In koha-testing-docker there is biblionumber=369 2) Go to page cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx 3) You see the page with a message : There is an error with this bibliographic record, the view may be degraded. Error: Invalid data, cannot decode metadata object ... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #2 from Fridolin Somers <fridolin.somers@biblibre.com> --- If we are OK with this implementation, I will amend unit tests (any help is welcome) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27266 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Could we not move the eval up a level into the record accessor itself this catching all cases? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Martin Renvoize from comment #3)
Could we not move the eval up a level into the record accessor itself this catching all cases?
OK, we already do and throw an exception.. hence your evals here.. my mistake -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128484|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128485 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128485&action=edit Bug 29690: Add eval on metadata record fetch in details.pl when invalid MARCXML Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. Test plan : 1) Create a biblio record with invalid MARCXML (see Bug 29690) In koha-testing-docker there is biblionumber=369 2) Go to page cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx 3) You see the page with a message : There is an error with this bibliographic record, the view may be degraded. Error: Invalid data, cannot decode metadata object ... 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=29690 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128486 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128486&action=edit Bug 29690: (follow-up) Catch other cases This patch catches the rest of the cases in Koha::Biblio where `->metadata->record` may fail. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Martin Renvoize <martin.renvoize@ptfs-europe.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=29690 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It looks to me like `opac/opac-ISBDdetail.pl` and `opac/opac-showmarc.pl` are also affected by this.. should we fix those here too.. and perhaps introduce a QA script check for any `->metadata->record` call not wrapped in eval or try/catch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #8 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Martin Renvoize from comment #7)
It looks to me like `opac/opac-ISBDdetail.pl` and `opac/opac-showmarc.pl` are also affected by this.. should we fix those here too.. and perhaps introduce a QA script check for any `->metadata->record` call not wrapped in eval or try/catch?
Mmm lets focus for this bug on regression in detail.pl. We may open other reports :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128487 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128487&action=edit Bug 29690: Unit test for get_components_query -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> ---
1) Create a biblio record with invalid MARCXML (see Bug 29690)
Referring to bug 23846? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Should not we at least warn? There is nothing else more frustrating that investigating a problem and find that it's coming from an exception that is being ignored. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #11)
Should not we at least warn?
Probably a good call.. I hate silent failures... do we not already throw a warning from the parent as part of the exception it throws though? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It would appear not.. so yes, throwing a warning in would be beneficial. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 128488 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128488&action=edit Bug 29690: Always warn if something is wrong with the metadata -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Would something like this patch work for you, Martin? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Looks great to me Jonathan, thanks for taking it on :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128488|0 |1 is obsolete| | --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 128497 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128497&action=edit Bug 29690: Always warn if something is wrong with the metadata -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128485|0 |1 is obsolete| | --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 128530 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128530&action=edit Bug 29690: Add eval on metadata record fetch in details.pl when invalid MARCXML Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. Test plan : 1) Create a biblio record with invalid MARCXML (see Bug 29690) In koha-testing-docker there is biblionumber=369 2) Go to page cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx 3) You see the page with a message : There is an error with this bibliographic record, the view may be degraded. Error: Invalid data, cannot decode metadata object ... Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=29690 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128486|0 |1 is obsolete| | --- Comment #19 from David Nind <david@davidnind.com> --- Created attachment 128531 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128531&action=edit Bug 29690: (follow-up) Catch other cases This patch catches the rest of the cases in Koha::Biblio where `->metadata->record` may fail. 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=29690 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128487|0 |1 is obsolete| | --- Comment #20 from David Nind <david@davidnind.com> --- Created attachment 128532 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128532&action=edit Bug 29690: Unit test for get_components_query 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=29690 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128497|0 |1 is obsolete| | --- Comment #21 from David Nind <david@davidnind.com> --- Created attachment 128533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128533&action=edit Bug 29690: Always warn if something is wrong with the metadata 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=29690 --- Comment #22 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Martin Renvoize from comment #9)
Created attachment 128487 [details] [review] Bug 29690: Unit test for get_components_query
Thanks a lot. We need to also impact other unit tests right ? get_coins, is_serial ... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 128541 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128541&action=edit Bug 29690: Add selenium test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Fridolin Somers from comment #22)
(In reply to Martin Renvoize from comment #9)
Created attachment 128487 [details] [review] [review] Bug 29690: Unit test for get_components_query
Thanks a lot. We need to also impact other unit tests right ? get_coins, is_serial ...
I actually prefer a selenium test, it will catch possible future regressions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29697 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29697 [Bug 29697] Excessive use of StripNonXmlChars -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #25 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Jonathan Druart from comment #24)
(In reply to Fridolin Somers from comment #22)
(In reply to Martin Renvoize from comment #9)
Created attachment 128487 [details] [review] [review] [review] Bug 29690: Unit test for get_components_query
Thanks a lot. We need to also impact other unit tests right ? get_coins, is_serial ...
I actually prefer a selenium test, it will catch possible future regressions.
Super giga. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27266 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27266 [Bug 27266] Move C4::Biblio::GetMarcAuthors to Koha namespace -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=29690 --- Comment #26 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 128578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128578&action=edit Bug 29690: (follow-up) Add eval on metadata record fetch in get_marc_authors() -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #27 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Fridolin Somers from comment #26)
Created attachment 128578 [details] [review] Bug 29690: (follow-up) Add eval on metadata record fetch in get_marc_authors()
@Rmaints : When backporting this, skip this patch if branch does not contain Bug 27266 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Bug 29690 depends on bug 27266, which changed state. Bug 27266 Summary: Move C4::Biblio::GetMarcAuthors to Koha namespace https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27266 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Hayley Pelham <hayleypelham@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hayleypelham@catalyst.net.n | |z --- Comment #28 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Hello, What needs signing off here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #29 from Fridolin Somers <fridolin.somers@biblibre.com> --- ((In reply to Hayley Pelham from comment #28)
Hello,
What needs signing off here?
Last patch : Bug 29690: (follow-up) Add eval on metadata record fetch in get_marc_authors() I bet one can not sign the Selenuim patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Hayley Pelham <hayleypelham@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #30 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- I gave testing the Selenium test regressions.t a go. It failed, here is the output: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/selenium/regressions.t t/db_dependent/selenium/regressions.t .. 1/7 Label not found for "last SKIP" at /usr/share/perl/5.28/Test/More.pm line 1370. Use of uninitialized value $value in string eq at /home/vagrant/kohaclone/C4/Context.pm line 423. # Looks like your test exited with 2 just after 7. t/db_dependent/selenium/regressions.t .. Dubious, test returned 2 (wstat 512, 0x200) All 7 subtests passed (less 7 skipped subtests: 0 okay) Test Summary Report ------------------- t/db_dependent/selenium/regressions.t (Wstat: 512 Tests: 7 Failed: 0) Non-zero exit status: 2 Files=1, Tests=7, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.28 cusr 0.17 csys = 1.47 CPU) Result: FAIL I think I set up the Selenium server correctly, here is the output from another Selenium test: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/selenium/basic_workflow.t t/db_dependent/selenium/basic_workflow.t .. ok All tests successful. Files=1, Tests=22, 1 wallclock secs ( 0.03 usr 0.00 sys + 1.27 cusr 0.11 csys = 1.41 CPU) Please let me know if you think I have missed a step here Cheers, Hayley -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #31 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Hayley, your dev box is not configured correctly for selenium tests. You should first install Selenium::Remote::Driver. However there is a bug as the tests should return green even if the module is not installed. But it's not related to these patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #32 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #31)
However there is a bug as the tests should return green even if the module is not installed. But it's not related to these patches.
Opened bug 29779. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29779 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #33 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Hi Jonathan, Thanks for letting me know. I've gone through and followed this: https://wiki.koha-community.org/wiki/Using_Selenium_with_Koha And made sure to install that module now. I can get a server running, but get an exception when trying to prove the test: org.openqa.selenium.firefox.NotConnectedException: Unable to connect to host 127.0.0.1 on port 7055 after 45000 ms So I may have to leave the testing here to someone else. Are the Selenium wiki docs up to date? Cheers, Hayley -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Tomás Cohen Arazi <tomascohen@gmail.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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128578|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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128530|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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128531|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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128532|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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128533|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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128541|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=29690 --- Comment #34 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129339&action=edit Bug 29690: Add eval on metadata record fetch in details.pl when invalid MARCXML Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. Test plan : 1) Create a biblio record with invalid MARCXML (see Bug 29690) In koha-testing-docker there is biblionumber=369 2) Go to page cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx 3) You see the page with a message : There is an error with this bibliographic record, the view may be degraded. Error: Invalid data, cannot decode metadata object ... Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas 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=29690 --- Comment #35 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129340 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129340&action=edit Bug 29690: (follow-up) Catch other cases This patch catches the rest of the cases in Koha::Biblio where `->metadata->record` may fail. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas 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=29690 --- Comment #36 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129341 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129341&action=edit Bug 29690: Unit test for get_components_query Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas 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=29690 --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129342 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129342&action=edit Bug 29690: Always warn if something is wrong with the metadata Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas 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=29690 --- Comment #38 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129343&action=edit Bug 29690: Add selenium test Signed-off-by: Tomas 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=29690 --- Comment #39 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129344 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129344&action=edit Bug 29690: (follow-up) Add eval on metadata record fetch in get_marc_authors() Signed-off-by: Tomas 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=29690 --- Comment #40 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129345 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129345&action=edit Bug 29690: Fix tests count Signed-off-by: Tomas 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=29690 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|tomascohen@gmail.com |testopia@bugs.koha-communit | |y.org --- Comment #41 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I saw David's signature so was about to PQA it, but it seems it was NSO. Not sure what is the status. Adding my signature as it does the job, looks correct, tests pass and the QA scripts are happy. My only doubt is about Koha::Biblio calling warn... I'm still trying to figure if it should use the logger instead. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #42 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Thinking a bit more about it. We decided to not explode on detail.pl to provide a nice message to the end user. But we decided to not care about the other views, there is a script already to catch such problems, etc. Here I am wondering if we are not actually fixing the problem at the wrong level. Methods should not deal with the errors (or we are going to add checks everywhere), it's controller's job to catch the error IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Minor but worth noting: the patches rename $marc with $record. It should actually be $marc_record I think, $record is too generic. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #44 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129365&action=edit Bug 29690: [ALTERNATIVE PATCH] Prevent detail.pl to crash if MARCXLM in invalid -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #45 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #44)
Created attachment 129365 [details] [review] Bug 29690: [ALTERNATIVE PATCH] Prevent detail.pl to crash if MARCXLM in invalid
Tomas, Martin, Fridolin, what about this patch? It would obsolete the other patches except the selenium tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #46 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Jonathan Druart from comment #45)
(In reply to Jonathan Druart from comment #44)
Created attachment 129365 [details] [review] [review] Bug 29690: [ALTERNATIVE PATCH] Prevent detail.pl to crash if MARCXLM in invalid
Tomas, Martin, Fridolin, what about this patch? It would obsolete the other patches except the selenium tests.
Hi, I like this shorter patch. We deal with a regression, I think the most simple fix is the best. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129339|0 |1 is obsolete| | Attachment #129340|0 |1 is obsolete| | Attachment #129341|0 |1 is obsolete| | Attachment #129342|0 |1 is obsolete| | Attachment #129343|0 |1 is obsolete| | Attachment #129344|0 |1 is obsolete| | Attachment #129345|0 |1 is obsolete| | Attachment #129365|0 |1 is obsolete| | --- Comment #47 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129381 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129381&action=edit Bug 29690: Add selenium test Signed-off-by: Tomas 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=29690 --- Comment #48 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129382&action=edit Bug 29690: Prevent detail.pl to crash if MARCXLM in invalid -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #49 from Fridolin Somers <fridolin.somers@biblibre.com> --- Small typo in commit message : "if MARCXLM in invalid" => MARCXML -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.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=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129381|0 |1 is obsolete| | --- Comment #50 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 129542 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129542&action=edit Bug 29690: Add selenium test Run prove t/db_dependent/selenium/regressions.t on KTD with selenium Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129382|0 |1 is obsolete| | --- Comment #51 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 129543 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129543&action=edit Bug 29690: Prevent detail.pl to crash if MARCXML is invalid Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. Test plan : 1) Create a biblio record with invalid MARCXML (see Bug 29690) In koha-testing-docker there is biblionumber=369 2) Go to page cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx 3) You see the page with a message : There is an error with this bibliographic record, the view may be degraded. Error: Invalid data, cannot decode metadata object ... Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 --- Comment #52 from Fridolin Somers <fridolin.somers@biblibre.com> --- I signoff. Test successful with koha-testing-docker. Fixed number of tests in selenium test file, from 7 to 8. Copied test plan from old patch to new main patch. Fixed typo in main patch message "if MARCXLM in invalid" Go go team :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129542|0 |1 is obsolete| | --- Comment #53 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 129730 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129730&action=edit Bug 29690: Add selenium test Run prove t/db_dependent/selenium/regressions.t on KTD with selenium Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.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=29690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129543|0 |1 is obsolete| | --- Comment #54 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 129731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129731&action=edit Bug 29690: Prevent detail.pl to crash if MARCXML is invalid Bug 23846 added support for invalid MARCXML. But now page details.pl fails again with software error. This comes from several Koha::Biblio calling $self->metadata->record without eval. Test plan : 1) Create a biblio record with invalid MARCXML (see Bug 29690) In koha-testing-docker there is biblionumber=369 2) Go to page cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx 3) You see the page with a message : There is an error with this bibliographic record, the view may be degraded. Error: Invalid data, cannot decode metadata object ... Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.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=29690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #55 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Read over the history to remind myself how we got to this point.. all appear to be in order.. Tested. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.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=29690 --- Comment #56 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #57 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 29970 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30008 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30008 [Bug 30008] Software error in details.pl when invalid MARCXML and showing component records -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30009 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30009 [Bug 30009] Records with invalid MarcXML show notes tab 'Descriptions(1)' but tab is empty -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.05.00 |22.05.00,21.11.03 released in| | CC| |kyle@bywatersolutions.com Status|Pushed to master |Pushed to stable --- Comment #58 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29690 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com Status|Pushed to stable |RESOLVED Resolution|--- |FIXED --- Comment #59 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Missing dependency, not backported to 21.05 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org