From 26bf8460ce29d1c9c4257838d3f9e284eddda8c7 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 31 May 2018 10:55:18 +0000 Subject: [PATCH] Bug 17168: (follow-up) Tidy, clean params/options, use date tools --- Koha/Patrons.pm | 23 ++-- misc/cronjobs/update_patrons_category.pl | 176 +++++++++++++++++-------------- t/db_dependent/Patrons.t | 10 +- 3 files changed, 118 insertions(+), 91 deletions(-) diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 1c855a8..ca18189 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -236,15 +236,22 @@ sub search_patrons_to_update { my $cat_from = Koha::Patron::Categories->find($params->{from}); $search_params->{categorycode}=$params->{from}; - if ($params->{ao} || $params->{au}){ - my ($y,$m,$d) = Today(); - if( $cat_from->dateofbirthrequired && $params->{au} ) { - my ($dy,$dm,$dd) =Add_Delta_YMD($y,$m,$d,-$cat_from->dateofbirthrequired,0,0); - $search_params->{dateofbirth}{'>'} = $dy."-".sprintf("%02d",$dm)."-".sprintf("%02d",$dd); + if ($params->{ageover} || $params->{ageunder}){ + if( $cat_from->dateofbirthrequired && $params->{ageunder} ) { + my $date_after = output_pref({ + dt => dt_from_string()->subtract( years => $cat_from->dateofbirthrequired), + dateonly => 1, + dateformat => 'sql' + }); + $search_params->{dateofbirth}{'>'} = $date_after; } - if( $cat_from->upperagelimit && $params->{ao} ) { - my ($dy,$dm,$dd) =Add_Delta_YMD($y,$m,$d,-$cat_from->upperagelimit,0,0); - $search_params->{dateofbirth}{'<'} = $dy."-".sprintf("%02d",$dm)."-".sprintf("%02d",$dd); + if( $cat_from->upperagelimit && $params->{ageover} ) { + my $date_before = output_pref({ + dt => dt_from_string()->subtract( years => $cat_from->upperagelimit), + dateonly => 1, + dateformat => 'sql' + }); + $search_params->{dateofbirth}{'<'} = $date_before; } } if ($params->{fine_min} || $params->{fine_max}) { diff --git a/misc/cronjobs/update_patrons_category.pl b/misc/cronjobs/update_patrons_category.pl index c49cb0c..e426803 100644 --- a/misc/cronjobs/update_patrons_category.pl +++ b/misc/cronjobs/update_patrons_category.pl @@ -47,20 +47,20 @@ update_patrons_category.pl -f=categorycode -t=categorycode update_patrons_category.pl --help | --man Options: - --help brief help message - --man full documentation - -ao update if over maximum age for current category - -au update if under minimuum age current category - -fo=X update if fines over X amount - -fu=X update if fines under X amount - -rb=date update if registration date is before given date - -ra=date update if registration date is after a given date - --field name=value where is a column in the borrowers table, patrons will be updated if the field is equal to given - -v verbose mode - -confirm commit changes to db, no action will be taken unless this switch is included - -b only deal with patrons from this library/branch - -f change patron category from this category - -t change patron category to this category + --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 + -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 + -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 + -f --from change patron category from this category + -t --to change patron category to this category =head1 OPTIONS @@ -74,52 +74,52 @@ Print a brief help message and exits. Prints the manual page and exits. -=item B<-v> +=item B<--verbosse | -v> Verbose. Without this flag set, only fatal errors are reported. -=item B<--confirm> +=item B<--confirm | -c> Commit changes. Unless this flag set is, the script will report changes but not actually execute them on the database. -=item B<-b> +=item B<--branch | -b> changes patrons for one specific branch. Use the value in the branches.branchcode table. -=item B<-f> +=item B<--from | -f> *required* defines the category to update. Expects the code from categories.categorycode. -=item B<-t> +=item B<--to | -t> *required* defines the category patrons will be converted to. Expects the code from categories.categorycode. -=item B<-ao> +=item B<--ageover | -ao> Update patron only if they are above the maximum age range specified for the 'from' category. -=item B<-au> +=item B<--ageunde | -au> Update patron only if they are below the minimum age range specified for the 'from' category. -=item B<-fo=X> +=item B<--fineover=X | -fo=X> Supply a number and only account with fines over this number will be updated. -=item B<-fu=X> +=item B<==fineunder=X | -fu=X> Supply a number and only account with fines under this number will be updated. -=item B<-rb=date> +=item B<--regbefore=date | -rb=date> Enter a date in ISO format YYYY-MM-DD and only patrons registered before this date wil be updated. -=item B<-ra=date> +=item B<--regafter=date | -ra=date> Enter a date in ISO format YYYY-MM-DD and only patrons registered after this date wil be updated. -=item B<--field column=value> +=item B<--field column=value | -d column=value> Use this flag to specify a column in the borrowers table and update only patrons whose value in that column matches the value supplied (repeatable) @@ -148,10 +148,10 @@ C -f= -t= -v - Process # These variables are set by command line options. # They are initially set to default values. -my $help = 0; -my $man = 0; -my $verbose = 0; -my $doit = 0; +my $help = 0; +my $man = 0; +my $verbose = 0; +my $doit = 0; my $au; my $ao; my $min_dob; @@ -167,46 +167,59 @@ my $branch_lim; my %fields; GetOptions( - 'help|?' => \$help, - 'man' => \$man, - 'v' => \$verbose, - 'confirm' => \$doit, - 'f=s' => \$fromcat, - 't=s' => \$tocat, - 'ao' => \$ao, - 'au' => \$au, - 'fo=s' => \$fine_min, - 'fu=s' => \$fine_max, - 'rb=s' => \$reg_bef, - 'ra=s' => \$reg_aft, - 'b=s' => \$branch_lim, - 'field=s' => \%fields + 'help|?' => \$help, + 'man' => \$man, + 'v|verbose' => \$verbose, + 'c|confirm' => \$doit, + 'f|from=s' => \$fromcat, + 't|to=s' => \$tocat, + 'ao|ageover' => \$ageover, + 'au|ageunder' => \$ageunder, + 'fo|finesover=s' => \$fine_min, + 'fu|finesunder=s' => \$fine_max, + 'rb|regbefore=s' => \$reg_bef, + 'ra|regafter=s' => \$reg_aft, + 'b|branch=s' => \$branch_lim, + 'd|field=s' => \%fields ) or pod2usage(2); pod2usage(1) if $help; pod2usage( -verbose => 2 ) if $man; +warn "v $verbose c $doit f $fromcat t $tocat"; +warn Data::Dumper::Dumper(%fields); +exit; + if ( not $fromcat && $tocat ) { #make sure we've specified the info we need. - print "Must supply category from and to (-f & -t) please specify -help for usage tips.\n"; + print +"Must supply category from and to (-f & -t) please specify -help for usage tips.\n"; exit; } -($verbose && !$doit) and print "No actions will be taken (test mode)\n"; +( $verbose && !$doit ) and print "No actions will be taken (test mode)\n"; -$verbose and print "Will update patrons from $fromcat to $tocat with conditions below (if any)\n"; +$verbose + and print +"Will update patrons from $fromcat to $tocat with conditions below (if any)\n"; my %params; -if ( $reg_bef || $reg_aft ){ +if ( $reg_bef || $reg_aft ) { my $date_bef; my $date_aft; - if (defined $reg_bef) {eval { $date_bef = dt_from_string( $reg_bef, 'iso' ); };} - die "$reg_bef is not a valid date before, aborting! Use a date in format YYYY-MM-DD.$@" - if $@; - if (defined $reg_aft) {eval { $date_aft = dt_from_string( $reg_aft, 'iso' ); };} - die "$reg_bef is not a valid date after, aborting! Use a date in format YYYY-MM-DD.$@" - if $@; - $params{dateenrolled}{'<='}=$reg_bef if defined $date_bef; - $params{dateenrolled}{'>='}=$reg_aft if defined $date_aft; + if ( defined $reg_bef ) { + eval { $date_bef = dt_from_string( $reg_bef, 'iso' ); }; + } + die +"$reg_bef is not a valid date before, aborting! Use a date in format YYYY-MM-DD.$@" + if $@; + if ( defined $reg_aft ) { + eval { $date_aft = dt_from_string( $reg_aft, 'iso' ); }; + } + die +"$reg_bef is not a valid date after, aborting! Use a date in format YYYY-MM-DD.$@" + if $@; + $params{dateenrolled}{'<='} = $reg_bef if defined $date_bef; + $params{dateenrolled}{'>='} = $reg_aft if defined $date_aft; } my $cat_from = Koha::Patron::Categories->find($fromcat); @@ -218,42 +231,49 @@ $params{"me.branchcode"} = $branch_lim if $branch_lim; if ($verbose) { print "Conditions:\n"; - print " Registered before $reg_bef\n" if $reg_bef; - print " Registered after $reg_aft\n" if $reg_aft; + print " Registered before $reg_bef\n" if $reg_bef; + print " Registered after $reg_aft\n" if $reg_aft; print " Total fines more than $fine_min\n" if $fine_min; print " Total fines less than $fine_max\n" if $fine_max; - print " Age below minimum for ".$cat_from->description."\n" if $au; - print " Age above maximum for ".$cat_from->description."\n" if $ao; - if (defined $branch_lim) { + print " Age below minimum for " . $cat_from->description . "\n" if $ageunder; + print " Age above maximum for " . $cat_from->description . "\n" if $ageover; + if ( defined $branch_lim ) { print " Branchcode of patron is $branch_lim\n"; } } -while (my ($key,$value) = each %fields ) { +while ( my ( $key, $value ) = each %fields ) { $verbose and print " Borrower column $key is equal to $value\n"; - $params{"me.".$key} = $value; + $params{ "me." . $key } = $value; } -my $target_patrons = Koha::Patrons->search_patrons_to_update({ - from => $fromcat, +my $target_patrons = Koha::Patrons->search_patrons_to_update( + { + from => $fromcat, search_params => \%params, - au => $au, - ao => $ao, - fine_min => $fine_min, - fine_max => $fine_max, - }); -my $patrons_found = $target_patrons->count; + ageunder => $ageunder, + ageover => $ageover, + fine_min => $fine_min, + fine_max => $fine_max, + } +); +my $patrons_found = $target_patrons->count; my $actually_updated = 0; -my $testdisplay = $doit ? "" : "WOULD HAVE "; -if ( $verbose ) { - while ( my $target_patron = $target_patrons->next() ){ - my $target = Koha::Patrons->find( $target_patron->borrowernumber ); - $verbose and print $testdisplay."Updated ".$target->firstname." ".$target->surname." from $fromcat to $tocat\n"; +my $testdisplay = $doit ? "" : "WOULD HAVE "; +if ($verbose) { + while ( my $target_patron = $target_patrons->next() ) { + my $target = Koha::Patrons->find( $target_patron->borrowernumber ); + $verbose + and print $testdisplay + . "Updated " + . $target->firstname . " " + . $target->surname + . " from $fromcat to $tocat\n"; } $target_patrons->reset; } -if ( $doit ) { - $actually_updated = $target_patrons->update_category({to=>$tocat}); +if ($doit) { + $actually_updated = $target_patrons->update_category( { to => $tocat } ); } $verbose and print "$patrons_found found, $actually_updated updated\n"; diff --git a/t/db_dependent/Patrons.t b/t/db_dependent/Patrons.t index 0e842c0..3420b49 100755 --- a/t/db_dependent/Patrons.t +++ b/t/db_dependent/Patrons.t @@ -168,10 +168,10 @@ subtest "Update patron categories" => sub { $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,au=>1})->count,1,'One under age patron in child category'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,au=>1})->next->borrowernumber,$child1->{borrowernumber},'Under age patron in child category is expected one'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ao=>1})->count,1,'One over age patron in child category'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,ao=>1})->next->borrowernumber,$child3->{borrowernumber},'Over age patron in child category is expected one'); + 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'); @@ -187,7 +187,7 @@ subtest "Update patron categories" => sub { 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->find($adult1->{borrowernumber})->guarantees->count,3,'Guarantor has 3 guarantees'); - is( Koha::Patrons->search_patrons_to_update({from=>$c_categorycode,au=>1})->update_category({to=>$a_categorycode}),1,'One child patron updated to adult category'); + 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->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'); -- 2.1.4