C4::Serials::GetSubscriptionsFromBorrower is only used for the display of the routing list tab in staff. Bug 20400 implements this for the OPAC using Koha::Object. After switching staff the routine can be safely removed.
Created attachment 73119 [details] [review] Bug 20456: Switch routling list tab in staff to use Koha::Object Bug 20400 added a routing list tab to the patron account in the OPAC using Koha::Object. This patch switches the routing list tab in the patron account in intranet over to the new code. It also adds an additional column showing the position of the patron on the routing list and fixes the search. To test: - Create some subscriptions with routing lists - Take a look at the patron accounts of several patrons having - no entries on routing lists - 1 entry on a routing list - entries on several routing lists - Make sure the display works correctly. - Search for a subscription and make sure search works.
Created attachment 73120 [details] [review] Bug 20456: Remove the C4::Serials::GetSubscriptionsFromBorrower Bug 20400 added a routing list tab to the patron account in the OPAC using Koha::Object. After changing the staff view too, we can now safely remove GetSubscriptionsFromBorrower as it is no longer used. To test: - Make sure that GetSubscriptionsFromBorrower is no longer used.
I assume this is a false positive as removing a method should reduce POD :) FAIL C4/Serials.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK spelling OK valid
Created attachment 74667 [details] [review] Bug 20456: Switch routling list tab in staff to use Koha::Object Bug 20400 added a routing list tab to the patron account in the OPAC using Koha::Object. This patch switches the routing list tab in the patron account in intranet over to the new code. It also adds an additional column showing the position of the patron on the routing list and fixes the search. To test: - Create some subscriptions with routing lists - Take a look at the patron accounts of several patrons having - no entries on routing lists - 1 entry on a routing list - entries on several routing lists - Make sure the display works correctly. - Search for a subscription and make sure search works.
Created attachment 74668 [details] [review] Bug 20456: Remove the C4::Serials::GetSubscriptionsFromBorrower Bug 20400 added a routing list tab to the patron account in the OPAC using Koha::Object. After changing the staff view too, we can now safely remove GetSubscriptionsFromBorrower as it is no longer used. To test: - Make sure that GetSubscriptionsFromBorrower is no longer used.
No problem to use routing lists, but several display issues an patron's account : - the number of subscritpion lists always display "0", whatever the number of lists visible is - the position priority is always 1, even if the patron is not the first on list - routing lists are displayed even on patron's account that are not part of them Probably should not write that point here, but "Your routing list" tab in the OPAC always leads to an "Internal server error" (bug 20400).
Created attachment 74675 [details] [review] Bug 20456: (follow-up) Fix number of subscriptions and position display TT variables needed to be adjusted because of changes from bug 20400. Also: while we want logged_in_user in OPAC, we don't want that in staff. 1) Check the right number of subscriptions is displayed on top of the table 2) check the right position number is display for the routing list
(In reply to Séverine Queune from comment #6) > - the number of subscription lists always display "0", whatever the number > of lists visible is Thx for catching that - fixed! > - the position priority is always 1, even if the patron is not the first > on list Fixed! > - routing lists are displayed even on patron's account that are not part > of them The tab in staff is only dependent on the pref, not on the existence. I didn't touch that logic, could be a separate bug? But as we have the search option in staff, maybe we want to keep it? > Probably should not write that point here, but "Your routing list" tab in > the OPAC always leads to an "Internal server error" (bug 20400). This is a hard one. I can see it in the sandbox, but I can't replicate it on my installation with the same conditions (patron with one routing list, being first position). Better to move that to bug 20400.
Created attachment 74683 [details] [review] Bug 20456: Switch routling list tab in staff to use Koha::Object Bug 20400 added a routing list tab to the patron account in the OPAC using Koha::Object. This patch switches the routing list tab in the patron account in intranet over to the new code. It also adds an additional column showing the position of the patron on the routing list and fixes the search. To test: - Create some subscriptions with routing lists - Take a look at the patron accounts of several patrons having - no entries on routing lists - 1 entry on a routing list - entries on several routing lists - Make sure the display works correctly. - Search for a subscription and make sure search works. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74684 [details] [review] Bug 20456: Remove the C4::Serials::GetSubscriptionsFromBorrower Bug 20400 added a routing list tab to the patron account in the OPAC using Koha::Object. After changing the staff view too, we can now safely remove GetSubscriptionsFromBorrower as it is no longer used. To test: - Make sure that GetSubscriptionsFromBorrower is no longer used. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74685 [details] [review] Bug 20456: (follow-up) Fix number of subscriptions and position display TT variables needed to be adjusted because of changes from bug 20400. Also: while we want logged_in_user in OPAC, we don't want that in staff. 1) Check the right number of subscriptions is displayed on top of the table 2) check the right position number is display for the routing list Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
No more count problems ! (In reply to Katrin Fischer from comment #8) > (In reply to Séverine Queune from comment #6) > > > - routing lists are displayed even on patron's account that are not part > > of them > > The tab in staff is only dependent on the pref, not on the existence. I > didn't touch that logic, could be a separate bug? But as we have the search > option in staff, maybe we want to keep it? > I didn't notice that before, as I only use routing list for tests and not in production. Behaviour is now different, only the routing lists patron is on are now displayed (empty list for patron that have no routing list). If that changes the original behaviour, maybe I shouldn't have signed it...
All is ok for me. Passed QA
String changes, skipping for 18.05
Awesome work all! Pushed to master for 18.11.x
Pushed to 18.05.x for 18.05.02 Thanks guys, I've made the decision to port this one back as it only just missed the 18.05.00 release due to string changes and doesn't fundamentally change functionality but rather tidies up code.
Depends on Bug 20400 not in 17.11.x