https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kelly@bywatersolutions.com --- Comment #2 from Kelly McElligott <kelly@bywatersolutions.com> --- Aleisha- something odd- nothing is showing in the Holds To Pull report. I have tried this multiple times and unable to get the holds to populate. Both biblio and Item level holds. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #4 from Kelly McElligott <kelly@bywatersolutions.com> --- Hi Aleisha, I have done what you have recommended. I am now getting an error when I try to go to the Holds to Pull report DBIx::Class::Storage::DBI::_dbh_execute(): 'koha_hold.reserve.reserve_id' isn't in GROUP BY at /kohadevbox/koha/Koha/Objects.pm line 130 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #5 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Kelly McElligott from comment #4)

Hi Aleisha, I have done what you have recommended. I am now getting an error when I try to go to the Holds to Pull report

DBIx::Class::Storage::DBI::_dbh_execute(): 'koha_hold.reserve.reserve_id' isn't in GROUP BY at /kohadevbox/koha/Koha/Objects.pm line 130 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

Hi Kelly, I can't reproduce this error. There might be a problem in your database? Sorry! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Lucy Vaux-Harvey <lucy.vaux-harvey@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucy.vaux-harvey@ptfs-europ | |e.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---

DBIx::Class::Storage::DBI::_dbh_execute(): 'koha_hold.reserve.reserve_id' isn't in GROUP BY at /kohadevbox/koha/Koha/Objects.pm line 130 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

Issues with GROUP BY usually point to problems with stricter DB settings - that's what I meant. It's possible the tester had a different DBMS version / different settings which made it explode - trying to help because you said you cannot reproduce. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #10 from Aleisha Amohia <aleisha@catalyst.net.nz> --- I've misunderstood here - I thought that Katrin's comment was talking about the potential cause of the original bug reported, not in relation to reproducing Kelly's error. Also I'm more than happy to write tests. I had simply forgotten in the first instance. Will take another look when I get a chance. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Kelly's error exists with and without the patch (if strict_sql_modes is set). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |black23@gmail.com --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 24301 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=24488 --- Comment #14 from Michal Denar <black23@gmail.com> --- Hello Aleisha, is this patch ready to test? I applied patch, but name of first patron was same after change of holds order as before. Tested on Kohadevbox after restart_all. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #16 from Michal Denar <black23@gmail.com> --- OK, I'll wait on "Needs signoff" status :-) But I think that similar bug has Holds queue page. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #18 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Just rebasing - still need to try with strict_sql_mode. Will come back to this tomorrow. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> 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=24488 --- Comment #21 from Aleisha Amohia <aleisha@catalyst.net.nz> --- I enabled strict_sql_modes but unfortunately I still can't reproduce the errors. Can someone give me some instructions for this? I'm happy to keep working on it. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #23 from Michal Denar <black23@gmail.com> --- Hi Aleisha, I'm so sorry but this patch didn't solve problem with first patron on my kohadevbox. If I change order of borrower with hold, list without change. I've problems with 'dbic' on kohadevbox, so I tried make schema change manually by !sudo koha-upgrade-schema kohadev'. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #25 from Michal Denar <black23@gmail.com> --- Hi Katrin, yes,QA faild for me. Thank you. I saw on IRC, that you fight with 'dbic' too. Did you solve it? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |26020 --- Comment #27 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Katrin Fischer from comment #22)

Actually I think we have a bigger problem here: I was confusing holds to pull and holds queue earlier. Now testing another patch...

Holds to pull fails for me with and without this patch in strict mode (error on bug 26020)

Thanks for pointing this out and reporting it, Katrin. I still can't seem to enable strict mode, can you please tell me how to do this? I suppose we want to fix the bug on Bug 26020 before getting this patch through (in fact fixing Bug 26020 might fix this problem) so I'll set 26020 as blocking this bug. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26020 [Bug 26020] Holds to pull report is broken in sql strict mode -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #28)

Hi Aleiha, all I do is changing the setting in the koha-conf.xml file and restarted all the things including the mysql server. Not sure if that's needed. I later turend it off again so I coudl continue testing the other patch and get around the bug.

