[Bug 34223] New: ILL status filter does not load immediately after selecting a backend filter
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Bug ID: 34223 Summary: ILL status filter does not load immediately after selecting a backend filter Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: ILL Assignee: koha-bugs@lists.koha-community.org Reporter: Laura.escamilla@bywatersolutions.com CC: pedro.amorim@ptfs-europe.com Selecting a backend filter in ILL is required in order for the status filter to become enabled. Once a provider is chosen in the backend filter, it takes the status filter around 7s to populate instead of populating immediately. This is confusing to staff members and can also disrupt the workflow. -- 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=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com --- Comment #1 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- The solution I found for this at the time was prevented, I believe, by ONLY_FULL_GROUP_BY in https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Da... There is currently a FIXME in https://github.com/Koha-Community/Koha/blob/master/Koha/Illbackend.pm This FIXME is for the samne issue reported by this bug. Need help with this. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |magnus@libriotech.no, | |nick@bywatersolutions.com Depends on| |22440 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22440 [Bug 22440] Improve ILL page performance by moving to server side filtering -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153289&action=edit Bug 34223: PROOF OF CONCEPT The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #3 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- The patch is just a proof of concept detailing the solution I had at the time of developing this but was ultimately prevented by the ONLY_FULL_GROUP_BY. The patch is NOT the fix, as I don't think we want to change Database.pm for this. It's just demonstrating how it should work. -- 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=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153289|0 |1 is obsolete| | --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153953 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153953&action=edit Bug 34223: Fix filter performance We are now only fetching the existing statuses for the provided backend. Then, we fetch 1 illrequest object to make use of ->strings_map. Before, we were fetching all illrequests in the database and removing redundant afterwards -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- @Laura Escamilla Can you test my most recent patch? I've tested it myself with 10k fake requests and it fixed the issue. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Found an issue with the patch. Trying to fix. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- 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=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153953|0 |1 is obsolete| | --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153956 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153956&action=edit Bug 34223: Fix filter performance We are now only fetching the existing statuses for the provided backend. Then, we fetch 1 illrequest object to make use of ->strings_map. Before, we were fetching all illrequests in the database and removing redundant afterwards -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153957 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153957&action=edit Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast -- 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=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153956|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |normal Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pedro and I had another look at this and I managed to come up with a slightly cleaner and I believe more performant alternative now attached above. We're not getting just the status's we want in just one DB hit.. it's a fun work around for the `ONLY_FULL_GROUP_BY` problem. Please test as per Pedro's test plan.. now copied into my submission too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Issue spotted.. Just merging parts of Pedro's patch and parts of my own and we'll cross-sign each others. Also.. there appears to be no unit tests for this method.. so I'm going to have a look at writing one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34431 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34431 [Bug 34431] Distinguish between status and status_alias in ILL UI -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153961&action=edit Bug 34223: (follow-up) Restore status_alias handling By reducing the original call to all distinct 'status' we also removed the status_alias combinations. This patch adds an additional distinct query to fetch all 'status_alias' and add them to the returned data structure. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153957|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153962&action=edit Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153961|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153963&action=edit Bug 34223: (follow-up) Restore status_alias handling By reducing the original call to all distinct 'status' we also removed the status_alias combinations. This patch adds an additional distinct query to fetch all 'status_alias' and add them to the returned data structure. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153968&action=edit Bug 34223: Add unit test for existing_statuses This patch adds a unit test for the 'existing_statuses' method. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Martin Renvoize <martin.renvoize@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=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153968|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153973 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153973&action=edit Bug 34223: Add unit test for existing_statuses This patch adds a unit test for the 'existing_statuses' method. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #16 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153978&action=edit Bug 34223: (follow-up) Mock backend in tests Add a new request with a backend specific status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Laura Escamilla <Laura.escamilla@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=34223 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153962|0 |1 is obsolete| | --- Comment #17 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 153979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153979&action=edit Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153963|0 |1 is obsolete| | --- Comment #18 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 153980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153980&action=edit Bug 34223: (follow-up) Restore status_alias handling By reducing the original call to all distinct 'status' we also removed the status_alias combinations. This patch adds an additional distinct query to fetch all 'status_alias' and add them to the returned data structure. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153973|0 |1 is obsolete| | --- Comment #19 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 153981 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153981&action=edit Bug 34223: Add unit test for existing_statuses This patch adds a unit test for the 'existing_statuses' method. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153978|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #20 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153989 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153989&action=edit Bug 34223: (follow-up) Mock backend in tests Add a new request with a backend specific status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153989|0 |1 is obsolete| | --- Comment #21 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153990&action=edit Bug 34223: (follow-up) Mock backend in tests Add a new request with a backend specific status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153979|0 |1 is obsolete| | --- Comment #22 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154026 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154026&action=edit Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153980|0 |1 is obsolete| | --- Comment #23 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154027 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154027&action=edit Bug 34223: (follow-up) Restore status_alias handling By reducing the original call to all distinct 'status' we also removed the status_alias combinations. This patch adds an additional distinct query to fetch all 'status_alias' and add them to the returned data structure. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153981|0 |1 is obsolete| | --- Comment #24 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154028 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154028&action=edit Bug 34223: Add unit test for existing_statuses This patch adds a unit test for the 'existing_statuses' method. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153990|0 |1 is obsolete| | --- Comment #25 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154029 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154029&action=edit Bug 34223: (follow-up) Mock backend in tests Add a new request with a backend specific status Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #26 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- It appears something went wrong with patch "Add unit test for existing_statuses" upon Laura's sign-off, it currently includes all other patches AND Illbackend.t (which is the only file it should contain). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154026|0 |1 is obsolete| | --- Comment #27 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154030 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154030&action=edit Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154027|0 |1 is obsolete| | --- Comment #28 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154031 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154031&action=edit Bug 34223: (follow-up) Restore status_alias handling By reducing the original call to all distinct 'status' we also removed the status_alias combinations. This patch adds an additional distinct query to fetch all 'status_alias' and add them to the returned data structure. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154028|0 |1 is obsolete| | --- Comment #29 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154032 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154032&action=edit Bug 34223: Add unit test for existing_statuses This patch adds a unit test for the 'existing_statuses' method. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154029|0 |1 is obsolete| | --- Comment #30 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154033 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154033&action=edit Bug 34223: (follow-up) Mock backend in tests Add a new request with a backend specific status Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154030|0 |1 is obsolete| | --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 154068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154068&action=edit Bug 34223: Work around ONLY_FULL_GROUP_BY in Illbackend This patch re-works the query in the existing_statuses method to remove the FIXME and improve performance. We pass an SQL literal into the query to make it explicit which illrequest_id we're looking for (which dampens the SQL warning) even though we really don't mind which request is returned here. Test plan: The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) On an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-d...) 2) Pet a cat 3) Visit /cgi-bin/koha/ill/ill-requests.pl and select a backend on the left side filters 4) Notice how the status filter takes a while (3-5 secs) to load 5) Apply patch and koha-plack --restart kohadev 6) Repeat 3, notice how the status filter now loads fast Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154031|0 |1 is obsolete| | --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 154069 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154069&action=edit Bug 34223: (follow-up) Restore status_alias handling By reducing the original call to all distinct 'status' we also removed the status_alias combinations. This patch adds an additional distinct query to fetch all 'status_alias' and add them to the returned data structure. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154032|0 |1 is obsolete| | --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 154070 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154070&action=edit Bug 34223: Add unit test for existing_statuses This patch adds a unit test for the 'existing_statuses' method. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154033|0 |1 is obsolete| | --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 154071 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154071&action=edit Bug 34223: (follow-up) Mock backend in tests Add a new request with a backend specific status Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |katrin.fischer@bsz-bw.de --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I almost got stuck on step 2... no cat! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.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=34223 --- Comment #36 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 155280 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155280&action=edit Bug 34223: (QA follow-up) Fix file permissions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|23.11.00 |23.11.00,23.05.04 released in| | Status|Pushed to master |Pushed to stable --- Comment #38 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 155493 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155493&action=edit Bug 34223: (follow-up) Ensure consistent ordering of statuses This patch adds an order_by clause to ensure consistent ordering of the returned status and status_alias statuses from the database between MySQL 8 and other DB engines. This fixes the failing tests introducd in bug 34223. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |ASSIGNED --- Comment #40 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Follow-up pushed to master. Please backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=34223 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable --- Comment #41 from Fridolin Somers <fridolin.somers@biblibre.com> --- Follow-up pushed to 23.05.x for 23.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00,23.05.04 |23.11.00,23.05.04,22.11.10 released in| | 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=34223 --- Comment #42 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34223 Irene Tortorella <i.tortorella@arts.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |i.tortorella@arts.ac.uk -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org