Bug 20617 - Add 'Search Engine configuration' link to administration menu
Summary: Add 'Search Engine configuration' link to administration menu
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-19 09:50 UTC by Séverine Queune
Modified: 2018-06-14 13:10 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Missing link for Search engine configuration (9.98 KB, image/png)
2018-04-19 09:50 UTC, Séverine Queune
Details
Bug 20617: Add 'Search Engine configuration' link to administration menu (2.73 KB, patch)
2018-04-20 18:43 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20617: Add 'Search Engine configuration' link to administration menu (2.79 KB, patch)
2018-04-20 20:47 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 20617: Add 'Search Engine configuration' link to administration menu (2.94 KB, patch)
2018-04-21 07:01 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 20617: Add 'Search Engine configuration' link to administration menu (2.99 KB, patch)
2018-04-22 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Séverine Queune 2018-04-19 09:50:39 UTC
Created attachment 74530 [details]
Missing link for Search engine configuration

On the #navmenulist of the different administration pages, link to 'Search engine configuration' menu is missing (should be under 'Item search fileds' link).
It should be displayed only if the 'Searchengine' preference is set on 'ElasticSearch' (just like the menu on /admin/admin-home.pl).
Comment 1 Owen Leonard 2018-04-20 18:43:24 UTC Comment hidden (obsolete)
Comment 2 David Bourgault 2018-04-20 20:47:20 UTC Comment hidden (obsolete)
Comment 3 Séverine Queune 2018-04-21 07:01:33 UTC
Created attachment 74669 [details] [review]
Bug 20617: Add 'Search Engine configuration' link to administration menu

This patch adds a link to the Elasticsearch configuration page in the
administration sidebar menu when Elasticsearch is enabled.

This patch also corrects the title of the Elasticsearch page to match
the page heading. "Search engine configuration" instead of
"Elasticsearch mappings."

To test, apply the patch and set the SearchEngine system preference to
"Elasticsearch."

  - View any page in the administration module which shows the
    left-hand sidebar menu. The link to search engine configuration
    should be there.
  - View the search engine configuration page and confirm that the page
    title is correct.
  - Set the SearchEngine system preferenc to "Zebra."
  - The search engine configuration link should no longer appear in the
    administration sidebar menu.

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 4 Séverine Queune 2018-04-21 07:05:54 UTC
Thanks Owen, it works as described !
Just a naive question left :

(In reply to Owen Leonard from comment #1)
> This patch also corrects the title of the Elasticsearch page to match
> the page heading. "Search engine configuration" instead of
> "Elasticsearch mappings."

That's a good idea, but as it only concerns Elasticsearch configuration, why do you prefer to keep "Search engine configuration" instead of "Elasticsearch configuration" ?
Comment 5 Owen Leonard 2018-04-21 18:21:25 UTC
(In reply to Séverine Queune from comment #4)

> That's a good idea, but as it only concerns Elasticsearch configuration, why
> do you prefer to keep "Search engine configuration" instead of
> "Elasticsearch configuration" ?

I wasn't sure which to go with for the same reason. I chose to match the existing page heading because I think "Search engine configuration" is a more user-friendly description. 

I assume that once Elasticsearch is enabled the librarian/administrator doesn't need to know they're configuring Elasticsearch. They only need to configure "the search engine."
Comment 6 Katrin Fischer 2018-04-22 21:46:39 UTC
Created attachment 74708 [details] [review]
Bug 20617: Add 'Search Engine configuration' link to administration menu

This patch adds a link to the Elasticsearch configuration page in the
administration sidebar menu when Elasticsearch is enabled.

This patch also corrects the title of the Elasticsearch page to match
the page heading. "Search engine configuration" instead of
"Elasticsearch mappings."

To test, apply the patch and set the SearchEngine system preference to
"Elasticsearch."

  - View any page in the administration module which shows the
    left-hand sidebar menu. The link to search engine configuration
    should be there.
  - View the search engine configuration page and confirm that the page
    title is correct.
  - Set the SearchEngine system preferenc to "Zebra."
  - The search engine configuration link should no longer appear in the
    administration sidebar menu.

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Séverine Queune 2018-04-23 12:27:56 UTC
(In reply to Owen Leonard from comment #5)
> (In reply to Séverine Queune from comment #4)
> 
> > That's a good idea, but as it only concerns Elasticsearch configuration, why
> > do you prefer to keep "Search engine configuration" instead of
> > "Elasticsearch configuration" ?
> 
> I wasn't sure which to go with for the same reason. I chose to match the
> existing page heading because I think "Search engine configuration" is a
> more user-friendly description. 
> 
> I assume that once Elasticsearch is enabled the librarian/administrator
> doesn't need to know they're configuring Elasticsearch. They only need to
> configure "the search engine."

Your logic makes sense, I'm ok with it :)
Comment 8 Jonathan Druart 2018-04-23 17:29:32 UTC
Pushed to master for 18.05, thanks to everybody involved!