Summary: | "Get all requests" API call fired when loading any ILL page | ||
---|---|---|---|
Product: | Koha | Reporter: | Andrew Isherwood <bugzilla> |
Component: | ILL | Assignee: | Andrew Isherwood <bugzilla> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | major | ||
Priority: | P5 - low | CC: | fridolin.somers, josef.moravec, katrin.fischer, lucas, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
19.11.00,19.05.03,18.11.09
|
|
Bug Depends on: | |||
Bug Blocks: | 23529 | ||
Attachments: |
Bug 23229: Only load ILL table JS when needed
Bug 23229: Only load ILL table JS when needed Bug 23229: (QA follow-up) Fix indentation Bug 23229: Only load ILL table JS when needed Bug 23229: (QA follow-up) Fix indentation Bug 23229: (follow-up) Narrow scope of changes Bug 23229: Only load ILL table JS when needed Bug 23229: (QA follow-up) Fix indentation Bug 23229: (follow-up) Narrow scope of changes |
Description
Andrew Isherwood
2019-06-28 09:20:09 UTC
Created attachment 91062 [details] [review] Bug 23229: Only load ILL table JS when needed This patch prevents the JS associated with the ILL table from loading unless we're actually displaying the table. This has the knock on effect of not causing the "get all ILL requests" API request to fire when loading any ILL page. Test plan: - Go to "ILL requests" page - TEST: Observe that the table loads all ILL requests - Go to the "New ILL request" page for any installed backend - TEST: Check the browser dev tools "Network" panel, observe that the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias,comments,requested_partners request is not made. Created attachment 91096 [details] [review] Bug 23229: Only load ILL table JS when needed This patch prevents the JS associated with the ILL table from loading unless we're actually displaying the table. This has the knock on effect of not causing the "get all ILL requests" API request to fire when loading any ILL page. Test plan: - Go to "ILL requests" page - TEST: Observe that the table loads all ILL requests - Go to the "New ILL request" page for any installed backend - TEST: Check the browser dev tools "Network" panel, observe that the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias,comments,requested_partners request is not made. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 91097 [details] [review] Bug 23229: (QA follow-up) Fix indentation Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 91769 [details] [review] Bug 23229: Only load ILL table JS when needed This patch prevents the JS associated with the ILL table from loading unless we're actually displaying the table. This has the knock on effect of not causing the "get all ILL requests" API request to fire when loading any ILL page. Test plan: - Go to "ILL requests" page - TEST: Observe that the table loads all ILL requests - Go to the "New ILL request" page for any installed backend - TEST: Check the browser dev tools "Network" panel, observe that the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias,comments,requested_partners request is not made. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 91770 [details] [review] Bug 23229: (QA follow-up) Fix indentation Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.03 backported to 18.11.x for 18.11.09 Created attachment 92491 [details] [review] Bug 23229: (follow-up) Narrow scope of changes Sorry guys! Whilst working on something else, it came to light that the previous fix in this bug was waaaaay too heavy handed and basically kills all ILL related JS :( This is obviously a big problem, this patch fixes that. The main problem is that all the ILL related JS is in a single file, which isn't ideal as bits of it are used on different pages. Ideally, we'd refactor it out into more manageable chunks which can be included as required. Until such a time, this patch will do, it just prevents the "get all requests" AJAX request unless you're on the ILL list page. Created attachment 92492 [details] [review] Bug 23229: Only load ILL table JS when needed This patch prevents the JS associated with the ILL table from loading unless we're actually displaying the table. This has the knock on effect of not causing the "get all ILL requests" API request to fire when loading any ILL page. Test plan: - Go to "ILL requests" page - TEST: Observe that the table loads all ILL requests - Go to the "New ILL request" page for any installed backend - TEST: Check the browser dev tools "Network" panel, observe that the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias,comments,requested_partners request is not made. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 92493 [details] [review] Bug 23229: (QA follow-up) Fix indentation Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 92494 [details] [review] Bug 23229: (follow-up) Narrow scope of changes Sorry guys! Whilst working on something else, it came to light that the previous fix in this bug was waaaaay too heavy handed and basically kills all ILL related JS :( This is obviously a big problem, this patch fixes that. The main problem is that all the ILL related JS is in a single file, which isn't ideal as bits of it are used on different pages. Ideally, we'd refactor it out into more manageable chunks which can be included as required. Until such a time, this patch will do, it just prevents the "get all requests" AJAX request unless you're on the ILL list page. New test plan: - Go to "ILL requests" page - TEST: Observe that the table loads all ILL requests - Go to the "New ILL request" page for any installed backend - TEST: Check the browser dev tools "Network" panel, observe that the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias,comments,requested_partners request is not made. - Go to "Manage request" page for a request - TEST: Check that the "ILL request log" and "Display supplier metadata" buttons work Anyone got any idea how I can set this back to "Signed off" so the last patch can be tested? The code was just released - so you need to file a new bug - this is no longer a follow-up patch, it's now a bug fix and should be treated with high priority. (In reply to Katrin Fischer from comment #15) > The code was just released - so you need to file a new bug - this is no > longer a follow-up patch, it's now a bug fix and should be treated with high > priority. Ok, it was a bug fix before. What I am trying ot explain: once code is in a released version, you should not reuse the bug - it will be confusing, especially in release notes. Better to have a new distinctive bug that describes what is fixed. Thanks Katrin, Bug 23529 now addresses this. |