Bug 20400 - Add routing list tab to the patron account in OPAC
Summary: Add routing list tab to the patron account in OPAC
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 20456
  Show dependency treegraph
 
Reported: 2018-03-14 13:54 UTC by Katrin Fischer
Modified: 2019-10-14 19:58 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Adds a routing list tab to the patron account in the OPAC that will be visible if RoutingSerials is turned on and the user is at least on one routing list.
Version(s) released in:


Attachments
Bug 20400: Add routing list tab in OPAC (7.48 KB, patch)
2018-03-15 15:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (7.90 KB, patch)
2018-03-15 15:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add routing list tab in OPAC (7.72 KB, patch)
2018-03-20 23:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (8.16 KB, patch)
2018-03-20 23:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add unit tests (5.21 KB, patch)
2018-03-20 23:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Display new tab in OPAC only for patrons with routing lists (2.67 KB, patch)
2018-03-20 23:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add routing list tab in OPAC (7.74 KB, patch)
2018-03-20 23:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (8.16 KB, patch)
2018-03-20 23:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add unit tests (5.19 KB, patch)
2018-03-20 23:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Display new tab in OPAC only for patrons with routing lists (2.68 KB, patch)
2018-03-20 23:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add routing list tab in OPAC (7.74 KB, patch)
2018-04-17 06:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (8.16 KB, patch)
2018-04-17 06:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add unit tests (5.19 KB, patch)
2018-04-17 06:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Display new tab in OPAC only for patrons with routing lists (2.68 KB, patch)
2018-04-17 06:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt (1.15 KB, patch)
2018-04-17 06:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add routing list tab in OPAC (7.72 KB, patch)
2018-04-17 06:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (8.13 KB, patch)
2018-04-17 06:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add unit tests (5.17 KB, patch)
2018-04-17 06:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Display new tab in OPAC only for patrons with routing lists (2.67 KB, patch)
2018-04-17 06:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt (1.15 KB, patch)
2018-04-17 06:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt (1.33 KB, patch)
2018-04-18 02:14 UTC, Dilan Johnpullé
Details | Diff | Splinter Review
Bug 20400: Add routing list tab in OPAC (7.86 KB, patch)
2018-04-20 06:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (8.27 KB, patch)
2018-04-20 06:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add unit tests (5.31 KB, patch)
2018-04-20 06:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Display new tab in OPAC only for patrons with routing lists (2.81 KB, patch)
2018-04-20 06:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt (1.33 KB, patch)
2018-04-20 06:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: Add routing list tab in OPAC (7.92 KB, patch)
2018-04-20 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20400: Rewrite using Koha::Objects (8.32 KB, patch)
2018-04-20 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20400: Add unit tests (5.36 KB, patch)
2018-04-20 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20400: Display new tab in OPAC only for patrons with routing lists (2.86 KB, patch)
2018-04-20 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt (1.38 KB, patch)
2018-04-20 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20400: (QA follow-up) Redirect to 404 if routing is disabled (836 bytes, patch)
2018-04-20 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20400: (follow-up) Several fixes from RM review (6.31 KB, patch)
2018-04-20 20:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: (follow-up) Several fixes from RM review (6.83 KB, patch)
2018-04-20 20:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20400: (follow-up) Several fixes from RM review (7.80 KB, patch)
2018-04-20 20:57 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-03-14 13:54:17 UTC

    
Comment 1 Katrin Fischer 2018-03-14 14:42:54 UTC
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.
Comment 2 Katrin Fischer 2018-03-15 15:34:51 UTC
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
Comment 3 Katrin Fischer 2018-03-15 15:34:54 UTC
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
Comment 4 Katrin Fischer 2018-03-15 15:35:26 UTC
Not ready for sign-off yet!
Comment 5 Katrin Fischer 2018-03-20 23:31:38 UTC
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
Comment 6 Katrin Fischer 2018-03-20 23:31:41 UTC
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
Comment 7 Katrin Fischer 2018-03-20 23:31:45 UTC
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
Comment 8 Katrin Fischer 2018-03-20 23:31:49 UTC
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.
Comment 9 Katrin Fischer 2018-03-20 23:47:52 UTC
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
Comment 10 Katrin Fischer 2018-03-20 23:47:56 UTC
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
Comment 11 Katrin Fischer 2018-03-20 23:47:59 UTC
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
Comment 12 Katrin Fischer 2018-03-20 23:48:03 UTC
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 13 paxed 2018-03-21 07:09:52 UTC
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 &rsaquo; 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
Comment 14 Katrin Fischer 2018-03-21 08:30:03 UTC
(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 &rsaquo; 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 15 Tomás Cohen Arazi 2018-03-23 13:46:56 UTC
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' });
Comment 16 Séverine Queune 2018-03-23 21:48:11 UTC
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
Comment 17 Katrin Fischer 2018-04-17 06:18:47 UTC
(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?
Comment 18 Katrin Fischer 2018-04-17 06:33:03 UTC
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
Comment 19 Katrin Fischer 2018-04-17 06:33:07 UTC
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
Comment 20 Katrin Fischer 2018-04-17 06:33:10 UTC
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
Comment 21 Katrin Fischer 2018-04-17 06:33:14 UTC
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.
Comment 22 Katrin Fischer 2018-04-17 06:33:18 UTC
Created attachment 74295 [details] [review]
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt
Comment 23 Katrin Fischer 2018-04-17 06:36:21 UTC
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
Comment 24 Katrin Fischer 2018-04-17 06:36:24 UTC
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
Comment 25 Katrin Fischer 2018-04-17 06:36:28 UTC
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
Comment 26 Katrin Fischer 2018-04-17 06:36:31 UTC
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.
Comment 27 Katrin Fischer 2018-04-17 06:36:35 UTC
Created attachment 74300 [details] [review]
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt
Comment 28 Dilan Johnpullé 2018-04-18 02:14:04 UTC
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>
Comment 29 Tomás Cohen Arazi 2018-04-18 18:59:55 UTC
(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!
Comment 30 Katrin Fischer 2018-04-20 06:28:37 UTC
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>
Comment 31 Katrin Fischer 2018-04-20 06:28:40 UTC
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>
Comment 32 Katrin Fischer 2018-04-20 06:28:44 UTC
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>
Comment 33 Katrin Fischer 2018-04-20 06:28:48 UTC
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>
Comment 34 Katrin Fischer 2018-04-20 06:28:52 UTC
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>
Comment 35 Katrin Fischer 2018-04-20 06:29:58 UTC
(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!
Comment 36 Nick Clemens 2018-04-20 12:58:24 UTC
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>
Comment 37 Nick Clemens 2018-04-20 12:58:28 UTC
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>
Comment 38 Nick Clemens 2018-04-20 12:58:31 UTC
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>
Comment 39 Nick Clemens 2018-04-20 12:58:34 UTC
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>
Comment 40 Nick Clemens 2018-04-20 12:58:37 UTC
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>
Comment 41 Nick Clemens 2018-04-20 12:58:40 UTC
Created attachment 74637 [details] [review]
Bug 20400: (QA follow-up) Redirect to 404 if routing is disabled
Comment 42 Nick Clemens 2018-04-20 13:00:43 UTC
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
Comment 43 Jonathan Druart 2018-04-20 16:34:08 UTC
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)
Comment 44 Jonathan Druart 2018-04-20 16:48:15 UTC
I have pushed a squashed version of these patches by mistake...
Comment 45 Katrin Fischer 2018-04-20 20:27:35 UTC
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
Comment 46 Katrin Fischer 2018-04-20 20:33:45 UTC
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
Comment 47 Katrin Fischer 2018-04-20 20:57:34 UTC
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
Comment 48 Jonathan Druart 2018-04-20 21:05:44 UTC
Patches pushed to master for 18.05.

Thanks Katrin and sorry again for the confusion :)

6 first patches have been pushed squashed (sorry Nick!)
Comment 49 Jonathan Druart 2018-04-20 21:08:04 UTC
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).
Comment 50 Séverine Queune 2018-04-21 11:27:52 UTC
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 ?
Comment 51 Jonathan Druart 2018-04-21 14:08:57 UTC
What's the error in the log?
Comment 52 Katrin Fischer 2018-04-21 15:47:12 UTC
I can't see the error and no way to see it on the sandbox :(