Bug 13775

Summary: Set main headings to mandatory in authority frameworks
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: CatalogingAssignee: Jonathan Druart <jonathan.druart>
Status: Pushed to master --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: bgkriegel, frank.hansen, j.kylmala, J.P.Knight, jonathan.druart, joy, katrin.fischer, m.de.rooy, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
20.05.00
Attachments: Bug 13775: Set main headings to mandatory in authority frameworks
Bug 13775: Set main headings to mandatory in authority frameworks (UNIMARC)
Bug 13775: Set main headings to mandatory in authority frameworks
Bug 13775: Set main headings to mandatory in authority frameworks (UNIMARC)
Bug 13775: Set main headings to mandatory in authority frameworks
Bug 13775: Set main headings to mandatory in authority frameworks (UNIMARC)

Description Nicole C. Engard 2015-03-02 12:32:32 UTC
When cataloging if you use the authority chooser plugin you get the following error when clicking 'choose'



Software error:

Can't call method "field" on an undefined value at /home/nengard/kohaclone/authorities/blinddetail-biblio-search.pl line 82.





My URL looks like this: cgi-bin/koha/authorities/blinddetail-biblio-search.pl?authid=948&index=tag_600_subfield_a_847208_166743&repet=
Comment 1 Jonathan Druart 2015-07-10 11:57:02 UTC
Nicole, 
Which plugin are you talking about?
There is no plugin linked to 600$a by default.
Comment 2 Jonathan Druart 2015-07-10 11:58:22 UTC
Looking at the code, it happens when the authority does not exist (authid=948 for you). Maybe it has been deleted but not removed from the Zebra index yet?
Comment 3 j.kylmala 2015-08-20 07:38:17 UTC
I get also similar error:
Can't call method "subfields" on an undefined value at ~/kohaclone/authorities/blinddetail-biblio-search.pl line 88.

First I edit marc record's field 648 -> chronological term, then I try to add there one by clicking a link on the left side of the field. From there it opens me to a new window: cgi-bin/koha/authorities/auth_finder.pl. In that window I click Create a new authority, I click on the authority creating fields randomly, I don't input there anything but the window puts there some randomly looking data itself, and after that I try to save the new authority and it gives the error.
Comment 4 Jonathan Druart 2015-08-20 09:06:33 UTC
Debug: It's because the field 148 is not filled

# If 148 is not set, @fields = ();
 82     my @fields = $record->field( $auth_type->{auth_tag_to_report} );
# Repet is never set, so $repet = 0
 83     my $repet = ($query->param('repet') || 1) - 1;
 84     my $field = $fields[$repet];
 85     
 86     # Get all values for each distinct subfield
 87     my %subfields;
