From 436ad74399c5ca14cbbeaa21c6de82a08ea878ee Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 28 Feb 2019 08:56:04 -0500 Subject: [PATCH] Bug 17168: (follow-up) Address QA concerns and update for strict mode Signed-off-by: Martin Renvoize --- Koha/Patrons.pm | 58 ++++++++++++------------ misc/cronjobs/update_patrons_category.pl | 24 +++++----- t/db_dependent/Patrons.t | 38 ++++++---------- 3 files changed, 55 insertions(+), 65 deletions(-) diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index f02dfad4e4..ca759a6b7a 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -360,36 +360,37 @@ sub anonymize { if( $params->{verbose} ) { warn "Anonymized $count patrons\n"; } +} -=head3 search_patrons_to_update +=head3 search_patrons_to_update_category - my $patrons = Koha::Patrons->search_patrons_to_anonymise( { - from => $from_category, - fine_max => $fine_max, - fine_min => $fin_min, - au => $au, - ao => $ao, + my $patrons = Koha::Patrons->search_patrons_to_update_category( { + from => $from_category, + fine_max => $fine_max, + fine_min => $fin_min, + too_young => $too_young, + too_old => $too_old, }); -This method returns all patron who should be updated form one category to another meeting criteria: +This method returns all patron who should be updated from one category to another meeting criteria: -from - original category -fine_min - with fines totaling at least this amount -fine_max - with fines above this amount -au - under the age limit for 'from' -ao - over the agelimit for 'from' +from - borrower categorycode +fine_min - with fines totaling at least this amount +fine_max - with fines above this amount +too_young - if passed, select patrons who are under the age limit for the current category +too_old - if passed, select patrons who are over the age limit for the current category =cut -sub search_patrons_to_update { +sub search_patrons_to_update_category { my ( $self, $params ) = @_; my %query; - my $search_params = $params->{search_params};; + my $search_params; my $cat_from = Koha::Patron::Categories->find($params->{from}); $search_params->{categorycode}=$params->{from}; - if ($params->{ageover} || $params->{ageunder}){ - if( $cat_from->dateofbirthrequired && $params->{ageunder} ) { + if ($params->{too_young} || $params->{too_old}){ + if( $cat_from->dateofbirthrequired && $params->{too_young} ) { my $date_after = output_pref({ dt => dt_from_string()->subtract( years => $cat_from->dateofbirthrequired), dateonly => 1, @@ -397,7 +398,7 @@ sub search_patrons_to_update { }); $search_params->{dateofbirth}{'>'} = $date_after; } - if( $cat_from->upperagelimit && $params->{ageover} ) { + if( $cat_from->upperagelimit && $params->{too_old} ) { my $date_before = output_pref({ dt => dt_from_string()->subtract( years => $cat_from->upperagelimit), dateonly => 1, @@ -408,30 +409,29 @@ sub search_patrons_to_update { } if ($params->{fine_min} || $params->{fine_max}) { $query{join} = ["accountlines"]; - $query{select} = ["borrowernumber", { sum => 'amountoutstanding', -as => 'total_fines'} ]; - $query{as} = [qw/borrowernumber total_fines/]; + $query{select} = ["borrowernumber", "accountlines.amountoutstanding" ]; $query{group_by} = ["borrowernumber"]; - $query{having}{total_fines}{'<='}=$params->{fine_max} if defined $params->{fine_max}; - $query{having}{total_fines}{'>='}=$params->{fine_min} if defined $params->{fine_min}; + $query{having} = \['sum(accountlines.amountoutstanding) <= ?',$params->{fine_max}] if defined $params->{fine_max}; + $query{having} = \['sum(accountlines.amountoutstanding) >= ?',$params->{fine_min}] if defined $params->{fine_min}; } - return Koha::Patrons->search($search_params,\%query); + return $self->search($search_params,\%query); } -=head3 update_category +=head3 update_category_to - Koha::Patrons->search->update_category( { - to => $to, + Koha::Patrons->search->update_category_to( { + category => $to_category, }); -Update supplied patrons from one category to another and take care of guarantor info. +Update supplied patrons from current category to another and take care of guarantor info. To make sure all the conditions are met, the caller has the responsibility to call search_patrons_to_update to filter the Koha::Patrons set =cut -sub update_category { +sub update_category_to { my ( $self, $params ) = @_; - my $to = $params->{to}; + my $to = $params->{category}; my $to_cat = Koha::Patron::Categories->find($to); return unless $to_cat; diff --git a/misc/cronjobs/update_patrons_category.pl b/misc/cronjobs/update_patrons_category.pl index 202de57830..25b70d2008 100644 --- a/misc/cronjobs/update_patrons_category.pl +++ b/misc/cronjobs/update_patrons_category.pl @@ -40,7 +40,7 @@ update_patrons_category.pl - Given a set of parameters update selected patrons f =head1 SYNOPSIS update_patrons_category.pl -f=categorycode -t=categorycode - [-b=branchcode] [-au] [-ao] [-fo=X] [-fu=X] + [-b=branchcode] [--too_old] [--too_young] [-fo=X] [-fu=X] [-rb=date] [-ra=date] [-v] [--field column=value ...] @@ -49,13 +49,13 @@ update_patrons_category.pl --help | --man Options: --help brief help message --man full documentation - -ao --ageover update if over maximum age for current category - -au --ageunder update if under minimuum age current category + -too_old update if over maximum age for current category + -too_young update if under minimuum age current category -fo=X --fineover=X update if fines over X amount -fu=X --fineunder=X update if fines under X amount -rb=date --regbefore update if registration date is before given date -ra=date --regafter update if registration date is after a given date - -d --dbfield name=value where is a column in the borrowers table, patrons will be updated if the field is equal to given + -d --dbfield name=value where is a column in the borrowers table, patrons will be updated if the field is equal to given -v -verbose verbose mode -c --confirm commit changes to db, no action will be taken unless this switch is included -b --branch only deal with patrons from this library/branch @@ -95,11 +95,11 @@ branches.branchcode table. *required* defines the category patrons will be converted to. Expects the code from categories.categorycode. -=item B<--ageover | -ao> +=item B<--too_old> Update patron only if they are above the maximum age range specified for the 'from' category. -=item B<--ageunde | -au> +=item B<--too_young> Update patron only if they are below the minimum age range specified for the 'from' category. @@ -139,7 +139,7 @@ C - Suggests that you read this help. :) C -b= -f= -t= --confirm - Processes a single branch, and updates the patron categories from fromcat to tocat. -C -b= -f= -t= -a --confirm - Processes a single branch, and updates the patron categories from fromcat to tocat for patrons outside the age range of fromcat. +C -b= -f= -t= --too_old --confirm - Processes a single branch, and updates the patron categories from fromcat to tocat for patrons over the age range of fromcat. C -f= -t= -v - Processes all branches, shows all messages, and reports the patrons who would be affected. Takes no action on the database. @@ -171,8 +171,8 @@ GetOptions( 'c|confirm' => \$doit, 'f|from=s' => \$fromcat, 't|to=s' => \$tocat, - 'ao|ageover' => \$ageover, - 'au|ageunder' => \$ageunder, + 'too_old' => \$ageover, + 'too_young' => \$ageunder, 'fo|finesover=s' => \$fine_min, 'fu|finesunder=s' => \$fine_max, 'rb|regbefore=s' => \$reg_bef, @@ -244,12 +244,12 @@ while ( my ( $key, $value ) = each %fields ) { $params{ "me." . $key } = $value; } -my $target_patrons = Koha::Patrons->search_patrons_to_update( +my $target_patrons = Koha::Patrons->search({ search_params => \%params })->search_patrons_to_update( { from => $fromcat, search_params => \%params, - ageunder => $ageunder, - ageover => $ageover, + too_young => $ageunder, + too_old => $ageover, fine_min => $fine_min, fine_max => $fine_max, } diff --git a/t/db_dependent/Patrons.t b/t/db_dependent/Patrons.t index 3420b499b7..549a83bbe8 100755 --- a/t/db_dependent/Patrons.t +++ b/t/db_dependent/Patrons.t @@ -105,10 +105,7 @@ foreach my $b ( $patrons->as_list() ) { } subtest "Update patron categories" => sub { - plan tests => 23; - $builder->schema->resultset( 'Issue' )->delete_all; - $builder->schema->resultset( 'Borrower' )->delete_all; - $builder->schema->resultset( 'Category' )->delete_all; + plan tests => 17; my $c_categorycode = $builder->build({ source => 'Category', value => { category_type=>'C', upperagelimit=>17, @@ -167,31 +164,24 @@ subtest "Update patron categories" => sub { $builder->build({source=>'Accountline',value => {amountoutstanding=>4.99,borrowernumber=>$adult1->{borrowernumber}}}); $builder->build({source=>'Accountline',value => {amountoutstanding=>5.01,borrowernumber=>$adult2->{borrowernumber}}}); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode})->count,3,'Three patrons in child category'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ageunder=>1})->count,1,'One under age patron in child category'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ageunder=>1})->next->borrowernumber,$child1->{borrowernumber},'Under age patron in child category is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ageover=>1})->count,1,'One over age patron in child category'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ageover=>1})->next->borrowernumber,$child3->{borrowernumber},'Over age patron in child category is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{branchcode=>$branchcode2}})->count,1,'One patron in branch 2'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{branchcode=>$branchcode2}})->next->borrowernumber,$adult2->{borrowernumber},'Adult patron in branch 2 is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,fine_min=>5})->count,1,'One patron with fines over $5'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,fine_min=>5})->next->borrowernumber,$adult2->{borrowernumber},'One patron with fines over $5 is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,fine_max=>5})->count,1,'One patron with fines under $5'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,fine_max=>5})->next->borrowernumber,$adult1->{borrowernumber},'One patron with fines under $5 is expected one'); - - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{dateenrolled=>{'<'=>'2018-01-01'}}})->count,1,'One adult patron enrolled before date'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{dateenrolled=>{'<'=>'2018-01-01'}}})->next->borrowernumber,$adult2->{borrowernumber},'One adult patron enrolled before date is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{dateenrolled=>{'>'=>'2017-01-01'}}})->count,1,'One adult patron enrolled after date'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{dateenrolled=>{'>'=>'2017-01-01'}}})->next->borrowernumber,$adult1->{borrowernumber},'One adult patron enrolled after date is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{'sort1'=>'quack'}})->count,1,'One adult patron has a quack'); - is( Koha::Patrons->search_patrons_to_update({from=>$a_categorycode,search_params=>{'sort1'=>'quack'}})->next->borrowernumber,$adult1->{borrowernumber},'One adult patron with a quack is expected one'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode})->count,3,'Three patrons in child category'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode,too_young=>1})->count,1,'One under age patron in child category'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode,too_young=>1})->next->borrowernumber,$child1->{borrowernumber},'Under age patron in child category is expected one'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode,too_old=>1})->count,1,'One over age patron in child category'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode,too_old=>1})->next->borrowernumber,$child3->{borrowernumber},'Over age patron in child category is expected one'); + is( Koha::Patrons->search({branchcode=>$branchcode2})->search_patrons_to_update_category({from=>$a_categorycode})->count,1,'One patron in branch 2'); + is( Koha::Patrons->search({branchcode=>$branchcode2})->search_patrons_to_update_category({from=>$a_categorycode})->next->borrowernumber,$adult2->{borrowernumber},'Adult patron in branch 2 is expected one'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$a_categorycode,fine_min=>5})->count,1,'One patron with fines over $5'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$a_categorycode,fine_min=>5})->next->borrowernumber,$adult2->{borrowernumber},'One patron with fines over $5 is expected one'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$a_categorycode,fine_max=>5})->count,1,'One patron with fines under $5'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$a_categorycode,fine_max=>5})->next->borrowernumber,$adult1->{borrowernumber},'One patron with fines under $5 is expected one'); is( Koha::Patrons->find($adult1->{borrowernumber})->guarantees->count,3,'Guarantor has 3 guarantees'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ageunder=>1})->update_category({to=>$a_categorycode}),1,'One child patron updated to adult category'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category'); is( Koha::Patrons->find($adult1->{borrowernumber})->guarantees->count,2,'Guarantee was removed when made adult'); is( Koha::Patrons->find($inst->{borrowernumber})->guarantees->count,1,'Guarantor has 1 guarantees'); - is( Koha::Patrons->search_patrons_to_update({from=>$p_categorycode})->update_category({to=>$a_categorycode}),1,'One professional patron updated to adult category'); + is( Koha::Patrons->search_patrons_to_update_category({from=>$p_categorycode})->update_category_to({category=>$a_categorycode}),1,'One professional patron updated to adult category'); is( Koha::Patrons->find($inst->{borrowernumber})->guarantees->count,0,'Guarantee was removed when made adult'); }; -- 2.39.5