Bug 30008 - Software error in details.pl when invalid MARCXML and showing component records
Summary: Software error in details.pl when invalid MARCXML and showing component records
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 29690
Blocks: 29697
  Show dependency treegraph
 
Reported: 2022-02-02 15:10 UTC by Nick Clemens
Modified: 2022-03-25 13:52 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.05


Attachments
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object (1.54 KB, patch)
2022-03-17 15:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object (1.60 KB, patch)
2022-03-18 06:46 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object (1.60 KB, patch)
2022-03-18 06:51 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object (1.69 KB, patch)
2022-03-18 08:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-02-02 15:10:23 UTC
To recreate:
1 - Find a record in the staff catalog
2 - Edit the record
3 - In the 520 notes field, add an ASCII escape character (27 decimal, 1b hex)
    On ubuntu Ctrl+Shift+u, then 1b, enter
4 - Save the record
5 - Set syspref ShowComponentRecords to 'both...'
6 - On record details: New->New child record
7 - Save the record
8 - Attempt to view the parent record in the staff client and opac - FAIL
Comment 1 Jonathan Druart 2022-03-17 15:20:58 UTC
Created attachment 131844 [details] [review]
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object

Test plan:
1 - Find a record in the staff catalog
2 - Edit the record
3 - In the 520 notes field, add an ASCII escape character (27 decimal, 1b hex)
    On ubuntu Ctrl+Shift+u, then 1b, enter
4 - Save the record
5 - Set syspref ShowComponentRecords to 'both...'
6 - On record details: New->New child record
7 - Save the record
8 - Attempt to view the parent record in the staff client and opac
Comment 2 Fridolin Somers 2022-03-18 06:46:30 UTC
Created attachment 131853 [details] [review]
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object

Test plan:
1 - Find a record in the staff catalog
2 - Edit the record
3 - In the 520 notes field, add an ASCII escape character (27 decimal, 1b hex)
    On ubuntu Ctrl+Shift+u, then 1b, enter
4 - Save the record
5 - Set syspref ShowComponentRecords to 'both...'
6 - On record details: New->New child record
7 - Save the record
8 - Attempt to view the parent record in the staff client and opac

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 3 Fridolin Somers 2022-03-18 06:51:43 UTC
Created attachment 131855 [details] [review]
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object

Test plan:
1 - Find a record in the staff catalog
2 - Edit the record
3 - In the 520 notes field, add an ASCII escape character (27 decimal, 1b hex)
    On ubuntu Ctrl+Shift+u, then 1b, enter
4 - Save the record
5 - Set syspref ShowComponentRecords to 'both...'
6 - On record details: New->New child record
7 - Save the record
8 - Attempt to view the parent record in the staff client and opac

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 4 Marcel de Rooy 2022-03-18 08:13:48 UTC
Created attachment 131859 [details] [review]
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object

Test plan:
1 - Find a record in the staff catalog
2 - Edit the record
3 - In the 520 notes field, add an ASCII escape character (27 decimal, 1b hex)
    On ubuntu Ctrl+Shift+u, then 1b, enter
4 - Save the record
5 - Set syspref ShowComponentRecords to 'both...'
6 - On record details: New->New child record
7 - Save the record
8 - Attempt to view the parent record in the staff client and opac

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2022-03-18 08:15:35 UTC
Small note:

+    if ( my $components = $marc_record ? $biblio->get_marc_components(C4::Context->preference('MaxComponentRecords')) : undef ) {

From a theoretical perspective I would prefer to test $biblio here instead of $marc_record.
If there is a biblio object, I can call it and if there is somehow no marc record, then I expect no components back ;)
But it works, ok.
Comment 6 Jonathan Druart 2022-03-18 08:40:39 UTC
(In reply to Marcel de Rooy from comment #5)
> Small note:
> 
> +    if ( my $components = $marc_record ?
> $biblio->get_marc_components(C4::Context->preference('MaxComponentRecords'))
> : undef ) {
> 
> From a theoretical perspective I would prefer to test $biblio here instead
> of $marc_record.
> If there is a biblio object, I can call it and if there is somehow no marc
> record, then I expect no components back ;)
> But it works, ok.

No, that's the point. You can have a $biblio but no $marc_record.
The error will happen later in get_components_query.

It's what we decided earlier on bug 29690, we are dealing on invalid MARC *only* on this view. We add a big warning and data has to be fixed.
There is also misc/maintenance/search_for_data_inconsistencies.pl that can catch such problems.
Comment 7 Marcel de Rooy 2022-03-18 08:57:43 UTC
(In reply to Jonathan Druart from comment #6)
> 
> No, that's the point. You can have a $biblio but no $marc_record.
> The error will happen later in get_components_query.

If there is an error in that sub, it needs a fix :)
Currently, Koha does not like biblios without MARC record. It will certainly trigger problems elsewhere too.
But simply stated, if there is no MARC, there are no components.
And as I said, this is just a theoretical discussion.

> It's what we decided earlier on bug 29690, we are dealing on invalid MARC
> *only* on this view. We add a big warning and data has to be fixed.
> There is also misc/maintenance/search_for_data_inconsistencies.pl that can
> catch such problems.

Good.
Comment 8 Fridolin Somers 2022-03-22 20:19:10 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 9 Kyle M Hall 2022-03-25 13:52:43 UTC
Pushed to 21.11.x for 21.11.05