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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 9ee5c2b..00dedcb 100644 (file)
--- 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 fb278aa..35cfce4 100644 (file)
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -667,18 +667,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 4c081e5..eb18e4b 100644 (file)
--- 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 5914f04..0783438 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc
@@ -96,7 +96,7 @@
                 <a href="/cgi-bin/koha/opac-shelves.pl?op=list&amp;category=1">your lists</a></li>
             [% END %]
 
-            [% IF Koha.Preference( 'RoutingSerials' ) && routing_lists_exist %]
+            [% IF Koha.Preference( 'RoutingSerials' ) && logged_in_user && logged_in_user.get_routing_lists.count %]
                 [% IF ( routinglistsview ) %]
                     <li class="active">
                 [% 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 ba1e34a..a0e09e5 100644 (file)
--- 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 @@
 
                     <h3>Routing lists</h3>
 
-                    [% IF ( routinglists ) %]
+                    [% SET routinglists = logged_in_user.get_routing_lists %]
+                    [% IF ( routinglists.count ) %]
                         <p id="routing-list-intro">You are subscribed to the routing lists for following serial titles. If you wish to make changes, please contact the library.</p>
 
                         <table class="table table-bordered table-striped" id="routingtable">

diff --git a/opac/opac-routing-lists.pl b/opac/opac-routing-lists.pl
index 331cc4d..67bb5fa 100644 (file)
--- a/opac/opac-routing-lists.pl
+++ b/opac/opac-routing-lists.pl
@@ -48,11 +48,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 c97d3b4..c3ff817 100644 (file)
--- 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