Bug 25709 - Rename systempreference from NotesBlacklist to NotesToHide
Summary: Rename systempreference from NotesBlacklist to NotesToHide
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 9162
Blocks: 25708
  Show dependency treegraph
 
Reported: 2020-06-10 15:58 UTC by Nick Clemens
Modified: 2020-09-18 22:18 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
This patchset updates a syspref name to be clearer about what it does and to follow community guidelines on using inclusive language. https://wiki.koha-community.org/wiki/Coding_Guidelines#TERM3:_Inclusive_Language
Version(s) released in:
20.11.00, 20.05.03, 19.11.09, 19.05.15


Attachments
Bug 25709: Rename systemprefrence to NotesDenylist (8.80 KB, patch)
2020-06-10 16:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25709: Rename systemprefrence to NotesDenylist (8.85 KB, patch)
2020-06-10 16:40 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25709: Rename systemprefrence to OpacHiddenNotes (6.12 KB, patch)
2020-07-06 17:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25709: Rename systempreference to NotesToHide (5.10 KB, patch)
2020-07-07 11:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25709: Rename systempreference to NotesToHide (5.15 KB, patch)
2020-07-07 13:41 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25709: Rename systempreference to NotesToHide (5.20 KB, patch)
2020-07-15 06:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25709: Rename systempreference to NotesToHide (5.45 KB, patch)
2020-07-27 15:33 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-06-10 15:58:11 UTC

    
Comment 1 Nick Clemens 2020-06-10 16:03:54 UTC
Created attachment 105701 [details] [review]
Bug 25709: Rename systemprefrence to NotesDenylist
Comment 2 Nick Clemens 2020-06-10 16:06:18 UTC
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
Comment 3 Owen Leonard 2020-06-10 16:40:14 UTC
Created attachment 105702 [details] [review]
Bug 25709: Rename systemprefrence to NotesDenylist

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Katrin Fischer 2020-06-11 12:11:53 UTC
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.
Comment 5 Julian Maurice 2020-07-03 07:35:56 UTC
(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.
Comment 6 Julian Maurice 2020-07-03 07:41:36 UTC
Changing status to Failed QA as there are things to fix (comment 4)
Comment 7 David Cook 2020-07-06 02:50:30 UTC
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.
Comment 8 Katrin Fischer 2020-07-06 05:53:00 UTC
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 :)
Comment 9 Nick Clemens 2020-07-06 17:58:03 UTC
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
Comment 10 Nick Clemens 2020-07-06 17:59:31 UTC
Removed old update and release note changes

Renamed to OpacHiddenNotes (like OpacHiddenItems)

There is an atomic update to change existing installs
Comment 11 Julian Maurice 2020-07-07 07:10:10 UTC
(In reply to Nick Clemens from comment #10)
> Renamed to OpacHiddenNotes (like OpacHiddenItems)

Is this only for OPAC ? Syspref description mentions staff interface too.
Comment 12 Nick Clemens 2020-07-07 11:35:04 UTC
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
Comment 13 Owen Leonard 2020-07-07 13:41:41 UTC
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>
Comment 14 Katrin Fischer 2020-07-15 06:11:03 UTC
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>
Comment 15 Jonathan Druart 2020-07-20 15:10:08 UTC
Please update the commit message to explain why.
Comment 16 Nick Clemens 2020-07-27 15:33:46 UTC
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>
Comment 17 Jonathan Druart 2020-07-30 15:46:57 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 18 Lucas Gass 2020-07-31 21:23:03 UTC
I feel like this enhancement should be backported 

backported to 20.05.x for 20.05.03
Comment 19 Aleisha Amohia 2020-08-04 00:34:01 UTC
backported to 19.11.x for 19.11.09
Comment 20 Caroline Cyr La Rose 2020-08-20 20:52:14 UTC
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 :)
Comment 21 Victor Grousset/tuxayo 2020-09-18 22:18:55 UTC
Backported to 19.05.x branch for 19.05.15