Created attachment 94420 [details]
broken marcxml record
Created attachment 95291 [details] [review] Bug 23846: Display degraded view when MARCXML is invalid (staff detail) When an invalid bibliographic record is imported into the catalogue there is not warning or error. However the bibliographic record detail page will explode (Koha::Biblio::Metadata->record will raise an exception). This patch proposes to catch the exception on this view and display a warning about the situation. Note that editing/saving the record will fix the MARCXML data and so removes the warning (some black magic we should get rid of I suspect). Test plan: - Import a bibliographic record with invalid XML, you can add non printable characters, like 0x1F (CTRL-V 1F with vim) - Go to the detail page => Without this patch you get a 500 => With this patch applied you get a "degraded view" with a warning message, telling you what the error is. Created attachment 95292 [details] [review] Bug 23846: Add a check to the data inconsistencies script This may be quite long for big catalogue, but I think it is a good one to have. Test plan: Same as first patch, then execute search_for_data_inconsistencies.pl Notice the error. Bonjour Dominique, There is certainly much more to do here, but this quick fix will at least prevent the page to crash. Created attachment 95329 [details] [review] Bug 23846: Display degraded view when MARCXML is invalid (staff detail) When an invalid bibliographic record is imported into the catalogue there is not warning or error. However the bibliographic record detail page will explode (Koha::Biblio::Metadata->record will raise an exception). This patch proposes to catch the exception on this view and display a warning about the situation. Note that editing/saving the record will fix the MARCXML data and so removes the warning (some black magic we should get rid of I suspect). Test plan: - Import a bibliographic record with invalid XML, you can add non printable characters, like 0x1F (CTRL-V 1F with vim) - Go to the detail page => Without this patch you get a 500 => With this patch applied you get a "degraded view" with a warning message, telling you what the error is. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 95330 [details] [review] Bug 23846: Add a check to the data inconsistencies script This may be quite long for big catalogue, but I think it is a good one to have. Test plan: Same as first patch, then execute search_for_data_inconsistencies.pl Notice the error. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Works as expected, thanks Jonathan.. Signing off. I tested the change in catalogue/detail.pl and it works as expected. I loaded the failed marcxml into a sample record and had the maintenance script display the problem, and the staff UI display it as well. I have doubts about how useful the degraded view is or in what circumstances it will display anything. I would like it to display references to the introduced maintenance script so people can take actions (for example: It is recommended that you run the following maintenance script to find (this and maybe other) problems on your data: maintenance/search_for_data_inconsistencies.pl /end_suggestion Minor issues: - Missing filter in the tt. - POD error on the script Created attachment 95340 [details] [review] Bug 23846: Handle exception gracefully at the OPAC I do not think we should have the same trick as the intranet, and display a message. This should be enough. Created attachment 95341 [details] [review] Bug 23846: Handle exception gracefully at the OPAC I do not think we should have the same trick as the intranet, and display a message. This should be enough. Created attachment 95342 [details] [review] Bug 23846: Display degraded view when MARCXML is invalid (staff detail) When an invalid bibliographic record is imported into the catalogue there is not warning or error. However the bibliographic record detail page will explode (Koha::Biblio::Metadata->record will raise an exception). This patch proposes to catch the exception on this view and display a warning about the situation. Note that editing/saving the record will fix the MARCXML data and so removes the warning (some black magic we should get rid of I suspect). Test plan: - Import a bibliographic record with invalid XML, you can add non printable characters, like 0x1F (CTRL-V 1F with vim) - Go to the detail page => Without this patch you get a 500 => With this patch applied you get a "degraded view" with a warning message, telling you what the error is. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 95343 [details] [review] Bug 23846: Add a check to the data inconsistencies script This may be quite long for big catalogue, but I think it is a good one to have. Test plan: Same as first patch, then execute search_for_data_inconsistencies.pl Notice the error. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 95344 [details] [review] Bug 23846: Handle exception gracefully at the OPAC I do not think we should have the same trick as the intranet, and display a message. This should be enough. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.05 This is great! Thanks Jonathan, Martin, and Tomas. While I try to handle all encoding issues on the backend, it's really nice to be able to people to self-manage this. |
Created attachment 94419 [details] iso 2709 with problematic record The (UNIMARC) ISO 2709 record in the attached file (26080509.txt) seems to have a problem : the 009 field should contain an ARK identifier but seems to be something else, with a subfield separator in it. When imported into koha, it is said to be imported all right, but when trying to access the notice, we get the following error message : Invalid data, cannot decode object (id=9865, format=marcxml, schema=UNIMARC) Investigating, I found that the marcxml in biblio_metadata.metadata is broken : the content of the <controlfield tag="009"> element is a verbatim copy of the 009 field, including the (non-printable, 0x1F) subfield separator (cf line 9 of the attached marcxml) When I deleted this element in the database, all seems to work fine. I think either the record should be rejected alltogether, with an explicit error message, or the erroneous field should be droped.