Bug 17793 - Make sysprefs search show on all Administration pages
Summary: Make sysprefs search show on all Administration pages
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks: 17864
  Show dependency treegraph
 
Reported: 2016-12-19 22:56 UTC by Aleisha Amohia
Modified: 2017-12-07 22:16 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17793: Make sysprefs search show on Admin home page (1.17 KB, patch)
2016-12-19 22:58 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17793: Make sysprefs search show on all Admin pages (21.89 KB, patch)
2016-12-21 01:33 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17793: Make sysprefs search show on all Admin pages (22.63 KB, patch)
2016-12-21 20:52 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17793: Test Cases (4.52 KB, patch)
2016-12-23 00:57 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 17793: Test Cases (4.57 KB, patch)
2016-12-23 01:04 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 17793: Make sysprefs search show on all Admin pages (22.69 KB, patch)
2016-12-23 01:04 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17793: Test Cases (4.64 KB, patch)
2017-01-09 07:25 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17793: Follow up for identified missing changes (4.95 KB, patch)
2017-01-09 14:58 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17793: Follow up for identified missing changes (5.01 KB, patch)
2017-01-24 15:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17793: Make sysprefs search show on all Admin pages (22.76 KB, patch)
2017-01-27 09:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17793: Follow up for identified missing changes (5.07 KB, patch)
2017-01-27 09:27 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2016-12-19 22:56:09 UTC
At the moment, admin/admin-home.pl shows the cataloguing search with check out, check in, renew and catalogue search options. It seems more appropriate to show the prefs-admin search here instead.
Comment 1 Aleisha Amohia 2016-12-19 22:58:20 UTC
Created attachment 58278 [details] [review]
Bug 17793: Make sysprefs search show on Admin home page

To test:
1) Go to Administration
2) Notice Catalogue search at the top - seems out of place.
3) Apply patch and refresh page.
4) Notice admin / sysprefs search now shows and is more appropriate.
5) Confirm searching for sysprefs still works

Sponsored-by: Catalyst IT
Comment 2 Aleisha Amohia 2016-12-21 01:33:02 UTC
Created attachment 58327 [details] [review]
Bug 17793: Make sysprefs search show on all Admin pages

This patch is amended to use the sysprefs search on all Administration
pages that do not have their own custom search.

To test:
1) Go to Administration
2) Notice Catalogue search at the top - seems out of place.
3) Apply patch and refresh page.
4) Notice admin / sysprefs search now shows and is more appropriate.
5) Confirm searching for sysprefs still works

This patch affects the following pages:
- admin-home.pl
- audio_alerts.pl
- authtypes.pl
- auth_tag_structure.pl
- authorised_values.pl
- biblio_framework.pl
- marctagstructure.pl
- branch_transfer_limits.pl
- branches.pl
- checkmarc.pl
- classsources.pl
- columns_settings.pl
- didyoumean.pl
- edi_accounts.pl
- edi_ean_accounts.pl
- fieldmapping.pl
- item_circulation_alerts.pl
- items_search_fields.pl
- items_search_field.pl
- item_types.pl
- koha2marklinks.pl
- matching-rules.pl
- oai_sets.pl
- oai_set_mappings.pl
- patron-attr-types.pl
- smart-rules.pl
- transport-cost-matrix.pl

Sponsored-by: Catalyst IT
Comment 3 Owen Leonard 2016-12-21 16:12:24 UTC
Looks good to me, but found one missing:

sms_providers.pl
Comment 4 Aleisha Amohia 2016-12-21 20:52:08 UTC
Created attachment 58371 [details] [review]
Bug 17793: Make sysprefs search show on all Admin pages

This patch is amended to use the sysprefs search on all Administration
pages that do not have their own custom search.

To test:
1) Go to Administration
2) Notice Catalogue search at the top - seems out of place.
3) Apply patch and refresh page.
4) Notice admin / sysprefs search now shows and is more appropriate.
5) Confirm searching for sysprefs still works

