gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 29083: Update article requests-related Koha::Patron methods to use relationships 

This patch makes Koha::Patron->article_requests use the underlying DBIC
relationship and _new_from_dbic instead of a plain search. It also
refactors 'article_requests_current' and 'article_requests_finished' to
use ->article_requests, as well as the new methods introduced by bug
29082 for filtering.

No behavior change should take place.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Patron.t \
           t/db_dependent/ArticleRequests.t
=> SUCCESS: Tests pass!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Bug 29083: Unit tests

This patch adds missing tests for Koha::Patron->article_requests and
moves (and extends) tests for 'article_requests_current' and
'article_requests_finished' that were originally in ArticleRequests.t
into Koha/Patron.t as we now do.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/ArticleRequests.t \
           t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Bug 29083: (QA follow-up) Remove unused param

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Bug 29083: Fix OPAC listing of article requests

This patch makes the OPAC template reuse a precalculated value for the
active article requests for the patron (and its count).

The original code relied on the methods returning a list, which is not
the case for _new_from_dbic until bug 28883 is pushed.

This patch fixes that.

Note: there was an odd behavior when ArticleRequests was enabled but no
active article requests were present: the tab wasn't rendered but the
'empty table' with the 'You have no article requests currently.' message
was displayed below the Checkouts tab. I'm not sure that was caused by
this patches, or other. Fixed on this patch.

To test:
1. In the OPAC, go to 'your summary'
=> FAIL: Things don't show for article requests
2. Add some article requests and repeat 1
=> FAIL: Something's wrong there
3. Apply this patch and repeat 1
=> Yes! Things show correctly!
4. Cancel all your article requests
=> SUCCESS: Things render as they should
5. Re-enter the 'your summary' page (to force re-rendering)
=> SUCCESS: Things render correctly for empty article requests
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Bug 29083: Remove article_requests_finished and article_requests_current

This patch removes those methods that are not really needed. Templates
are adjusted to use the expected combination of
->article_requests->filter_by_current.

To test:
1. Apply this patch
2. Visit a patron with article requests
=> SUCCESS: All works
3. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass, less tests.
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Tomas Cohen Arazi 2021-09-22 15:46:26 -03:00 committed by Jonathan Druart
parent f74dff4537
commit 02db36b95c
8 changed files with 62 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
Koha/Patron.pm
View File

