Bugzilla – Attachment 115041 Details for
Bug 11983
Code to select patrons to purge needs to be centralized (use of filter_by)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11983: (follow-up) Terminology fixes
Bug-11983-follow-up-Terminology-fixes.patch (text/plain), 35.82 KB, created by
Martin Renvoize (ashimema)
on 2021-01-11 15:54:45 UTC
(
hide
)
Description:
Bug 11983: (follow-up) Terminology fixes
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-01-11 15:54:45 UTC
Size:
35.82 KB
patch
obsolete
>From 030230831ca6d96d12aeec9197aae0460cb7d912 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 11 Jan 2021 15:16:27 +0000 >Subject: [PATCH] Bug 11983: (follow-up) Terminology fixes > >As discussed, this patch updates the class (and tests) to reflect the >agreed upon terms >--- > Koha/Patrons.pm | 425 ++++++++++++++++++++++------------ > t/db_dependent/Koha/Patrons.t | 159 +++++++++---- > 2 files changed, 398 insertions(+), 186 deletions(-) > >diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm >index 5da77178b1..e5e6529feb 100644 >--- a/Koha/Patrons.pm >+++ b/Koha/Patrons.pm >@@ -59,36 +59,38 @@ sub search_limited { > my @restricted_branchcodes; > if ( $userenv and $userenv->{number} ) { > my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); >- @restricted_branchcodes = $logged_in_user->libraries_where_can_see_patrons; >+ @restricted_branchcodes = >+ $logged_in_user->libraries_where_can_see_patrons; > } >- $params->{'me.branchcode'} = { -in => \@restricted_branchcodes } if @restricted_branchcodes; >+ $params->{'me.branchcode'} = { -in => \@restricted_branchcodes } >+ if @restricted_branchcodes; > return $self->search( $params, $attributes ); > } > > =head3 filter_by_is_guarantor > >- Koha::Patrons->filter_by_is_guarantor(1); >+ $guarantors = Koha::Patrons->filter_by_is_guarantor; >+ $guarantors = Koha::Patrons->filter_by_is_guarantor($enabled); > >-Returns patrons filtered by whether they have a guarantor, are a guarantor or are not a guarantor. >+Returns patrons that are guarantors. > > =head4 options > > =over 4 > >-=item undef - do not apply filter >- >-=item 1 - boolean to limit to NOT guarantors >- >-=item 0 - boolean limit to ONLY guarantors >- >+=item 0 - boolean to disable the filter >+ >+=item 1 - boolean to enable the filter (default) >+ > =back > > =cut > > sub filter_by_is_guarantor { >- my ( $self, $option ) = @_; >+ my ( $self, $enabled ) = @_; > >- return $self unless defined($option); >+ # Noop if explicitly disabled by param >+ return $self if ( defined($enabled) && !$enabled ); > > my $guarantorList = Koha::Patron::Relationships->search( > {}, >@@ -100,38 +102,72 @@ sub filter_by_is_guarantor { > > # Patrons who are guarantors > return $self->search( >- { 'me.borrowernumber' => { '-in' => $guarantorList } } ) >- if $option; >+ { 'me.borrowernumber' => { '-in' => $guarantorList } } ); > >- # Patrons who are not guarantors >- return $self->search( >- { 'me.borrowernumber' => { '-not_in' => $guarantorList } } ); > } > >-=head3 filter_by_has_guarantor >+=head3 filter_by_is_not_guarantor > >- Koha::Patrons->filter_by_has_guarantor(1); >+ $non_guarantors = Koha::Patrons->filter_by_is_not_guarantor; >+ $non_guarantors = Koha::Patrons->filter_by_is_not_guarantor($enabled); > >-Returns patrons filtered by whether they have a guarantor, are a guarantor or are not a guarantor. >+Returns patrons that are not guarantors. > > =head4 options > > =over 4 > >-=item undef - do not apply filter >+=item 0 - boolean to disable the filter >+ >+=item 1 - boolean to enable the filter (default) >+ >+=back >+ >+=cut >+ >+sub filter_by_is_not_guarantor { >+ my ( $self, $enabled ) = @_; >+ >+ # Noop if explicitly disabled by param >+ return $self if ( defined($enabled) && !$enabled ); >+ >+ my $guarantorList = Koha::Patron::Relationships->search( >+ {}, >+ { >+ columns => ['guarantor_id'], >+ distinct => 1 >+ } >+ )->_resultset->as_query; >+ >+ # Patrons who are not guarantors >+ return $self->search( >+ { 'me.borrowernumber' => { '-not_in' => $guarantorList } } ); >+} >+ >+=head3 filter_by_is_guarantee > >-=item 1 - boolean to limit to users who have a guarantor >+ $guarantees = Koha::Patrons->filter_by_is_guarantee; >+ $guarantees = Koha::Patrons->filter_by_is_guarantee($enabled); > >-=item 0 - boolean to limit to users who do not have a guarantor >+Returns patrons that are guaranteed, have a guarantor. > >+=head4 options >+ >+=over 4 >+ >+=item 0 - boolean to disable the filter >+ >+=item 1 - boolean to enable the filter (default) >+ > =back > > =cut > >-sub filter_by_has_guarantor { >- my ( $self, $option ) = @_; >+sub filter_by_is_guarantee { >+ my ( $self, $enabled ) = @_; > >- return $self unless defined($option); >+ # Noop if explicitly disabled by param >+ return $self if ( defined($enabled) && !$enabled ); > > my $guaranteeList = Koha::Patron::Relationships->search( > {}, >@@ -143,19 +179,52 @@ sub filter_by_has_guarantor { > > # Patrons who have guarantors > return $self->search( >- { 'me.borrowernumber' => { '-in' => $guaranteeList } } ) >- if $option; >+ { 'me.borrowernumber' => { '-in' => $guaranteeList } } ); >+} >+ >+=head3 filter_by_is_not_guarantee >+ >+ $non_guarantees = Koha::Patrons->filter_by_is_not_guarantee; >+ $non_guarantees = Koha::Patrons->filter_by_is_not_guarantee($enabled); >+ >+Returns patrons that are not guaranteed, do not have a guarantor. >+ >+=head4 options >+ >+=over 4 >+ >+=item 0 - boolean to disable the filter >+ >+=item 1 - boolean to enable the filter (default) >+ >+=back >+ >+=cut >+ >+sub filter_by_is_not_guarantee { >+ my ( $self, $enabled ) = @_; >+ >+ # Noop if explicitly disabled by param >+ return $self if ( defined($enabled) && !$enabled ); >+ >+ my $guaranteeList = Koha::Patron::Relationships->search( >+ {}, >+ { >+ columns => ['guarantee_id'], >+ distinct => 1 >+ } >+ )->_resultset->as_query; > > # Patrons who do not have guarantors > return $self->search( > { 'me.borrowernumber' => { '-not_in' => $guaranteeList } } ); > } > >-=head3 filter_by_last_issued >+=head3 filter_by_date_last_checkout > >- Koha::Patrons->filter_by_last_issued( { after => DateTime, before => DateTime } ); >+ Koha::Patrons->filter_by_date_last_checkout( { after => DateTime, before => DateTime } ); > >-Returns patrons filtered by whether their last issue falls betwen the passed limits. >+Returns patrons filtered by when their last issue date falls. > > =head4 arguments hashref > >@@ -169,7 +238,7 @@ Returns patrons filtered by whether their last issue falls betwen the passed lim > > =cut > >-sub filter_by_last_issued { >+sub filter_by_date_last_checkout { > my ( $self, $options ) = @_; > > return $self >@@ -177,7 +246,8 @@ sub filter_by_last_issued { > && ( $options->{before} || $options->{after} ) ); > > my $where = {}; >- my $group_by = [ map { 'me.' . $_ } $self->_resultset->result_source->columns ]; >+ my $group_by = >+ [ map { 'me.' . $_ } $self->_resultset->result_source->columns ]; > my $attrs = { > join => 'old_issues', > group_by => $group_by, >@@ -227,31 +297,65 @@ sub filter_by_last_issued { > # return $self->search( $where, $attrs ); > #} > >-=head3 filter_by_has_issues >+=head3 filter_by_has_pending_checkouts > >- Koha::Patrons->filter_by_has_issues(1); >+ $patrons_with_checkouts = Koha::Patrons->filter_by_has_pending_checkout; > >-Returns patrons filtered by whether they have current issues or not. >+Returns patrons filtered by whether they have one or more current checkouts. > > =head4 options > > =over 4 > >-=item undef - do not apply filter >+=item 0 - boolean to disable the filter >+ >+=item 1 - boolean to enable the filter (default) >+ >+=back >+ >+=cut >+ >+sub filter_by_has_pending_checkouts { >+ my ( $self, $enabled ) = @_; >+ >+ # Noop if explicitly disabled by param >+ return $self if ( defined($enabled) && !$enabled ); >+ >+ my $issueList = Koha::Checkouts->search( >+ {}, >+ { >+ columns => ['borrowernumber'], >+ distinct => 1 >+ } >+ )->_resultset->as_query; > >-=item 0 - limit to patrons WITHOUT current issues >+ # Patrons who have current issues >+ return $self->search( { 'me.borrowernumber' => { '-in' => $issueList } } ); >+} >+ >+=head3 filter_by_has_no_pending_checkouts >+ >+ Koha::Patrons->filter_by_has_no_ending_checkout(1); > >-=item 1 - limit to patrons WITH current issues >+Returns patrons filtered to those who do not have any current checkouts. > >+=head4 options >+ >+=over 4 >+ >+=item 0 - boolean to disable the filter >+ >+=item 1 - boolean to enable the filter (default) >+ > =back > > =cut > >-sub filter_by_has_issues { >- my ( $self, $option ) = @_; >+sub filter_by_has_no_pending_checkouts { >+ my ( $self, $enabled ) = @_; > >- return $self >- unless defined($option); >+ # Noop if explicitly disabled by param >+ return $self if ( defined($enabled) && !$enabled ); > > my $issueList = Koha::Checkouts->search( > {}, >@@ -261,10 +365,6 @@ sub filter_by_has_issues { > } > )->_resultset->as_query; > >- # Patrons who have current issues >- return $self->search( { 'me.borrowernumber' => { '-in' => $issueList } } ) >- if $option; >- > # Patrons who do not have current issues > return $self->search( > { 'me.borrowernumber' => { '-not_in' => $issueList } } ); >@@ -300,9 +400,9 @@ sub filter_by_has_issues { > # return $self->search($where,$attrs); > #} > >-=head3 filter_by_when_expired >+=head3 filter_by_date_expired > >- Koha::Patrons->filter_by_when_expired( { after => DateTime, before => DateTime ); >+ Koha::Patrons->filter_by_date_expired( { after => DateTime, before => DateTime ); > > Returns patrons filtered by whether their accounts expired between between the passed limits. > >@@ -318,7 +418,7 @@ Returns patrons filtered by whether their accounts expired between between the p > > =cut > >-sub filter_by_when_expired { >+sub filter_by_date_expired { > my ( $self, $options ) = @_; > > return $self >@@ -350,7 +450,6 @@ sub filter_by_when_expired { > return $self->search( $where, $attrs ); > } > >- > =head3 filter_by_amount_owed > > Koha::Patrons->filter_by_amount_owed( { less_than => '2.00', more_than => '0.50' } ); >@@ -418,7 +517,9 @@ sub filter_by_amount_owed { > && ( $options->{less_than} || $options->{more_than} ) ); > > my $where = {}; >- my $group_by = [ map { 'me.' . $_ } $self->_resultset->result_source->columns ]; >+ my $group_by = >+ [ map { 'me.' . $_ } $self->_resultset->result_source->columns ]; >+ > #push @{$group_by}, 'outstanding'; > my $attrs = { > join => 'accountlines', >@@ -514,11 +615,13 @@ Returns all Patrons which are Housebound choosers. > =cut > > sub search_housebound_choosers { >- my ( $self ) = @_; >- my $cho = $self->_resultset >- ->search_related('housebound_role', { >+ my ($self) = @_; >+ my $cho = $self->_resultset->search_related( >+ 'housebound_role', >+ { > housebound_chooser => 1, >- })->search_related('borrowernumber'); >+ } >+ )->search_related('borrowernumber'); > return Koha::Patrons->_new_from_dbic($cho); > } > >@@ -529,11 +632,13 @@ Returns all Patrons which are Housebound deliverers. > =cut > > sub search_housebound_deliverers { >- my ( $self ) = @_; >- my $del = $self->_resultset >- ->search_related('housebound_role', { >+ my ($self) = @_; >+ my $del = $self->_resultset->search_related( >+ 'housebound_role', >+ { > housebound_deliverer => 1, >- })->search_related('borrowernumber'); >+ } >+ )->search_related('borrowernumber'); > return Koha::Patrons->_new_from_dbic($del); > } > >@@ -551,22 +656,21 @@ from 12 to 17 days. > sub search_upcoming_membership_expires { > my ( $self, $params ) = @_; > my $before = $params->{before} || 0; >- my $after = $params->{after} || 0; >+ my $after = $params->{after} || 0; > delete $params->{before}; > delete $params->{after}; > > my $days = C4::Context->preference("MembershipExpiryDaysNotice") || 0; > my $date_before = dt_from_string->add( days => $days - $before ); >- my $date_after = dt_from_string->add( days => $days + $after ); >- my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+ my $date_after = dt_from_string->add( days => $days + $after ); >+ my $dtf = Koha::Database->new->schema->storage->datetime_parser; > > $params->{dateexpiry} = { >- ">=" => $dtf->format_date( $date_before ), >- "<=" => $dtf->format_date( $date_after ), >+ ">=" => $dtf->format_date($date_before), >+ "<=" => $dtf->format_date($date_after), > }; >- return $self->SUPER::search( >- $params, { join => ['branchcode', 'categorycode'] } >- ); >+ return $self->SUPER::search( $params, >+ { join => [ 'branchcode', 'categorycode' ] } ); > } > > =head3 search_patrons_to_anonymise >@@ -581,22 +685,33 @@ sub search_patrons_to_anonymise { > my ( $class, $params ) = @_; > my $older_than_date = $params->{before}; > my $library = $params->{library}; >- $older_than_date = $older_than_date ? dt_from_string($older_than_date) : dt_from_string; >+ $older_than_date = >+ $older_than_date ? dt_from_string($older_than_date) : dt_from_string; > $library ||= >- ( C4::Context->preference('IndependentBranches') && C4::Context->userenv && !C4::Context->IsSuperLibrarian() && C4::Context->userenv->{branch} ) >+ ( C4::Context->preference('IndependentBranches') >+ && C4::Context->userenv >+ && !C4::Context->IsSuperLibrarian() >+ && C4::Context->userenv->{branch} ) > ? C4::Context->userenv->{branch} > : undef; > my $anonymous_patron = C4::Context->preference('AnonymousPatron') || undef; > > my $dtf = Koha::Database->new->schema->storage->datetime_parser; >- my $rs = $class->_resultset->search( >- { returndate => { '<' => $dtf->format_datetime($older_than_date), }, >+ my $rs = $class->_resultset->search( >+ { >+ returndate => { '<' => $dtf->format_datetime($older_than_date), }, > 'old_issues.borrowernumber' => { 'not' => undef }, >- privacy => { '<>' => 0 }, # Keep forever >+ privacy => { '<>' => 0 }, # Keep forever > ( $library ? ( 'old_issues.branchcode' => $library ) : () ), >- ( $anonymous_patron ? ( 'old_issues.borrowernumber' => { '!=' => $anonymous_patron } ) : () ), >+ ( >+ $anonymous_patron >+ ? ( 'old_issues.borrowernumber' => >+ { '!=' => $anonymous_patron } ) >+ : () >+ ), > }, >- { join => ["old_issues"], >+ { >+ join => ["old_issues"], > distinct => 1, > } > ); >@@ -620,24 +735,26 @@ sub anonymise_issue_history { > > $older_than_date = dt_from_string $older_than_date if $older_than_date; > >- # The default of 0 does not work due to foreign key constraints >- # The anonymisation should not fail quietly if AnonymousPatron is not a valid entry >- # Set it to undef (NULL) >- my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+# The default of 0 does not work due to foreign key constraints >+# The anonymisation should not fail quietly if AnonymousPatron is not a valid entry >+# Set it to undef (NULL) >+ my $dtf = Koha::Database->new->schema->storage->datetime_parser; > my $nb_rows = 0; > while ( my $patron = $self->next ) { > my $old_issues_to_anonymise = $patron->old_checkouts->search( >- { >- ( >- $older_than_date >- ? ( returndate => >- { '<' => $dtf->format_datetime($older_than_date) } ) >- : () >- ) >- } >+ { >+ ( >+ $older_than_date >+ ? ( returndate => >+ { '<' => $dtf->format_datetime($older_than_date) } ) >+ : () >+ ) >+ } > ); >- my $anonymous_patron = C4::Context->preference('AnonymousPatron') || undef; >- $nb_rows += $old_issues_to_anonymise->update( { 'old_issues.borrowernumber' => $anonymous_patron } ); >+ my $anonymous_patron = >+ C4::Context->preference('AnonymousPatron') || undef; >+ $nb_rows += $old_issues_to_anonymise->update( >+ { 'old_issues.borrowernumber' => $anonymous_patron } ); > } > return $nb_rows; > } >@@ -659,19 +776,23 @@ sub anonymise_issue_history { > sub delete { > my ( $self, $params ) = @_; > my $patrons_deleted; >- $self->_resultset->result_source->schema->txn_do( sub { >- my ( $set, $params ) = @_; >- my $count = $set->count; >- while ( my $patron = $set->next ) { >+ $self->_resultset->result_source->schema->txn_do( >+ sub { >+ my ( $set, $params ) = @_; >+ my $count = $set->count; >+ while ( my $patron = $set->next ) { > >- next unless $patron->in_storage; >+ next unless $patron->in_storage; > >- $patron->move_to_deleted if $params->{move}; >- $patron->delete; >+ $patron->move_to_deleted if $params->{move}; >+ $patron->delete; > >- $patrons_deleted++; >- } >- }, $self, $params ); >+ $patrons_deleted++; >+ } >+ }, >+ $self, >+ $params >+ ); > return $patrons_deleted; > } > >@@ -686,22 +807,23 @@ sub delete { > =cut > > sub search_unsubscribed { >- my ( $class ) = @_; >+ my ($class) = @_; > > my $delay = C4::Context->preference('UnsubscribeReflectionDelay'); >- if( !defined($delay) || $delay eq q{} ) { >+ if ( !defined($delay) || $delay eq q{} ) { >+ > # return empty set >- return $class->search({ borrowernumber => undef }); >+ return $class->search( { borrowernumber => undef } ); > } > my $parser = Koha::Database->new->schema->storage->datetime_parser; >- my $dt = dt_from_string()->subtract( days => $delay ); >- my $str = $parser->format_datetime($dt); >- my $fails = C4::Context->preference('FailedLoginAttempts') || 0; >- my $cond = [ undef, 0, 1..$fails-1 ]; # NULL, 0, 1..fails-1 (if fails>0) >+ my $dt = dt_from_string()->subtract( days => $delay ); >+ my $str = $parser->format_datetime($dt); >+ my $fails = C4::Context->preference('FailedLoginAttempts') || 0; >+ my $cond = [ undef, 0, 1 .. $fails - 1 ]; # NULL, 0, 1..fails-1 (if fails>0) > return $class->search( > { > 'patron_consents.refused_on' => { '<=' => $str }, >- 'login_attempts' => $cond, >+ 'login_attempts' => $cond, > }, > { join => 'patron_consents' }, > ); >@@ -721,21 +843,24 @@ sub search_anonymize_candidates { > my ( $class, $params ) = @_; > > my $delay = C4::Context->preference('PatronAnonymizeDelay'); >- if( !defined($delay) || $delay eq q{} ) { >+ if ( !defined($delay) || $delay eq q{} ) { >+ > # return empty set >- return $class->search({ borrowernumber => undef }); >+ return $class->search( { borrowernumber => undef } ); > } >- my $cond = {}; >+ my $cond = {}; > my $parser = Koha::Database->new->schema->storage->datetime_parser; >- my $dt = dt_from_string()->subtract( days => $delay ); >- my $str = $parser->format_datetime($dt); >+ my $dt = dt_from_string()->subtract( days => $delay ); >+ my $str = $parser->format_datetime($dt); > $cond->{dateexpiry} = { '<=' => $str }; >- $cond->{anonymized} = 0; # not yet done >- if( $params->{locked} ) { >+ $cond->{anonymized} = 0; # not yet done >+ if ( $params->{locked} ) { > my $fails = C4::Context->preference('FailedLoginAttempts') || 0; >- $cond->{login_attempts} = [ -and => { '!=' => undef }, { -not_in => [0, 1..$fails-1 ] } ]; # -not_in does not like undef >+ $cond->{login_attempts} = >+ [ -and => { '!=' => undef }, { -not_in => [ 0, 1 .. $fails - 1 ] } ] >+ ; # -not_in does not like undef > } >- return $class->search( $cond ); >+ return $class->search($cond); > } > > =head3 search_anonymized >@@ -749,20 +874,21 @@ sub search_anonymize_candidates { > =cut > > sub search_anonymized { >- my ( $class ) = @_; >+ my ($class) = @_; > > my $delay = C4::Context->preference('PatronRemovalDelay'); >- if( !defined($delay) || $delay eq q{} ) { >+ if ( !defined($delay) || $delay eq q{} ) { >+ > # return empty set >- return $class->search({ borrowernumber => undef }); >+ return $class->search( { borrowernumber => undef } ); > } >- my $cond = {}; >+ my $cond = {}; > my $parser = Koha::Database->new->schema->storage->datetime_parser; >- my $dt = dt_from_string()->subtract( days => $delay ); >- my $str = $parser->format_datetime($dt); >+ my $dt = dt_from_string()->subtract( days => $delay ); >+ my $str = $parser->format_datetime($dt); > $cond->{dateexpiry} = { '<=' => $str }; > $cond->{anonymized} = 1; >- return $class->search( $cond ); >+ return $class->search($cond); > } > > =head3 lock >@@ -777,7 +903,7 @@ sub search_anonymized { > sub lock { > my ( $self, $params ) = @_; > my $count = $self->count; >- while( my $patron = $self->next ) { >+ while ( my $patron = $self->next ) { > $patron->lock($params); > } > } >@@ -792,9 +918,9 @@ sub lock { > =cut > > sub anonymize { >- my ( $self ) = @_; >+ my ($self) = @_; > my $count = $self->count; >- while( my $patron = $self->next ) { >+ while ( my $patron = $self->next ) { > $patron->anonymize; > } > } >@@ -824,27 +950,39 @@ sub search_patrons_to_update_category { > my %query; > my $search_params; > >- my $cat_from = Koha::Patron::Categories->find($params->{from}); >- $search_params->{categorycode}=$params->{from}; >- if ($params->{too_young} || $params->{too_old}){ >+ my $cat_from = Koha::Patron::Categories->find( $params->{from} ); >+ $search_params->{categorycode} = $params->{from}; >+ if ( $params->{too_young} || $params->{too_old} ) { > my $dtf = Koha::Database->new->schema->storage->datetime_parser; >- if( $cat_from->dateofbirthrequired && $params->{too_young} ) { >- my $date_after = dt_from_string()->subtract( years => $cat_from->dateofbirthrequired); >- $search_params->{dateofbirth}{'>'} = $dtf->format_datetime( $date_after ); >+ if ( $cat_from->dateofbirthrequired && $params->{too_young} ) { >+ my $date_after = dt_from_string() >+ ->subtract( years => $cat_from->dateofbirthrequired ); >+ $search_params->{dateofbirth}{'>'} = >+ $dtf->format_datetime($date_after); > } >- if( $cat_from->upperagelimit && $params->{too_old} ) { >- my $date_before = dt_from_string()->subtract( years => $cat_from->upperagelimit); >- $search_params->{dateofbirth}{'<'} = $dtf->format_datetime( $date_before ); >+ if ( $cat_from->upperagelimit && $params->{too_old} ) { >+ my $date_before = >+ dt_from_string()->subtract( years => $cat_from->upperagelimit ); >+ $search_params->{dateofbirth}{'<'} = >+ $dtf->format_datetime($date_before); > } > } >- if ($params->{fine_min} || $params->{fine_max}) { >- $query{join} = ["accountlines"]; >- $query{columns} = ["borrowernumber"]; >+ if ( $params->{fine_min} || $params->{fine_max} ) { >+ $query{join} = ["accountlines"]; >+ $query{columns} = ["borrowernumber"]; > $query{group_by} = ["borrowernumber"]; >- $query{having} = \['COALESCE(sum(accountlines.amountoutstanding),0) <= ?',$params->{fine_max}] if defined $params->{fine_max}; >- $query{having} = \['COALESCE(sum(accountlines.amountoutstanding),0) >= ?',$params->{fine_min}] if defined $params->{fine_min}; >+ $query{having} = \[ >+ 'COALESCE(sum(accountlines.amountoutstanding),0) <= ?', >+ $params->{fine_max} >+ ] >+ if defined $params->{fine_max}; >+ $query{having} = \[ >+ 'COALESCE(sum(accountlines.amountoutstanding),0) >= ?', >+ $params->{fine_min} >+ ] >+ if defined $params->{fine_min}; > } >- return $self->search($search_params,\%query); >+ return $self->search( $search_params, \%query ); > } > > =head3 update_category_to >@@ -862,9 +1000,9 @@ call search_patrons_to_update to filter the Koha::Patrons set > sub update_category_to { > my ( $self, $params ) = @_; > my $counter = 0; >- while( my $patron = $self->next ) { >+ while ( my $patron = $self->next ) { > $counter++; >- $patron->categorycode($params->{category})->store(); >+ $patron->categorycode( $params->{category} )->store(); > } > return $counter; > } >@@ -904,7 +1042,6 @@ sub filter_by_attribute_value { > return Koha::Patrons->_new_from_dbic($rs); > } > >- > =head3 _type > > =cut >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 69e8a88ebf..1990502200 100755 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -1603,7 +1603,7 @@ subtest 'BorrowersLog tests' => sub { > }; > > subtest 'complex filters' => sub { >- plan tests => 7; >+ plan tests => 9; > > my $now = DateTime->now; > my $new_patron_cf_1 = Koha::Patron->new( >@@ -1664,8 +1664,9 @@ subtest 'complex filters' => sub { > is( ref($filtered), 'Koha::Patrons', > 'Koha::Patrons->filter_by_is_guarantor should return a Koha::Patrons result set in a scalar context' > ); >- is( $filtered->count, 3, >- 'filter_by_is_guarantor() found both patrons' ); >+ is( $filtered->count, 1, 'filter_by_is_guarantor() found one patron' ); >+ is( $filtered->next->guarantee_relationships->count, >+ 1, 'filter_by_is_guarantor() found the correct patron' ); > > $filtered = $results->filter_by_is_guarantor(1); > is( $filtered->count, 1, 'filter_by_is_guarantor(1) found one patron' ); >@@ -1673,34 +1674,86 @@ subtest 'complex filters' => sub { > 1, 'filter_by_is_guarantor(1) found the correct patron' ); > > $filtered = $results->filter_by_is_guarantor(0); >+ is( $filtered->count, 3, >+ 'filter_by_is_guarantor(0) found all patrons - filter disabled' ); >+ }; >+ >+ subtest 'filter_by_is_not_guarantor' => sub { >+ plan tests => 6; >+ >+ my $filtered = $results->filter_by_is_not_guarantor(); >+ is( ref($filtered), 'Koha::Patrons', >+'Koha::Patrons->filter_by_is_not_guarantor should return a Koha::Patrons result set in a scalar context' >+ ); >+ is( $filtered->count, 2, >+ 'filter_by_is_not_guarantor() found both patrons' ); >+ is( $filtered->next->guarantee_relationships->count, >+ 0, 'filter_by_is_not_guarantor() found the correct patrons' ); >+ >+ $filtered = $results->filter_by_is_not_guarantor(1); > is( $filtered->count, 2, >- 'filter_by_is_guarantor(0) found two patrons' ); >+ 'filter_by_is_not_guarantor(1) found two patrons' ); > is( $filtered->next->guarantee_relationships->count, >- 0, 'filter_by_is_guarantor(0) found the correct patrons' ); >+ 0, 'filter_by_is_not_guarantor(1) found the correct patrons' ); >+ >+ $filtered = $results->filter_by_is_not_guarantor(0); >+ is( $filtered->count, 3, >+ 'filter_by_is_not_guarantor(0) found all patrons - filter disabled' ); > }; > >- subtest 'filter_by_has_guarantor' => sub { >+ subtest 'filter_by_is_guarantee' => sub { > plan tests => 6; > >- my $filtered = $results->filter_by_has_guarantor(); >+ my $filtered = $results->filter_by_is_guarantee(); > is( ref($filtered), 'Koha::Patrons', >-'filter_by_has_guarantor() should return a Koha::Patrons result set in a scalar context' >+'filter_by_is_guarantee() should return a Koha::Patrons result set in a scalar context' >+ ); >+ is( $filtered->count, 1, >+ 'filter_by_is_guarantee() found one patron' ); >+ is( >+ $filtered->next->borrowernumber, >+ $new_patron_cf_2->borrowernumber, >+ 'filter_by_is_guarantee() found the correct patron' > ); >- is( $filtered->count, 3, >- 'filter_by_has_guarantor() found both patrons' ); > >- $filtered = $results->filter_by_has_guarantor(1); >+ $filtered = $results->filter_by_is_guarantee(1); > is( $filtered->count, 1, >- 'filter_by_has_guarantor(1) found one patron' ); >+ 'filter_by_is_guarantee(1) found one patron' ); > is( > $filtered->next->borrowernumber, > $new_patron_cf_2->borrowernumber, >- 'filter_by_has_guarantor(1) found the correct patron' >+ 'filter_by_is_guarantee(1) found the correct patron' >+ ); >+ >+ $filtered = $results->filter_by_is_guarantee(0); >+ is( $filtered->count, 3, >+ 'filter_by_is_guarantee(0) found all patrons - filter disabled' ); >+ }; >+ >+ subtest 'filter_by_is_not_guarantee' => sub { >+ plan tests => 6; >+ >+ my $filtered = $results->filter_by_is_not_guarantee(); >+ is( ref($filtered), 'Koha::Patrons', >+'filter_by_is_not_guarantee() should return a Koha::Patrons result set in a scalar context' >+ ); >+ is( $filtered->count, 2, >+ 'filter_by_is_not_guarantee() found two patrons' ); >+ is_deeply( >+ [ >+ sort( $filtered->next->borrowernumber, >+ $filtered->next->borrowernumber ) >+ ], >+ [ >+ sort ( $new_patron_cf_1->borrowernumber, >+ $new_patron_cf_3->borrowernumber ) >+ ], >+ 'filter_by_is_not_guarantee() found the correct patrons' > ); > >- $filtered = $results->filter_by_has_guarantor(0); >+ $filtered = $results->filter_by_is_not_guarantee(1); > is( $filtered->count, 2, >- 'filter_by_has_guarantor(0) found two patrons' ); >+ 'filter_by_is_not_guarantee(1) found two patrons' ); > is_deeply( > [ > sort( $filtered->next->borrowernumber, >@@ -1710,11 +1763,15 @@ subtest 'complex filters' => sub { > sort ( $new_patron_cf_1->borrowernumber, > $new_patron_cf_3->borrowernumber ) > ], >- 'filter_by_has_guarantor(0) found the correct patrons' >+ 'filter_by_is_not_guarantee(1) found the correct patrons' > ); >+ >+ $filtered = $results->filter_by_is_not_guarantee(0); >+ is( $filtered->count, 3, >+ 'filter_by_is_not_guarantee(0) found all patrons - filter disabled' ); > }; > >- subtest 'filter_by_last_issued' => sub { >+ subtest 'filter_by_date_last_checkout' => sub { > plan tests => 4; > > my $first = DateTime->now()->subtract( days => 5 ); >@@ -1747,28 +1804,28 @@ subtest 'complex filters' => sub { > } > ); > >- my $filtered = $results->filter_by_last_issued(); >+ my $filtered = $results->filter_by_date_last_checkout(); > is( ref($filtered), 'Koha::Patrons', >-'filter_by_last_issued() should return a Koha::Patrons result set in a scalar context' >+'filter_by_date_last_checkout() should return a Koha::Patrons result set in a scalar context' > ); > >- $filtered = $results->filter_by_last_issued( { before => $last } ); >+ $filtered = $results->filter_by_date_last_checkout( { before => $last } ); > is( $filtered->_resultset->as_subselect_rs->count, >- 2, "filter_by_last_issued({ before => $last }) found two patrons" ); >+ 2, "filter_by_date_last_checkout({ before => $last }) found two patrons" ); > >- $filtered = $results->filter_by_last_issued( { after => $first } ); >+ $filtered = $results->filter_by_date_last_checkout( { after => $first } ); > is( $filtered->_resultset->as_subselect_rs->count, >- 2, "filter_by_last_issued({ after => $first }) found two patrons" ); >+ 2, "filter_by_date_last_checkout({ after => $first }) found two patrons" ); > >- $filtered = $results->filter_by_last_issued( >+ $filtered = $results->filter_by_date_last_checkout( > { after => $first, before => $last } ); > is( $filtered->_resultset->as_subselect_rs->count, 1, >-"filter_by_last_issued({ after => $first, before => $last }) found two patrons" >+"filter_by_date_last_checkout({ after => $first, before => $last }) found two patrons" > ); > }; > >- subtest 'filter_by_has_issues' => sub { >- plan tests => 3; >+ subtest 'filter_by_has_pending_checkouts/filter_by_has_no_pending_checkouts' => sub { >+ plan tests => 8; > > my $issue_1 = $builder->build( > { >@@ -1779,45 +1836,63 @@ subtest 'complex filters' => sub { > } > ); > >- my $filtered = $results->filter_by_has_issues(); >+ my $filtered = $results->filter_by_has_pending_checkouts(); > is( ref($filtered), 'Koha::Patrons', >-'filter_by_has_issues() should return a Koha::Patrons result set in a scalar context' >+'filter_by_has_pending_checkouts() should return a Koha::Patrons result set in a scalar context' > ); >+ is( $filtered->_resultset->as_subselect_rs->count, >+ 1, "filter_by_has_pending_checkouts() found one patron" ); >+ >+ >+ $filtered = $results->filter_by_has_pending_checkouts(1); >+ is( $filtered->_resultset->as_subselect_rs->count, >+ 1, "filter_by_has_pending_checkouts(1) found one patron" ); >+ >+ $filtered = $results->filter_by_has_pending_checkouts(0); >+ is( $filtered->_resultset->as_subselect_rs->count, >+ 3, "filter_by_has_pending_checkouts(0) found all patrons - filter disabled" ); >+ >+ $filtered = $results->filter_by_has_no_pending_checkouts(); >+ is( ref($filtered), 'Koha::Patrons', >+'filter_by_has_no_pending_checkouts() should return a Koha::Patrons result set in a scalar context' >+ ); >+ is( $filtered->_resultset->as_subselect_rs->count, >+ 2, "filter_by_has_no_pending_checkouts() found two patrons" ); > >- $filtered = $results->filter_by_has_issues(1); >+ $filtered = $results->filter_by_has_no_pending_checkouts(1); > is( $filtered->_resultset->as_subselect_rs->count, >- 1, "filter_by_has_issues(1) found one patron" ); >+ 2, "filter_by_has_no_pending_checkouts(1) found two patrons" ); > >- $filtered = $results->filter_by_has_issues(0); >+ $filtered = $results->filter_by_has_no_pending_checkouts(0); > is( $filtered->_resultset->as_subselect_rs->count, >- 2, "filter_by_has_issues(0) found two patrons" ); >+ 3, "filter_by_has_no_pending_checkouts(0) found all patrons - filter disabled" ); > }; > >- subtest 'filter_by_when_expired' => sub { >+ subtest 'filter_by_date_expired' => sub { > plan tests => 4; > > my $first = $now->clone->subtract( days => 12 ); > my $second = $now->clone->subtract( days => 4 ); > >- my $filtered = $results->filter_by_when_expired(); >+ my $filtered = $results->filter_by_date_expired(); > is( ref($filtered), 'Koha::Patrons', >-'Koha::Patrons->filter_by_when_expired should return a Koha::Patrons result set in a scalar context' >+'Koha::Patrons->filter_by_date_expired should return a Koha::Patrons result set in a scalar context' > ); > >- $filtered = $results->filter_by_when_expired( { before => $second } ); >+ $filtered = $results->filter_by_date_expired( { before => $second } ); > is( $filtered->_resultset->as_subselect_rs->count, > 2, >- "filter_by_when_expired({ before => $second }) found two patrons" ); >+ "filter_by_date_expired({ before => $second }) found two patrons" ); > >- $filtered = $results->filter_by_when_expired( { after => $first } ); >+ $filtered = $results->filter_by_date_expired( { after => $first } ); > is( $filtered->_resultset->as_subselect_rs->count, > 2, >- "filter_by_when_expired({ after => $first }) found two patrons" ); >+ "filter_by_date_expired({ after => $first }) found two patrons" ); > >- $filtered = $results->filter_by_when_expired( >+ $filtered = $results->filter_by_date_expired( > { after => $first, before => $second } ); > is( $filtered->_resultset->as_subselect_rs->count, 1, >-"filter_by_when_expired({ after => $first, before => $second }) found one patrons" >+"filter_by_date_expired({ after => $first, before => $second }) found one patrons" > ); > }; > >-- >2.20.1
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 11983
:
76227
|
76228
|
76701
|
114730
|
114731
|
114735
|
115040
| 115041