Currently, deleting a patron will create a missing value in the ranking of members in a routing list. For instance, the ranking will go 1, 2, 3, 5 in the database when the patron ranked at #4 is deleted. This causes the intranet to display these rankings as 1, 2, 3, 1, since 5 is not a valid option due to there only being 4 entries in the database. I've written up a little re-order function based on the reorder_members function, which just re-orders all the members for that routing list every time routing.pl or routing-preview.pl are called. It might be more ideal to only run it if errors are detected, but that error detection would require more code that might just add more bulk.
Actually, I'm not currently running it for routing-preview.pl, since there are no numbers displayed. (In reply to comment #0) > Currently, deleting a patron will create a missing value in the ranking of > members in a routing list. For instance, the ranking will go 1, 2, 3, 5 in > the database when the patron ranked at #4 is deleted. > > This causes the intranet to display these rankings as 1, 2, 3, 1, since 5 is > not a valid option due to there only being 4 entries in the database. > > I've written up a little re-order function based on the reorder_members > function, which just re-orders all the members for that routing list every > time routing.pl or routing-preview.pl are called. > > It might be more ideal to only run it if errors are detected, but that error > detection would require more code that might just add more bulk.
Created attachment 10342 [details] [review] Force re-order of routing list rankings on page load
This works well, and mimics the behaviour that happens with the reserves that are set if you have system preference set to create reserves for routing lists.
The only problem is that it is called unconditionally on every load of the page, even if we just reranked the list by deleting a member, or changing the rank. For that reason i'm hesitant to sign off, does anyone else have a better idea on how to solve this? I'm thinking fixing the borrower delete to not delete borrowers who are in a routing list might be better.
(In reply to comment #4) > The only problem is that it is called unconditionally on every load of the > page, even if we just reranked the list by deleting a member, or changing > the rank. > > For that reason i'm hesitant to sign off, does anyone else have a better > idea on how to solve this? I'm thinking fixing the borrower delete to not > delete borrowers who are in a routing list might be better. I agree with the problem, but I think it's still a good idea to be able to delete borrowers who are in routing lists. I think it would be a bit too finicky to have to go into each routing list and remove them before doing a complete delete. Why don't we call the new reorder function on patron delete? I see that there is already a function for handling patron deletes in regards to Lists/VirtualShelves. That probably makes the most sense.
(In reply to David Cook from comment #5) > Why don't we call the new reorder function on patron delete? I see that > there is already a function for handling patron deletes in regards to > Lists/VirtualShelves. That probably makes the most sense. I prefer this option.
Created attachment 90393 [details] [review] Bug 8260: user rank order instead of absolute rank to set dropdowns on routing.pl When deleting a borrower on a routing list we leave a gap in the numbering. It appears to only be a problem when viewing or editing the list in the staff client. Rather than use the rank in the DB to set the values, we use the implicit order To test: 1 - Create a subscription (or find one) 2 - Add a routing list 3 - Add five patrons 4 - Delete the patron who is third in the list 5 - Edit the routing list 6 - Note the priority for the last patron is 1 7 - Apply patch 8 - Reload the page 9 - Values look correct 10 - Confirm reordering works as expected
I think I like the idea of renumbering in the database a bit better, as it will also fix things for any reports and similar. And it always seems a little odd to display something different than what to is stored in the db. That said, I guess David's patch would have to be reworked for the Koha namespace?
(In reply to Katrin Fischer from comment #8) > I think I like the idea of renumbering in the database a bit better, as it > will also fix things for any reports and similar. And it always seems a > little odd to display something different than what to is stored in the db. > That said, I guess David's patch would have to be reworked for the Koha > namespace? Another issue: Printing routing lists. I think Nick's patch only fixes the view, not the print.
(In reply to Katrin Fischer from comment #9) > (In reply to Katrin Fischer from comment #8) > > That said, I guess David's patch would have to be reworked for the Koha > > namespace? > > Another issue: Printing routing lists. I think Nick's patch only fixes the > view, not the print. Looks like I wrote my patch back in 2012 and I think I had it applied locally in 3.8, but didn't bother to apply it locally after that. Seems that I haven't had any complaints about it since 2012 either, so not really a priority for me at the moment.
(In reply to Katrin Fischer from comment #9) > (In reply to Katrin Fischer from comment #8) > > I think I like the idea of renumbering in the database a bit better, as it > > will also fix things for any reports and similar. And it always seems a > > little odd to display something different than what to is stored in the db. > > That said, I guess David's patch would have to be reworked for the Koha > > namespace? > > Another issue: Printing routing lists. I think Nick's patch only fixes the > view, not the print. The print doesn't include the numbers, and the ordering will still be correct In reports too, the numbering will still be ordered correctly, it just may have a gap. happy to have someone supply an alternate
Created attachment 94215 [details] [review] Bug 8260: user rank order instead of absolute rank to set dropdowns on routing.pl When deleting a borrower on a routing list we leave a gap in the numbering. It appears to only be a problem when viewing or editing the list in the staff client. Rather than use the rank in the DB to set the values, we use the implicit order To test: 1 - Create a subscription (or find one) 2 - Add a routing list 3 - Add five patrons 4 - Delete the patron who is third in the list 5 - Edit the routing list 6 - Note the priority for the last patron is 1 7 - Apply patch 8 - Reload the page 9 - Values look correct 10 - Confirm reordering works as expected Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to Nick Clemens from comment #11) > (In reply to Katrin Fischer from comment #9) > > (In reply to Katrin Fischer from comment #8) > > > I think I like the idea of renumbering in the database a bit better, as it > > > will also fix things for any reports and similar. And it always seems a > > > little odd to display something different than what to is stored in the db. > > > That said, I guess David's patch would have to be reworked for the Koha > > > namespace? > > > > Another issue: Printing routing lists. I think Nick's patch only fixes the > > view, not the print. > > The print doesn't include the numbers, and the ordering will still be correct > > In reports too, the numbering will still be ordered correctly, it just may > have a gap. > > happy to have someone supply an alternate You got a point there. If we are sure it doesn't cause issues with reordering or similar, I am ok.
Created attachment 94226 [details] [review] Bug 8260: user rank order instead of absolute rank to set dropdowns on routing.pl When deleting a borrower on a routing list we leave a gap in the numbering. It appears to only be a problem when viewing or editing the list in the staff client. Rather than use the rank in the DB to set the values, we use the implicit order To test: 1 - Create a subscription (or find one) 2 - Add a routing list 3 - Add five patrons 4 - Delete the patron who is third in the list 5 - Edit the routing list 6 - Note the priority for the last patron is 1 7 - Apply patch 8 - Reload the page 9 - Values look correct 10 - Confirm reordering works as expected Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Nice work! Pushed to master for 19.11.00