Bugzilla – Attachment 125179 Details for
Bug 27945
Limit the number of active article requests per patron category
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27945: (QA follow-up) Fix and move tests to proper file
Bug-27945-QA-follow-up-Fix-and-move-tests-to-prope.patch (text/plain), 10.91 KB, created by
Marcel de Rooy
on 2021-09-23 06:21:29 UTC
(
hide
)
Description:
Bug 27945: (QA follow-up) Fix and move tests to proper file
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2021-09-23 06:21:29 UTC
Size:
10.91 KB
patch
obsolete
>From 18a8d57566f0f3e57a3155b3141f6cd1b3cd1c2d Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Wed, 22 Sep 2021 10:41:48 -0300 >Subject: [PATCH] Bug 27945: (QA follow-up) Fix and move tests to proper file >Content-Type: text/plain; charset=utf-8 > >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 <tomascohen@theke.io> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > 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.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 27945
:
123217
|
123218
|
123219
|
123220
|
123714
|
123715
|
123774
|
123775
|
123776
|
123777
|
123778
|
123820
|
124066
|
124067
|
124068
|
124069
|
124070
|
124071
|
124072
|
124073
|
124074
|
124075
|
124076
|
124077
|
124078
|
124079
|
124284
|
124285
|
124633
|
124673
|
124674
|
124675
|
124676
|
124677
|
124678
|
124679
|
124680
|
124681
|
124682
|
125140
|
125142
|
125143
|
125144
|
125145
|
125146
|
125147
|
125148
|
125149
|
125150
|
125151
|
125152
|
125178
|
125179
|
125569
|
125571
|
125638
|
125690
|
125702
|
125703
|
125704
|
125705
|
125706
|
125707
|
125726
|
125741
|
125761