Bug 17127

Summary: Can't hide MARC21 500 and others with NotesToHide
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: OPACAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: andrewfh, david, florian.bontemps, kyle, liz, magnus, paliotti, pierre.genty, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17697
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes hiding notes fields (5XX in MARC21 and 3XX in UNIMARC) using NotesToHide. Before this you could hide one field and it worked. However, when hiding multiple fields one field would still always be visible. Now hiding notes fields works as expected.
Version(s) released in:
22.05.00,21.11.02,21.05.09
Attachments: Bug 17127: Remove notes from detail.tt to only use MARCNOTES
Bug 17127: Remove notes from detail.tt to only use MARCNOTES
Bug 17127: Remove notes from detail.tt to only use MARCNOTES
Bug 17127: Remove notes from detail.tt to only use MARCNOTES

Description Katrin Fischer 2016-08-15 11:02:49 UTC
NotesBlacklist will hide fields from the descriptions tab in the OPAC. This works fine for all fields, but 500 in our tests.
Comment 1 Liz Rea 2016-11-30 00:45:17 UTC
I confirm this, moreover it seems like it's 5xx fields, not just 500.
Comment 2 Magnus Enger 2019-02-25 10:33:43 UTC
I have a record with two 5xx fields: 500 and 518. 

If I set NotesBlacklist = 500 I get this:
<div id="marcnotes"><p>Text of 518</p></div>
OK

If I set NotesBlacklist = 518 I get this:
<div id="marcnotes"><p>Text of 500</p></div>
OK

