[Bug 23229] New: "Get all requests" API call fired when loading any ILL page
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Bug ID: 23229 Summary: "Get all requests" API call fired when loading any ILL page Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: ILL Assignee: koha-bugs@lists.koha-community.org Reporter: andrew.isherwood@ptfs-europe.com This is a substantial bug that, as far as I can determine, has always existed. The JS that retrieves all ILL requests for the main ILL table (ill-list-table.js) runs on every ILL page that utilises ill-requests.tt (pretty much every ILL page). This is clearly wrong, it should only run when the ILL list table is being displayed. This has significant performance implications since the cost in constructing and delivering the response to this request can be substantial. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |andrew.isherwood@ptfs-europ |ity.org |e.com Patch complexity|--- |Trivial patch Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #1 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91062 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91062&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91062|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 91096 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91096&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 91097 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91097&action=edit Bug 23229: (QA follow-up) Fix indentation Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91096|0 |1 is obsolete| | --- Comment #4 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 91769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91769&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91097|0 |1 is obsolete| | --- Comment #5 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 91770 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91770&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|19.11.00 |19.11.00,19.05.03 released in| | Status|Pushed to master |Pushed to stable --- Comment #7 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 19.05.x for 19.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #8 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 18.11.x for 18.11.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #9 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92491 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92491&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91769|0 |1 is obsolete| | --- Comment #10 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92492 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92492&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91770|0 |1 is obsolete| | --- Comment #11 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92493 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92493&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92491|0 |1 is obsolete| | --- Comment #12 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92494 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92494&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #13 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #14 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Anyone got any idea how I can set this back to "Signed off" so the last patch can be tested? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|19.11.00,19.05.03 |19.11.00,19.05.03,18.11.09 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23529 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23529 [Bug 23529] Interlibrary loan javascript is broken -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 --- Comment #17 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Thanks Katrin, Bug 23529 now addresses this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23229 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org