Oh, and your patch didn't fix it for me - still showing the same problem with the group by. (it's not changed, I think) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #32 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Sorry team I really cannot seem to enable strict_sql_modes and reproduce that error, I tried a bunch of different things including MariaDB specific things but I couldn't reproduce. I've reworked the patch a bit, and really curious if it stops that error from showing. Please test and let me know how it goes. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|26020 | --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Aleisha Amohia from comment #27)

(In reply to Katrin Fischer from comment #22)

...

Actually I think we have a bigger problem here: I was confusing holds to pull and holds queue earlier. Now testing another patch...

Holds to pull fails for me with and without this patch in strict mode (error on bug 26020)

Thanks for pointing this out and reporting it, Katrin. I still can't seem to enable strict mode, can you please tell me how to do this?

I suppose we want to fix the bug on Bug 26020 before getting this patch through (in fact fixing Bug 26020 might fix this problem) so I'll set 26020 as blocking this bug.

I have the hope to fix the problematic group by with your patches, Aleisha. We already tried to fix it on bug 22431 but failed, so I don't think we should depend on another bug report. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26020 [Bug 26020] Holds to pull report is broken in sql strict mode -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #35 from Michal Denar <black23@gmail.com> --- Hi, sorry Aleisha and Jonathan. But patch don't work for me. If you change priority of holds, name on List to Pull is still same as befere change. It's not on test plan, I know. But I should work too. Maybe someone test it too. Katrin? https://recordit.co/RA8bCjXekD -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I am expecting the first patron in the queue to be displayed. The query did not deal with the priority indeed, but this query is plain wrong :D -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #39 from Aleisha Amohia <aleisha@catalyst.net.nz> --- So to confirm, this patch will not only fix the group_by error but we also want to fix the Holds to Pull to show the highest priority hold instead of the earliest placed hold? Because currently it shows the earliest placed hold. Happy to make this change. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #41 from Michal Denar <black23@gmail.com> --- Hi Aleisha, any updates? Thank you. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #43 from Michal Denar <black23@gmail.com> --- OK -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107206|0 |1 is obsolete| | Attachment #107213|0 |1 is obsolete| | --- Comment #44 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 111341 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111341&action=edit Bug 24488: Show correct first patron details on Holds to pull This patch rewrites the complex search query to reduce the risk of reordering the results multiple times. This patch includes a change to schema files so may need to upgrade schema and/or restart memcached To test: 1) Reproduce problem following test plan in Description 2) Apply patch and refresh page 3) Notice the correct patron is now shown 4) Play with date selection, confirm correct results are still shown 5) Test cancelling holds 6) Test filtering table results 7) Test with biblios with multiple items 8) Test with making items unavailable (i.e. not for loan, checked out) Sponsored by: IHC New Zealand -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #46 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 111343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111343&action=edit Bug 24488: Display patron with the highest priority hold -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #48 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 111347 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111347&action=edit Bug 24488: (follow-up) Reduce repeated code -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111347|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=24488 --- Comment #51 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 111349 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111349&action=edit Bug 24488: [DISCUSSION] For comparaison I'd like someone to run a benchmark with and without this patch on a quite big database and see which approach is the best -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Michal Denar <black23@gmail.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=24488 --- Comment #54 from Michal Denar <black23@gmail.com> --- Created attachment 111359 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111359&action=edit Bug 24488: Fix reserve.reserve_id' isn't in GROUP BY Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #56 from Michal Denar <black23@gmail.com> --- Created attachment 111361 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111361&action=edit Bug 24488: Simplify searches It also: Use dtf for date handling add missing td Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #58 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Neither the original patches nor Joubu's alternative(?) apply at the moment. I feel like we got a bit stuck here. Aleisha, could you take a look at comment#50 too when rebasing and maybe Joubu's patch? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111358|0 |1 is obsolete| | Attachment #111359|0 |1 is obsolete| | Attachment #111360|0 |1 is obsolete| | Attachment #111361|0 |1 is obsolete| | Attachment #111362|0 |1 is obsolete| | --- Comment #59 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 112815 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112815&action=edit Bug 24488: Show correct first patron details on Holds to pull This patch rewrites the complex search query to reduce the risk of reordering the results multiple times. This patch includes a change to schema files so may need to upgrade schema and/or restart memcached To test: 1) Reproduce problem following test plan in Description 2) Apply patch and refresh page 3) Notice the correct patron is now shown 4) Play with date selection, confirm correct results are still shown 5) Test cancelling holds 6) Test filtering table results 7) Test with biblios with multiple items 8) Test with making items unavailable (i.e. not for loan, checked out) Sponsored by: IHC New Zealand Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #61 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 112817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112817&action=edit Bug 24488: Display patron with the highest priority hold Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #63 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 112819 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112819&action=edit Bug 24488: [DISCUSSION] For comparaison I'd like someone to run a benchmark with and without this patch on a quite big database and see which approach is the best Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112815|0 |1 is obsolete| | --- Comment #65 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113695&action=edit Bug 24488: Show correct first patron details on Holds to pull This patch rewrites the complex search query to reduce the risk of reordering the results multiple times. This patch includes a change to schema files so may need to upgrade schema and/or restart memcached To test: 1) Reproduce problem following test plan in Description 2) Apply patch and refresh page 3) Notice the correct patron is now shown 4) Play with date selection, confirm correct results are still shown 5) Test cancelling holds 6) Test filtering table results 7) Test with biblios with multiple items 8) Test with making items unavailable (i.e. not for loan, checked out) Sponsored by: IHC New Zealand Signed-off-by: Michal Denar <black23@gmail.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=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112817|0 |1 is obsolete| | --- Comment #67 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113697&action=edit Bug 24488: Display patron with the highest priority hold Signed-off-by: Michal Denar <black23@gmail.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=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112819|0 |1 is obsolete| | --- Comment #69 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113699&action=edit Bug 24488: [DISCUSSION] For comparaison I'd like someone to run a benchmark with and without this patch on a quite big database and see which approach is the best Signed-off-by: Michal Denar <black23@gmail.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=24488 --- Comment #71 from Josef Moravec <josef.moravec@gmail.com> --- I have done a little benchmark Testing DB - copy from production 85k biblios 125k items 15k reserves I ran twice a batch of 10 runs of pendingreserves.pl and measure clean time of this script running (from begin to end of whole script) Original version from Aleisha: Run 1: 7.57 s avg Run 1: 8.1 s avg Joubu's alternative: Run 1: 5,96 s Run 2: 6,04 s That is between 20 and 25 percent gain for Joubu's alternative patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #73 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Josef Moravec from comment #71)

I have done a little benchmark

Testing DB - copy from production 85k biblios 125k items 15k reserves

I ran twice a batch of 10 runs of pendingreserves.pl and measure clean time of this script running (from begin to end of whole script)

Original version from Aleisha: Run 1: 7.57 s avg Run 1: 8.1 s avg

Joubu's alternative: Run 1: 5,96 s Run 2: 6,04 s

That is between 20 and 25 percent gain for Joubu's alternative patch

How many holds were reported on the report for you? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #74 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113756&action=edit Bug 24488: (QA follow-up) Make pending reserves faster 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=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113757|0 |1 is obsolete| | --- Comment #77 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113763&action=edit Bug 24488: (QA follow-up) Remove two queries out of foreach cycle 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=24488 --- Comment #79 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Josef, I am expecting this last patch to improve perf only in the case where all items are coming from the same biblio. Which don't happen on a real-life situation :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED --- Comment #81 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 17-18s (josef) 18-19 (mine) 5s (master) Working on this. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #83 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113774 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113774&action=edit Bug 24488: Fix count distinct patron on a given hold -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #85 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113776 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113776&action=edit Bug 24488: perf - move patrons_count out of the loop -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #87 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113778 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113778&action=edit Bug 24488: perf - Move holds search out of the loop This will need attention from QA. Are we sure it's doing the same thing (especially the order by!) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #89 from Michal Denar <black23@gmail.com> --- Hi, I get this errors on Hold to pull page https://lh3.googleusercontent.com/-Ka99Dm0l93w/X7VZsqPoAoI/AAAAAAAAHvM/btn2w... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #91 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Josef Moravec from comment #90)

This does not work correctly - in fact the behaviour is same as before patch (only slower)

When we get all holds at once, it will be the same as before... we need to do one of this things:

- get hold by biblionumber in each loop - revert last patch - bad performance - add 'priority' => 1, to where clause - best performance, but can we - add subquery and get only the holds with priority = min(priority) for given biblionumber - probably the optimal solution?

Sorry, commited comment too early ;) ad second point: - add 'priority' => 1, to where clause - best performance, but can we assume the lowest priority is always 1? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113778|0 |1 is obsolete| | --- Comment #92 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113798 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113798&action=edit Bug 24488: perf - Move holds search out of the loop This will need attention from QA. We are assuming that the hold with the highest priority has priority=1 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=24488 --- Comment #95 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #94)

