From 59acb48d9f67aef1a4b8cbf99ee0cbd7df5ccd36 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Sun, 15 May 2016 10:44:48 -0400 Subject: [PATCH] Bug 14874 - Add ability to search for patrons by date of birth from checkout and patron quick searches This patch adds a new syspef 'DefaultPatronSearchFields' which defines the fields that should be used when searching for a patron if none are defined. To test: 1 - Aply patch, updatedatabase 2 - Ensure patron search has not changed 3 - Add dateofbirth to new pref 4 - Ensure things work wll 5 - Experiment with adding and removing other fields from borrowers table 6 - prove t/db_dependent/Utils/Datatables_Members.t --- C4/Utils/DataTables/Members.pm | 17 +- ...bug14874-addDefaultPatronSearchFieldspreference | 1 + installer/data/mysql/sysprefs.sql | 1 + .../en/includes/circ-patron-search-results.inc | 2 + .../prog/en/modules/admin/preferences/patrons.pref | 4 + t/db_dependent/Utils/Datatables_Members.t | 243 +++++++++++++-------- 6 files changed, 166 insertions(+), 102 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug14874-addDefaultPatronSearchFieldspreference diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm index d63ce51..a421dea 100644 --- a/C4/Utils/DataTables/Members.pm +++ b/C4/Utils/DataTables/Members.pm @@ -59,7 +59,7 @@ sub search { } my $searchfields = { - standard => 'surname,firstname,othernames,cardnumber,userid', + standard => C4::Context->preference('DefaultPatronSearchFields'), surname => 'surname', email => 'email,emailpro,B_email', borrowernumber => 'borrowernumber', @@ -87,10 +87,16 @@ sub search { foreach my $term (@terms) { next unless $term; - $term .= '%' # end with anything - if $term !~ /%$/; - $term = "%$term" # begin with anythin unless start_with - if $searchtype eq 'contain' && $term !~ /^%/; + my $term_dt = eval { local $SIG{__WARN__} = {}; output_pref( { str => $term, dateonly => 1, dateformat => 'sql' } ); }; + + if ($term_dt) { + $term = $term_dt; + } else { + $term .= '%' # end with anything + if $term !~ /%$/; + $term = "%$term" # begin with anythin unless start_with + if $searchtype eq 'contain' && $term !~ /^%/; + } my @where_strs_or; for my $searchfield ( split /,/, $searchfields->{$searchfieldstype} ) { @@ -135,6 +141,7 @@ sub search { ($limit ? $limit : "") ); my $sth = $dbh->prepare($query); + $sth->execute(@where_args); my $patrons = $sth->fetchall_arrayref({}); diff --git a/installer/data/mysql/atomicupdate/bug14874-addDefaultPatronSearchFieldspreference b/installer/data/mysql/atomicupdate/bug14874-addDefaultPatronSearchFieldspreference new file mode 100644 index 0000000..1790e03 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug14874-addDefaultPatronSearchFieldspreference @@ -0,0 +1 @@ +INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('DefaultPatronSearchFields','surname,firstname,othernames,cardnumber,userid',NULL,'Comma separated list defining the default fields to be used during a patron search','free'); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 169665f..ba48a09 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -116,6 +116,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('DefaultLongOverdueChargeValue', '', NULL, "Charge a lost item to the borrower's account when the LOST value of the item changes to n.", 'integer'), ('DefaultLongOverdueDays', '', NULL, "Set the LOST value of an item when the item has been overdue for more than n days.", 'integer'), ('DefaultLongOverdueLostValue', '', NULL, "Set the LOST value of an item to n when the item has been overdue for more than defaultlongoverduedays days.", 'integer'), +('DefaultPatronSearchFields', 'surname,firstname,othernames,cardnumber,userid',NULL,'Comma separated list defining the default fields to be used during a patron search','free'); ('defaultSortField','relevance','relevance|popularity|call_number|pubdate|acqdate|title|author','Specify the default field used for sorting','Choice'), ('defaultSortOrder','dsc','asc|dsc|az|za','Specify the default sort order','Choice'), ('DefaultToLoggedInLibraryCircRules', '0', NULL , 'If enabled, circ rules editor will default to the logged in library''s rules, rather than the ''all libraries'' rules.', 'YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc index eadeefb..8ff24be 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc @@ -16,6 +16,7 @@ Name Card number + Date of birth Category Library Address @@ -31,6 +32,7 @@ [% borrower.surname %], [% borrower.firstname %] [% END %] [% borrower.cardnumber %] + [% borrower.dateofbirth %] [% Categories.GetName( borrower.categorycode ) %] [% Branches.GetName( borrower.branchcode ) %] [% borrower.address %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index 2f2faa0..b8a2a30 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -109,6 +109,10 @@ Patrons: class: integer - results per page in the staff client. - + - pref: DefaultPatronSearchFields + class: multi + - "Comma separated list defining the default fields to be used during a patron search" + - - pref: PatronQuickAddFields class: multi - (separate columns with |) diff --git a/t/db_dependent/Utils/Datatables_Members.t b/t/db_dependent/Utils/Datatables_Members.t index acef35b..8f2aa50 100644 --- a/t/db_dependent/Utils/Datatables_Members.t +++ b/t/db_dependent/Utils/Datatables_Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 27; +use Test::More tests => 47; use C4::Context; use C4::Members; @@ -29,97 +29,80 @@ use Koha::Library; use Koha::Patron::Categories; use t::lib::Mocks; +use t::lib::TestBuilder; + +use Koha::Database; use_ok( "C4::Utils::DataTables::Members" ); -my $dbh = C4::Context->dbh; - -# Start transaction -$dbh->{AutoCommit} = 0; -$dbh->{RaiseError} = 1; - -# Pick a categorycode from the DB -my @categories = Koha::Patron::Categories->search_limited; -my $categorycode = $categories[0]->categorycode; -# Add a new branch so we control what borrowers it has -my $branchcode = "UNC"; -my $branch_data = { - branchcode => $branchcode, - branchname => 'Universidad Nacional de Cordoba', - branchaddress1 => 'Haya de la Torre', - branchaddress2 => 'S/N', - branchzip => '5000', - branchcity => 'Cordoba', - branchstate => 'Cordoba', - branchcountry => 'Argentina' -}; -Koha::Library->new( $branch_data )->store; - -my %john_doe = ( - cardnumber => '123456', - firstname => 'John', - surname => 'Doe', - categorycode => $categorycode, - branchcode => $branchcode, - dateofbirth => '', - dateexpiry => '9999-12-31', - userid => 'john.doe' -); +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; -my %john_smith = ( - cardnumber => '234567', - firstname => 'John', - surname => 'Smith', - categorycode => $categorycode, - branchcode => $branchcode, - dateofbirth => '', - dateexpiry => '9999-12-31', - userid => 'john.smith' -); +my $builder = t::lib::TestBuilder->new; -my %jane_doe = ( - cardnumber => '345678', - firstname => 'Jane', - surname => 'Doe', - categorycode => $categorycode, - branchcode => $branchcode, - dateofbirth => '', - dateexpiry => '9999-12-31', - userid => 'jane.doe' -); +my $library = $builder->build({ + source => "Branch", +}); -my %jeanpaul_dupont = ( - cardnumber => '456789', - firstname => 'Jean Paul', - surname => 'Dupont', - categorycode => $categorycode, - branchcode => $branchcode, - dateofbirth => '', - dateexpiry => '9999-12-31', - userid => 'jeanpaul.dupont' -); +my $branchcode=$library->{branchcode}; + +my $john_doe = $builder->build({ + source => "Borrower", + value => { + cardnumber => '123456', + firstname => 'John', + surname => 'Doe', + branchcode => $branchcode, + dateofbirth => '1983-03-01', + userid => 'john.doe' + }, +}); -my %dupont_brown = ( - cardnumber => '567890', - firstname => 'Dupont', - surname => 'Brown', - categorycode => $categorycode, - branchcode => $branchcode, - dateofbirth => '', - dateexpiry => '9999-12-31', - userid => 'dupont.brown' -); +my $john_smith = $builder->build({ + source => "Borrower", + value => { + cardnumber => '234567', + firstname => 'John', + surname => 'Smith', + branchcode => $branchcode, + dateofbirth => '1982-02-01', + userid => 'john.smith' + }, +}); -$john_doe{borrowernumber} = AddMember( %john_doe ); -warn "Error adding John Doe, check your tests" unless $john_doe{borrowernumber}; -$john_smith{borrowernumber} = AddMember( %john_smith ); -warn "Error adding John Smith, check your tests" unless $john_smith{borrowernumber}; -$jane_doe{borrowernumber} = AddMember( %jane_doe ); -warn "Error adding Jane Doe, check your tests" unless $jane_doe{borrowernumber}; -$jeanpaul_dupont{borrowernumber} = AddMember( %jeanpaul_dupont ); -warn "Error adding Jean Paul Dupont, check your tests" unless $jeanpaul_dupont{borrowernumber}; -$dupont_brown{borrowernumber} = AddMember( %dupont_brown ); -warn "Error adding Dupont Brown, check your tests" unless $dupont_brown{borrowernumber}; +my $jane_doe = $builder->build({ + source => "Borrower", + value => { + cardnumber => '345678', + firstname => 'Jane', + surname => 'Doe', + branchcode => $branchcode, + dateofbirth => '1983-03-01', + userid => 'jane.doe' + }, +}); +my $jeanpaul_dupont = $builder->build({ + source => "Borrower", + value => { + cardnumber => '456789', + firstname => 'Jean Paul', + surname => 'Dupont', + branchcode => $branchcode, + dateofbirth => '1982-02-01', + userid => 'jeanpaul.dupont' + }, +}); +my $dupont_brown = $builder->build({ + source => "Borrower", + value => { + cardnumber => '567890', + firstname => 'Dupont', + surname => 'Brown', + branchcode => $branchcode, + dateofbirth => '1979-01-01', + userid => 'dupont.brown' + }, +}); # Set common datatables params my %dt_params = ( @@ -139,7 +122,7 @@ my $search_results = C4::Utils::DataTables::Members::search({ is( $search_results->{ iTotalDisplayRecords }, 1, "John Doe has only one match on $branchcode (Bug 12595)"); -ok( $search_results->{ patrons }[0]->{ cardnumber } eq $john_doe{ cardnumber } +ok( $search_results->{ patrons }[0]->{ cardnumber } eq $john_doe->{ cardnumber } && ! $search_results->{ patrons }[1], "John Doe is the only match (Bug 12595)"); @@ -156,7 +139,7 @@ is( $search_results->{ iTotalDisplayRecords }, 1, "Jane Doe has only one match on $branchcode (Bug 12595)"); is( $search_results->{ patrons }[0]->{ cardnumber }, - $jane_doe{ cardnumber }, + $jane_doe->{ cardnumber }, "Jane Doe is the only match (Bug 12595)"); # Search "John" @@ -172,11 +155,11 @@ is( $search_results->{ iTotalDisplayRecords }, 2, "There are two John at $branchcode"); is( $search_results->{ patrons }[0]->{ cardnumber }, - $john_doe{ cardnumber }, + $john_doe->{ cardnumber }, "John Doe is the first result"); is( $search_results->{ patrons }[1]->{ cardnumber }, - $john_smith{ cardnumber }, + $john_smith->{ cardnumber }, "John Smith is the second result"); # Search "Doe" @@ -192,11 +175,11 @@ is( $search_results->{ iTotalDisplayRecords }, 2, "There are two Doe at $branchcode"); is( $search_results->{ patrons }[0]->{ cardnumber }, - $john_doe{ cardnumber }, + $john_doe->{ cardnumber }, "John Doe is the first result"); is( $search_results->{ patrons }[1]->{ cardnumber }, - $jane_doe{ cardnumber }, + $jane_doe->{ cardnumber }, "Jane Doe is the second result"); # Search "Smith" as surname - there is only one occurrence of Smith @@ -212,7 +195,7 @@ is( $search_results->{ iTotalDisplayRecords }, 1, "There is one Smith at $branchcode when searching for surname"); is( $search_results->{ patrons }[0]->{ cardnumber }, - $john_smith{ cardnumber }, + $john_smith->{ cardnumber }, "John Smith is the first result"); # Search "Dupont" as surname - Dupont is used both as firstname and surname, we @@ -229,7 +212,7 @@ is( $search_results->{ iTotalDisplayRecords }, 1, "There is one Dupont at $branchcode when searching for surname"); is( $search_results->{ patrons }[0]->{ cardnumber }, - $jeanpaul_dupont{ cardnumber }, + $jeanpaul_dupont->{ cardnumber }, "Jean Paul Dupont is the first result"); # Search "Doe" as surname - Doe is used twice as surname @@ -245,11 +228,11 @@ is( $search_results->{ iTotalDisplayRecords }, 2, "There are two Doe at $branchcode when searching for surname"); is( $search_results->{ patrons }[0]->{ cardnumber }, - $john_doe{ cardnumber }, + $john_doe->{ cardnumber }, "John Doe is the first result"); is( $search_results->{ patrons }[1]->{ cardnumber }, - $jane_doe{ cardnumber }, + $jane_doe->{ cardnumber }, "Jane Doe is the second result"); # Search by userid @@ -292,10 +275,10 @@ $attribute_type->store; C4::Members::Attributes::SetBorrowerAttributes( - $john_doe{borrowernumber}, [ { code => $attribute_type->{code}, value => 'the default value for a common user' } ] + $john_doe->{borrowernumber}, [ { code => $attribute_type->{code}, value => 'the default value for a common user' } ] ); C4::Members::Attributes::SetBorrowerAttributes( - $jane_doe{borrowernumber}, [ { code => $attribute_type->{code}, value => 'the default value for another common user' } ] + $jane_doe->{borrowernumber}, [ { code => $attribute_type->{code}, value => 'the default value for another common user' } ] ); t::lib::Mocks::mock_preference('ExtendedPatronAttributes', 1); @@ -363,7 +346,73 @@ $search_results = C4::Utils::DataTables::Members::search({ is( $search_results->{ iTotalDisplayRecords }, 1, "Jean Paul Dupont is found using contains and two terms search 'Jea Pau' (Bug 15252)"); +my @datetimeprefs = ("dmydot","iso","metric","us"); +my %dates_in_pref = ( + dmydot => ["01.02.1982","01.03.1983","01.01.1979","01.01.1988"], + iso => ["1982-02-01","1983-03-01","1979-01-01","1988-01-01"], + metric => ["01/02/1982","01/03/1983","01/01/1979","01/01/1988"], + us => ["02/01/1982","03/01/1983","01/01/1979","01/01/1988"], + ); +foreach my $dateformloo (@datetimeprefs){ + t::lib::Mocks::mock_preference('dateformat', $dateformloo); + t::lib::Mocks::mock_preference('DefaultPatronSearchFields', 'surname,firstname,othernames,userid,dateofbirth'); + $search_results = C4::Utils::DataTables::Members::search({ + searchmember => $dates_in_pref{$dateformloo}[0], + searchfieldstype => 'standard', + searchtype => 'contain', + branchcode => $branchcode, + dt_params => \%dt_params + }); + + is( $search_results->{ iTotalDisplayRecords }, 2, + "dateformat: $dateformloo Two borrowers have dob $dates_in_pref{$dateformloo}[0], standard search fields plus dob works"); + + $search_results = C4::Utils::DataTables::Members::search({ + searchmember => $dates_in_pref{$dateformloo}[2], + searchfieldstype => 'standard', + searchtype => 'contain', + branchcode => $branchcode, + dt_params => \%dt_params + }); + + is( $search_results->{ iTotalDisplayRecords }, 1, + "dateformat: $dateformloo One borrower has dob $dates_in_pref{$dateformloo}[2], standard search fields plus dob works"); + + $search_results = C4::Utils::DataTables::Members::search({ + searchmember => $dates_in_pref{$dateformloo}[1], + searchfieldstype => 'dateofbirth', + searchtype => 'contain', + branchcode => $branchcode, + dt_params => \%dt_params + }); + + is( $search_results->{ iTotalDisplayRecords }, 2, + "dateformat: $dateformloo Two borrowers have dob $dates_in_pref{$dateformloo}[1], dateofbirth search field works"); + + $search_results = C4::Utils::DataTables::Members::search({ + searchmember => $dates_in_pref{$dateformloo}[3], + searchfieldstype => 'dateofbirth', + searchtype => 'contain', + branchcode => $branchcode, + dt_params => \%dt_params + }); + + is( $search_results->{ iTotalDisplayRecords }, 0, + "dateformat: $dateformloo No borrowers have dob $dates_in_pref{$dateformloo}[3], dateofbirth search field works"); + + $search_results = C4::Utils::DataTables::Members::search({ + searchmember => $dates_in_pref{$dateformloo}[3], + searchfieldstype => 'standard', + searchtype => 'contain', + branchcode => $branchcode, + dt_params => \%dt_params + }); + + is( $search_results->{ iTotalDisplayRecords }, 0, + "dateformat: $dateformloo No borrowers have dob $dates_in_pref{$dateformloo}[3], standard search fields plus dob works"); +} + # End -$dbh->rollback; +$schema->storage->txn_rollback; 1; -- 2.1.4