if 148 is not set, boom here
 88     for ( $field->subfields ) {
Comment 5 Jonathan Druart 2015-09-02 14:13:31 UTC
Should we set 148 as mandatory for the CHRON_TERM auth type?
Comment 6 Katrin Fischer 2015-09-02 21:13:50 UTC
Hm, sounds good to me!
Comment 7 Jonathan Druart 2015-09-15 10:06:35 UTC
I can make the change, globally.
Someone should tell me which fields should be mandatory for each authority type.
Comment 8 Katrin Fischer 2017-08-15 18:09:26 UTC
Hi Jonathan, those are the ones for MARC21:

SELECT authtypecode, auth_tag_to_report FROM koha_kohadev.auth_types where authtypecode != ""

'CHRON_TERM', '148'
'CORPO_NAME', '110'
'GENRE/FORM', '155'
'GEOGR_NAME', '151'
'MEETI_NAME', '111'
'PERSO_NAME', '100'
'TOPIC_TERM', '150'
'UNIF_TITLE', '130'
Comment 9 Martin Renvoize 2019-11-07 21:49:03 UTC
Wondering if this one is still relevant and whether Katrins information was enough and you wanted to take this forward Jonathan?
Comment 10 Jonathan Druart 2019-11-08 10:11:12 UTC
(In reply to Katrin Fischer from comment #8)
> Hi Jonathan, those are the ones for MARC21:
> 
> SELECT authtypecode, auth_tag_to_report FROM koha_kohadev.auth_types where
> authtypecode != ""
> 
> 'CHRON_TERM', '148'
> 'CORPO_NAME', '110'
> 'GENRE/FORM', '155'
> 'GEOGR_NAME', '151'
> 'MEETI_NAME', '111'
> 'PERSO_NAME', '100'
> 'TOPIC_TERM', '150'
> 'UNIF_TITLE', '130'

It that correct for all marc flavors? Only for existing installs I guess?
Fix should be provided for existing installs, at code level then. What kind message should we display to the user?
Comment 11 Katrin Fischer 2019-11-24 14:05:29 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Katrin Fischer from comment #8)
> > Hi Jonathan, those are the ones for MARC21:
> > 
> > SELECT authtypecode, auth_tag_to_report FROM koha_kohadev.auth_types where
> > authtypecode != ""
> > 
> > 'CHRON_TERM', '148'
> > 'CORPO_NAME', '110'
> > 'GENRE/FORM', '155'
> > 'GEOGR_NAME', '151'
> > 'MEETI_NAME', '111'
> > 'PERSO_NAME', '100'
> > 'TOPIC_TERM', '150'
> > 'UNIF_TITLE', '130'
> 
> It that correct for all marc flavors? Only for existing installs I guess?
> Fix should be provided for existing installs, at code level then. What kind
> message should we display to the user?

This only applies to MARC21 - I am not sure if UNIMARC is different, but as it usually is... the SQL I provided could help with UNIMARC:

SELECT authtypecode, auth_tag_to_report FROM koha_kohadev.auth_types where authtypecode != ""
Comment 12 Katrin Fischer 2019-12-12 07:23:22 UTC
Lowering severity here as this can be easily avoided by updating your configuration (setting fields to mandatory). Would still be a nice addition to our sample data.
Comment 13 Jonathan Druart 2019-12-12 09:18:52 UTC
Created attachment 96213 [details] [review]
Bug 13775: Set main headings to mandatory in authority frameworks

For MARC21 we need to set the following tags as mandatory:
  CHRON_TERM => 148
  CORPO_NAME => 110
  GENRE/FORM => 155
  GEOGR_NAME => 151
  MEETI_NAME => 111
  PERSO_NAME => 100
  TOPIC_TERM => 150
  UNIF_TITLE => 130

Note that it also fixes an error on the UI, which was initially reported
on bug 13775.

Test plan:
- Create a fresh install in the language of your choice.
- Confirm that when you create a new authority you cannot save without
entering a value for the main heading

- Edit marc record's field 648 -> chronological term
- Try to add there one by clicking a link on the left side of the field.
From there it opens me to a new window, click "Create a new authority"
Click on the authority creating fields randomly, save.
=> Without this patch an error occurred "Can't call method "subfields"
on an undefined value at authorities/blinddetail-biblio-search.pl line 88."
Comment 14 Jonathan Druart 2019-12-12 09:18:56 UTC
Created attachment 96214 [details] [review]
Bug 13775: Set main headings to mandatory in authority frameworks (UNIMARC)

Same as previously, for UNNIMARC

For unimarc_complet:
CLASS => 245
CO => 210
CO_UNI_TI => 235
EXP => 232
FAM => 220
GENRE/FORM => 280
NAME_COL => 245
NP => 200
NTEXP => 242
NTWORK => 241
PA => 260
PERS => 223
PUB => 217
SAUTTIT => 240
SNC => 250
SNG => 215
TM => 216
TU => 230
WORK => 231

For unimarc_lecture_pub:
CO => 210
NP => 200
SAUT => 200
SAUTTIT => 240
SCO => 210
SNC => 250
SNG => 215
STU => 230
TU => 230
Comment 15 Jonathan Druart 2019-12-12 09:19:14 UTC
(In reply to Katrin Fischer from comment #12)
> Lowering severity here as this can be easily avoided by updating your
> configuration (setting fields to mandatory). Would still be a nice addition
> to our sample data.

It was on my list for today :)
Comment 16 Jon Knight 2020-02-26 17:17:51 UTC
I've just looked at this to sign it off and applied it on the Bywater Solutions sandbox.  The patch applied OK, but when I create a new authority it doesn't seem to enforce the main heading restriction.  For example I just created a new Corporate Name authority with no 110 field:

Authority #1709 (Corporate Name)
This authority is not used in any records.
0
000 - LEADER
  @ 00188nz a2200097n 4500

001 - CONTROL NUMBER
  @ 1709

003 - CONTROL NUMBER IDENTIFIER
  @ OSt

005 - DATE AND TIME OF LATEST TRANSACTION
  @ 20200226170818.0

008 - FIXED-LENGTH DATA ELEMENTS
  @ 200226|| aca||aabn | a|a d

040 ## - CATALOGING SOURCE
  a Original cataloging OSt


Same with a new Chronological Term and Personal Name authorities.  I've checked that the patch is applied (by trying to apply it again and getting the error that it is already applied) and restarted all the service just in case, but the same behaviour.  Is there a configuration option that needs to be set first (which should probably be in the test plan)?
Comment 17 Martin Renvoize 2020-02-26 22:23:32 UTC
I believe the change is just for new installs John, so to see it in a sandbox you'll need to reset the database and run through the installer. That will test that the updated defaults are loaded for new installs.
Comment 18 Bernardo Gonzalez Kriegel 2020-03-17 15:24:22 UTC
Created attachment 100908 [details] [review]
Bug 13775: Set main headings to mandatory in authority frameworks

For MARC21 we need to set the following tags as mandatory:
  CHRON_TERM => 148
  CORPO_NAME => 110
  GENRE/FORM => 155
  GEOGR_NAME => 151
  MEETI_NAME => 111
  PERSO_NAME => 100
  TOPIC_TERM => 150
  UNIF_TITLE => 130

Note that it also fixes an error on the UI, which was initially reported
on bug 13775.

Test plan:
- Create a fresh install in the language of your choice.
- Confirm that when you create a new authority you cannot save without
entering a value for the main heading

- Edit marc record's field 648 -> chronological term
- Try to add there one by clicking a link on the left side of the field.
From there it opens me to a new window, click "Create a new authority"
Click on the authority creating fields randomly, save.
=> Without this patch an error occurred "Can't call method "subfields"
on an undefined value at authorities/blinddetail-biblio-search.pl line 88."

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Fixed conflict in english version.
Comment 19 Bernardo Gonzalez Kriegel 2020-03-17 15:24:26 UTC
Created attachment 100909 [details] [review]
Bug 13775: Set main headings to mandatory in authority frameworks (UNIMARC)

Same as previously, for UNNIMARC

For unimarc_complet:
CLASS => 245
CO => 210
CO_UNI_TI => 235
EXP => 232
FAM => 220
GENRE/FORM => 280
NAME_COL => 245
NP => 200
NTEXP => 242
NTWORK => 241
PA => 260
PERS => 223
PUB => 217
SAUTTIT => 240
SNC => 250
SNG => 215
TM => 216
TU => 230
WORK => 231

For unimarc_lecture_pub:
CO => 210
NP => 200
SAUT => 200
SAUTTIT => 240
SCO => 210
SNC => 250
SNG => 215
STU => 230
TU => 230

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 20 Kyle M Hall 2020-03-20 13:20:29 UTC
Created attachment 101104 [details] [review]
Bug 13775: Set main headings to mandatory in authority frameworks

For MARC21 we need to set the following tags as mandatory:
  CHRON_TERM => 148
  CORPO_NAME => 110
  GENRE/FORM => 155
  GEOGR_NAME => 151
  MEETI_NAME => 111
  PERSO_NAME => 100
  TOPIC_TERM => 150
  UNIF_TITLE => 130

Note that it also fixes an error on the UI, which was initially reported
on bug 13775.

Test plan:
- Create a fresh install in the language of your choice.
- Confirm that when you create a new authority you cannot save without
entering a value for the main heading

- Edit marc record's field 648 -> chronological term
- Try to add there one by clicking a link on the left side of the field.
From there it opens me to a new window, click "Create a new authority"
Click on the authority creating fields randomly, save.
=> Without this patch an error occurred "Can't call method "subfields"
on an undefined value at authorities/blinddetail-biblio-search.pl line 88."

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Fixed conflict in english version.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall 2020-03-20 13:20:38 UTC
Created attachment 101105 [details] [review]
Bug 13775: Set main headings to mandatory in authority frameworks (UNIMARC)

Same as previously, for UNNIMARC

For unimarc_complet:
CLASS => 245
CO => 210
CO_UNI_TI => 235
EXP => 232
FAM => 220
GENRE/FORM => 280
NAME_COL => 245
NP => 200
NTEXP => 242
NTWORK => 241
PA => 260
PERS => 223
PUB => 217
SAUTTIT => 240
SNC => 250
SNG => 215
TM => 216
TU => 230
WORK => 231

For unimarc_lecture_pub:
CO => 210
NP => 200
SAUT => 200
SAUTTIT => 240
SCO => 210
SNC => 250
SNG => 215
STU => 230
TU => 230

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Martin Renvoize 2020-03-20 15:43:15 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 23 Joy Nelson 2020-04-03 21:10:25 UTC
does not apply to 19.11.x branch  not backported