From f710f170746eaa315dfb5014173867796f6b3fa5 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 21 Jun 2018 14:18:17 +0100 Subject: [PATCH] Bug 11983: Centralised Koha::Patrons method OPTION 2 --- Koha/Patrons.pm | 359 ++++++++++++++++++++++++++++++++++ t/db_dependent/Koha/Patrons.t | 338 +++++++++++++++++++++++++++++++- 2 files changed, 696 insertions(+), 1 deletion(-) diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 0d2d9ec6b4..938767b6e5 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -62,6 +62,365 @@ sub search_limited { return $self->search( $params, $attributes ); } +=head3 filter_by_is_guarantor + + Koha::Patrons->filter_by_is_guarantor(1); + +Returns patrons filtered by whether they have a guarantor, are a guarantor or are not a guarantor. + +=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 + +=back + +=cut + +sub filter_by_is_guarantor { + my ( $self, $option ) = @_; + + return $self unless defined($option); + + my $guarantorList = Koha::Patrons->search( + { guarantorid => [ { '!=' => 0 }, { '!=' => undef } ] }, + { select => ['guarantorid'] } )->_resultset->as_query; + + # Patrons who are guarantors + return $self->search( + { 'me.borrowernumber' => { '-in' => $guarantorList } } ) + if $option; + + # Patrons who are not guarantors + return $self->search( + { 'me.borrowernumber' => { '-not_in' => $guarantorList } } ); +} + +=head3 filter_by_has_guarantor + + Koha::Patrons->filter_by_has_guarantor(1); + +Returns patrons filtered by whether they have a guarantor, are a guarantor or are not a guarantor. + +=head4 options + +=over 4 + +=item undef - do not apply filter + +=item 1 - boolean to limit to users who have a guarantor + +=item 0 - boolean to limit to users who do not have a guarantor + +=back + +=cut + +sub filter_by_has_guarantor { + my ( $self, $option ) = @_; + + return $self unless defined($option); + + # Patrons who have guarantors + return $self->search( + { 'me.guarantorid' => [ { '!=' => 0 }, { '!=' => undef } ] } ) + if $option; + + # Patrons who do not have guarantors + return $self->search( { 'me.guarantorid' => [ 0, undef ] } ); +} + +=head3 filter_by_last_issued + + Koha::Patrons->filter_by_last_issued( { after => DateTime, before => DateTime } ); + +Returns patrons filtered by whether their last issue falls betwen the passed limits. + +=head4 arguments hashref + +=over 4 + +=item before (optional) - filter out patrons whose last_issue was since DateTime + +=item after (optional) - filter out patrons whose last_issue has been since DateTime + +=back + +=cut + +sub filter_by_last_issued { + my ( $self, $options ) = @_; + + return $self + unless ( defined($options) + && ( $options->{before} || $options->{after} ) ); + + my $where = {}; + my $attrs = { + join => 'old_issues', + group_by => 'me.borrowernumber', + order_by => { '-desc' => 'old_issues.timestamp' } + }; + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + + push @{ $where->{'-and'} }, + { 'old_issues.timestamp' => + { '<' => $dtf->format_datetime( $options->{before} ) } } + if $options->{before}; + push @{ $where->{'-and'} }, + { 'old_issues.timestamp' => + { '>' => $dtf->format_datetime( $options->{after} ) } } + if $options->{after}; + + return $self->search( $where, $attrs ); +} + +#sub filter_by_last_issued { +# my ( $self, $options ) = @_; +# +# return $self +# unless ( defined($options) +# && ( $options->{before} || $options->{after} ) ); +# +# my $where = {}; +# my $attrs = { +# join => 'old_issues', +# group_by => 'me.borrowernumber', +# '+select' => [ { max => 'old_issues.timestamp', '-as' => 'last_issued' } ], +# '+as' => [ 'last_issued' ] +# }; +# my $dtf = Koha::Database->new->schema->storage->datetime_parser; +# +# push @{ $attrs->{'having'}->{'-and'} }, +# { 'last_issued' => { '<' => $dtf->format_datetime( $options->{before} ) } +# } +# if $options->{before}; +# +# push @{ $attrs->{'having'}->{'-and'} }, +# { 'last_issued' => { '>' => $dtf->format_datetime( $options->{after} ) } } +# if $options->{after}; +# +# return $self->search( $where, $attrs ); +#} + +=head3 filter_by_has_issues + + Koha::Patrons->filter_by_has_issues(1); + +Returns patrons filtered by whether they have current issues or not. + +=head4 options + +=over 4 + +=item undef - do not apply filter + +=item 0 - limit to patrons WITHOUT current issues + +=item 1 - limit to patrons WITH current issues + +=back + +=cut + +sub filter_by_has_issues { + my ( $self, $option ) = @_; + + return $self + unless defined($option); + + my $where = {}; + my $attrs = { + join => 'issues', + group_by => 'me.borrowernumber', + '+select' => [ { count => 'issues.issue_id', -as => 'current_issues' } ], + '+as' => 'current_issues' + }; + + # Patrons who have current issues + if ($option) { + $attrs->{'having'} = { 'current_issues' => { '>' => 0 } }; + } + + # Patrons who do not have current issues + else { + $attrs->{'having'} = { 'current_issues' => 0 }; + } + + return $self->search($where,$attrs); +} + + +=head3 filter_by_when_expired + + Koha::Patrons->filter_by_when_expired( { after => DateTime, before => DateTime ); + +Returns patrons filtered by whether their accounts expired between between the passed limits. + +=head4 arguments hashref + +=over 4 + +=item before (optional) - filter out patrons whose expired_date was since DateTime + +=item after (optional) - filter out patrons whose expired_date has been since DateTime + +=back + +=cut + +sub filter_by_when_expired { + my ( $self, $options ) = @_; + + return $self + unless ( defined($options) + && ( $options->{before} || $options->{after} ) ); + + my $where = {}; + my $attrs = {}; + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + + # Before only + $where->{'me.dateexpiry'} = + { '<' => $dtf->format_datetime( $options->{before} ) } + if ( $options->{before} && !$options->{after} ); + + # After only (includes null) + $where->{'me.dateexpiry'} = + [ undef, { '>' => $dtf->format_datetime( $options->{after} ) } ] + if ( !$options->{before} && $options->{after} ); + + # Before and After (between) + $where->{'me.dateexpiry'} = [ + '-and', + { '<' => $dtf->format_datetime( $options->{before} ) }, + { '>' => $dtf->format_datetime( $options->{after} ) } + ] + if ( $options->{before} && $options->{after} ); + + return $self->search( $where, $attrs ); +} + + +=head3 filter_by_amount_owed + + Koha::Patrons->filter_by_amount_owed( { less_than => '2.00', more_than => '0.50' } ); + +Returns patrons filtered by how much money they owe, between passed limits. + +=head4 arguments hashref + +=over 4 + +=item less_than (optional) - filter out patrons who owe less than Amount + +=item more_than (optional) - filter out patrons who owe more than Amount + +=back + +=cut + +sub filter_by_amount_owed { + my ( $self, $options ) = @_; + + return $self + unless ( defined($options) + && ( $options->{less_than} || $options->{more_than} ) ); + + my $where = {}; + my $attrs = { + join => 'accountlines', + group_by => 'me.borrowernumber', + '+select' => + { sum => 'accountlines.amountoutstanding', '-as' => 'outstanding' }, + '+as' => 'outstanding' + }; + + $attrs->{'having'} = [ + { 'outstanding' => { '<' => $options->{less_than} } }, + { 'outstanding' => undef } + ] + if ( defined( $options->{less_than} ) + && !defined( $options->{more_than} ) ); + + $attrs->{'having'} = { 'outstanding' => { '>' => $options->{more_than} } } + if (!defined( $options->{less_than} ) + && defined( $options->{more_than} ) ); + + $attrs->{'having'}->{'-and'} = [ + { 'outstanding' => { '>' => $options->{more_than} } }, + { 'outstanding' => { '<' => $options->{less_than} } } + ] + if ( defined( $options->{less_than} ) + && defined( $options->{more_than} ) ); + + return $self->search( $where, $attrs ); +} + + +=head3 filter_by_in_lists + + Koha::Patrons->filter_by_in_lists(\@lists); + +Returns patrons filtered by whether that appear in ANY of the lists specified. + +=head4 arguments arrayref + +Arrayref of list ids + +=back + +=cut + +sub filter_by_in_lists { + my ( $self, $lists ) = @_; + + return $self + unless defined($lists); + + my $where = {}; + my $attrs = { join => 'patron_list_patrons' }; + + push @{ $where->{'-and'} }, + { 'patron_list_patrons.patron_list_id' => { '-in' => $lists } }; + + return $self->search( $where, $attrs ); +} + +=head3 filter_by_not_in_lists + + Koha::Patrons->filter_by_not_in_lists(\@lists); + +Returns patrons filtered by whether they do not appear in ANY of the lists specified. + +=head4 arguments arrayref + +Arrayref of list ids + +=back + +=cut + +sub filter_by_not_in_lists { + my ( $self, $lists ) = @_; + + return $self + unless defined($lists); + + my $where = {}; + my $attrs = { join => 'patron_list_patrons' }; + + push @{ $where->{'-and'} }, + { 'patron_list_patrons.patron_list_id' => { '-not_in' => $lists } }; + + return $self->search( $where, $attrs ); +} + =head3 search_housebound_choosers Returns all Patrons which are Housebound choosers. diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index bb6e2bcf98..c0aaa16fcc 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 31; +use Test::More tests => 32; use Test::Warn; use Time::Fake; use DateTime; @@ -1377,6 +1377,342 @@ subtest 'Log cardnumber change' => sub { is( scalar @logs, 2, 'With BorrowerLogs, Change in cardnumber should be logged, as well as general alert of patron mod.' ); }; +subtest 'complex filters' => sub { + plan tests => 7; + + my $now = DateTime->now; + my $new_patron_cf_1 = Koha::Patron->new( + { + cardnumber => 'test_cn_cf_1', + branchcode => $library->{branchcode}, + categorycode => $category->{categorycode}, + surname => 'surname for patron1', + firstname => 'firstname for patron1', + userid => 'a_nonexistent_userid_cf_1', + dateexpiry => $now->clone->subtract( days => 14 ) + } + )->store; + my $new_patron_cf_2 = Koha::Patron->new( + { + cardnumber => 'test_cn_cf_2', + branchcode => $library->{branchcode}, + categorycode => $category->{categorycode}, + surname => 'surname for patron2', + firstname => 'firstname for patron2', + userid => 'a_nonexistent_userid_cf_2', + dateexpiry => $now->clone->subtract( days => 7 ) + } + )->store; + my $new_patron_cf_3 = Koha::Patron->new( + { + cardnumber => 'test_cn_cf_3', + branchcode => $library->{branchcode}, + categorycode => $category->{categorycode}, + surname => 'surname for patron3', + firstname => 'firstname for patron3', + userid => 'a_nonexistent_userid_cf_3', + } + )->store; + + my $results = Koha::Patrons->search( + { + 'me.borrowernumber' => [ + $new_patron_cf_1->borrowernumber, + $new_patron_cf_2->borrowernumber, + $new_patron_cf_3->borrowernumber + ] + } + ); + + subtest 'filter_by_is_guarantor' => sub { + plan tests => 6; + + # Set new_patron_cf_1 to be the guarantor for new_patron_cf_2 + $new_patron_cf_2->guarantorid( $new_patron_cf_1->borrowernumber ) + ->store; + + my $filtered = $results->filter_by_is_guarantor(); + 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' ); + + $filtered = $results->filter_by_is_guarantor(1); + is( $filtered->count, 1, 'filter_by_is_guarantor(1) found one patron' ); + is( $filtered->next->guarantees->count, + 1, 'filter_by_is_guarantor(1) found the correct patron' ); + + $filtered = $results->filter_by_is_guarantor(0); + is( $filtered->count, 2, + 'filter_by_is_guarantor(0) found two patrons' ); + is( $filtered->next->guarantees->count, + 0, 'filter_by_is_guarantor(0) found the correct patrons' ); + }; + + subtest 'filter_by_has_guarantor' => sub { + plan tests => 6; + + my $filtered = $results->filter_by_has_guarantor(); + is( ref($filtered), 'Koha::Patrons', +'filter_by_has_guarantor() should return a Koha::Patrons result set in a scalar context' + ); + is( $filtered->count, 3, + 'filter_by_has_guarantor() found both patrons' ); + + $filtered = $results->filter_by_has_guarantor(1); + is( $filtered->count, 1, + 'filter_by_has_guarantor(1) found one patron' ); + is( + $filtered->next->guarantorid, + $new_patron_cf_1->borrowernumber, + 'filter_by_has_guarantor(1) found the correct patron' + ); + + $filtered = $results->filter_by_has_guarantor(0); + is( $filtered->count, 2, + 'filter_by_has_guarantor(0) found two patrons' ); + is( $filtered->next->guarantorid, + undef, 'filter_by_has_guarantor(0) found the correct patrons' ); + }; + + subtest 'filter_by_last_issued' => sub { + plan tests => 4; + + my $first = DateTime->now()->subtract( days => 5 ); + my $last = $first->clone->add( days => 2 ); + my $old_issue_1 = $builder->build( + { + source => 'OldIssue', + value => { + borrowernumber => $new_patron_cf_1->borrowernumber, + timestamp => $first->clone->subtract( days => 1 ) + }, + } + ); + my $old_issue_2 = $builder->build( + { + source => 'OldIssue', + value => { + borrowernumber => $new_patron_cf_2->borrowernumber, + timestamp => $last->clone->add( days => 1 ) + }, + } + ); + my $old_issue_3 = $builder->build( + { + source => 'OldIssue', + value => { + borrowernumber => $new_patron_cf_3->borrowernumber, + timestamp => $first->clone->add( days => 1 ) + }, + } + ); + + my $filtered = $results->filter_by_last_issued(); + is( ref($filtered), 'Koha::Patrons', +'filter_by_last_issued() should return a Koha::Patrons result set in a scalar context' + ); + + $filtered = $results->filter_by_last_issued( { before => $last } ); + is( $filtered->_resultset->as_subselect_rs->count, + 2, "filter_by_last_issued({ before => $last }) found two patrons" ); + + $filtered = $results->filter_by_last_issued( { after => $first } ); + is( $filtered->_resultset->as_subselect_rs->count, + 2, "filter_by_last_issued({ after => $first }) found two patrons" ); + + $filtered = $results->filter_by_last_issued( + { after => $first, before => $last } ); + is( $filtered->_resultset->as_subselect_rs->count, 1, +"filter_by_last_issued({ after => $first, before => $last }) found two patrons" + ); + }; + + subtest 'filter_by_has_issues' => sub { + plan tests => 3; + + my $issue_1 = $builder->build( + { + source => 'Issue', + value => { + borrowernumber => $new_patron_cf_1->borrowernumber + }, + } + ); + + my $filtered = $results->filter_by_has_issues(); + is( ref($filtered), 'Koha::Patrons', +'filter_by_has_issues() should return a Koha::Patrons result set in a scalar context' + ); + + $filtered = $results->filter_by_has_issues(1); + is( $filtered->_resultset->as_subselect_rs->count, + 1, "filter_by_has_issues(1) found one patron" ); + + $filtered = $results->filter_by_has_issues(0); + is( $filtered->_resultset->as_subselect_rs->count, + 2, "filter_by_has_issues(0) found two patrons" ); + }; + + subtest 'filter_by_when_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(); + is( ref($filtered), 'Koha::Patrons', +'Koha::Patrons->filter_by_when_expired should return a Koha::Patrons result set in a scalar context' + ); + + $filtered = $results->filter_by_when_expired( { before => $second } ); + is( $filtered->_resultset->as_subselect_rs->count, + 2, + "filter_by_when_expired({ before => $second }) found two patrons" ); + + $filtered = $results->filter_by_when_expired( { after => $first } ); + is( $filtered->_resultset->as_subselect_rs->count, + 2, + "filter_by_when_expired({ after => $first }) found two patrons" ); + + $filtered = $results->filter_by_when_expired( + { after => $first, before => $second } ); + is( $filtered->_resultset->as_subselect_rs->count, 1, +"filter_by_when_expired({ after => $first, before => $second }) found one patrons" + ); + }; + + subtest 'filter_by_amount_owed' => sub { + plan tests => 4; + + my $fine1 = $builder->build( + { + source => 'Accountline', + value => { + borrowernumber => $new_patron_cf_1->borrowernumber, + amountoutstanding => 12.00 + }, + } + ); + my $fine2 = $builder->build( + { + source => 'Accountline', + value => { + borrowernumber => $new_patron_cf_2->borrowernumber, + amountoutstanding => 8.00 + }, + } + ); + my $fine3 = $builder->build( + { + source => 'Accountline', + value => { + borrowernumber => $new_patron_cf_2->borrowernumber, + amountoutstanding => 10.00 + }, + } + ); + + my $filtered = $results->filter_by_amount_owed(); + is( ref($filtered), 'Koha::Patrons', +'Koha::Patrons->filter_by_amount_owed should return a Koha::Patrons result set in a scalar context' + ); + + my $lower_limit = 12.00; + my $upper_limit = 16.00; + + # Catch user with 1 x 12.00 fine and user with no fines. + $filtered = + $results->filter_by_amount_owed( { less_than => $upper_limit } ); + is( $filtered->_resultset->as_subselect_rs->count, 2, +"filter_by_amount_owed({ less_than => $upper_limit }) found two patrons" + ); + + # Catch user with 1 x 8.00 and 1 x 10.00 fine + $filtered = + $results->filter_by_amount_owed( { more_than => $lower_limit } ); + is( $filtered->_resultset->as_subselect_rs->count, 1, +"filter_by_amount_owed({ more_than => $lower_limit }) found two patrons" + ); + + # User with 2 fines falls above upper limit, + # user with 1 fine falls below lower limit + # and user with no fines falls below lower limit. + $filtered = $results->filter_by_amount_owed( + { more_than => $lower_limit, less_than => $upper_limit } ); + is( $filtered->_resultset->as_subselect_rs->count, 0, +"filter_by_amount_owed({ more_than => $lower_limit, less_than => $upper_limit }) found one patrons" + ); + }; + + subtest 'filter_by_lists' => sub { + plan tests => 6; + + my $patronListPatron1 = $builder->build( + { + source => 'PatronListPatron', + value => { + borrowernumber => $new_patron_cf_1->borrowernumber + }, + } + ); + my $patronListPatron2 = $builder->build( + { + source => 'PatronListPatron', + value => { + borrowernumber => $new_patron_cf_2->borrowernumber + }, + } + ); + #my $patronListPatron3 = $builder->build( + # { + # source => 'PatronListPatron', + # value => { + # borrowernumber => $new_patron_cf_2->borrowernumber, + # patron_list_id => $patronListPatron1->patron_list_id + # }, + # } + #); + + my $filtered = $results->filter_by_in_lists(); + is( ref($filtered), 'Koha::Patrons', +'Koha::Patrons->filter_by_in_lists should return a Koha::Patrons result set in a scalar context' + ); + + $filtered = $results->filter_by_not_in_lists(); + is( ref($filtered), 'Koha::Patrons', +'Koha::Patrons->filter_by_not_in_lists should return a Koha::Patrons result set in a scalar context' + ); + + $filtered = + $results->filter_by_in_lists( + [ $patronListPatron1->{patron_list_id} ] ); + is( $filtered->_resultset->as_subselect_rs->count, 1, +"filter_by_in_lists([$patronListPatron1->patron_list_id]) found one patron" + ); + is( $filtered->next->borrowernumber, $new_patron_cf_1->borrowernumber, +"filter_by_in_lists([$patronListPatron1->{patron_list_id}]) found the correct patron" + ); + + $filtered = + $results->filter_by_in_lists( + [ $patronListPatron1->{patron_list_id} ] ); + is( $filtered->_resultset->as_subselect_rs->count, 1, +"filter_by_in_lists([$patronListPatron1->{patron_list_id}]) found one patron" + ); + is( $filtered->next->borrowernumber, $new_patron_cf_1->borrowernumber, +"filter_by_in_lists([$patronListPatron1->{patron_list_id}]) found the correct patron" + ); + + }; + + # subtest 'chaining' => sub { + # plan tests => 0; + # + #}; +}; + $schema->storage->txn_rollback; subtest 'Test Koha::Patrons::merge' => sub { -- 2.17.1