(In reply to Jonathan Druart from comment #93)

...

Sorry, the last patch was silly indeed. I hadn't turned on the strict mode.

Your suggestion, Josef, of assuming that the highest priority hold has priority=1 is great actually. I think we never wanted to assume that (in case the data is corrupted?) in this script, but we should go for it IMO, that simplifies the code, the queries, and it makes sense to me!

I believe in the past we had issues with the priorities not being correct, but believe it was fixed (hopefully). Nick or Kyle might be able to tell.

Wondering what happens with the local holds priority settings - do they change the priority in the db or do they check highest priority for local?

I checked in our production sites, there are few cases where the highest priority is not 1, and they mostly seem to be related to other issues (suspended holds, holds in the future, missing items) so nothing that should show on holds to pull. Local holds don't alter the priority, they get checked first when filling holds or building the queue, so this report won't catch them, but I think that is fine as this report isn't branch specific Other options (for another bug) would be: 1 - List all the holds - just show the first and maybe add a modal to see all the others 2 - Create a cron like the holds queue, build this page on the backend and simply display - it adds a delay to new holds showing, but allows us to calculate the exact holds to be pulled 3 - Add a branch limiter to the report and default to local branches items -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113695|0 |1 is obsolete| | Attachment #113696|0 |1 is obsolete| | Attachment #113697|0 |1 is obsolete| | Attachment #113698|0 |1 is obsolete| | Attachment #113699|0 |1 is obsolete| | Attachment #113700|0 |1 is obsolete| | Attachment #113756|0 |1 is obsolete| | Attachment #113763|0 |1 is obsolete| | Attachment #113764|0 |1 is obsolete| | Attachment #113774|0 |1 is obsolete| | Attachment #113775|0 |1 is obsolete| | Attachment #113776|0 |1 is obsolete| | Attachment #113777|0 |1 is obsolete| | Attachment #113798|0 |1 is obsolete| | --- Comment #96 from Michal Denar <black23@gmail.com> --- Created attachment 113829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113829&action=edit Bug 24488: Show correct first patron details on Holds to pull This patch rewrites the complex search query to reduce the risk of reordering the results multiple times. This patch includes a change to schema files so may need to upgrade schema and/or restart memcached To test: 1) Reproduce problem following test plan in Description 2) Apply patch and refresh page 3) Notice the correct patron is now shown 4) Play with date selection, confirm correct results are still shown 5) Test cancelling holds 6) Test filtering table results 7) Test with biblios with multiple items 8) Test with making items unavailable (i.e. not for loan, checked out) Sponsored by: IHC New Zealand Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #98 from Michal Denar <black23@gmail.com> --- Created attachment 113831 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113831&action=edit Bug 24488: Display patron with the highest priority hold Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denra <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #100 from Michal Denar <black23@gmail.com> --- Created attachment 113833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113833&action=edit Bug 24488: [DISCUSSION] For comparaison I'd like someone to run a benchmark with and without this patch on a quite big database and see which approach is the best Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denra <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #102 from Michal Denar <black23@gmail.com> --- Created attachment 113835 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113835&action=edit Bug 24488: (QA follow-up) Make pending reserves faster Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denra <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #104 from Michal Denar <black23@gmail.com> --- Created attachment 113837 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113837&action=edit Bug 24488: (QA follow-up) Move getting items out of cycle Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denra <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #106 from Michal Denar <black23@gmail.com> --- Created attachment 113839 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113839&action=edit Bug 24488: perf - Group by at DBMS level Signed-off-by: Michal Denra <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #108 from Michal Denar <black23@gmail.com> --- Created attachment 113841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113841&action=edit Bug 24488: Simplify structure passed to the template Signed-off-by: Michal Denra <black23@gmail.comm> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #110 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Michal Denar from comment #108)

Created attachment 113841 [details] [review] Bug 24488: Simplify structure passed to the template

Signed-off-by: Michal Denra <black23@gmail.comm>

Thanks Michal for testing, just a note, you have misspelled your name and email in your git configuration -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113830|0 |1 is obsolete| | --- Comment #112 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113851 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113851&action=edit Bug 24488: Fix reserve.reserve_id' isn't in GROUP BY Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113832|0 |1 is obsolete| | --- Comment #114 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113853&action=edit Bug 24488: Simplify searches It also: Use dtf for date handling add missing td Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113834|0 |1 is obsolete| | --- Comment #116 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113855 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113855&action=edit Bug 24488: (QA follow-up): Fix small bugs, add checkk for waiting holds Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113836|0 |1 is obsolete| | --- Comment #118 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113857 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113857&action=edit Bug 24488: (QA follow-up) Remove two queries out of foreach cycle Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113838|0 |1 is obsolete| | --- Comment #120 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113859 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113859&action=edit Bug 24488: Fix count distinct patron on a given hold Signed-off-by: Michal Denar <black23@gmail.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=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113840|0 |1 is obsolete| | --- Comment #122 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113861 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113861&action=edit Bug 24488: perf - move patrons_count out of the loop Signed-off-by: Michal Denar <black23@gmail.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=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113842|0 |1 is obsolete| | --- Comment #124 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 113863 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113863&action=edit Bug 24488: perf - Move holds search out of the loop This will need attention from QA. We are assuming that the hold with the highest priority has priority=1 Signed-off-by: Michal Denar <black23@gmail.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=24488 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=24488 --- Comment #127 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This looks like a great improvement, movin us to searching via objects etc... but... dare I say it, can we move this to a module and thus add tests? There's a lot of functionality here in filtering down to the right level, improving performance etc.. be great to have that solidly unit/regression tested ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #131 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Joonas Kylmälä from comment #130)

(In reply to Martin Renvoize from comment #127)

...

This looks like a great improvement, movin us to searching via objects etc... but... dare I say it, can we move this to a module and thus add tests?

I think this definitely needs to use a module, the code population of @all_items should be replaced with an object's method call that returns items that can fill holds. Then it will make super easy to read code where you just iterate over all the items and add the callnumber, shelving location, etc. info to the $reserves variable that goes.

Yes, it would be nice to have tests and make the code even more clean. But given from when we started, I think it's already a great improvement. I really would like to have this into master at the beginning of the release cycle, and I am not sure we are going to find volunteers to write tests soon...

Btw, to resolve the original issue, can't we just order by reserve priority? This refactoring could be in a separate bug.

No, I don't think it will work, because of the group by. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #133 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Jonathan Druart from comment #132)

I had a (quick) look at how we could move the @all_items construction into a Koha::Items->filter_by_ method. I didn't find a simply way to do it, keeping code readable and without duplication.

Having it under Koha::Items was not my intention and would not lead to any clearer result. It should be something like Koha::Holds->find($biblionumber)->items_that_can_fill and Koha::Hold->items_that_can_fill. But for this we need the CanItemFillHold from https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26659#c6 I think. And of course some other naming scheme might be better, take items_that_can_fill as just an example. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113850|0 |1 is obsolete| | Attachment #113851|0 |1 is obsolete| | Attachment #113852|0 |1 is obsolete| | Attachment #113853|0 |1 is obsolete| | Attachment #113854|0 |1 is obsolete| | Attachment #113855|0 |1 is obsolete| | Attachment #113856|0 |1 is obsolete| | Attachment #113857|0 |1 is obsolete| | Attachment #113858|0 |1 is obsolete| | Attachment #113859|0 |1 is obsolete| | Attachment #113860|0 |1 is obsolete| | Attachment #113861|0 |1 is obsolete| | Attachment #113862|0 |1 is obsolete| | Attachment #113863|0 |1 is obsolete| | Attachment #113864|0 |1 is obsolete| | --- Comment #135 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114835 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114835&action=edit Bug 24488: Show correct first patron details on Holds to pull This patch rewrites the complex search query to reduce the risk of reordering the results multiple times. This patch includes a change to schema files so may need to upgrade schema and/or restart memcached To test: 1) Reproduce problem following test plan in Description 2) Apply patch and refresh page 3) Notice the correct patron is now shown 4) Play with date selection, confirm correct results are still shown 5) Test cancelling holds 6) Test filtering table results 7) Test with biblios with multiple items 8) Test with making items unavailable (i.e. not for loan, checked out) Sponsored by: IHC New Zealand Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #137 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114837 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114837&action=edit Bug 24488: Display patron with the highest priority hold Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #139 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114839 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114839&action=edit Bug 24488: [DISCUSSION] For comparaison I'd like someone to run a benchmark with and without this patch on a quite big database and see which approach is the best Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #141 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114841&action=edit Bug 24488: (QA follow-up) Make pending reserves faster Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #143 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114843&action=edit Bug 24488: (QA follow-up) Move getting items out of cycle Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #145 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114845&action=edit Bug 24488: perf - Group by at DBMS level Signed-off-by: Michal Denar <black23@gmail.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=24488 --- Comment #147 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114847&action=edit Bug 24488: Simplify structure passed to the template Signed-off-by: Michal Denar <black23@gmail.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=24488 --- Comment #149 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114849&action=edit Bug 24488: (QA follow-up) Go to next loop as early as possible 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=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #151 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Patches rebased. I have provided a small enhancement on bug 27131 to move some code to a method. I don't think we could do more here, if you look at the code it read good and is pretty trivial (it's just a matter a few ->search calls). I have done all what I could do get this ready, hopefully it will be approved and pushed soon :) Sending back to the NQA queue. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #153 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- my ( $k, $f ) = each %$fff ) Clear variables names btw :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114836|0 |1 is obsolete| | --- Comment #156 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116363 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116363&action=edit Bug 24488: Fix reserve.reserve_id' isn't in GROUP BY Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114838|0 |1 is obsolete| | --- Comment #158 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116365&action=edit Bug 24488: Simplify searches It also: Use dtf for date handling add missing td Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114840|0 |1 is obsolete| | --- Comment #160 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116367&action=edit Bug 24488: (QA follow-up): Fix small bugs, add checkk for waiting holds Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114842|0 |1 is obsolete| | --- Comment #162 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116369 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116369&action=edit Bug 24488: (QA follow-up) Remove two queries out of foreach cycle Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114844|0 |1 is obsolete| | --- Comment #164 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116371 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116371&action=edit Bug 24488: Fix count distinct patron on a given hold Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114846|0 |1 is obsolete| | --- Comment #166 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116373 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116373&action=edit Bug 24488: perf - move patrons_count out of the loop Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114848|0 |1 is obsolete| | --- Comment #168 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116375 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116375&action=edit Bug 24488: perf - Move holds search out of the loop This will need attention from QA. We are assuming that the hold with the highest priority has priority=1 Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114850|0 |1 is obsolete| | --- Comment #170 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 116377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116377&action=edit Bug 24488: create a loop to avoid copy paste Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #172 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- In view of the attention given already, passing QA. This should get further follow-up. Move essential code into a module, test it. The original query was terrible, but the new code is far from ideal too. The performance has been degraded. Hard to say when it is no longer acceptable. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116377|0 |1 is obsolete| | --- Comment #174 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 116384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116384&action=edit Bug 24488: create a loop to avoid copy paste Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> JD amended patch: Rename variables -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #175 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #178 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Fridolin Somers from comment #177)

Do you think this is safe for backport on 20.11.x ?

No, it's too big. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 --- Comment #179 from Fridolin Somers <fridolin.somers@biblibre.com> --- Not backported to 20.11.x -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27718 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27718 [Bug 27718] Holds to Pull list doesn't respect holdallowed circulation rule anymore -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24488 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24301 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24301 [Bug 24301] Holds to pull: Name of borrower on list -- You are receiving this mail because: You are watching all bug changes.