Bug 20398 - Add a system preference to disable search result highlighting in the staff interface
Summary: Add a system preference to disable search result highlighting in the staff in...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Thibaud Guillot (thibaud_g)
QA Contact: Testopia
URL:
Keywords:
: 11525 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-03-14 13:41 UTC by Gaetan Boisson
Modified: 2022-12-12 21:23 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a new system preference StaffHighlightWords. This enables highlighting of words in search results for the staff interface to be turned on or off.
Version(s) released in:
22.05.00
Circulation function:


Attachments
Added a syspref to highlight or not highlight search terms in results on the staff interface (8.37 KB, patch)
2021-12-08 10:21 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review
Added a syspref to highlight or not highlight search terms in results on the staff interface (8.32 KB, patch)
2022-02-09 15:56 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface (8.30 KB, patch)
2022-02-10 08:22 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface (8.31 KB, patch)
2022-02-10 10:57 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface (8.45 KB, patch)
2022-02-10 11:16 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface (8.32 KB, patch)
2022-02-10 12:31 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface (8.36 KB, patch)
2022-02-10 15:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface (8.42 KB, patch)
2022-02-13 12:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20398: (QA follow-up) Fix alphabetical order in sysprefs.sql (2.60 KB, patch)
2022-02-13 12:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20398: (QA Follow-up) Fix position of 'if' statement (1007 bytes, patch)
2022-03-29 13:41 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Gaetan Boisson 2018-03-14 13:41:53 UTC
Search result highlighting is broken and has been broken for a long time:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11270

Adding insult to injury there is no way to deactivate it on the staff interface. We are so used to see it that we don't pay attention, but new users notice it, and complain.

It would be good to remove it until it is fixed. (As a general rule i think features that aren't entirely functional shouldn't make it into the release until they are actually ready.)
Comment 1 Gaetan Boisson 2018-03-14 16:28:06 UTC
Discussed with Owen at the hackfest, removing it could be disturbing for some users even though it is clearly not working.

The way to go would be to add a system preference so we can disable it. Changing the title of the bug accordingly.
Comment 2 Katrin Fischer 2018-03-17 20:11:15 UTC
It works to a certain extend - it higlights words from your search in the results. It could work better, yes, but it's not totally broken. A pref seems fine to me.
Comment 3 Andreas Hedström Mace 2018-04-08 20:27:07 UTC
+1 on adding a pref to disable!

