The current transfer limits editor works great for Koha instances with small numbers of branches. However, for consortiums with dozens or even hundreds of libraries, the editor does not work well or effectively. We should provide an "advanced" editor displays all to/from library combinations in a grid and allows them to be edited in a manner somewhat similar to the transport cost matrix editor.
Created attachment 111380 [details] [review] Bug 26633: Add REST API for managing transfer limits Test Plan: 1) prove t/db_dependent/api/v1/transfer_limits.t
Created attachment 111381 [details] [review] Bug 26633: Add advanced editor for transfer limits The current transfer limits editor works great for Koha instances with small numbers of branches. However, for consortiums with dozens or even hundreds of libraries, the editor does not work well or effectively. We should provide an "advanced" editor displays all to/from library combinations in a grid and allows them to be edited in a manner somewhat similar to the transport cost matrix editor. Test Plan: 1) Apply this patch 2) Browse to the transfer limits editor 3) Click the new "Switch to advanced editor" link 4) Select a collection code/item type to edit limits for 5) Test the "Check all" function 6) Test the "Uncheck all" function 7) Test the "Check" column function 8) Test the "Uncheck" column function 9) Test the "Check" row function 10) Test the "Uncheck" row function 11) Test individual checkboxes/table cells
Created attachment 112327 [details] [review] Bug 26633: Add REST API for managing transfer limits Test Plan: 1) prove t/db_dependent/api/v1/transfer_limits.t Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Created attachment 112328 [details] [review] Bug 26633: Add REST API for managing transfer limits Test Plan: 1) prove t/db_dependent/api/v1/transfer_limits.t Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Created attachment 112330 [details] [review] Bug 26633: Add advanced editor for transfer limits The current transfer limits editor works great for Koha instances with small numbers of branches. However, for consortiums with dozens or even hundreds of libraries, the editor does not work well or effectively. We should provide an "advanced" editor displays all to/from library combinations in a grid and allows them to be edited in a manner somewhat similar to the transport cost matrix editor. Test Plan: 1) Apply this patch 2) Browse to the transfer limits editor 3) Click the new "Switch to advanced editor" link 4) Select a collection code/item type to edit limits for 5) Test the "Check all" function 6) Test the "Uncheck all" function 7) Test the "Check" column function 8) Test the "Uncheck" column function 9) Test the "Check" row function 10) Test the "Uncheck" row function 11) Test individual checkboxes/table cells Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Worked great, our consortium will be happy to have this, especially if we have to close again like we did last spring.
Created attachment 112437 [details] [review] Bug 26633: Fix indirect object notation
Created attachment 112438 [details] [review] Bug 26633: (QA follow-up) Change label to "collection" as we display the description
Hi Kyle, I have posted 2 small follow-ups, but then got stuck on an issue: Whenever you click any of the buttons ("Uncheck all" , "Check all") a second time, all the buttons get greyed out and no other action can be performed until the page is reloaded.
Created attachment 112587 [details] [review] Bug 26633: (QA follow-up) Action buttons should not remain grayed out if the same action is used multiple times in a row
Created attachment 112778 [details] [review] Bug 26633: Add REST API for managing transfer limits Test Plan: 1) prove t/db_dependent/api/v1/transfer_limits.t Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 112779 [details] [review] Bug 26633: Add advanced editor for transfer limits The current transfer limits editor works great for Koha instances with small numbers of branches. However, for consortiums with dozens or even hundreds of libraries, the editor does not work well or effectively. We should provide an "advanced" editor displays all to/from library combinations in a grid and allows them to be edited in a manner somewhat similar to the transport cost matrix editor. Test Plan: 1) Apply this patch 2) Browse to the transfer limits editor 3) Click the new "Switch to advanced editor" link 4) Select a collection code/item type to edit limits for 5) Test the "Check all" function 6) Test the "Uncheck all" function 7) Test the "Check" column function 8) Test the "Uncheck" column function 9) Test the "Check" row function 10) Test the "Uncheck" row function 11) Test individual checkboxes/table cells Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 112780 [details] [review] Bug 26633: Fix indirect object notation Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 112781 [details] [review] Bug 26633: (QA follow-up) Change label to "collection" as we display the description Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 112782 [details] [review] Bug 26633: (QA follow-up) Action buttons should not remain grayed out if the same action is used multiple times in a row Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Do we need any documentation for the API routes here?
Tomas, can I get your stamp on this one please?
(In reply to Jonathan Druart from comment #17) > Tomas, can I get your stamp on this one please? I've been tracking this dev closely with Kyle and he responded to my suggestions, on the API part of it. I will review it altogether now.
Created attachment 112923 [details] [review] Bug 26633: Add REST API for managing transfer limits Test Plan: 1) prove t/db_dependent/api/v1/transfer_limits.t Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 112924 [details] [review] Bug 26633: Add advanced editor for transfer limits The current transfer limits editor works great for Koha instances with small numbers of branches. However, for consortiums with dozens or even hundreds of libraries, the editor does not work well or effectively. We should provide an "advanced" editor displays all to/from library combinations in a grid and allows them to be edited in a manner somewhat similar to the transport cost matrix editor. Test Plan: 1) Apply this patch 2) Browse to the transfer limits editor 3) Click the new "Switch to advanced editor" link 4) Select a collection code/item type to edit limits for 5) Test the "Check all" function 6) Test the "Uncheck all" function 7) Test the "Check" column function 8) Test the "Uncheck" column function 9) Test the "Check" row function 10) Test the "Uncheck" row function 11) Test individual checkboxes/table cells Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 112925 [details] [review] Bug 26633: Fix indirect object notation Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 112926 [details] [review] Bug 26633: (QA follow-up) Change label to "collection" as we display the description Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 112927 [details] [review] Bug 26633: (QA follow-up) Action buttons should not remain grayed out if the same action is used multiple times in a row Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 112928 [details] [review] Bug 26633: (QA follow-up) Tests on error condition This patch highlights a problem with the try/catch conditions in the ad() controlled method. Tests are added, and the controller is fixed to reflect the right behaviour. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/transfer_limits.t => SUCCESS: Tests pass! 409 is returned as appropriate! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 112929 [details] [review] Bug 26633: POD for the exceptions file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Jonathan Druart from comment #17) > Tomas, can I get your stamp on this one please? Yes, we need them, Kyle?
Created attachment 113053 [details] [review] Bug 26633: Remove jquery.checkboxes.min.js include No longer exists and not used in this template
1. Please provide documentation for the API 2. There is no error handling when the page is loaded. I first tried with D11 (which has the REST API broken) and the "loading..." was running forever, the table was loaded but without any checkboxes. Maybe we should not display anything in that case. 3. I am a bit concern that there is no "save" or "undo" button. If you click "Check all" or "Uncheck all", you may have lost your work! It's not blocker but worth noting it here. 4. There is no way to bookmark or share the URL to display a given table. That would have been useful. Is that something we could implement easily? 5. (In reply to Kyle M Hall from comment #0) > The current transfer limits editor works great for Koha instances with small > numbers of branches. However, for consortiums with dozens or even hundreds > of libraries, the editor does not work well or effectively. I've added around 30 libraries and it creates a huge table. I am not sure we are answering the need as the table is not readable easily if you are starting to scroll. Maybe it would have been useful to have "floating th"'s, which are always displayed when you scroll. Not blocker however, but feel free to submit if you think it would be useful.
(In reply to Jonathan Druart from comment #28) > 1. Please provide documentation for the API I'll see what I can do! > 2. There is no error handling when the page is loaded. I first tried with > D11 (which has the REST API broken) and the "loading..." was running > forever, the table was loaded but without any checkboxes. > Maybe we should not display anything in that case. I'm not sure why we'd do this here, when we don't do it for any other ajax table in Koha. > 4. There is no way to bookmark or share the URL to display a given table. > That would have been useful. Is that something we could implement easily? I'll see what I can do! > 5. > (In reply to Kyle M Hall from comment #0) > > The current transfer limits editor works great for Koha instances with small > > numbers of branches. However, for consortiums with dozens or even hundreds > > of libraries, the editor does not work well or effectively. > > I've added around 30 libraries and it creates a huge table. I am not sure we > are answering the need as the table is not readable easily if you are > starting to scroll. Maybe it would have been useful to have "floating th"'s, > which are always displayed when you scroll. > Not blocker however, but feel free to submit if you think it would be useful. I really like that idea! I'll give it a shot but I might need some help from Lucas or Owen ;)
(In reply to Kyle M Hall from comment #29) > (In reply to Jonathan Druart from comment #28) > > 2. There is no error handling when the page is loaded. I first tried with > > D11 (which has the REST API broken) and the "loading..." was running > > forever, the table was loaded but without any checkboxes. > > Maybe we should not display anything in that case. > > I'm not sure why we'd do this here, when we don't do it for any other ajax > table in Koha. I think the other tables (with the checkout list in mind) will raise a DataTable alert box if the svc script/REST API route does not return something valid (or 500). So we have a the interface telling us something went wrong.
Created attachment 113265 [details] [review] Bug 26633: Add REST API for managing transfer limits Test Plan: 1) prove t/db_dependent/api/v1/transfer_limits.t Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113266 [details] [review] Bug 26633: Add advanced editor for transfer limits The current transfer limits editor works great for Koha instances with small numbers of branches. However, for consortiums with dozens or even hundreds of libraries, the editor does not work well or effectively. We should provide an "advanced" editor displays all to/from library combinations in a grid and allows them to be edited in a manner somewhat similar to the transport cost matrix editor. Test Plan: 1) Apply this patch 2) Browse to the transfer limits editor 3) Click the new "Switch to advanced editor" link 4) Select a collection code/item type to edit limits for 5) Test the "Check all" function 6) Test the "Uncheck all" function 7) Test the "Check" column function 8) Test the "Uncheck" column function 9) Test the "Check" row function 10) Test the "Uncheck" row function 11) Test individual checkboxes/table cells Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113267 [details] [review] Bug 26633: Fix indirect object notation Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113268 [details] [review] Bug 26633: (QA follow-up) Change label to "collection" as we display the description Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113269 [details] [review] Bug 26633: (QA follow-up) Action buttons should not remain grayed out if the same action is used multiple times in a row Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113270 [details] [review] Bug 26633: (QA follow-up) Tests on error condition This patch highlights a problem with the try/catch conditions in the ad() controlled method. Tests are added, and the controller is fixed to reflect the right behaviour. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/transfer_limits.t => SUCCESS: Tests pass! 409 is returned as appropriate! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113271 [details] [review] Bug 26633: POD for the exceptions file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113272 [details] [review] Bug 26633: Remove jquery.checkboxes.min.js include No longer exists and not used in this template
Created attachment 113273 [details] [review] Bug 26633: Convert transfer_limit.json to YAML
Created attachment 113274 [details] [review] Bug 26633: Add API documentation
Created attachment 113275 [details] [review] Bug 26633: Add ability to bookmark the page you are viewing
(In reply to Jonathan Druart from comment #30) > (In reply to Kyle M Hall from comment #29) > > (In reply to Jonathan Druart from comment #28) > > > 2. There is no error handling when the page is loaded. I first tried with > > > D11 (which has the REST API broken) and the "loading..." was running > > > forever, the table was loaded but without any checkboxes. > > > Maybe we should not display anything in that case. > > > > I'm not sure why we'd do this here, when we don't do it for any other ajax > > table in Koha. > > I think the other tables (with the checkout list in mind) will raise a > DataTable alert box if the svc script/REST API route does not return > something valid (or 500). > So we have a the interface telling us something went wrong. Ah, I see. I'll see what I can do about that!
Created attachment 113676 [details] [review] Bug 26633 (QA follow-up) Add error handling for loading transfer limits table
Created attachment 113677 [details] [review] Bug 26633 (QA follow-up) Add error handling for loading transfer limits table
Created attachment 113961 [details] [review] Bug 26633 - Add advanced editor link on admin home page