@ -1003,70 +1003,16 @@ sub can_request_article {
=head3 article_requests
my @requests = $borrower->article_requests();
my $requests = $borrower->article_requests();
my $article_requests = $patron->article_requests;
Returns either a list of ArticleRequests objects,
or an ArtitleRequests object, depending on the
calling context.
Returns the patron article requests.
=cut
sub article_requests {
my ( $self ) = @_;
my ($self) = @_;
$self->{_article_requests} ||= Koha::ArticleRequests->search({ borrowernumber => $self->borrowernumber() });
return $self->{_article_requests};
}
=head3 article_requests_current
my @requests = $patron->article_requests_current
Returns the article requests associated with this patron that are incomplete
=cut
sub article_requests_current {
my ( $self ) = @_;
$self->{_article_requests_current} ||= Koha::ArticleRequests->search(
{
borrowernumber => $self->id(),
-or => [
{ status => Koha::ArticleRequest::Status::Requested },
{ status => Koha::ArticleRequest::Status::Pending },
{ status => Koha::ArticleRequest::Status::Processing }
]
}
);
return $self->{_article_requests_current};
}
=head3 article_requests_finished
my @requests = $biblio->article_requests_finished
Returns the article requests associated with this patron that are completed
=cut
sub article_requests_finished {
my ( $self, $borrower ) = @_;
$self->{_article_requests_finished} ||= Koha::ArticleRequests->search(
{
borrowernumber => $self->id(),
-or => [
{ status => Koha::ArticleRequest::Status::Completed },
{ status => Koha::ArticleRequest::Status::Canceled }
]
}
);
return $self->{_article_requests_finished};
return Koha::ArticleRequests->_new_from_dbic( scalar $self->_result->article_requests );
}
=head3 add_enrolment_fee_if_needed

6
koha-tmpl/intranet-tmpl/prog/en/includes/patron-article-requests.inc
View File

@ -1,5 +1,7 @@
[% USE Context %]
[% SET current_article_requests = Context.Scalar( Context.Scalar( patron, 'article_requests' ), 'filter_by_current' ) %]
<div id="article-requests">
[% IF patron.article_requests_current.count %]
[% IF current_article_requests.count > 0 %]
<table id="article-requests-table" class="table table-bordered table-striped">
<thead>
<tr>
@ -20,7 +22,7 @@
</thead>
<tbody>
[% FOREACH ar IN patron.article_requests_current %]
[% FOREACH ar IN current_article_requests %]
<tr>
<td class="article-request-record-title">
<a class="article-request-title" href="/cgi-bin/koha/circ/request-article.pl?biblionumber=[% ar.biblionumber | html %]">

4
koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
View File

@ -1,5 +1,6 @@
[% USE raw %]
[% USE Asset %]
[% USE Context %]
[% USE Koha %]
[% USE Branches %]
[% USE KohaDates %]
@ -780,8 +781,9 @@
</li>
[% IF Koha.Preference('ArticleRequests') %]
[% SET current_article_requests = Context.Scalar( Context.Scalar( patron, 'article_requests' ), 'filter_by_current' ) %]
<li>
<a href="#article-requests" id="article-requests-tab"> [% patron.article_requests_current.count | html %] Article requests</a>
<a href="#article-requests" id="article-requests-tab"> [% current_article_requests.count | html %] Article requests</a>
</li>
[% END %]

4
koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
View File

@ -1,5 +1,6 @@
[% USE raw %]
[% USE Asset %]
[% USE Context %]
[% USE Koha %]
[% USE Branches %]
[% USE ItemTypes %]
@ -695,8 +696,9 @@
</li>
[% END %]
[% IF Koha.Preference('ArticleRequests') %]
[% SET article_requests = Context.Scalar( Context.Scalar( patron, 'article_requests' ), 'filter_by_current') %]
<li>
<a href="#article-requests" id="article-requests-tab"> [% patron.article_requests_current.count | html %] Article requests</a>
<a href="#article-requests" id="article-requests-tab"> [% article_requests.count | html %] Article requests</a>
</li>
[% END %]

10
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
View File

@ -230,7 +230,7 @@
[% END %]
[% IF ( RESERVES.count ) %]<li><a href="#opac-user-holds">Holds ([% RESERVES.count | html %])</a></li>[% END %]
[% IF Koha.Preference('ArticleRequests') && logged_in_user.article_requests_current %]<li><a href="#opac-user-article-requests">Article requests ([% logged_in_user.article_requests_current.count | html %])</a></li>[% END %]
[% IF Koha.Preference('ArticleRequests') %]<li><a href="#opac-user-article-requests">Article requests ([% current_article_requests_count || 0 | html %])</a></li>[% END %]
[% IF ( OverDriveCirculation ) %]
<li><a href="#opac-user-overdrive">OverDrive account</a></li>
[% END %]
@ -765,7 +765,7 @@
[% IF Koha.Preference('ArticleRequests') %]
<div id="opac-user-article-requests">
[% IF logged_in_user.article_requests_current.count %]
[% IF current_article_requests_count %]
<table id="article-requests-table" class="table table-bordered table-striped">
<caption>Article requests <span class="count"></span></caption>
<thead>
@ -788,7 +788,7 @@
</thead>
<tbody>
[% FOREACH ar IN logged_in_user.article_requests_current %]
[% FOREACH ar IN current_article_requests %]
<td class="article-request-record-title">
<a class="article-request-title" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% ar.biblionumber | html %]">
[% INCLUDE 'biblio-title.inc' biblio=ar.biblio %]
@ -866,7 +866,7 @@
<caption class="sr-only">Article requests</caption>
<tr><td>You have no article requests currently.</td></tr>
</table>
[% END # IF article_requests_current.count %]
[% END # IF current_article_requests_count %]
</div> <!-- / #opac-user-article-requests -->
[% END %]
@ -904,7 +904,7 @@
}
}
$(document).ready(function(){
$('#opac-user-article-requests caption .count').html(AR_CAPTION_COUNT.format('[% logged_in_user.article_requests_current.count | html %]'));
$('#opac-user-article-requests caption .count').html(AR_CAPTION_COUNT.format('[% current_article_requests_count | html %]'));
$('#opac-user-views').tabs();
$(".modal-nojs").addClass("modal").addClass("hide").removeClass("modal-nojs");
$(".suspend-until").prop("readonly",1);

7
opac/opac-user.pl
View File

@ -386,6 +386,13 @@ if ( C4::Context->preference('AllowPatronToSetFinesVisibilityForGuarantor')
$template->param( relatives_with_fines => \@relatives_with_fines );
}
if ( C4::Context->preference("ArticleRequests") ) {
my @current_article_requests = $patron->article_requests->filter_by_current->as_list;
$template->param(
current_article_requests => \@current_article_requests,
current_article_requests_count => scalar @current_article_requests,
);
}
$template->param(
patron_messages => $patron_messages,

27
t/db_dependent/ArticleRequests.t
View File

@ -19,7 +19,7 @@ use Modern::Perl;
use POSIX qw(strftime);
use Test::More tests => 54;
use Test::More tests => 45;
use Test::MockModule;
use t::lib::TestBuilder;
@ -110,30 +110,7 @@ is( $article_request->biblio->id, $biblio->id, '$ar->biblio() gets correspondi
is( $article_request->item->id, $item->id, '$ar->item() gets corresponding Koha::Item object' );
is( $article_request->borrower->id, $patron->id, '$ar->borrower() gets corresponding Koha::Patron object' );
my $ar = $patron->article_requests();
is( ref($ar), 'Koha::ArticleRequests', '$patron->article_requests returns Koha::ArticleRequests object' );
is( $ar->next->id, $article_request->id, 'Returned article request matches' );
is( $patron->article_requests_current()->count(), 1, 'Open request returned for article_requests_current' );
$article_request->process();
is( $patron->article_requests_current()->count(), 1, 'Processing request returned for article_requests_current' );
$article_request->complete();
is( $patron->article_requests_current()->count(), 0, 'Completed request not returned for article_requests_current' );
$article_request->cancel();
is( $patron->article_requests_current()->count(), 0, 'Canceled request not returned for article_requests_current' );
$article_request->set_pending();
is( $patron->article_requests_finished()->count(), 0, 'Open request returned for article_requests_finished' );
$article_request->process();
is( $patron->article_requests_finished()->count(), 0, 'Processing request returned for article_requests_finished' );
$article_request->complete();
$article_request->cancel();
is( $patron->article_requests_finished()->count(), 1, 'Canceled request not returned for article_requests_finished' );
$article_request->set_pending();
$ar = $biblio->article_requests();
my $ar = $biblio->article_requests();
is( ref($ar), 'Koha::ArticleRequests', '$biblio->article_requests returns Koha::ArticleRequests object' );
is( $ar->next->id, $article_request->id, 'Returned article request matches' );

35
t/db_dependent/Koha/Patron.t
View File

@ -19,7 +19,7 @@
use Modern::Perl;
use Test::More tests => 9;
use Test::More tests => 10;
use Test::Exception;
use Test::Warn;
@ -813,3 +813,36 @@ subtest 'can_request_article() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'article_requests() tests' => sub {
plan tests => 3;
$schema->storage->txn_begin;
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $article_requests = $patron->article_requests;
is( ref($article_requests), 'Koha::ArticleRequests',
'In scalar context, type is correct' );
is( $article_requests->count, 0, 'No article requests' );
foreach my $i ( 0 .. 3 ) {
my $item = $builder->build_sample_item;
Koha::ArticleRequest->new(
{
borrowernumber => $patron->id,
biblionumber => $item->biblionumber,
itemnumber => $item->id,
title => "Title",
}
)->request;
}
$article_requests = $patron->article_requests;
is( $article_requests->count, 4, '4 article requests' );
$schema->storage->txn_rollback;
};