Bug 28022 - MARC subfield 9 not honoring visibility
Summary: MARC subfield 9 not honoring visibility
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Thomas Klausner
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-23 11:15 UTC by Thomas Klausner
Modified: 2022-06-06 20:25 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Circulation function:


Attachments
only show subfield "9" if subfield "a" is visibile (bug 28022) (3.66 KB, patch)
2021-04-02 14:17 UTC, Thomas Klausner
Details | Diff | Splinter Review
Bug 28022: Only show subfield "9" if subfield "a" is visible (3.72 KB, patch)
2021-04-20 10:59 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28022: Only show subfield "9" if subfield "a" is visible (3.79 KB, patch)
2021-09-10 15:11 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 (ashimema) 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 (ashimema) 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 (khall) 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!