This patch affects the following pages:
- admin-home.pl
- audio_alerts.pl
- authtypes.pl
- auth_tag_structure.pl
- authorised_values.pl
- biblio_framework.pl
- marctagstructure.pl
- branch_transfer_limits.pl
- branches.pl
- checkmarc.pl
- classsources.pl
- columns_settings.pl
- didyoumean.pl
- edi_accounts.pl
- edi_ean_accounts.pl
- fieldmapping.pl
- item_circulation_alerts.pl
- items_search_fields.pl
- items_search_field.pl
- item_types.pl
- koha2marklinks.pl
- matching-rules.pl
- oai_sets.pl
- oai_set_mappings.pl
- patron-attr-types.pl
- smart-rules.pl
- transport-cost-matrix.pl
- sms_providers.pl

Sponsored-by: Catalyst IT
Comment 5 Mark Tompsett 2016-12-23 00:57:50 UTC
Created attachment 58405 [details] [review]
Bug 17793: Test Cases

There was no form of testing for the change. In order to test
the perl-side of the changes, I just went to a variety of
URLs to see if they have the expected three tabs.

TEST PLAN
---------
1) Apply this patch
2) At the bash prompt, export KOHA_INTRANET_URL=http://localhost:8080/
   -- Or whatever yours is. With or without the trailing /
3) prove -v t/db_dependent/www/prefs-admin_search.t
   -- before applying the other patch, there should be failures.
4) Apply both patches
5) prove -v t/db_dependent/www/prefs-admin_search.t
   -- there should be no failures now.
6) run koha qa test tools
Comment 6 Mark Tompsett 2016-12-23 01:04:37 UTC
Created attachment 58406 [details] [review]
Bug 17793: Test Cases

There was no form of testing for the change. In order to test
the perl-side of the changes, I just went to a variety of
URLs to see if they have the expected three tabs.

TEST PLAN
---------
1) Apply this patch
2) At the bash prompt, export KOHA_INTRANET_URL=http://localhost:8080/
   -- Or whatever yours is. With or without the trailing /
3) prove -v t/db_dependent/www/prefs-admin_search.t
   -- before applying the other patch, there should be failures.
4) Apply both patches
5) prove -v t/db_dependent/www/prefs-admin_search.t
   -- there should be no failures now.
6) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 7 Mark Tompsett 2016-12-23 01:04:47 UTC
Created attachment 58407 [details] [review]
Bug 17793: Make sysprefs search show on all Admin pages

This patch is amended to use the sysprefs search on all Administration
pages that do not have their own custom search.

To test:
1) Go to Administration
2) Notice Catalogue search at the top - seems out of place.
3) Apply patch and refresh page.
4) Notice admin / sysprefs search now shows and is more appropriate.
5) Confirm searching for sysprefs still works

This patch affects the following pages:
- admin-home.pl
- audio_alerts.pl
- authtypes.pl
- auth_tag_structure.pl
- authorised_values.pl
- biblio_framework.pl
- marctagstructure.pl
- branch_transfer_limits.pl
- branches.pl
- checkmarc.pl
- classsources.pl
- columns_settings.pl
- didyoumean.pl
- edi_accounts.pl
- edi_ean_accounts.pl
- fieldmapping.pl
- item_circulation_alerts.pl
- items_search_fields.pl
- items_search_field.pl
- item_types.pl
- koha2marklinks.pl
- matching-rules.pl
- oai_sets.pl
- oai_set_mappings.pl
- patron-attr-types.pl
- smart-rules.pl
- transport-cost-matrix.pl
- sms_providers.pl

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 8 Mark Tompsett 2016-12-23 01:06:10 UTC
Follow the test plan in comment #6 and sign off. :)
Comment 9 Katrin Fischer 2017-01-07 22:55:38 UTC
Marc, your tests don't work for me in koha-shell on kohadevbox, but the feature itself works nicely.

