Bug 23846 - Handle records with broken MARCXML on the bibliographic detail view
Summary: Handle records with broken MARCXML on the bibliographic detail view
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic record staging/import (show other bugs)
Version: master
Hardware: PC Linux
: P3 major (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 23425
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-20 16:46 UTC by Dominique Leducq
Modified: 2019-11-15 10:43 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05


Attachments
iso 2709 with problematic record (1.01 KB, application/octet-stream)
2019-10-20 16:46 UTC, Dominique Leducq
Details
broken marcxml record (3.44 KB, application/xml)
2019-10-20 16:48 UTC, Dominique Leducq
Details
Bug 23846: Display degraded view when MARCXML is invalid (staff detail) (2.90 KB, patch)
2019-11-12 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23846: Add a check to the data inconsistencies script (2.04 KB, patch)
2019-11-12 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23846: Display degraded view when MARCXML is invalid (staff detail) (2.96 KB, patch)
2019-11-12 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23846: Add a check to the data inconsistencies script (2.11 KB, patch)
2019-11-12 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23846: Handle exception gracefully at the OPAC (1.72 KB, patch)
2019-11-12 17:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23846: Handle exception gracefully at the OPAC (2.32 KB, patch)
2019-11-12 17:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23846: Display degraded view when MARCXML is invalid (staff detail) (3.01 KB, patch)
2019-11-12 17:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23846: Add a check to the data inconsistencies script (2.16 KB, patch)
2019-11-12 17:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23846: Handle exception gracefully at the OPAC (2.38 KB, patch)
2019-11-12 17:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Dominique Leducq 2019-10-20 16:46:24 UTC
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.
Comment 1 Dominique Leducq 2019-10-20 16:48:29 UTC
Created attachment 94420 [details]
broken marcxml record
Comment 2 Jonathan Druart 2019-11-12 10:22:46 UTC
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.
Comment 3 Jonathan Druart 2019-11-12 10:22:50 UTC
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.
Comment 4 Jonathan Druart 2019-11-12 10:24:38 UTC
Bonjour Dominique,
There is certainly much more to do here, but this quick fix will at least prevent the page to crash.
Comment 5 Martin Renvoize 2019-11-12 16:07:15 UTC
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>
Comment 6 Martin Renvoize 2019-11-12 16:07:19 UTC
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>
Comment 7 Martin Renvoize 2019-11-12 16:08:29 UTC
Works as expected, thanks Jonathan.. Signing off.
Comment 8 Tomás Cohen Arazi 2019-11-12 16:59:45 UTC
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
Comment 9 Jonathan Druart 2019-11-12 17:19:19 UTC
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.
Comment 10 Jonathan Druart 2019-11-12 17:21:02 UTC
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.
Comment 11 Tomás Cohen Arazi 2019-11-12 17:57:18 UTC
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>
Comment 12 Tomás Cohen Arazi 2019-11-12 17:57:22 UTC
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>
Comment 13 Tomás Cohen Arazi 2019-11-12 17:57:26 UTC
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>
Comment 14 Martin Renvoize 2019-11-13 08:04:41 UTC
Nice work!

Pushed to master for 19.11.00
Comment 15 Fridolin SOMERS 2019-11-15 10:43:46 UTC
Pushed to 19.05.x for 19.05.05