Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests

Browse source 
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>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Martin Renvoize 2022-04-22 15:19:29 +01:00 committed by Fridolin Somers
parent a54646ff69
commit 83bd88e040
3 changed files with 31 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-header.inc
View file

@ -24,7 +24,8 @@
<p><label for="searchfieldstype">Search fields:</label> <p><label for="searchfieldstype">Search fields:</label>
<select name="searchfieldstype" id="searchfieldstype"> <select name="searchfieldstype" id="searchfieldstype">
[% SET standard = Koha.Preference('DefaultPatronSearchFields') || 'firstname,surname,othernames,cardnumber,userid' %] [% 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 %] [% FOREACH s_o IN search_options %]
[% display_name = PROCESS patron_fields name=s_o %] [% display_name = PROCESS patron_fields name=s_o %]
[% NEXT IF !display_name.length %] [% NEXT IF !display_name.length %]

3
koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc
View file

@ -61,7 +61,8 @@
<label for="searchfieldstype_filter">Search field:</label> <label for="searchfieldstype_filter">Search field:</label>
<select name="searchfieldstype" id="searchfieldstype_filter"> <select name="searchfieldstype" id="searchfieldstype_filter">
[% SET standard = Koha.Preference('DefaultPatronSearchFields') || 'firstname,surname,othernames,cardnumber,userid' %] [% 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 %] [% FOREACH s_o IN search_options %]
[% display_name = PROCESS patron_fields name=s_o %] [% display_name = PROCESS patron_fields name=s_o %]
[% NEXT IF !display_name %] [% NEXT IF !display_name %]

47
t/db_dependent/selenium/patrons_search.t
View file

@ -51,7 +51,7 @@ my $base_url = $s->base_url;
my $builder = t::lib::TestBuilder->new; my $builder = t::lib::TestBuilder->new;
subtest 'Search patrons' => sub { subtest 'Search patrons' => sub {
plan tests => 19; plan tests => 23;
if ( Koha::Patrons->search({surname => {-like => "test_patron_%"}})->count ) { 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"); 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); C4::Context->set_preference('PatronsPerPage', $PatronsPerPage);
$driver->get( $base_url . "/members/members-home.pl" ); $driver->get( $base_url . "/members/members-home.pl" );
my @adv_options = $driver->find_elements('//select[@id="searchfieldstype"]/option'); 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'); 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, 13, 'All standard fields are searchable by filter if DefaultPatronSearchFields not set');
is( scalar @filter_options, 11, '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',"initials"); 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 @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" ); $driver->get( $base_url . "/members/members-home.pl" );
@adv_options = $driver->find_elements('//select[@id="searchfieldstype"]/option'); @adv_options = $driver->find_elements('//select[@id="searchfieldstype"]/option');
@filter_options = $driver->find_elements('//select[@id="searchfieldstype_filter"]/option'); @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 @adv_options, 13, 'Invalid option not added when DefaultPatronSearchFields is populated with an invalid field');
is( scalar @filter_options, 12, 'New filter option added when DefaultPatronSearchFields is populated with a field'); is( scalar @filter_options, 13, 'Invalid filter option not added when DefaultPatronSearchFields is populated with an invalid field');
C4::Context->set_preference('DefaultPatronSearchFields',"initials,horses"); # NOTE: We should probably ensure the bad field is removed from 'standard' search here, else searches are broken
$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');
C4::Context->set_preference('DefaultPatronSearchFields',""); C4::Context->set_preference('DefaultPatronSearchFields',"");
$driver->get( $base_url . "/members/members-home.pl" );
$s->fill_form( { search_patron_filter => 'test_patron' } ); $s->fill_form( { search_patron_filter => 'test_patron' } );
$s->submit_form; $s->submit_form;
my $first_patron = $patrons[0]; my $first_patron = $patrons[0];
@ -190,7 +196,8 @@ subtest 'Search patrons' => sub {
"Modify patron %s %s (%s) %s (%s) (%s) › Patrons › Koha", "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->title, $first_patron->firstname, $first_patron->othernames, $first_patron->surname, $first_patron->cardnumber,
$first_patron->category->description, $first_patron->category->description,
) ),
'Page title is correct after following modification link'
); );
$driver->get( $base_url . "/members/members-home.pl" ); $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->driver->find_element('//*[@id="'.$table_id.'_filter"]//input')->send_keys('test_patron');
$s->wait_for_ajax; $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->driver->find_element('//table[@id="'.$table_id.'"]//th[@data-filter="libraries"]/select/option[@value="'.$library->branchcode.'"]')->click;
$s->wait_for_ajax; $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 # Reset the filters
$driver->find_element('//form[@id="patron_search_form"]//*[@id="clear_search"]')->click(); $driver->find_element('//form[@id="patron_search_form"]//*[@id="clear_search"]')->click();
@ -212,14 +219,14 @@ subtest 'Search patrons' => sub {
$s->wait_for_ajax; $s->wait_for_ajax;
# And make sure all the patrons are present # 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! # Search on non-searchable attribute, we expect no result!
$s->fill_form( { search_patron_filter => 'test_attr_1' } ); $s->fill_form( { search_patron_filter => 'test_attr_1' } );
$s->submit_form; $s->submit_form;
$s->wait_for_ajax; $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 # clear form
$driver->find_element('//form[@id="patron_search_form"]//*[@id="clear_search"]')->click(); $driver->find_element('//form[@id="patron_search_form"]//*[@id="clear_search"]')->click();
@ -228,19 +235,19 @@ subtest 'Search patrons' => sub {
$s->submit_form; $s->submit_form;
$s->wait_for_ajax; $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 # Refine search and search for test_patron in all the data using the DT global search
# No change in result expected, still 2 patrons # No change in result expected, still 2 patrons
$s->driver->find_element('//*[@id="'.$table_id.'_filter"]//input')->send_keys('test_patron'); $s->driver->find_element('//*[@id="'.$table_id.'_filter"]//input')->send_keys('test_patron');
$s->wait_for_ajax; $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 # Adding the surname of the first patron in the "Name" column
# We expect only 1 result # We expect only 1 result
$s->driver->find_element('//table[@id="'.$table_id.'"]//input[@placeholder="Name search"]')->send_keys($patrons[0]->surname); $s->driver->find_element('//table[@id="'.$table_id.'"]//input[@placeholder="Name search"]')->send_keys($patrons[0]->surname);
$s->wait_for_ajax; $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, $_ for @patrons;
push @cleanup, $library; push @cleanup, $library;