not ok 191 - Expecting no \#checkin_search
#   Failed test 'Expecting no #checkin_search'
#   at t/db_dependent/www/prefs-admin_search.t line 99.
#     searched: "<!DOCTYPE html>\x{0a}<!-- TEMPLATE FILE: sms_providers."...
#    and found: "#checkin_search"
#  at position: 10048 (line 278 column 33)
not ok 192 - Expecting no \#renew_search
#   Failed test 'Expecting no #renew_search'
#   at t/db_dependent/www/prefs-admin_search.t line 100.
#     searched: "<!DOCTYPE html>\x{0a}<!-- TEMPLATE FILE: sms_providers."...
#    and found: "#renew_search"
#  at position: 10115 (line 279 column 33)
not ok 193 - Checking sms_providers.pl failed!
#   Failed test 'Checking sms_providers.pl failed!'
#   at t/db_dependent/www/prefs-admin_search.t line 94.
1..193
# Looks like you failed 109 tests of 193.

Maybe we should fix them on another bug so this can move forward?

Aleisha, failing because I found some more missing - quick to fix:
/cgi-bin/koha/plugins/plugins-home.pl
/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl
Comment 10 Mark Tompsett 2017-01-07 23:15:29 UTC
Comment on attachment 58406 [details] [review]
Bug 17793: Test Cases

moved to bug 17864, as per Katrin's suggestion. I'll tweak up the test coverage once the other two cases Katrin mentioned are dealt with.
Comment 11 Josef Moravec 2017-01-09 07:25:48 UTC
Created attachment 58670 [details] [review]
[SIGNED-OFF] Bug 17793: Test Cases

There was no form of testing for the change. In order to test
the perl-side of the changes, I just went to a variety of
URLs to see if they have the expected three tabs.

TEST PLAN
---------
1) Apply this patch
2) At the bash prompt, export KOHA_INTRANET_URL=http://localhost:8080/
   -- Or whatever yours is. With or without the trailing /
3) prove -v t/db_dependent/www/prefs-admin_search.t
   -- before applying the other patch, there should be failures.
4) Apply both patches
5) prove -v t/db_dependent/www/prefs-admin_search.t
   -- there should be no failures now.
6) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2017-01-09 07:29:12 UTC
Sorry, I didn't notice, the changes here...

Katrin, the test is working fine in koha-shell on kohadevbox for me... 

I noticed the missing elasticsearch mapping page, and think we should add it to clone-rules.pl too - it's not used directly, but can be, so for the case, it doesn't hurt to make the same change I think...
Comment 13 Mark Tompsett 2017-01-09 14:56:00 UTC
I ran:
$ git grep -l cat-search.inc | grep admin

This means I believe the outstanding ones are (koha-tmpl/intranet-tmpl/prog/en/modules/):
- admin/auth_subfields_structure.tt
- admin/clone-rules.tt
- admin/marc_subfields_structure.tt
- admin/searchengine/elasticsearch/mappings.tt
- plugins/plugins-home.tt
Comment 14 Mark Tompsett 2017-01-09 14:58:45 UTC
Created attachment 58684 [details] [review]
Bug 17793: Follow up for identified missing changes

I ran:
$ git grep -l cat-search.inc | grep admin

This means I believe the outstanding ones are
(koha-tmpl/intranet-tmpl/prog/en/modules/):
- admin/auth_subfields_structure.tt
- admin/clone-rules.tt
- admin/marc_subfields_structure.tt
- admin/searchengine/elasticsearch/mappings.tt

One other was recommended by Katrin in comment #9:
- plugins/plugins-home.tt
Comment 15 Owen Leonard 2017-01-24 15:34:24 UTC
Created attachment 59507 [details] [review]
[SIGNED-OFF] Bug 17793: Follow up for identified missing changes

I ran:
$ git grep -l cat-search.inc | grep admin

This means I believe the outstanding ones are
(koha-tmpl/intranet-tmpl/prog/en/modules/):
- admin/auth_subfields_structure.tt
- admin/clone-rules.tt
- admin/marc_subfields_structure.tt
- admin/searchengine/elasticsearch/mappings.tt

One other was recommended by Katrin in comment #9:
- plugins/plugins-home.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 16 Jonathan Druart 2017-01-25 11:55:57 UTC
So we are going to loose the 'check in' and 'renew' actions from the header search and the syspref search will become the default one.

Does everybody agree with that?
Comment 17 Mark Tompsett 2017-01-26 13:26:32 UTC
(In reply to Jonathan Druart from comment #16)
> So we are going to loose the 'check in' and 'renew' actions from the header
> search and the syspref search will become the default one.
> 
> Does everybody agree with that?

When you are in the admin section, that makes more sense. So yes, I agree with that.
Comment 18 Marcel de Rooy 2017-01-27 08:53:56 UTC
I think it is somewhat arbitrary choice.
If you leave checkout, I would have preferred to leave checkin too e.g.
But the syspref search is handy.
So I do not disagree :)
Comment 19 Josef Moravec 2017-01-27 09:20:20 UTC
Hm, what about

syspref|checkout|checkin|catsearch combination?
Comment 20 Jonathan Druart 2017-01-27 09:27:46 UTC
Created attachment 59608 [details] [review]
Bug 17793: Make sysprefs search show on all Admin pages

This patch is amended to use the sysprefs search on all Administration
pages that do not have their own custom search.

To test:
1) Go to Administration
2) Notice Catalogue search at the top - seems out of place.
3) Apply patch and refresh page.
4) Notice admin / sysprefs search now shows and is more appropriate.
5) Confirm searching for sysprefs still works

This patch affects the following pages:
- admin-home.pl
- audio_alerts.pl
- authtypes.pl
- auth_tag_structure.pl
- authorised_values.pl
- biblio_framework.pl
- marctagstructure.pl
- branch_transfer_limits.pl
- branches.pl
- checkmarc.pl
- classsources.pl
- columns_settings.pl
- didyoumean.pl
- edi_accounts.pl
- edi_ean_accounts.pl
- fieldmapping.pl
- item_circulation_alerts.pl
- items_search_fields.pl
- items_search_field.pl
- item_types.pl
- koha2marklinks.pl
- matching-rules.pl
- oai_sets.pl
- oai_set_mappings.pl
- patron-attr-types.pl
- smart-rules.pl
- transport-cost-matrix.pl
- sms_providers.pl

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2017-01-27 09:27:52 UTC
Created attachment 59609 [details] [review]
Bug 17793: Follow up for identified missing changes

I ran:
$ git grep -l cat-search.inc | grep admin

This means I believe the outstanding ones are
(koha-tmpl/intranet-tmpl/prog/en/modules/):
- admin/auth_subfields_structure.tt
- admin/clone-rules.tt
- admin/marc_subfields_structure.tt
- admin/searchengine/elasticsearch/mappings.tt

One other was recommended by Katrin in comment #9:
- plugins/plugins-home.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Jonathan Druart 2017-01-27 09:29:03 UTC
(In reply to Josef Moravec from comment #19)
> Hm, what about
> 
> syspref|checkout|checkin|catsearch combination?

Oops, did not see your comment before passing QA.
Anybody has opinions?
Comment 23 Mark Tompsett 2017-01-27 13:09:08 UTC
(In reply to Jonathan Druart from comment #22)
> Oops, did not see [Josef Moravec's] comment before passing QA.
> Anybody has opinions?

A new enhancement that actually has configurable search bar letting you also set the default active tab? In short, let this pass, and fix it elsewhere.
Comment 24 Kyle M Hall 2017-02-17 15:34:46 UTC
Pushed to master for 17.05, thanks Aleisha, Mark!
Comment 25 Katrin Fischer 2017-02-19 20:33:00 UTC
This won't get ported back to 16.11.x as it is an enhancement.