It would be nice if a patron could see the routing lists he is on in the OPAC. Currently this is only possible from the staff client.
Created attachment 72973 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: A list of the routling lists a patron appears on in the OPAC. To test: - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Turn the preference on - the tab should appear now - Add some subscriptions and add your patron to the routing list - Check tab again - it should now show a list of the serials with links to the detail page in the OPAC
Created attachment 72974 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription
Not ready for sign-off yet!
Created attachment 73115 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: Show a list of the serial titles a patron is on routing lists for in the OPAC. Test plan applies to the complete patch set: To test: - Apply all patches - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Activate RoutingSerials - Create subscriptions and different routing lists, test with: - Patron with no routing list entries = no tab - Patron with one or more routing list entries = tab appears
Created attachment 73116 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription
Created attachment 73117 [details] [review] Bug 20400: Add unit tests prove t/db_dependent/Koha/Subscription/Routinglists.t prove t/db_dependent/Koha/Patrons.t
Created attachment 73118 [details] [review] Bug 20400: Display new tab in OPAC only for patrons with routing lists The visibility of the routing list tab in the OPAC depends on the system preference RoutingSerials and the existence of routing list entries for the patron. Some libraries only offer routing lists to certain user groups and would not want it generally visible. As there are currently no actions you can perform from the list, this appears to be a reasonable behaviour. See test plan in first patch.
Created attachment 73121 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: Show a list of the serial titles a patron is on routing lists for in the OPAC. Test plan applies to the complete patch set: To test: - Apply all patches - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Activate RoutingSerials - Create subscriptions and different routing lists, test with: - Patron with no routing list entries = no tab - Patron with one or more routing list entries = tab appears
Created attachment 73122 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription
Created attachment 73123 [details] [review] Bug 20400: Add unit tests prove t/db_dependent/Koha/Subscription/Routinglists.t prove t/db_dependent/Koha/Patrons.t
Created attachment 73124 [details] [review] Bug 20400: Display new tab in OPAC only for patrons with routing lists The visibility of the routing list tab in the OPAC depends on the system preference RoutingSerials and the existence of routing list entries for the patron. Some libraries only offer routing lists to certain user groups and would not want it generally visible. As there are currently no actions you can perform from the list, this appears to be a reasonable behaviour. See test plan in first patch.
Comment on attachment 73121 [details] [review] Bug 20400: Add routing list tab in OPAC I'm not quite happy with the title: +<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog › Your routing lists</title> in Finnish, the word "catalog" could be translated in several different ways, and "Koha online catalog" vs. "$LIBRARY catalog" would use two different words for it to sound more natural ... so perhaps moving the word "catalog" into both parts of the IF - THEN - ELSE block? I know it probably shows up as a single msgid in the po-file, but still
(In reply to paxed from comment #13) > Comment on attachment 73121 [details] [review] [review] > Bug 20400: Add routing list tab in OPAC > > I'm not quite happy with the title: > > +<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha > online[% END %] catalog › Your routing lists</title> > > in Finnish, the word "catalog" could be translated in several different > ways, and "Koha online catalog" vs. "$LIBRARY catalog" would use two > different words for it to sound more natural ... so perhaps moving the word > "catalog" into both parts of the IF - THEN - ELSE block? > > I know it probably shows up as a single msgid in the po-file, but still Hi Pasi, I am using the standard construct here, copied from another page. It's not specific to this patch set. I am not so happy with it either for German translation, but I think this needs a global fix and is outside of scope here.
Comment on attachment 73123 [details] [review] Bug 20400: Add unit tests Review of attachment 73123 [details] [review]: ----------------------------------------------------------------- This + my $patron = $builder->build( { source => 'Borrower' } ); + $patron = Koha::Patrons->find( $patron->{borrowernumber} ); should be replaced by + my $patron = $builder->build_object({ class => 'Koha::Patrons' });
The new tab correctly appears on opac but when I click on it, I get an Error 500 trying to access this page /cgi-bin/koha/opac-routing-lists.pl
(In reply to Séverine Queune from comment #16) > The new tab correctly appears on opac but when I click on it, I get an Error > 500 trying to access this page /cgi-bin/koha/opac-routing-lists.pl Hi Severine, I just retested and don't see the problem. I will fix some tests and maybe you can take another look?
Created attachment 74291 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: Show a list of the serial titles a patron is on routing lists for in the OPAC. Test plan applies to the complete patch set: To test: - Apply all patches - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Activate RoutingSerials - Create subscriptions and different routing lists, test with: - Patron with no routing list entries = no tab - Patron with one or more routing list entries = tab appears
Created attachment 74292 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription
Created attachment 74293 [details] [review] Bug 20400: Add unit tests prove t/db_dependent/Koha/Subscription/Routinglists.t prove t/db_dependent/Koha/Patrons.t
Created attachment 74294 [details] [review] Bug 20400: Display new tab in OPAC only for patrons with routing lists The visibility of the routing list tab in the OPAC depends on the system preference RoutingSerials and the existence of routing list entries for the patron. Some libraries only offer routing lists to certain user groups and would not want it generally visible. As there are currently no actions you can perform from the list, this appears to be a reasonable behaviour. See test plan in first patch.
Created attachment 74295 [details] [review] Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt
Created attachment 74296 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: Show a list of the serial titles a patron is on routing lists for in the OPAC. Test plan applies to the complete patch set: To test: - Apply all patches - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Activate RoutingSerials - Create subscriptions and different routing lists, test with: - Patron with no routing list entries = no tab - Patron with one or more routing list entries = tab appears
Created attachment 74297 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription
Created attachment 74298 [details] [review] Bug 20400: Add unit tests prove t/db_dependent/Koha/Subscription/Routinglists.t prove t/db_dependent/Koha/Patrons.t
Created attachment 74299 [details] [review] Bug 20400: Display new tab in OPAC only for patrons with routing lists The visibility of the routing list tab in the OPAC depends on the system preference RoutingSerials and the existence of routing list entries for the patron. Some libraries only offer routing lists to certain user groups and would not want it generally visible. As there are currently no actions you can perform from the list, this appears to be a reasonable behaviour. See test plan in first patch.
Created attachment 74300 [details] [review] Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt
Created attachment 74402 [details] [review] Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt Patch applies and functions as described. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
(In reply to Dilan Johnpullé from comment #28) > Created attachment 74402 [details] [review] [review] > Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt > > Patch applies and functions as described. > Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Dilan, it seems you only uploaded the last signed patch. You need to do the same with all the patches. You can try $ cd kohaclone $ git fetch $ git checkout origin/master -b qa_20400 $ git bz apply -s 20400 # then $ git bz attach 20400 HEAD~5.. Ask on IRC if you have trouble doing it. Thanks!
Created attachment 74588 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: Show a list of the serial titles a patron is on routing lists for in the OPAC. Test plan applies to the complete patch set: To test: - Apply all patches - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Activate RoutingSerials - Create subscriptions and different routing lists, test with: - Patron with no routing list entries = no tab - Patron with one or more routing list entries = tab appears Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Created attachment 74589 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Created attachment 74590 [details] [review] Bug 20400: Add unit tests prove t/db_dependent/Koha/Subscription/Routinglists.t prove t/db_dependent/Koha/Patrons.t Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Created attachment 74592 [details] [review] Bug 20400: Display new tab in OPAC only for patrons with routing lists The visibility of the routing list tab in the OPAC depends on the system preference RoutingSerials and the existence of routing list entries for the patron. Some libraries only offer routing lists to certain user groups and would not want it generally visible. As there are currently no actions you can perform from the list, this appears to be a reasonable behaviour. See test plan in first patch. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Created attachment 74593 [details] [review] Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt Patch applies and functions as described. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
(In reply to Tomás Cohen Arazi from comment #29) > (In reply to Dilan Johnpullé from comment #28) > > Created attachment 74402 [details] [review] [review] [review] > > Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt > > > > Patch applies and functions as described. > > Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> > > Dilan, it seems you only uploaded the last signed patch. You need to do the > same with all the patches. You can try > > $ cd kohaclone > $ git fetch > $ git checkout origin/master -b qa_20400 > $ git bz apply -s 20400 > # then > $ git bz attach 20400 HEAD~5.. > > Ask on IRC if you have trouble doing it. Thanks! I've gone ahead and added the sign-off lines on the other Patches. Dilan, I hope that's ok for you and thx a lot for testing!
Created attachment 74632 [details] [review] Bug 20400: Add routing list tab in OPAC This patch adds the base for the new feature: Show a list of the serial titles a patron is on routing lists for in the OPAC. Test plan applies to the complete patch set: To test: - Apply all patches - Make sure RoutingSerials is not activated - Check patron account in OPAC - no tab should appear - Activate RoutingSerials - Create subscriptions and different routing lists, test with: - Patron with no routing list entries = no tab - Patron with one or more routing list entries = tab appears Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 74633 [details] [review] Bug 20400: Rewrite using Koha::Objects Adds - Koha::Subscription::Routinglist - Koha::Subscription::Routinglists Adds 2 methods - Koha::Patron::get_routinglists - Koha::Routinglist::subscription Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 74634 [details] [review] Bug 20400: Add unit tests prove t/db_dependent/Koha/Subscription/Routinglists.t prove t/db_dependent/Koha/Patrons.t Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 74635 [details] [review] Bug 20400: Display new tab in OPAC only for patrons with routing lists The visibility of the routing list tab in the OPAC depends on the system preference RoutingSerials and the existence of routing list entries for the patron. Some libraries only offer routing lists to certain user groups and would not want it generally visible. As there are currently no actions you can perform from the list, this appears to be a reasonable behaviour. See test plan in first patch. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 74636 [details] [review] Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt Patch applies and functions as described. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 74637 [details] [review] Bug 20400: (QA follow-up) Redirect to 404 if routing is disabled
One minor comment - not entirely sure about testing for routinglists in get_template_and_user - but we don't want to show the tab for all (since only a minority will have them) but this seems to be the easiest method - noting for RM feedback
Few things: 1. get_routinglists should not be get_routing_lists instead? 2. changes to C4::Auth are not needed, since bug 18403 we have a logged_in_user Koha::Patron sent to the template (from C4::Auth, so we have access to it from everywhere). Yes, it's awesome :) So from the template: [% IF logged_in_user && logged_in_user.get_routing_lists.count %] should do the trick. 3. Koha::Patron->get_routing_lists should return the ->search result directly, to let callers use it in scalar or list context. 4. Koha::Subscription::RoutingList->subscription should use DBIC relationship: return Koha::Subscription->_new_from_dbic($self->_result->subscriptionid); 5. "your routing lists" tab is not marked as "active" when needed (routinglistview vs routinglistsview)
I have pushed a squashed version of these patches by mistake...
Created attachment 74662 [details] [review] 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
Created attachment 74664 [details] [review] 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
Created attachment 74666 [details] [review] 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
Patches pushed to master for 18.05. Thanks Katrin and sorry again for the confusion :) 6 first patches have been pushed squashed (sorry Nick!)
Additional notes (for me): there are some stuffs to remove, the BORROWER_INFO and ->unblessed are not needed. We should rewrite patron-title.inc to take a Koha::Patron and adapt the other templates (looks easy to do).
I'm curious and wanted to see what this new feature looks like but it doesn't work for me (Lewis Caroll was right : it ofter leads to trouble...). I tried with 3 different users with different routing lists configurations (0, 1 and 2 lists) on different browsers (Mozilla, Edge, Chrome) for both my own test instance and Biblibre sandbox#11 (20456 applied), and I always get "Error 500" for my Koha and "Internal Server Error" for Biblibre's. Does someone have an idea of what is going wrong ?
What's the error in the log?
I can't see the error and no way to see it on the sandbox :(