|
Description
Katrin Fischer
2020-02-14 21:34:06 UTC
+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 |