Created attachment 105701 [details] [review] Bug 25709: Rename systemprefrence to NotesDenylist
To test: 1 - Apply patch and updatedatabase 2 - git grep NotesBlacklist 3 - Note all remaining occurrences are translations 4 - Add a field (e.g. '520' ) to the NotesDenylist systempreference 5 - View a record with a 520 field on the opac 6 - Confirm the field does not show in the 'Title notes' tab
Created attachment 105702 [details] [review] Bug 25709: Rename systemprefrence to NotesDenylist Signed-off-by: Owen Leonard <oleonard@myacpl.org>
First: I very much agree with renaming this one! Just the new name is not really 'talking' to me. I'd like to suggest we maybe use something like: OpacNotesToHide or OpacNotesUnwanted There is also a tiny thing (would have fixed in a follow-up): FAIL installer/data/mysql/updatedatabase.pl OK critic FAIL forbidden patterns forbidden pattern: Use INSERT IGNORE INTO on inserting a new syspref (see bug 9071) (line 6380) It could be discussed if we want to change the former release notes - we had named it badly, we fixed it after realizing our error. But I can see it was a sponsored feature, so maybe this will be nicer for the library.
(In reply to Katrin Fischer from comment #4) > It could be discussed if we want to change the former release notes - we had > named it badly, we fixed it after realizing our error. But I can see it was > a sponsored feature, so maybe this will be nicer for the library. Are we trying to rewrite history and to *deny* that we ever used the now forbidden word "blacklist" ? :) Seriously, please do not rewrite release notes. In 20.05.00 this syspref is named NotesBlacklist and this will forever be true for that particular version. This could only add confusion.
Changing status to Failed QA as there are things to fix (comment 4)
Note that this patch doesn't actually rename the system preference. The updatedatabase.pl change is for version 3.12. It needs an UPDATE statement for the present. I also agree with Katrin. I don't think that NotesDenylist is very intuitive. Perhaps NotesExcludeList or HideNotesList. (I don't think we can use OpacNotesToHide since NotesBlacklist currently works for both the Intranet and OPAC I believe?) I also agree that rewriting release notes doesn't make sense. It would be better to just say that we're renaming it now. Alternatively, we could also deprecate this system preference, since the framework visibility is working better now, although perhaps it's not quite comprehensive enough yet.
This has a different purpose than the frameworks visibility - we use it to hide notes that also appear in the normal view so they are not doubled up. By default all 5xx display in the description tab, but some can be in the XSLT too. So I'd like to keep the functionality with a new name :)
Created attachment 106601 [details] [review] Bug 25709: Rename systemprefrence to OpacHiddenNotes To test: 1 - Apply patch and updatedatabase 2 - git grep NotesBlacklist 3 - Note all remaining occurrences are translations, db updates, and release notes 4 - Add a field (e.g. '520' ) to the OpacHiddenNotes systempreference 5 - View a record with a 520 field on the opac 6 - Confirm the field does not show in the 'Title notes' tab
Removed old update and release note changes Renamed to OpacHiddenNotes (like OpacHiddenItems) There is an atomic update to change existing installs
(In reply to Nick Clemens from comment #10) > Renamed to OpacHiddenNotes (like OpacHiddenItems) Is this only for OPAC ? Syspref description mentions staff interface too.
Created attachment 106625 [details] [review] Bug 25709: Rename systempreference to NotesToHide To test: 1 - Apply patch and updatedatabase 2 - git grep NotesBlacklist 3 - Note all remaining occurrences are translations, db updates, and release notes 4 - Add a field (e.g. '520' ) to the NotesToHide systempreference 5 - View a record with a 520 field on the opac 6 - Confirm the field does not show in the 'Title notes' tab
Created attachment 106632 [details] [review] Bug 25709: Rename systempreference to NotesToHide To test: 1 - Apply patch and updatedatabase 2 - git grep NotesBlacklist 3 - Note all remaining occurrences are translations, db updates, and release notes 4 - Add a field (e.g. '520' ) to the NotesToHide systempreference 5 - View a record with a 520 field on the opac 6 - Confirm the field does not show in the 'Title notes' tab Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 106892 [details] [review] Bug 25709: Rename systempreference to NotesToHide To test: 1 - Apply patch and updatedatabase 2 - git grep NotesBlacklist 3 - Note all remaining occurrences are translations, db updates, and release notes 4 - Add a field (e.g. '520' ) to the NotesToHide systempreference 5 - View a record with a 520 field on the opac 6 - Confirm the field does not show in the 'Title notes' tab Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Please update the commit message to explain why.
Created attachment 107439 [details] [review] Bug 25709: Rename systempreference to NotesToHide In an effort to use more inclusive language we have added a new coding guideline: https://wiki.koha-community.org/wiki/Coding_Guidelines#TERM3:_Inclusive_Language This patchset renames a syspref to be clearer and follow the guideline To test: 1 - Apply patch and updatedatabase 2 - git grep NotesBlacklist 3 - Note all remaining occurrences are translations, db updates, and release notes 4 - Add a field (e.g. '520' ) to the NotesToHide systempreference 5 - View a record with a 520 field on the opac 6 - Confirm the field does not show in the 'Title notes' tab Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 20.11, thanks to everybody involved!
I feel like this enhancement should be backported backported to 20.05.x for 20.05.03
backported to 19.11.x for 19.11.09
I changed the component from "Architecture, internals and plumbing" to "System administration" so that it appears under Administration in the release notes. From a user stand point, I think it makes more sense to have it under Administration. If I were a system administrator, I would check the administration section of the release notes and I would want to know that a syspref changed name. I hope this is ok. If not, feel free to change it back :)
Backported to 19.05.x branch for 19.05.15