If I set NotesBlacklist = 500,518 I get this:
<p>Text of 500</p>
Not so OK...
Comment 3 Magnus Enger 2019-02-25 11:28:03 UTC
The template code that outputs the notes in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt looks like this:

 704 [% IF ( MARCNOTES || notes || ( SyndeticsEnabled && SyndeticsSummary && SYNDETICS_SUMMARY )) %]
 705     <div id="descriptions">
 706         <div class="content_set">
 707 
 708         [% IF ( SyndeticsEnabled && SyndeticsSummary && SYNDETICS_SUMMARY ) %]
 709             <h4>Enhanced descriptions from Syndetics:</h4>
 710             <p>[% SYNDETICS_SUMMARY | html %]</p>
 711         [% END %]
 712 
 713         [% IF ( MARCNOTES ) %]
 714             <div id="marcnotes">
 715             [% FOREACH MARCNOTE IN MARCNOTES %]
 716                 <p>
 717                 [% IF MARCNOTE.marcnote.match('^https?://\S+$') %]
 718                     <a href="[% MARCNOTE.marcnote | url %]">[% MARCNOTE.marcnote | html %]</a>
 719                 [% ELSE %]
 720                     [% MARCNOTE.marcnote | html | html_line_break %]
 721                 [% END %]
 722                 </p>
 723             [% END %]
 724             </div>
 725         [% ELSE %]
 726             [% IF ( notes ) %]
 727                 <p>[% notes | html %]</p>
 728             [% END %]
 729         [% END %]
 730 
 731         </div>
 732     </div> <!-- / #descriptions -->
 733 [% END # / IF MARCNOTES || notes %]

If I have notes in e.g. 500 and 518, and NotesBlacklist holds a single field, the remaining field is output by the line "[% MARCNOTE.marcnote | html | html_line_break %]", as it should be. 

If NotesBlacklist is given a list of fields, like "500,518", nothing is output by "[% MARCNOTE.marcnote | html | html_line_break %]", BUT the text of the 500 field is output by this line: "<p>[% notes | html %]</p>". I have not been able to figure out where the "notes" variable comes from!
Comment 4 Katrin Fischer 2019-04-09 21:57:09 UTC
Magnus, interesting riddle...
I haven't been able to find out where it comes from either. I was thinking it could be biblioitems.notes.
Comment 5 Katrin Fischer 2021-04-02 15:18:09 UTC
*** Bug 28067 has been marked as a duplicate of this bug. ***
Comment 6 Pablo López Liotti 2021-07-07 14:43:33 UTC
Today this still don`t work. koha 20.05.11

NotesToHide syspref set to 500,520,505 works fine with 520 and 505 but 500 is in description tab.

Is there an advance on this little bug?

Thks
Comment 7 Florian 2021-10-27 13:07:04 UTC
Created attachment 126990 [details] [review]
Bug 17127: Remove notes from detail.tt to only use MARCNOTES

Notes come from the biblio table, but that table is already used to fill in MARCNOTES through Biblio.pm and get_marc_notes.
Get_marc_notes does check for NotesToHide and already read every note on the records, but .notes doesn't go through that same filter.
I don't see the point of keeping notes as a condition when MARCNOTES does the same job but better.

To test:
    1) Take any record, or create one
    2) Input something in the 500 field (or 300 in UNIMARC)
    3) In Systempreferences -> NotesToHide, fill in the number 500 (or 300).
    4) Save, then go look at the record detail in the OPAC and admin website.
    5) You should still see the 500 or 300 field under the Description tab.
    6) Apply patch.
    7) Reload the record detail page.
    8) Observe the error is gone.
Comment 8 David Nind 2021-11-24 23:57:18 UTC
With no patch applied and using the latest master, if only 500 is hidden using the NotesToHide system preference then it works as expected.

As per comment #2, if I have two 5XX fields with data (say 500 and 504, but I tested and it can be any two fields) and hide all of them 500 still displays.
 
From what I can tell, if all the available 5XX fields are added to NotesToHide, then the first available field still displays. That is, it needs to display at least one field under the description tab in the OPAC and staff interface, and you can't hide them all.

To replicate (using koha-testing-docker (KTD) and the default book framework):
1. Add data to each available field for a record (with KTD I used record 262 - Programming Perl).
2. Progressively hide each field until all the 5XX fields available are included in NotesToHide (500,504,505,520,521,522,526,538,541,546,583,586).
3. Up until 586 was added to NotesToHide, everything worked as expected.
4. However, once I added 586, 500 was no longer hidden (all the other fields remained hidden).
5. If I only have data in two fields, say 504 and 526, then 504 is still displayed.

The patch no longer applies 8-(..

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17127: Remove notes from detail.tt to only use MARCNOTES
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 17127: Remove notes from detail.tt to only use MARCNOTES
Comment 9 Florian 2021-11-26 14:31:12 UTC
Created attachment 128041 [details] [review]
Bug 17127: Remove notes from detail.tt to only use MARCNOTES

Notes come from the biblio table, but that table is already used to fill in MARCNOTES through Biblio.pm and get_marc_notes.
Get_marc_notes does check for NotesToHide and already read every note on the records, but .notes doesn't go through that same filter.
I don't see the point of keeping notes as a condition when MARCNOTES does the same job but better.

To test:
    1) Take any record, or create one
    2) Input something in the 500 field (or 300 in UNIMARC)
    3) In Systempreferences -> NotesToHide, fill in the number 500 (or 300).
    4) Save, then go look at the record detail in the OPAC and admin website.
    5) You should still see the 500 or 300 field under the Description tab.
    6) Apply patch.
    7) Reload the record detail page.
    8) Observe the error is gone.
Comment 10 Florian 2021-11-26 14:32:18 UTC
Thank you David, a pesky new line had lodged itself in the code since then :p

I posted the rebased version.
Comment 11 David Nind 2021-11-26 16:23:12 UTC
Created attachment 128049 [details] [review]
Bug 17127: Remove notes from detail.tt to only use MARCNOTES

Notes come from the biblio table, but that table is already used to fill in MARCNOTES through Biblio.pm and get_marc_notes.
Get_marc_notes does check for NotesToHide and already read every note on the records, but .notes doesn't go through that same filter.
I don't see the point of keeping notes as a condition when MARCNOTES does the same job but better.

To test:
    1) Take any record, or create one
    2) Input something in the 500 field (or 300 in UNIMARC)
    3) In Systempreferences -> NotesToHide, fill in the number 500 (or 300).
    4) Save, then go look at the record detail in the OPAC and admin website.
    5) You should still see the 500 or 300 field under the Description tab.
    6) Apply patch.
    7) Reload the record detail page.
    8) Observe the error is gone.

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2021-11-26 16:25:48 UTC
(In reply to Florian from comment #10)
> Thank you David, a pesky new line had lodged itself in the code since then :p
> 
> I posted the rebased version.

Thanks Florian! Things now work as expected and you can hide 500 fields.

Note: I only tested the 500 fields in MARC21, and didn't test .
Comment 13 David Nind 2021-11-26 16:26:26 UTC
... UNIMARC.
Comment 14 David Nind 2021-11-26 17:13:33 UTC
Now tested on a UNIMARC system, and everything works as expected.
Comment 15 Katrin Fischer 2021-12-18 23:00:06 UTC
Created attachment 128705 [details] [review]
Bug 17127: Remove notes from detail.tt to only use MARCNOTES

Notes come from the biblio table, but that table is already used to fill in MARCNOTES through Biblio.pm and get_marc_notes.
Get_marc_notes does check for NotesToHide and already read every note on the records, but .notes doesn't go through that same filter.
I don't see the point of keeping notes as a condition when MARCNOTES does the same job but better.

To test:
    1) Take any record, or create one
    2) Input something in the 500 field (or 300 in UNIMARC)
    3) In Systempreferences -> NotesToHide, fill in the number 500 (or 300).
    4) Save, then go look at the record detail in the OPAC and admin website.
    5) You should still see the 500 or 300 field under the Description tab.
    6) Apply patch.
    7) Reload the record detail page.
    8) Observe the error is gone.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2021-12-18 23:00:38 UTC
Really love to see this one finally fixed!
Comment 17 Fridolin Somers 2022-01-11 22:51:30 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 18 Kyle M Hall 2022-01-14 14:11:54 UTC
Pushed to 21.11.x for 21.11.02
Comment 19 Andrew Fuerste-Henry 2022-01-21 18:24:54 UTC
Pushed to 21.05.x for 21.05.09
Comment 20 Victor Grousset/tuxayo 2022-02-07 18:55:31 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.