(I wouldn't mind if the functionality was removed altogether either.)
Comment 4 Elaine Bradtke 2018-04-23 18:10:26 UTC
+1 we've actually had comments / complaints from users who don't like it.  Would love to switch it off or remove it completely.
(In reply to Andreas Hedström Mace from comment #3)
> +1 on adding a pref to disable!
> 
> (I wouldn't mind if the functionality was removed altogether either.)
Comment 5 Katrin Fischer 2018-04-23 19:11:10 UTC
(In reply to Elaine Bradtke from comment #4)
> +1 we've actually had comments / complaints from users who don't like it. 
> Would love to switch it off or remove it completely.
> (In reply to Andreas Hedström Mace from comment #3)
> > +1 on adding a pref to disable!
> > 
> > (I wouldn't mind if the functionality was removed altogether either.)

You can already deactivate it on the OPAC:  HighlightOwnItemsOnOPAC
Comment 6 Elaine Bradtke 2018-04-23 22:02:29 UTC
(In reply to Katrin Fischer from comment #5)
We deactivated it in the OPAC. Shortly after we activated it.  

> You can already deactivate it on the OPAC:  HighlightOwnItemsOnOPAC
Comment 7 Owen Leonard 2020-07-28 20:23:21 UTC
*** Bug 11525 has been marked as a duplicate of this bug. ***
Comment 8 Thibaud Guillot (thibaud_g) 2021-12-08 10:21:31 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2021-12-15 09:56:18 UTC
Shouldn't we store it as a user's setting in local storage instead?
Comment 10 Thibaud Guillot (thibaud_g) 2021-12-15 12:36:04 UTC
I reproduced the same behavior than OPAC side and from the discussion it coincided with the integration of a system preference. If we put this parameter in the localstorage could problems arise? Like using a browser that is set to not store anything in the localstorage or simply the library that would prefer not to store anything there. I am not against it but I question to raise possible problems with its storage.
Especially since the system preference allows not to highlight certain words.
Comment 11 Jonathan Druart 2021-12-15 12:53:49 UTC
In my opinion sysprefs should not drive UI's preferences. We are using local storage for several features already so that should not be a problem.

See bug 5697 for instance, that have been pushed recently, or bug 24958.
Comment 12 Owen Leonard 2021-12-15 13:01:52 UTC
I think that the existence of OpacHighlightedWords makes it logical to add a preference for the staff interface. I agree that the "stopwords" option is important to have.

I think it might also be good to preserve the user's highlight/unhighlight choice in local storage so that it is preserved across their session.
Comment 13 Jonathan Druart 2021-12-15 13:19:29 UTC
Why do we need the syspref if each user can decide if they want the feature?
We could (later) remove OpacHighlightedWords and use local storage for OPAC as well.

The reason I see would be to turn it off globally, but I don't think it's useful.
Comment 14 Thibaud Guillot (thibaud_g) 2021-12-15 13:28:18 UTC
I agree, it's like "killing an ant with a tank" to remove the highlight by adding a syspref, run updatedatabase.pl, add a conditioner... etc.

I wanted to keep the same behavior as OPAC for some unity of operation.

If we plan to remove OPACHighlightedWords later, it may be easier to add it to localstorage by a little js code for now.
Comment 15 Katrin Fischer 2021-12-15 13:30:15 UTC
(In reply to Jonathan Druart from comment #13)
> Why do we need the syspref if each user can decide if they want the feature?
> We could (later) remove OpacHighlightedWords and use local storage for OPAC
> as well.
> 
> The reason I see would be to turn it off globally, but I don't think it's
> useful.

We had requests to turn it off in the OPAC and also the staff interface. I think that it's by default 'on' is an issue that can be annoying. People are often irritated that everything is highlighted instead of only highlighting the words int he fields they searched in. I think having a pref here would be helpful.
Comment 16 Katrin Fischer 2021-12-15 13:31:57 UTC
What I am trying to say: this is a feature that is not wanted by some and we have a lot of feature on/off switches. I don't see the difference with this one? 

Having it in localstorage would be nice, but then I think we also should have a pref to set the default (highlight or not highlight). The unhighlight option is often missed.
Comment 17 Thibaud Guillot (thibaud_g) 2021-12-15 13:42:01 UTC
For now maybe someone can test my patch and tell if it's appropriate or not ? "Highlighting" and "Unhighlighting" options are still possible on the go when syspref isn't set to "Highlight" (by default) but if you disable highlighting you will didn't see any highlighting after that
Comment 18 Thibaud Guillot (thibaud_g) 2021-12-15 13:54:21 UTC
Because it can concerned other syspref and this can be interesting to develop more I created https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29701
Comment 19 David Nind 2022-02-05 21:03:32 UTC
I have tested this and it works as expected. I would suggest these changes:
 
1. System preference name: change from HighlightedWords to StaffHighlightedWords
 
2.System preference wording:
  . Current: Don't highlight|Highlight words the patron searched for in their search results and detail pages.
  . Suggested: Don't highlight|Highlight words searched for in the staff interface search results pages.
  . Notes: For me the highlighting is only working on the search listing results pages, not on the record detail pages.
 
 - Location in the system preferences area: I would put under Staff interface > Appearance as this is consistent with the OPAC system preference, rather than under Searching > Results display.
 
 - Enable highlighting by default: as this is a change in the default behavour I would tend to leave the default (highlighting) enabled, rather than disabling it (that is, set the default to Highlight).
Comment 20 Thibaud Guillot (thibaud_g) 2022-02-09 15:56:06 UTC Comment hidden (obsolete)
Comment 21 Thibaud Guillot (thibaud_g) 2022-02-09 16:02:48 UTC
(In reply to David Nind from comment #19)
> I have tested this and it works as expected. I would suggest these changes:
>  
> 1. System preference name: change from HighlightedWords to
> StaffHighlightedWords
>  
> 2.System preference wording:
>   . Current: Don't highlight|Highlight words the patron searched for in
> their search results and detail pages.
>   . Suggested: Don't highlight|Highlight words searched for in the staff
> interface search results pages.
>   . Notes: For me the highlighting is only working on the search listing
> results pages, not on the record detail pages.
>  
>  - Location in the system preferences area: I would put under Staff
> interface > Appearance as this is consistent with the OPAC system
> preference, rather than under Searching > Results display.
>  
>  - Enable highlighting by default: as this is a change in the default
> behavour I would tend to leave the default (highlighting) enabled, rather
> than disabling it (that is, set the default to Highlight).

Hello David, 

Thanks for you feedback :)

I completely understand your notes, I'm agree for the syspref name : the more precise, the better it will be.

Also for the description, my english is not as good as i would like ^^ so I change this too.

And for the location... yes totally correct and adequate I concede it, like the default behavior as you notice.

So I push again the patch with all this changes, thanks to tell me your opinion :) 
And if other people test it of course we can discuss
Comment 22 David Nind 2022-02-09 18:58:48 UTC
(In reply to Thibaud Guillot from comment #21)

> Also for the description, my english is not as good as i would like ^^ so I
> change this too.
Hi Thibaud.

Thanks for incorporating my suggestions - writing things in another language can be challenging, so hopefully that is where testing can help!

I've retested, but for some reason highlighting doesn't seem to be working for me now - with either 'Highlight' or 'Don't highlight' selected for the new system preference.

Also, I would suggest:

- Setting the default to 'Highlight' (that is, leave the current behavour in place) (I'm not a developer, but from reading the code it should do that, but I'm not sure why it isn't).

- For the patch add the bug number at the start, that is: Bug 20398: Added... (the QA Team will most likely pick this up).

David
Comment 23 Thibaud Guillot (thibaud_g) 2022-02-10 08:22:34 UTC
Created attachment 130420 [details] [review]
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface

There is a possibility on OPAC but not yet on the staff interface, so I added this syspref which has the same behaviour as the OPAC highlighting syspref.
When you choose to disable highlighting on the syspref, the toggle anchor in the result page does not appear at all.
But if you keep the default state (highlighting on), you still have the option to switch between "Unhighlight" and "Highlight" modes.

Test plan :
1. Go to the syspref named "HighlightedWords" and see that the default state is "Highlight".
2. Look for something in the catalogue for example and see that the words are highlighted and that it is also possible to change between "Unhighlight" and "Highlight".
3. Go back to syspref and choose to disable highlighting by choosing "Don't Highlight".
4. Refresh your results page if you have not closed it or reload a new search.
5. There is no highlighting at all and anchors do not exist (same behaviour as the OPAC interface).
Comment 24 Thibaud Guillot (thibaud_g) 2022-02-10 08:26:58 UTC
(In reply to David Nind from comment #22)
> (In reply to Thibaud Guillot from comment #21)
> 
> > Also for the description, my english is not as good as i would like ^^ so I
> > change this too.
> Hi Thibaud.
> 
> Thanks for incorporating my suggestions - writing things in another language
> can be challenging, so hopefully that is where testing can help!
> 
> I've retested, but for some reason highlighting doesn't seem to be working
> for me now - with either 'Highlight' or 'Don't highlight' selected for the
> new system preference.
> 
> Also, I would suggest:
> 
> - Setting the default to 'Highlight' (that is, leave the current behavour in
> place) (I'm not a developer, but from reading the code it should do that,
> but I'm not sure why it isn't).
> 
> - For the patch add the bug number at the start, that is: Bug 20398:
> Added... (the QA Team will most likely pick this up).
> 
> David

Yes David you are absolutely right, yesterday when I rebased the patch I forgot to change some variable names on the script that manages the feature... my apologies, now its works. And I adjust the commit by the way with the bug number :)
Comment 25 David Nind 2022-02-10 10:26:08 UTC
Patch no longer applies 8-(... things move too fast sometimes!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/js/pages/results.js
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/js/pages/results.js
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/pages/results.js
error: Failed to merge in the changes.
Patch failed at 0001 Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface
Comment 26 David Nind 2022-02-10 10:30:23 UTC
(In reply to Thibaud Guillot from comment #24)
> 
> Yes David you are absolutely right, yesterday when I rebased the patch I
> forgot to change some variable names on the script that manages the
> feature... my apologies, now its works. And I adjust the commit by the way
> with the bug number :)

No worries and excellent!
Comment 27 Thibaud Guillot (thibaud_g) 2022-02-10 10:57:40 UTC
Created attachment 130426 [details] [review]
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface

There is a possibility on OPAC but not yet on the staff interface, so I added this syspref which has the same behaviour as the OPAC highlighting syspref.
When you choose to disable highlighting on the syspref, the toggle anchor in the result page does not appear at all.
But if you keep the default state (highlighting on), you still have the option to switch between "Unhighlight" and "Highlight" modes.

Test plan :
1. Go to the syspref named "HighlightedWords" and see that the default state is "Highlight".
2. Look for something in the catalogue for example and see that the words are highlighted and that it is also possible to change between "Unhighlight" and "Highlight".
3. Go back to syspref and choose to disable highlighting by choosing "Don't Highlight".
4. Refresh your results page if you have not closed it or reload a new search.
5. There is no highlighting at all and anchors do not exist (same behaviour as the OPAC interface).
Comment 28 Thibaud Guillot (thibaud_g) 2022-02-10 10:59:32 UTC
(In reply to David Nind from comment #25)
> Patch no longer applies 8-(... things move too fast sometimes!
> 
> Apply? [(y)es, (n)o, (i)nteractive] y
> Applying: Bug 20398 : Added a syspref to highlight or not highlight search
> terms in results on the staff interface
> Using index info to reconstruct a base tree...
> M	koha-tmpl/intranet-tmpl/prog/js/pages/results.js
> Falling back to patching base and 3-way merge...
> Auto-merging koha-tmpl/intranet-tmpl/prog/js/pages/results.js
> CONFLICT (content): Merge conflict in
> koha-tmpl/intranet-tmpl/prog/js/pages/results.js
> error: Failed to merge in the changes.
> Patch failed at 0001 Bug 20398 : Added a syspref to highlight or not
> highlight search terms in results on the staff interface

Normally I solved the conflict, thanks to you
Comment 29 Kyle M Hall (khall) 2022-02-10 11:16:37 UTC
Created attachment 130429 [details] [review]
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface

There is a possibility on OPAC but not yet on the staff interface, so I added this syspref which has the same behaviour as the OPAC highlighting syspref.
When you choose to disable highlighting on the syspref, the toggle anchor in the result page does not appear at all.
But if you keep the default state (highlighting on), you still have the option to switch between "Unhighlight" and "Highlight" modes.

Test plan :
1. Go to the syspref named "HighlightedWords" and see that the default state is "Highlight".
2. Look for something in the catalogue for example and see that the words are highlighted and that it is also possible to change between "Unhighlight" and "Highlight".
3. Go back to syspref and choose to disable highlighting by choosing "Don't Highlight".
4. Refresh your results page if you have not closed it or reload a new search.
5. There is no highlighting at all and anchors do not exist (same behaviour as the OPAC interface).

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 30 Emmanuel Bétemps 2022-02-10 11:18:59 UTC
This patch applies.

I encountered problems this morning (patch uninstalled or ineffective) but they seem solved.
Comment 31 Emmanuel Bétemps 2022-02-10 11:25:07 UTC
(but the name of the syspref is 'StaffHighlightedWords')
Comment 32 Thibaud Guillot (thibaud_g) 2022-02-10 12:31:50 UTC
Created attachment 130446 [details] [review]
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface

There is a possibility on OPAC but not yet on the staff interface, so I added this syspref which has the same behaviour as the OPAC highlighting syspref.
When you choose to disable highlighting on the syspref, the toggle anchor in the result page does not appear at all.
But if you keep the default state (highlighting on), you still have the option to switch between "Unhighlight" and "Highlight" modes.

Test plan :
1. Go to the syspref named "StaffHighlightedWords" and see that the default state is "Highlight".
2. Look for something in the catalogue for example and see that the words are highlighted and that it is also possible to change between "Unhighlight" and "Highlight".
3. Go back to syspref and choose to disable highlighting by choosing "Don't Highlight".
4. Refresh your results page if you have not closed it or reload a new search.
5. There is no highlighting at all and anchors do not exist (same behaviour as the OPAC interface).
Comment 33 Thibaud Guillot (thibaud_g) 2022-02-10 12:32:59 UTC
(In reply to e.betemps from comment #31)
> (but the name of the syspref is 'StaffHighlightedWords')

yes.. I saw that I forgot to change the name in my test plan with latest changes... this is fixed now, thanks for your feedback
Comment 34 Kyle M Hall (khall) 2022-02-10 15:39:00 UTC
Created attachment 130450 [details] [review]
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface

There is a possibility on OPAC but not yet on the staff interface, so I added this syspref which has the same behaviour as the OPAC highlighting syspref.
When you choose to disable highlighting on the syspref, the toggle anchor in the result page does not appear at all.
But if you keep the default state (highlighting on), you still have the option to switch between "Unhighlight" and "Highlight" modes.

Test plan :
1. Go to the syspref named "StaffHighlightedWords" and see that the default state is "Highlight".
2. Look for something in the catalogue for example and see that the words are highlighted and that it is also possible to change between "Unhighlight" and "Highlight".
3. Go back to syspref and choose to disable highlighting by choosing "Don't Highlight".
4. Refresh your results page if you have not closed it or reload a new search.
5. There is no highlighting at all and anchors do not exist (same behaviour as the OPAC interface).

Signed-off-by: ManuB <e.betemps@gmail.com>
Comment 35 Katrin Fischer 2022-02-13 12:47:16 UTC
Created attachment 130531 [details] [review]
Bug 20398 : Added a syspref to highlight or not highlight search terms in results on the staff interface

There is a possibility on OPAC but not yet on the staff interface, so I added this syspref which has the same behaviour as the OPAC highlighting syspref.
When you choose to disable highlighting on the syspref, the toggle anchor in the result page does not appear at all.
But if you keep the default state (highlighting on), you still have the option to switch between "Unhighlight" and "Highlight" modes.

Test plan :
1. Go to the syspref named "StaffHighlightedWords" and see that the default state is "Highlight".
2. Look for something in the catalogue for example and see that the words are highlighted and that it is also possible to change between "Unhighlight" and "Highlight".
3. Go back to syspref and choose to disable highlighting by choosing "Don't Highlight".
4. Refresh your results page if you have not closed it or reload a new search.
5. There is no highlighting at all and anchors do not exist (same behaviour as the OPAC interface).

Signed-off-by: ManuB <e.betemps@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Katrin Fischer 2022-02-13 12:47:22 UTC
Created attachment 130532 [details] [review]
Bug 20398: (QA follow-up) Fix alphabetical order in sysprefs.sql

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 37 Katrin Fischer 2022-02-13 12:47:54 UTC
Hi Thibaud,

1) Commit message 
Please make sure to run the QA test tools on your patches - they can highlight things like mistakes with the commit message and give other helpful hints. In this case there is an extra space that needs to be removed: Bug xxxx : should be Bug xxxx:

2) b/installer/data/mysql/mandatory/sysprefs.sql
This file is sorted alphabetically, as you are dealing with librarians here, we take this seriously :)

3) Database update
For some time we are using a new format for database updates, see here:
https://wiki.koha-community.org/wiki/Database_updates

That's for next time.
Comment 38 Thibaud Guillot (thibaud_g) 2022-02-14 08:04:05 UTC
Hi Katrin, 

Thanks to you, I will remember that :)
Comment 39 Fridolin Somers 2022-02-14 23:01:31 UTC
In results.js :

Shouldn't block :
    if( PREF_StaffHighlightedWords == 1 ){
        highlightOn();
    } else {
        highlightOff();
    }

Be inside :
  if( search_result.query_desc )

?
Comment 40 Thibaud Guillot (thibaud_g) 2022-03-29 13:41:56 UTC
Created attachment 132414 [details] [review]
Bug 20398: (QA Follow-up) Fix position of 'if' statement
Comment 41 Fridolin Somers 2022-04-08 12:59:11 UTC
Commit messages corrected :
no space before ':'
Follow-up => follow-up
Comment 42 Fridolin Somers 2022-04-08 13:52:14 UTC
Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