Bug 28022

Summary: MARC subfield 9 not honoring visibility
Product: Koha Reporter: Thomas Klausner <domm>
Component: CatalogingAssignee: Thomas Klausner <domm>
Status: Pushed to master --- QA Contact: Kyle M Hall <kyle>
Severity: enhancement    
Priority: P5 - low CC: dcook, jonathan.druart+koha, kyle, m.de.rooy, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=2206
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.11.00
Attachments: only show subfield "9" if subfield "a" is visibile (bug 28022)
Bug 28022: Only show subfield "9" if subfield "a" is visible
Bug 28022: Only show subfield "9" if subfield "a" is visible

Description Thomas Klausner 2021-03-23 11:15:05 UTC
It is not possible to hide eg field 600 (and several more) in the editor, because addbiblio.pl line 346 implements a special case (based on bug 2206, according to a code comment there): If there is a subfield '9' and a subfield 'a', and subfield 'a' has an 'authtypecode' set, a 'marc_value' is set, which seems to always force the display of the field (and subfield), even though none of the 'visibility' checkboxes (and esp not "Editor") are set.

The special case seems to ignore 'visibility' / 'hidden' completely.

I don't know enough about Koha/MARC internals to assess if this is intended behavior, but I would find it more sane if the condition would be changed to actually use the value of the Editor-checkbox.

The only other way I found to actually disable / hide field 600 was to remove the authtypecode from 600.a, which seems a bit harsh.

After reading more code in addbiblio.pl I see that this special case is implemented in several places. So I assume / fear this is intended behavior, and the correct "solution" is to in face remove the authtypecode?

Greetings,
Thomas Klausner
Comment 1 Katrin Fischer 2021-03-23 22:06:10 UTC
I believe the issue here is that if we hide the $9 which has the id of the linked authority record, the linking won't happen. So it is required to be visible.

I am not sure if the authority linker could be changed to also work with a hidden field, but I think it would require some work at least?
Comment 2 Thomas Klausner 2021-03-24 08:25:47 UTC
Would it work if we consider the visibility of $a?

i.e. change:

$subfield eq "9" and
           exists($tagslib->{$tag}->{'a'}->{authtypecode}) and
           defined($tagslib->{$tag}->{'a'}->{authtypecode}) and
           $tagslib->{$tag}->{'a'}->{authtypecode} ne ''


to something like 

$subfield eq "9" and
           is_visibile($tagslib->{$tag}->{'a'}) and
           exists($tagslib->{$tag}->{'a'}->{authtypecode}) and
           defined($tagslib->{$tag}->{'a'}->{authtypecode}) and
           $tagslib->{$tag}->{'a'}->{authtypecode} ne ''

where is_visibile would check the value of `hidden` of $tag->{a} ?

(probably this wouldn't even need to be a function, but just some bit-twiddling on `hidden`)

I was going to test something like this today. But if you say that this might have implications beyond the visibility of a field in the editor, we might end up removing the authtypecode from the various fields (which feels quite wrong..)

Greetings,
domm
Comment 3 Thomas Klausner 2021-03-29 12:10:55 UTC
I have now implemented a patch that adds a workaround the the workaround (or another corner-case to the corner-case?):

https://github.com/domm/Koha/commit/d19abcc23d5793becdd5edca26c4bd2d47fcd33f

In the three places where `cataloguing/addbiblio.pl` checks if the subfield `9` has a subfield `a` with an `authtypecode`, I have added an additional check for 
subfield `a`->`hidden`.

This works insofar as the fields (tags) are now no longer shown in the editor when subfield `a` does not have the `Editor` checkbox ticked.

But I have no idea if this has any implications on other parts of Koha.

So if somebody could please review the patch and comment on whether this the patch is acceptable or not (or what needs to be changed), I'd be very grateful!

I'm also not really sure if you want the patch via a github pull request, or via a plain old patch per mail (or some other way?)

Greetings,
Thomas Klausner
Comment 4 David Cook 2021-03-31 22:49:01 UTC
As Martin mentioned on the listserv, if you can use git-bz to attach a patch here (https://wiki.koha-community.org/wiki/Git_bz_configuration), then we can move things along.

At a glance, this is looking OK. I'm sure between Katrin, Martin, and myself that we can get this done quickly. 

Since the change only targets the addbiblio.pl script, there shouldn't really be any implications for other parts of Koha. The only scenario I can think that could be a problem would be where someone has added a thesaurus to a subfield other than $a and they've hidden the $a but that's quite an edge case and they are already arguably doing the wrong thing there...
Comment 5 Thomas Klausner 2021-04-02 14:17:21 UTC
Created attachment 119123 [details] [review]
only show subfield "9" if subfield "a" is visibile (bug 28022)

Amend the workaround (from but 2206) to only show subfield "9" in editor if subfield "a" has the editor-visibility bit set
Comment 6 Thomas Klausner 2021-04-20 08:59:42 UTC
Anything I can do to make sure this fix gets in the next release?
Comment 7 Martin Renvoize 2021-04-20 10:59:18 UTC
Created attachment 119903 [details] [review]
Bug 28022: Only show subfield "9" if subfield "a" is visible

Amend the workaround (from but 2206) to only show subfield "9" in editor if
subfield "a" has the editor-visibility bit set.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-04-20 11:01:48 UTC
Thanks for your submission Thomas, my apologies it took me a little while to come around to testing it... sooooo many things on the list.

Signing off as it works as described and I can't see any obvious regressions.
Comment 9 Kyle M Hall 2021-09-10 15:11:06 UTC
Created attachment 124768 [details] [review]
Bug 28022: Only show subfield "9" if subfield "a" is visible

Amend the workaround (from but 2206) to only show subfield "9" in editor if
subfield "a" has the editor-visibility bit set.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Jonathan Druart 2021-09-21 18:18:45 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 11 Jonathan Druart 2021-09-21 18:20:38 UTC
Congratulations, Thomas, for your first patch pushed!