Opening the circulation and fines rules page takes much longer than before bug 18936 even if there is only a single rule. Saving and editing a rule also takes longer.
+1 on this. We've spent some time exploring the new interface and it's painfully slow.
(In reply to Andrew Fuerste-Henry from comment #1) > +1 on this. We've spent some time exploring the new interface and it's > painfully slow. The new interface you are referring to is bug 15522. Not necessarily related to bug 18936. On bug 15522 the whole smart-rules.pl script is removed.
Created attachment 99082 [details] [review] Bug 24670: Fetch all circulation rules at once to fix perf regression
I do not have much circulation rules defined in my dev install, so it's a bit hard to notice the difference. Could you test this patch and tell me if it improves the performance?
When I try to load the page, I get this message: Could not compile /home/vagrant/kohaclone/admin/smart-rules.pl: Global symbol "$rules" requires explicit package name (did you forget to declare "my $rules"?) at /home/vagrant/kohaclone/admin/smart-rules.pl line 584. Global symbol "$rules" requires explicit package name (did you forget to declare "my $rules"?) at /home/vagrant/kohaclone/admin/smart-rules.pl line 595. BEGIN not safe after errors--compilation aborted at /home/vagrant/kohaclone/admin/smart-rules.pl line 633. at /usr/share/perl5/CGI/Compile.pm line 132 I reset_all and dbic and yarn build but still get this message. Is there another command I'm missing?
Created attachment 99278 [details] [review] Attachment to Bug 24670 - Circulation and fine rules page has performance issues since issuingrules change Bug 24670: Fetch all circulation rules at once to fix perf regression Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
I went back later and it worked this time. Much faster now!
Created attachment 99280 [details] [review] Bug 24670: Fetch all circulation rules at once to fix perf regression Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Ooops.. false positive there.. had the wrong branch checked out. I see the same error as Lisette I'm afraid.. looks like $rules is defined in a local scope but then assigned to in a global one.
Created attachment 99281 [details] [review] Bug 24670: Fetch all circulation rules at once to fix perf regression Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 99282 [details] [review] Bug 24670: (QA follow-up) Add missing $rules definition Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Now it's looking good to me.. Setting back to SO as I'd like Jonathan to check my followup.. seems strange it was missed.
Created attachment 99286 [details] [review] Bug 24670: (QA follow-up) Add missing $rules definition Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Created attachment 99300 [details] [review] Bug 24670: (QA follow-up) Add missing $rules definition Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Martin Renvoize from comment #13) > Now it's looking good to me.. Setting back to SO as I'd like Jonathan to > check my followup.. seems strange it was missed. Oops, I have the bad habit to modify my patches after I already tested them. I guess I renamed something just before attaching it here, sorry about that!
No worries, I often do the same :P
*puts hands over her ears and sings loudly*
(In reply to Katrin Fischer from comment #18) > *puts hands over her ears and sings loudly* ... hands over the eyes might work better in this case.
Nice work everyone! Pushed to master for 20.05
not backported to 19.11.x due to dependencies