Bug 30008

Summary: Software error in details.pl when invalid MARCXML and showing component records
Product: Koha Reporter: Nick Clemens <nick>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, kyle, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on: 29690    
Bug Blocks: 29697    
Attachments: Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object
Bug 30008: (bug 29690 follow-up) Fix Invalid data, cannot decode metadata object

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