From e6cd799a9366b5a82880143d021eb854dd9942b5 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 22 Sep 2021 10:41:48 -0300 Subject: [PATCH] Bug 27945: (QA follow-up) Fix and move tests to proper file Koha::Patron methods tests belong to the t/db_dependent/Koha/Patron.t tests file. This patch moves the tests, and also acknowledges the fact that we can use Test::Exception instead of a try/catch block. It also fixes the tests so they actually trigger the ->request method, which is the one that raises the exception if the limit is reached. At some point, because of the dependency mess, this bug ended up with the exception throwing in the wrong method, and that's why tests were also failing. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => FAIL: Patron.t passes, ArticleRequests.t doesn't 2. Apply this patch 3. Repeat 1 => SUCCESS: Both pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- Koha/ArticleRequest.pm | 8 +-- Koha/Patron.pm | 5 +- t/db_dependent/ArticleRequests.t | 97 ------------------------- t/db_dependent/Koha/Patron.t | 117 ++++++++++++++++++++++++++++++- 4 files changed, 122 insertions(+), 105 deletions(-) diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm index 1972f8f918..d3554132e9 100644 --- a/Koha/ArticleRequest.pm +++ b/Koha/ArticleRequest.pm @@ -48,6 +48,10 @@ Koha::ArticleRequest - Koha Article Request Object class sub request { my ($self) = @_; + Koha::Exceptions::ArticleRequest::LimitReached->throw( + error => 'Patron cannot request more articles for today' + ) unless $self->borrower->can_request_article; + $self->status(Koha::ArticleRequest::Status::Requested); $self->SUPER::store(); $self->notify(); @@ -61,10 +65,6 @@ sub request { sub set_pending { my ($self) = @_; - Koha::Exceptions::ArticleRequest::LimitReached->throw( - error => 'Patron cannot request more articles for today' - ) unless $self->borrower->can_request_article; - $self->status(Koha::ArticleRequest::Status::Pending); $self->SUPER::store(); $self->notify(); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 69edcfab10..967ae9a33b 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -970,18 +970,17 @@ sub can_request_article { my ($self) = @_; my $limit = $self->category->article_request_limit; - return 1 if !$limit; + return 1 unless defined $limit; my $dtf = Koha::Database->new->schema->storage->datetime_parser; my $compdate = dt_from_string->add( days => -1 ); my $count = Koha::ArticleRequests->search([ - { borrowernumber => $self->borrowernumber, status => ['PENDING','PROCESSING'] }, + { borrowernumber => $self->borrowernumber, status => ['REQUESTED','PENDING','PROCESSING'] }, { borrowernumber => $self->borrowernumber, status => 'COMPLETED', updated_on => { '>', $dtf->format_date($compdate) }}, ])->count; return $count < $limit ? 1 : 0; } - =head3 article_requests my @requests = $borrower->article_requests(); diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index 49f9e3e5ed..6fa935b0c2 100644 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -252,103 +252,6 @@ subtest 'may_article_request' => sub { $cache->clear_from_cache( Koha::CirculationRules::GUESSED_ITEMTYPES_KEY ); }; - -subtest 'article request limit' => sub { - plan tests => 13; - - t::lib::Mocks::mock_preference('ArticleRequests', 1); - - my $item = $builder->build_sample_item; - - my $category = $builder->build_object( - { - class => 'Koha::Patron::Categories', - value => { - article_request_limit => 1 - } - } - ); - my $patron = $builder->build_object( - { - class => 'Koha::Patrons', - value => { - categorycode => $category->categorycode - }, - } - ); - $patron->article_requests->delete(); - - is($patron->can_request_article, 1, 'There are no AR, so patron can request more articles'); - - my $article_request_1 = Koha::ArticleRequest->new( - { - borrowernumber => $patron->id, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - title => 'an article request', - } - )->store(); - - is($patron->can_request_article, 0, 'Limit is 1, so patron cannot request more articles'); - is($patron->article_requests->count, 1, 'There is one current article request'); - - try { - Koha::ArticleRequest->new( - { - borrowernumber => $patron->id, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - title => 'a second article request', - } - )->store(); - } - catch { - is(ref($_), 'Koha::Exceptions::ArticleRequest::LimitReached', 'When limit was reached and we ask for a new AR, Limit reached is thrown'); - }; - - is($patron->can_request_article, 0, 'There is still an AR, so patron cannot request more articles'); - is($patron->article_requests->count, 1, 'There is still one article request'); - - $article_request_1->complete(); - - is($patron->can_request_article, 0, 'AR was completed but within one day, so patron cannot request more articles'); - - $article_request_1->updated_on(dt_from_string->add(days => -2))->store(); - - is($patron->can_request_article, 1, 'There are no completed AR within one day, so patron can request more articles'); - - my $article_request_3 = Koha::ArticleRequest->new( - { - borrowernumber => $patron->id, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - title => 'a third article request', - } - )->store(); - - is($patron->can_request_article, 0, 'A new AR was created, so patron cannot request more articles'); - is($patron->article_requests->count, 2, 'There are 2 article requests'); - - $article_request_3->cancel(); - - is($patron->can_request_article, 1, 'New AR was cancelled, so patron can request more articles'); - - my $article_request_4 = Koha::ArticleRequest->new( - { - borrowernumber => $patron->id, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - title => 'an fourth article request', - } - )->store(); - - $article_request_4->updated_on(dt_from_string->add(days => -30))->store(); - - is($patron->can_request_article, 0, 'There is an old AR but not completed or cancelled, so patron cannot request more articles'); - is($patron->article_requests->count, 3, 'There are 3 current article requests'); - -}; - $schema->storage->txn_rollback(); subtest 'set_pending() tests' => sub { diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index f5a329960d..b088d2a5b2 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,12 +19,13 @@ use Modern::Perl; -use Test::More tests => 8; +use Test::More tests => 9; use Test::Exception; use Test::Warn; use Koha::Database; use Koha::DateUtils qw(dt_from_string); +use Koha::ArticleRequests; use Koha::Patrons; use Koha::Patron::Relationships; @@ -716,3 +717,117 @@ subtest 'can_log_into() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'can_request_article() tests' => sub { + + plan tests => 13; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'ArticleRequests', 1 ); + + my $item = $builder->build_sample_item; + + my $category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { + article_request_limit => 1 + } + } + ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category->categorycode + }, + } + ); + + is( $patron->can_request_article, + 1, 'There are no AR, so patron can request more articles' ); + + my $article_request_1 = Koha::ArticleRequest->new( + { + borrowernumber => $patron->id, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + title => 'an article request', + } + )->request; + + is( $patron->can_request_article, + 0, 'Limit is 1, so patron cannot request more articles' ); + is( $patron->article_requests->count, + 1, 'There is one current article request' ); + + throws_ok { + Koha::ArticleRequest->new( + { + borrowernumber => $patron->id, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + title => 'a second article request', + } + )->request; + } + 'Koha::Exceptions::ArticleRequest::LimitReached', + 'When limit was reached and we ask for a new AR, Limit reached is thrown'; + + is( $patron->can_request_article, + 0, 'There is still an AR, so patron cannot request more articles' ); + is( $patron->article_requests->count, + 1, 'There is still one article request' ); + + $article_request_1->complete(); + + is( $patron->can_request_article, 0, +'AR was completed but within one day, so patron cannot request more articles' + ); + + $article_request_1->updated_on( dt_from_string->add( days => -2 ) ) + ->store(); + + is( $patron->can_request_article, 1, +'There are no completed AR within one day, so patron can request more articles' + ); + + my $article_request_3 = Koha::ArticleRequest->new( + { + borrowernumber => $patron->id, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + title => 'a third article request', + } + )->request; + + is( $patron->can_request_article, + 0, 'A new AR was created, so patron cannot request more articles' ); + is( $patron->article_requests->count, 2, 'There are 2 article requests' ); + + $article_request_3->cancel(); + + is( $patron->can_request_article, + 1, 'New AR was cancelled, so patron can request more articles' ); + + my $article_request_4 = Koha::ArticleRequest->new( + { + borrowernumber => $patron->id, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + title => 'an fourth article request', + } + )->request; + + $article_request_4->updated_on( dt_from_string->add( days => -30 ) ) + ->store(); + + is( $patron->can_request_article, 0, +'There is an old AR but not completed or cancelled, so patron cannot request more articles' + ); + is( $patron->article_requests->count, + 3, 'There are 3 current article requests' ); + + $schema->storage->txn_rollback; +}; -- 2.30.2