From ab67689455970b02fe475a118435127c34dd0601 Mon Sep 17 00:00:00 2001 From: Katrin Fischer Date: Fri, 20 Apr 2018 22:25:08 +0200 Subject: [PATCH] Bug 20400: (follow-up) Several fixes from RM review - "your routing lists" tab is now highlighted when active - get_routinglists was renamed to get_routing_lists - Koha::Patron->get_routing_lists returns the ->search result directly - Koha::Subscription::RoutingList->subscription uses DBIC relationship - Undo changes to C4/Auth.pm --- C4/Auth.pm | 3 +-- Koha/Patron.pm | 10 +++++----- Koha/Subscription/Routinglist.pm | 2 +- koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc | 2 +- .../opac-tmpl/bootstrap/en/modules/opac-routing-lists.tt | 3 ++- opac/opac-routing-lists.pl | 5 +---- t/db_dependent/Koha/Patrons.t | 12 ++++++------ 7 files changed, 17 insertions(+), 20 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 0661031069..13c4652d2e 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -238,12 +238,12 @@ sub get_template_and_user { } my $borrowernumber; - my $patron; if ($user) { # It's possible for $user to be the borrowernumber if they don't have a # userid defined (and are logging in through some other method, such # as SSL certs against an email address) + my $patron; $borrowernumber = getborrowernumber($user) if defined($user); if ( !defined($borrowernumber) && defined($user) ) { $patron = Koha::Patrons->find( $user ); @@ -610,7 +610,6 @@ sub get_template_and_user { PatronSelfRegistration => C4::Context->preference("PatronSelfRegistration"), PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"), useDischarge => C4::Context->preference('useDischarge'), - routing_lists_exist => ( $patron and $patron->get_routinglists ), ); $template->param( OpacPublic => '1' ) if ( $user || C4::Context->preference("OpacPublic") ); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index dfa8bf1d0e..653de53c14 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -589,18 +589,18 @@ sub get_overdues { ); } -=head3 get_routinglists +=head3 get_routing_lists -my @routinglists = $patron->get_routinglists +my @routinglists = $patron->get_routing_lists Returns the routing lists a patron is subscribed to. =cut -sub get_routinglists { +sub get_routing_lists { my ($self) = @_; - my @subscribed_routings = Koha::Subscription::Routinglists->search({ borrowernumber => $self->borrowernumber }); - return @subscribed_routings; + my $routing_list_rs = $self->_result->subscriptionroutinglists; + return Koha::Subscription::Routinglists->_new_from_dbic($routing_list_rs); } =head3 get_age diff --git a/Koha/Subscription/Routinglist.pm b/Koha/Subscription/Routinglist.pm index 4c081e5774..eb18e4bc25 100644 --- a/Koha/Subscription/Routinglist.pm +++ b/Koha/Subscription/Routinglist.pm @@ -45,7 +45,7 @@ Returns the subscription for a routing list. sub subscription { my ( $self ) = @_; - return scalar Koha::Subscriptions->find( $self->subscriptionid ); + return Koha::Subscription->_new_from_dbic($self->_result->subscriptionid); } =head2 Internal methods diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc index 5914f04a2b..0783438ab7 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc @@ -96,7 +96,7 @@ your lists [% END %] - [% IF Koha.Preference( 'RoutingSerials' ) && routing_lists_exist %] + [% IF Koha.Preference( 'RoutingSerials' ) && logged_in_user && logged_in_user.get_routing_lists.count %] [% IF ( routinglistsview ) %]
  • [% ELSE %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-routing-lists.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-routing-lists.tt index ba1e34a108..a0e09e5903 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-routing-lists.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-routing-lists.tt @@ -32,7 +32,8 @@

    Routing lists

    - [% IF ( routinglists ) %] + [% SET routinglists = logged_in_user.get_routing_lists %] + [% IF ( routinglists.count ) %]

    You are subscribed to the routing lists for following serial titles. If you wish to make changes, please contact the library.

    diff --git a/opac/opac-routing-lists.pl b/opac/opac-routing-lists.pl index 2dc09c6003..95377712e9 100644 --- a/opac/opac-routing-lists.pl +++ b/opac/opac-routing-lists.pl @@ -41,11 +41,8 @@ $borrower->{description} = $category->description; $borrower->{category_type} = $category->category_type; $template->param( BORROWER_INFO => $borrower ); -my @routinglists = $patron->get_routinglists(); - $template->param( - routinglists => \@routinglists, - routinglistview => 1, + routinglistsview => 1, ); output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index ea57389b80..adad6c1faa 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -529,7 +529,7 @@ subtest 'checkouts + pending_checkouts + get_overdues + old_checkouts' => sub { $module->unmock('userenv'); }; -subtest 'get_routinglists' => sub { +subtest 'get_routing_lists' => sub { plan tests => 5; my $biblio = Koha::Biblio->new()->store(); @@ -541,7 +541,7 @@ subtest 'get_routinglists' => sub { my $patron = $builder->build( { source => 'Borrower' } ); $patron = Koha::Patrons->find( $patron->{borrowernumber} ); - is( $patron->get_routinglists, 0, 'Retrieves correct number of routing lists: 0' ); + is( $patron->get_routing_lists->count, 0, 'Retrieves correct number of routing lists: 0' ); my $routinglist_count = Koha::Subscription::Routinglists->count; my $routinglist = Koha::Subscription::Routinglist->new({ @@ -550,11 +550,11 @@ subtest 'get_routinglists' => sub { subscriptionid => $subscription->subscriptionid })->store; - is ($patron->get_routinglists, 1, "Retrieves correct number of routing lists: 1"); + is ($patron->get_routing_lists->count, 1, "Retrieves correct number of routing lists: 1"); - my @routinglists = $patron->get_routinglists; + my @routinglists = $patron->get_routing_lists; is ($routinglists[0]->ranking, 5, "Retrieves ranking: 5"); - is( ref($routinglists[0]), 'Koha::Subscription::Routinglist', 'get_routinglists returns Koha::Subscription::Routinglist objects' ); + is( ref($routinglists[0]), 'Koha::Subscription::Routinglist', 'get_routing_lists returns Koha::Subscription::Routinglist objects' ); my $subscription2 = Koha::Subscription->new({ biblionumber => $biblio->biblionumber, @@ -566,7 +566,7 @@ subtest 'get_routinglists' => sub { subscriptionid => $subscription2->subscriptionid })->store; - is ($patron->get_routinglists, 2, "Retrieves correct number of routing lists: 2"); + is ($patron->get_routing_lists->count, 2, "Retrieves correct number of routing lists: 2"); $patron->delete; # Clean up for later tests -- 2.14.1