|
Description
Kyle M Hall (khall)
2020-10-08 15:13:57 UTC
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 Hi Kyle, I have a QA script fail here: FAIL api/v1/swagger/paths/transfer_limits.json FAIL git manipulation The file has been added and deleted in the same patchset FAIL api/v1/swagger/paths/transfer_limits.yaml OK git manipulation FAIL yaml_valid YAML Error: Invalid element in map Can you please check? Can you please also fix the last 2 commit messages subjects while you are on it? Bug xxxx: I would have done it, but need your help with the other fail here :) * Commit title does not start with 'Bug XXXXX: ' - a6713283b5 * Commit title does not start with 'Bug XXXXX: ' - 20f1ef7e08 The first fail can be ignored (obv.) Created attachment 116612 [details] [review] Bug 26633: (QA follow-up) Add error handling for loading transfer limits table Created attachment 116613 [details] [review] Bug 26633: Add advanced editor link on admin home page (In reply to Katrin Fischer from comment #46) > Hi Kyle, > > I have a QA script fail here: > > FAIL api/v1/swagger/paths/transfer_limits.json > FAIL git manipulation > The file has been added and deleted in the same patchset > > FAIL api/v1/swagger/paths/transfer_limits.yaml > OK git manipulation > FAIL yaml_valid > YAML Error: Invalid element in map > > > Can you please check? > > Can you please also fix the last 2 commit messages subjects while you are on > it? Bug xxxx: I would have done it, but need your help with the other fail > here :) > > * Commit title does not start with 'Bug XXXXX: ' - a6713283b5 > * Commit title does not start with 'Bug XXXXX: ' - 20f1ef7e08 I've checked and the YAML is valid, so I think those are false flags from the QA script! How did you check, Kyle? The script is only returning the error from YAML::LoadFile YAML Error: Can't parse double quoted string Code: YAML_PARSE_ERR_BAD_DOUBLE Line: 149 Document: 1 at /usr/share/perl5/YAML/Loader.pm line 574. (In reply to Jonathan Druart from comment #51) > How did you check, Kyle? > > The script is only returning the error from YAML::LoadFile > > YAML Error: Can't parse double quoted string > Code: YAML_PARSE_ERR_BAD_DOUBLE > Line: 149 > Document: 1 > at /usr/share/perl5/YAML/Loader.pm line 574. I used yamllint. Looks like I'll have to do some more digging! (In reply to Kyle M Hall from comment #52) > (In reply to Jonathan Druart from comment #51) > > How did you check, Kyle? > > > > The script is only returning the error from YAML::LoadFile > > > > YAML Error: Can't parse double quoted string > > Code: YAML_PARSE_ERR_BAD_DOUBLE > > Line: 149 > > Document: 1 > > at /usr/share/perl5/YAML/Loader.pm line 574. > > I used yamllint. Looks like I'll have to do some more digging! So the problem is the YAML module is insufficient for running this test. I would recommend we switch to YAML::XS. The current YAML module is being deprecated anyway and is being move to YAML::Old. root@kohadevbox:koha(bug26633-qa)$ perl -e "use YAML qw(LoadFile); LoadFile('api/v1/swagger/paths/transfer_limits.yaml');" YAML Error: Invalid element in map Code: YAML_LOAD_ERR_BAD_MAP_ELEMENT Line: 8 Document: 1 at /usr/share/perl5/YAML/Loader.pm line 350. root@kohadevbox:koha(bug26633-qa)$ perl -e "use YAML::XS qw(LoadFile); LoadFile('api/v1/swagger/paths/transfer_limits.yaml');" root@kohadevbox:koha(bug26633-qa)$ Basically, YAML only supports YAML 1.0, where YAML::XS ( and Mojolicious::Plugin::OpenAPI ) support YAML 1.1. I would recommend as a first step that QohA::File::YAML be updated to use YAML::XS as a first step. I'm not sure where we handle change requests to the qa test tools, so I've created a pull request on GitHub: https://github.com/Koha-Community/qa-test-tools/pull/2 (In reply to Kyle M Hall from comment #54) > I'm not sure where we handle change requests to the qa test tools, so I've > created a pull request on GitHub: > https://github.com/Koha-Community/qa-test-tools/pull/2 It's https://gitlab.com/koha-community/qa-test-tools IMO we should first switch to YAML::XS in Koha code before adding it to the QA script. Reran the QA tools with the dependency applied, but still get: FAIL api/v1/swagger/paths/transfer_limits.yaml OK git manipulation FAIL yaml_valid YAML Error: Invalid element in map Do I need to install something new or do we need an update to the QA test tools? The changes from this merge request must be applied to the qa-test-tools repo: https://gitlab.com/koha-community/qa-test-tools/-/merge_requests/35 Created attachment 116952 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116953 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116954 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116955 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116956 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116957 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116958 [details] [review] Bug 26633: POD for the exceptions file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116959 [details] [review] Bug 26633: Remove jquery.checkboxes.min.js include No longer exists and not used in this template Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116960 [details] [review] Bug 26633: Convert transfer_limit.json to YAML Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116961 [details] [review] Bug 26633: Add API documentation Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116962 [details] [review] Bug 26633: Add ability to bookmark the page you are viewing Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116963 [details] [review] Bug 26633: (QA follow-up) Add error handling for loading transfer limits table Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 116964 [details] [review] Bug 26633: Add advanced editor link on admin home page Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x I am working with a client that has 447 branches and 25 itemtypes. Most transfers between branches are not allowed, so there are almost 5 million records in the branch_transfer_limits table, 199342 records by itemtype. The advanced editor runs out of memory in Chrome, producing errors that range from freezing the computer to crashing the tab or the browser. In Firefox, it starts loading the page but it doesn't end loading. This is without selecting the itemtype, so it's only building the default view to hide it. In the simple editor, I made changes in their installation of Koha so that instead of checking every combination with a SQL sentence (SELECT * FROM branch_transfer_limits WHERE toBranch = ? AND fromBranch = ? AND $limitType = ?), it checks all the limits for the branch with only one SQL sentence (SELECT * FROM branch_transfer_limits WHERE fromBranch = ?) and stores the result in a hash. This way the simple editor works properly for big networks like this one. I haven't found a way to apply a similar improvement in the performance of the advanced editor. |