Bug 37375

For context, one of our client libraries had a MARC framework without 952$8 and after upgrading to 24.05.1 could no longer see items for the records that were catalogued with this framework.

Koha should either way load the items, even if the framework was not configured as expected.

This may explain why some people are having issues after upgrading to Koha 24.05 - see bug 37416. I have added this as a related bug.

I've also changed the bug to major and linked to bug 33568, which introduced the change.

It is interesting to note that the developer tools in Firefox show the undefined property missing, but Chromium doesn't:
- Firefox: Uncaught TypeError: can't access property "str", row._strings.collection_code is undefined
- Chromium: Uncaught TypeError: Cannot read properties of undefined (reading 'str')

The patch no longer applies 8-(..

git bz apply 37375

Bug 37375 - Holdings table not loading if MARC framework is missing certain 952 subfields

169051 - Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc
error: Failed to merge in the changes.
Patch failed at 0001 Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields

Created attachment 170579 [details] [review]
Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields

When you've catalogued a record with MARC framework that is for some reason missing
certain 952 subfields (8, a, b, c, or y), holdings table is no longer loading but
stuck in the "Processing..." loader.

To test, open Koha in two browser tabs:

IN TAB 1 (framework configuration):
1. Create a new MARC framework "TEST", based on the default MARC framework

IN TAB 2 (record view):
2. Create a new record using MARC framework "TEST"
3. Add an item to the newly created record
4. Observe item visible on the record's holdings list

IN TAB 1 (framework configuration):
5. Delete all 952 subfields in framework "TEST" except: 8, a, b, c, y

IN TAB 2 (record view):
6. Refresh page
7. Observe item visible on the record's holdings list
8. Open your browser's console for upcoming error messages

Iterating remaining subfields in following order,
| <FIELD> | <NAME>
------------------------------------------------------
| 952$8   | row._strings.collection_code
| 952$c   | row._strings.location
| 952$a   | row._strings.home_library_id
| 952$b   | row._strings.holding_library_id
| 952$y   | row._strings.item_type_id

IN TAB 1 (framework configuration):
9. Delete subfield <FIELD> in framework "TEST"

IN TAB 2 (record view):
10. Refresh page
11. Observe error "Uncaught TypeError: <NAME> is undefined" (Firefox)
(or Cannot read properties of undefined (reading 'str') (Chromium))

12. Apply patch

IN TAB 2:
13. Refresh page
14. Observe item visible on the record's holdings list

(In reply to David Nind from comment #3)
> The patch no longer applies 8-(..

Fixed, conflicted with Bug 37399. Thanks.

> It is interesting to note that the developer tools in Firefox show the
> undefined property missing, but Chromium doesn't:
> - Firefox: Uncaught TypeError: can't access property "str",
> row._strings.collection_code is undefined
> - Chromium: Uncaught TypeError: Cannot read properties of undefined (reading
> 'str')
> 

Thank you for testing with both browser. I've included this information in the test plan.

Created attachment 170702 [details] [review]
Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields

When you've catalogued a record with MARC framework that is for some reason missing
certain 952 subfields (8, a, b, c, or y), holdings table is no longer loading but
stuck in the "Processing..." loader.

To test, open Koha in two browser tabs:

IN TAB 1 (framework configuration):
1. Create a new MARC framework "TEST", based on the default MARC framework

IN TAB 2 (record view):
2. Create a new record using MARC framework "TEST"
3. Add an item to the newly created record
4. Observe item visible on the record's holdings list

IN TAB 1 (framework configuration):
5. Delete all 952 subfields in framework "TEST" except: 8, a, b, c, y

IN TAB 2 (record view):
6. Refresh page
7. Observe item visible on the record's holdings list
8. Open your browser's console for upcoming error messages

Iterating remaining subfields in following order,
| <FIELD> | <NAME>
------------------------------------------------------
| 952$8   | row._strings.collection_code
| 952$c   | row._strings.location
| 952$a   | row._strings.home_library_id
| 952$b   | row._strings.holding_library_id
| 952$y   | row._strings.item_type_id

IN TAB 1 (framework configuration):
9. Delete subfield <FIELD> in framework "TEST"

IN TAB 2 (record view):
10. Refresh page
11. Observe error "Uncaught TypeError: <NAME> is undefined" (Firefox)
(or Cannot read properties of undefined (reading 'str') (Chromium))

12. Apply patch

IN TAB 2:
13. Refresh page
14. Observe item visible on the record's holdings list

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 170781 [details] [review]
Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields

When you've catalogued a record with MARC framework that is for some reason missing
certain 952 subfields (8, a, b, c, or y), holdings table is no longer loading but
stuck in the "Processing..." loader.

To test, open Koha in two browser tabs:

IN TAB 1 (framework configuration):
1. Create a new MARC framework "TEST", based on the default MARC framework

IN TAB 2 (record view):
2. Create a new record using MARC framework "TEST"
3. Add an item to the newly created record
4. Observe item visible on the record's holdings list

IN TAB 1 (framework configuration):
5. Delete all 952 subfields in framework "TEST" except: 8, a, b, c, y

IN TAB 2 (record view):
6. Refresh page
7. Observe item visible on the record's holdings list
8. Open your browser's console for upcoming error messages

Iterating remaining subfields in following order,
| <FIELD> | <NAME>
------------------------------------------------------
| 952$8   | row._strings.collection_code
| 952$c   | row._strings.location
| 952$a   | row._strings.home_library_id
| 952$b   | row._strings.holding_library_id
| 952$y   | row._strings.item_type_id

IN TAB 1 (framework configuration):
9. Delete subfield <FIELD> in framework "TEST"

IN TAB 2 (record view):
10. Refresh page
11. Observe error "Uncaught TypeError: <NAME> is undefined" (Firefox)
(or Cannot read properties of undefined (reading 'str') (Chromium))

12. Apply patch

IN TAB 2:
13. Refresh page
14. Observe item visible on the record's holdings list

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

I realise this does help fix part of the problem with 24.05+ since bug 33568. So thanks everyone for getting it this far!

However, since I'm not familiar enough with how bugs 33568 works (apart from helping speed up the loading of records with very large numbers of items and being API-based), is this what we want?

To me, this is saying you can still have issues with your underlying frameworks and record and item data, but we'll still load what we have anyway.

I don't think that should hold up this bug in any way, but I think it is perhaps a wider issue that we should think about outside this. Bug 33568 has made the issue of framework and data issues more visible.

Or maybe I'm overthinking this!

As an aside, do we need a bug to address the equivalent issue for a UNIMARC setup with 995(?) subfields? I'm happy to test and create a bug if you think we do.

ok so this is the same as bug 37544.

*** Bug 37544 has been marked as a duplicate of this bug. ***

Please see my patches there, which are similar as yours. However I am wondering if it's a config issue (shouldn't location and ccode exist and be linked with an authorised value category?)

For the item_type_id, holding_library_id and home_library_id I am not sure the test is needed. They subtag must be exist in the framework.

(In reply to Jonathan Druart from comment #12)
> They subtag must be exist in the framework.

The subtags must exist in the framework*

(In reply to Jonathan Druart from comment #12)
> Please see my patches there, which are similar as yours. However I am
> wondering if it's a config issue (shouldn't location and ccode exist and be
> linked with an authorised value category?)

I'm leaning towards this being a configuration issue - I think this is a case where authorized value lists must be used.

I've started a chat in the cataloguing channel to get wider input https://chat.koha-community.org/koha-community/pl/1qbrjph7hbgnikzyk3sajfrxqe

(In reply to David Nind from comment #14)
> (In reply to Jonathan Druart from comment #12)
> > Please see my patches there, which are similar as yours. However I am
> > wondering if it's a config issue (shouldn't location and ccode exist and be
> > linked with an authorised value category?)
> 
> I'm leaning towards this being a configuration issue - I think this is a
> case where authorized value lists must be used.
> 
> I've started a chat in the cataloguing channel to get wider input
> https://chat.koha-community.org/koha-community/pl/1qbrjph7hbgnikzyk3sajfrxqe

I know we've had some patches int he past so location could be used without being linked to an Authorised value category... might be a grey area. I remember it was a patch from Biblibre (Julian or Matthias maybe?) I think being a bit lenient here might be better than being strict.

We could add a warning to alert there are configuration issues with the framework.

Interesting by the way that "Administration -> MARC bibliographic framework test" doesn't complain about missing subfields.

But first I think it is important that we quickly fix the stuck processing loader and display the items regardless of unexpected configuration, and have this patch (or something similar) pushed.

We could then handle the discussed issues in a new Bug.

(In reply to Jonathan Druart from comment #12)
> For the item_type_id, holding_library_id and home_library_id I am not sure
> the test is needed. They subtag must be exist in the framework.
It seems there is currently nothing that prevents user from completely deleting the subfields - I don't know why anyone would do that but as long as they can such cases should be covered and not have the code die unhandled. That's why I added all of them to this patch.

I agree with Lari Taskula suggestions.

Meanwhile, this error forced me to review all the MARC mapping fields and tables. (No revision since 2003 ;) )

In addition to the location error, I also had this one:
row._strings.collection_code is undefined
Finally, it was resolved by associating the CCODE Authorized Value to 995$h (UNIMARC)

I didn't test the Patch because I preferred to correct the fields to don't have problems in the future.

However, I opened a new bug because there are differences in the UNIMARC fields from the documentation and the Wiki:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37772

Thank you all for your help.

Just tested and you can indeed remove 952$b and things seem to work (apart from this table). I was expecting a warning on the about page or the search_for_data_inconsistencies, but no alert.

Created attachment 170857 [details] [review]
Bug 37375: Fix holdings table not loading if MARC framework is missing certain 952 subfields

When you've catalogued a record with MARC framework that is for some reason missing
certain 952 subfields (8, a, b, c, or y), holdings table is no longer loading but
stuck in the "Processing..." loader.

To test, open Koha in two browser tabs:

IN TAB 1 (framework configuration):
1. Create a new MARC framework "TEST", based on the default MARC framework

IN TAB 2 (record view):
2. Create a new record using MARC framework "TEST"
3. Add an item to the newly created record
4. Observe item visible on the record's holdings list

IN TAB 1 (framework configuration):
5. Delete all 952 subfields in framework "TEST" except: 8, a, b, c, y

IN TAB 2 (record view):
6. Refresh page
7. Observe item visible on the record's holdings list
8. Open your browser's console for upcoming error messages

Iterating remaining subfields in following order,
| <FIELD> | <NAME>
------------------------------------------------------
| 952$8   | row._strings.collection_code
| 952$c   | row._strings.location
| 952$a   | row._strings.home_library_id
| 952$b   | row._strings.holding_library_id
| 952$y   | row._strings.item_type_id

IN TAB 1 (framework configuration):
9. Delete subfield <FIELD> in framework "TEST"

IN TAB 2 (record view):
10. Refresh page
11. Observe error "Uncaught TypeError: <NAME> is undefined" (Firefox)
(or Cannot read properties of undefined (reading 'str') (Chromium))

12. Apply patch

IN TAB 2:
13. Refresh page
14. Observe item visible on the record's holdings list

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

I am inclined to let this fix pushed ASAP because it's an important table, but I do not think we should fix all the issues that a bad configuration will create.

Without 952$b other bugs appear on this table (eg. the filter displays the dropdown list with the libraries, but the table shows the branchcode).

Pushed for 24.11!

Well done everyone, thank you!

Hi, can this be ported to 24.05 as well?

Backport +1.  We are running into this as well.

What about the w field?  We're running into an issue where we get a "month out of range" error if the record contains 0000-00-00, which I guess is a default value if nothing is entered.  This field has never been required, but suddenly it is causing problems.

(In reply to Christopher Brannon from comment #23)
> Backport +1.  We are running into this as well.
> 
> What about the w field?  We're running into an issue where we get a "month
> out of range" error if the record contains 0000-00-00, which I guess is a
> default value if nothing is entered.  This field has never been required,
> but suddenly it is causing problems.

You need to fix the 0000-00-00, those are invalid dates and not defaults and should not exist in your database. There is bug 31143 aiming to create a clean-up script for those. Replace with NULL.

Backported to 24.05.x for upcoming 24.05.05

Depends on Bug 33568 not in 23.11.x