Summary: | Can't hide MARC21 500 and others with NotesToHide | ||
---|---|---|---|
Product: | Koha | Reporter: | Katrin Fischer <katrin.fischer> |
Component: | OPAC | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | minor | ||
Priority: | P5 - low | CC: | andrew, 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
|
Circulation function: | |||
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
I confirm this, moreover it seems like it's 5xx fields, not just 500. 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... 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! Magnus, interesting riddle... I haven't been able to find out where it comes from either. I was thinking it could be biblioitems.notes. *** Bug 28067 has been marked as a duplicate of this bug. *** 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 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. 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 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. Thank you David, a pesky new line had lodged itself in the code since then :p I posted the rebased version. 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> (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 . ... UNIMARC. Now tested on a UNIMARC system, and everything works as expected. 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> Really love to see this one finally fixed! Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.02 Pushed to 21.05.x for 21.05.09 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |