Bugzilla – Attachment 134212 Details for
Bug 30576
DefaultPatronSearchFields no longer takes effect
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests
Bug-30576-follow-up-Corrections-to-behaviour-to-re.patch (text/plain), 12.60 KB, created by
Nick Clemens (kidclamp)
on 2022-04-28 10:04:29 UTC
(
hide
)
Description:
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-04-28 10:04:29 UTC
Size:
12.60 KB
patch
obsolete
>From bd753ecea85814f93291fcb2ea7f6ac855d86897 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Fri, 22 Apr 2022 15:19:29 +0100 >Subject: [PATCH] Bug 30576: (follow-up) Corrections to behaviour to reflect > unit tests > >The unit tests highlighted my original patch didn't cover the full >preference description. > >We now replace the 'standard' option with the fields from the preference >and we also add those fields as options to the field selection in >advanced searches. > >This patch also adjusts the tests to test for that and reflects the >expected changes to the number of options displayed in the select boxes. > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > .../prog/en/includes/patron-search-header.inc | 3 +- > .../prog/en/includes/patron-search.inc | 3 +- > t/db_dependent/selenium/patrons_search.t | 41 +++++++++++-------- > 3 files changed, 28 insertions(+), 19 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-header.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-header.inc >index 5a1df7600e..f5e8dddcde 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-header.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-header.inc >@@ -24,7 +24,8 @@ > <p><label for="searchfieldstype">Search fields:</label> > <select name="searchfieldstype" id="searchfieldstype"> > [% SET standard = Koha.Preference('DefaultPatronSearchFields') || 'firstname,surname,othernames,cardnumber,userid' %] >- [% SET search_options = [ standard, 'surname', 'cardnumber', 'email', 'borrowernumber', 'userid', 'phone', 'address', 'dateofbirth', 'sort1', 'sort2' ] %] >+ [% default_fields = [ standard, 'surname', 'cardnumber', 'email', 'borrowernumber', 'userid', 'phone', 'address', 'dateofbirth', 'sort1', 'sort2' ] %] >+ [% search_options = default_fields.merge(standard.split(',')).unique %] > [% FOREACH s_o IN search_options %] > [% display_name = PROCESS patron_fields name=s_o %] > [% NEXT IF !display_name.length %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc >index 07997bf945..ec68d3620a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc >@@ -61,7 +61,8 @@ > <label for="searchfieldstype_filter">Search field:</label> > <select name="searchfieldstype" id="searchfieldstype_filter"> > [% SET standard = Koha.Preference('DefaultPatronSearchFields') || 'firstname,surname,othernames,cardnumber,userid' %] >- [% SET search_options = [ standard, 'surname', 'cardnumber', 'email', 'borrowernumber', 'userid', 'phone', 'address', 'dateofbirth', 'sort1', 'sort2' ] %] >+ [% default_fields = [ standard, 'surname', 'cardnumber', 'email', 'borrowernumber', 'userid', 'phone', 'address', 'dateofbirth', 'sort1', 'sort2' ] %] >+ [% search_options = default_fields.merge(standard.split(',')).unique %] > [% FOREACH s_o IN search_options %] > [% display_name = PROCESS patron_fields name=s_o %] > [% NEXT IF !display_name %] >diff --git a/t/db_dependent/selenium/patrons_search.t b/t/db_dependent/selenium/patrons_search.t >index 8d64c7646d..eb2014fd97 100755 >--- a/t/db_dependent/selenium/patrons_search.t >+++ b/t/db_dependent/selenium/patrons_search.t >@@ -51,7 +51,7 @@ my $base_url = $s->base_url; > my $builder = t::lib::TestBuilder->new; > > subtest 'Search patrons' => sub { >- plan tests => 19; >+ plan tests => 23; > > if ( Koha::Patrons->search({surname => {-like => "test_patron_%"}})->count ) { > BAIL_OUT("Cannot run this test, data we need to create already exist in the DB"); >@@ -142,22 +142,28 @@ subtest 'Search patrons' => sub { > C4::Context->set_preference('PatronsPerPage', $PatronsPerPage); > $driver->get( $base_url . "/members/members-home.pl" ); > my @adv_options = $driver->find_elements('//select[@id="searchfieldstype"]/option'); >+ is( scalar @adv_options, 13, 'All standard fields are searchable if DefaultPatronSearchFields not set'); >+ is( $adv_options[0]->get_value(), 'firstname,surname,othernames,cardnumber,userid', 'Standard search uses hard coded list when DefaultPatronSearchFields not set'); > my @filter_options = $driver->find_elements('//select[@id="searchfieldstype_filter"]/option'); >- is( scalar @adv_options, 11, 'All standard fields are searchable if DefaultPatronSearchFields not set'); >- is( scalar @filter_options, 11, 'All standard fields are searchable by filter if DefaultPatronSearchFields not set'); >- C4::Context->set_preference('DefaultPatronSearchFields',"initials"); >+ is( scalar @filter_options, 13, 'All standard fields are searchable by filter if DefaultPatronSearchFields not set'); >+ is( $filter_options[0]->get_value(), 'firstname,surname,othernames,cardnumber,userid', 'Standard filter uses hard coded list when DefaultPatronSearchFields not set'); >+ C4::Context->set_preference('DefaultPatronSearchFields',"firstname,initials"); > $driver->get( $base_url . "/members/members-home.pl" ); > @adv_options = $driver->find_elements('//select[@id="searchfieldstype"]/option'); >+ is( scalar @adv_options, 13, 'New option added when DefaultPatronSearchFields is populated with a field'); >+ is( $adv_options[0]->get_value(), 'firstname,initials', 'Standard search uses DefaultPatronSearchFields when populated'); > @filter_options = $driver->find_elements('//select[@id="searchfieldstype_filter"]/option'); >- is( scalar @adv_options, 12, 'New option added when DefaultPatronSearchFields is populated with a field'); >- is( scalar @filter_options, 12, 'New filter option added when DefaultPatronSearchFields is populated with a field'); >- C4::Context->set_preference('DefaultPatronSearchFields',"initials,horses"); >+ is( scalar @filter_options, 13, 'New filter option added when DefaultPatronSearchFields is populated with a field'); >+ is( $filter_options[0]->get_value(), 'firstname,initials', 'Standard filter uses DefaultPatronSearchFields when populated'); >+ C4::Context->set_preference('DefaultPatronSearchFields',"firstname,initials,horses"); > $driver->get( $base_url . "/members/members-home.pl" ); > @adv_options = $driver->find_elements('//select[@id="searchfieldstype"]/option'); > @filter_options = $driver->find_elements('//select[@id="searchfieldstype_filter"]/option'); >- is( scalar @adv_options, 12, 'Invalid option not added when DefaultPatronSearchFields is populated with an invalid field'); >- is( scalar @filter_options, 12, 'Invalid filter option not added when DefaultPatronSearchFields is populated with an invalid field'); >+ is( scalar @adv_options, 13, 'Invalid option not added when DefaultPatronSearchFields is populated with an invalid field'); >+ is( scalar @filter_options, 13, 'Invalid filter option not added when DefaultPatronSearchFields is populated with an invalid field'); >+ # NOTE: We should probably ensure the bad field is removed from 'standard' search here, else searches are broken > C4::Context->set_preference('DefaultPatronSearchFields',""); >+ $driver->get( $base_url . "/members/members-home.pl" ); > $s->fill_form( { search_patron_filter => 'test_patron' } ); > $s->submit_form; > my $first_patron = $patrons[0]; >@@ -190,7 +196,8 @@ subtest 'Search patrons' => sub { > "Modify patron %s %s (%s) %s (%s) (%s) ⺠Patrons ⺠Koha", > $first_patron->title, $first_patron->firstname, $first_patron->othernames, $first_patron->surname, $first_patron->cardnumber, > $first_patron->category->description, >- ) >+ ), >+ 'Page title is correct after following modification link' > ); > > $driver->get( $base_url . "/members/members-home.pl" ); >@@ -200,11 +207,11 @@ subtest 'Search patrons' => sub { > > $s->driver->find_element('//*[@id="'.$table_id.'_filter"]//input')->send_keys('test_patron'); > $s->wait_for_ajax; >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', $PatronsPerPage, 26, $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', $PatronsPerPage, 26, $total_number_of_patrons), 'Searching in standard brings back correct results' ); > > $s->driver->find_element('//table[@id="'.$table_id.'"]//th[@data-filter="libraries"]/select/option[@value="'.$library->branchcode.'"]')->click; > $s->wait_for_ajax; >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', $PatronsPerPage, 25, $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', $PatronsPerPage, 25, $total_number_of_patrons), 'Filtering on library works in combination with main search' ); > > # Reset the filters > $driver->find_element('//form[@id="patron_search_form"]//*[@id="clear_search"]')->click(); >@@ -212,14 +219,14 @@ subtest 'Search patrons' => sub { > $s->wait_for_ajax; > > # And make sure all the patrons are present >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries', $PatronsPerPage, $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries', $PatronsPerPage, $total_number_of_patrons), 'Resetting filters works as expected' ); > > # Search on non-searchable attribute, we expect no result! > $s->fill_form( { search_patron_filter => 'test_attr_1' } ); > $s->submit_form; > $s->wait_for_ajax; > >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('No entries to show (filtered from %s total entries)', $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('No entries to show (filtered from %s total entries)', $total_number_of_patrons), 'Searching on a non-searchable attribute returns no results' ); > > # clear form > $driver->find_element('//form[@id="patron_search_form"]//*[@id="clear_search"]')->click(); >@@ -228,19 +235,19 @@ subtest 'Search patrons' => sub { > $s->submit_form; > $s->wait_for_ajax; > >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', 2, 2, $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', 2, 2, $total_number_of_patrons), 'Searching on a searchable attribute returns correct results' ); > > # Refine search and search for test_patron in all the data using the DT global search > # No change in result expected, still 2 patrons > $s->driver->find_element('//*[@id="'.$table_id.'_filter"]//input')->send_keys('test_patron'); > $s->wait_for_ajax; >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', 2, 2, $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', 2, 2, $total_number_of_patrons), 'Refining with DataTables search works to further filter the original query' ); > > # Adding the surname of the first patron in the "Name" column > # We expect only 1 result > $s->driver->find_element('//table[@id="'.$table_id.'"]//input[@placeholder="Name search"]')->send_keys($patrons[0]->surname); > $s->wait_for_ajax; >- is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', 1, 1, $total_number_of_patrons) ); >+ is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', 1, 1, $total_number_of_patrons), 'Refining with header filters works to further filter the original query' ); > > push @cleanup, $_ for @patrons; > push @cleanup, $library; >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30576
:
133543
|
133555
|
133684
|
133685
|
134059
|
134060
|
134084
|
134085
|
134211
| 134212