From e10291b8e5567fb37f73f2b8a80b4ca3951626d4 Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Tue, 27 Jul 2021 12:20:46 -0300 Subject: [PATCH] Bug 27945: Add limit article request feature This patch makes it possible to limit article requests per patron per day. To test: 1. Apply patches 2. updatedatabase 3. Enable ArticleRequests preference 4. Edit a patron category and set an article request limit to 1 CHECK => if you set the limit to anything else but a positive number or empty string, a warning appears 5. In staff search biblios and request an article for a patron of the modified category 6. Repeat step 5 SUCCESS => if limit is reached, when you select the user to request an article a warning appears saying that the limit was reached 7. Repeat steps 5 and 6 but this time in opac SUCCESS => Patron is not allowed to request another article if limit is reached 8. prove t/db_dependent/ArticleRequests.t Signed-off-by: Tomas Cohen Arazi Signed-off-by: David Nind Signed-off-by: Marcel de Rooy Edit: This patchset originally changed the 'categories' table structure and relied on that for limit calculation. I removed all that code and squashed into this one, as we moved everything to the circulation_rules table. --- Koha/ArticleRequest.pm | 5 + Koha/Exceptions/ArticleRequest.pm | 46 +++++++ Koha/Patron.pm | 23 ++++ circ/request-article.pl | 48 ++++--- .../prog/en/modules/circ/request-article.tt | 5 + .../en/modules/opac-request-article.tt | 7 +- opac/opac-request-article.pl | 54 +++++--- t/db_dependent/ArticleRequests.t | 91 -------------- t/db_dependent/Koha/Patron.t | 117 +++++++++++++++++- 9 files changed, 266 insertions(+), 130 deletions(-) create mode 100644 Koha/Exceptions/ArticleRequest.pm diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm index 82437089c9..8e9bd5fbb4 100644 --- a/Koha/ArticleRequest.pm +++ b/Koha/ArticleRequest.pm @@ -27,6 +27,7 @@ use Koha::Items; use Koha::Libraries; use Koha::DateUtils qw( dt_from_string ); use Koha::ArticleRequest::Status; +use Koha::Exceptions::ArticleRequest; use base qw(Koha::Object); @@ -51,6 +52,10 @@ Marks the article as requested. Send a notification if appropriate. 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(); diff --git a/Koha/Exceptions/ArticleRequest.pm b/Koha/Exceptions/ArticleRequest.pm new file mode 100644 index 0000000000..ef0f25769e --- /dev/null +++ b/Koha/Exceptions/ArticleRequest.pm @@ -0,0 +1,46 @@ +package Koha::Exceptions::ArticleRequest; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Exception::Class ( + 'Koha::Exceptions::ArticleRequest' => { + description => 'Something went wrong!', + }, + 'Koha::Exceptions::ArticleRequest::LimitReached' => { + isa => 'Koha::Exceptions::ArticleRequest', + description => 'Article request limit was reached' + }, +); + +=head1 NAME + +Koha::Exceptions::ArticleRequest - Base class for ArticleRequest exceptions + +=head1 Exceptions + +=head2 Koha::Exceptions::ArticleRequest + +Generic ArticleRequest exception + +=head2 Koha::Exceptions::ArticleRequest::LimitReached + +Exception to be used when the article request limit has been reached. + +=cut + +1; \ No newline at end of file diff --git a/Koha/Patron.pm b/Koha/Patron.pm index b861e53e9e..967ae9a33b 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -958,6 +958,29 @@ sub move_to_deleted { return Koha::Database->new->schema->resultset('Deletedborrower')->create($patron_infos); } +=head3 can_request_article + +my $can_request = $borrower->can_request_article + +Returns true if patron can request articles + +=cut + +sub can_request_article { + my ($self) = @_; + my $limit = $self->category->article_request_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 => ['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/circ/request-article.pl b/circ/request-article.pl index 678c4c9694..0cc4cc591d 100755 --- a/circ/request-article.pl +++ b/circ/request-article.pl @@ -27,6 +27,7 @@ use C4::Serials qw( CountSubscriptionFromBiblionumber ); use Koha::Biblios; use Koha::Patrons; use Koha::ArticleRequests; +use Try::Tiny; my $cgi = CGI->new; @@ -68,23 +69,29 @@ if ( $action eq 'create' ) { my $patron_notes = $cgi->param('patron_notes') || undef; my $format = $cgi->param('format') || undef; - my $ar = Koha::ArticleRequest->new( - { - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, - branchcode => $branchcode, - itemnumber => $itemnumber, - title => $title, - author => $author, - volume => $volume, - issue => $issue, - date => $date, - pages => $pages, - chapters => $chapters, - patron_notes => $patron_notes, - format => $format, - } - )->store(); + try { + my $ar = Koha::ArticleRequest->new( + { + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + branchcode => $branchcode, + itemnumber => $itemnumber, + title => $title, + author => $author, + volume => $volume, + issue => $issue, + date => $date, + pages => $pages, + chapters => $chapters, + patron_notes => $patron_notes, + format => $format, + } + )->store(); + } catch { + $template->param( + error_message => $_->{message} + ); + }; } @@ -109,6 +116,13 @@ if ( !$patron && $patron_cardnumber ) { } } +if( $patron && !$patron->can_request_article) { + $patron = undef; + $template->param( + error_message => 'Patron cannot request more articles for today' + ); +} + $template->param( biblio => $biblio, patron => $patron, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt index 224d2bb846..f26a10318d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt @@ -46,6 +46,11 @@

Request article from [% INCLUDE 'biblio-title.inc' link = 1 %]

+ [% IF error_message %] +
+

[% error_message | html %]

+
+ [% END %] [% IF no_patrons_found %]

Patron not found

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-request-article.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-request-article.tt index f33c470e87..e504556b74 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-request-article.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-request-article.tt @@ -33,7 +33,7 @@
- [% IF biblio.can_article_request( patron ) %] + [% IF !error_message && biblio.can_article_request( patron ) %]

Place article request for [% biblio.title | html %]

[% IF ( disclaimer && !action) %]
@@ -216,6 +216,11 @@ [% END %] + [% ELSIF error_message %] +

[% biblio.title | html %]

+
+ [% error_message | html %] +
[% ELSE %]

[% biblio.title | html %]

diff --git a/opac/opac-request-article.pl b/opac/opac-request-article.pl index dd02b4d7b9..146454f033 100755 --- a/opac/opac-request-article.pl +++ b/opac/opac-request-article.pl @@ -26,6 +26,7 @@ use C4::Output qw( output_html_with_http_headers ); use Koha::Biblios; use Koha::Patrons; +use Try::Tiny; my $cgi = CGI->new; @@ -59,26 +60,33 @@ if ( $action eq 'create' ) { my $patron_notes = $cgi->param('patron_notes') || undef; my $format = $cgi->param('format') || undef; - my $ar = Koha::ArticleRequest->new( - { - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, - branchcode => $branchcode, - itemnumber => $itemnumber, - title => $title, - author => $author, - volume => $volume, - issue => $issue, - date => $date, - pages => $pages, - chapters => $chapters, - patron_notes => $patron_notes, - format => $format, - } - )->store(); - - print $cgi->redirect("/cgi-bin/koha/opac-user.pl#opac-user-article-requests"); - exit; + try { + my $ar = Koha::ArticleRequest->new( + { + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + branchcode => $branchcode, + itemnumber => $itemnumber, + title => $title, + author => $author, + volume => $volume, + issue => $issue, + date => $date, + pages => $pages, + chapters => $chapters, + patron_notes => $patron_notes, + format => $format, + } + )->store(); + + print $cgi->redirect("/cgi-bin/koha/opac-user.pl#opac-user-article-requests"); + exit; + } catch { + exit unless $_->[0] && $_->[0] eq 'EXIT'; + $template->param( + error_message => $_->{message} + ); + }; # Should we redirect? } elsif ( !$action && C4::Context->preference('ArticleRequestsOpacHostRedirection') ) { @@ -96,6 +104,12 @@ elsif ( !$action && C4::Context->preference('ArticleRequestsOpacHostRedirection' my $patron = Koha::Patrons->find($borrowernumber); +if(!$patron->can_request_article) { + $template->param( + error_message => 'You cannot request more articles for today' + ); +} + $template->param( biblio => $biblio, patron => $patron, diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index 8c89182095..2313e1bc63 100644 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -252,95 +252,4 @@ subtest 'may_article_request' => sub { $cache->clear_from_cache( Koha::CirculationRules::GUESSED_ITEMTYPES_KEY ); }; - -subtest 'article request limit' => sub { - plan tests => 12; - - 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, '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, '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 => 'an second article request', - } - )->store(); - } - catch { - is(ref($_), 'Koha::Exceptions::ArticleRequest::LimitReached', 'Limit reached thrown'); - }; - - is($patron->can_request_article, 0, 'Patron cannot request more articles'); - is($patron->article_requests->count, 1, 'There is still one article request'); - - $article_request_1->created_on(dt_from_string->add(days => -1))->store(); - - is($patron->can_request_article, 1, 'Patron can request more articles'); - - my $article_request_3 = Koha::ArticleRequest->new( - { - borrowernumber => $patron->id, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - title => 'an third article request', - } - )->store(); - - is($patron->can_request_article, 0, '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, 'Patron can request more articles'); - - Koha::ArticleRequest->new( - { - borrowernumber => $patron->id, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - title => 'an fourth article request', - } - )->store(); - - is($patron->can_request_article, 0, 'Patron cannot request more articles'); - is($patron->article_requests->count, 3, 'There are 3 current article requests'); - -}; - $schema->storage->txn_rollback(); 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.32.0