Description
Jonathan Druart
2015-12-09 17:02:42 UTC
Created attachment 45553 [details] [review] Bug 15344: Remove unucessary call to GetMemberDetails This subroutine does a lot a processing and should only be called when necessary. In the get_template_and_user subroutine (so called from any pages of Koha), it is call to pass the branchcode, title, firstname, surname and borrowernumber values for the logged in user. This subroutine calls GetMemberAccountRecords which retrieve the items infos for all accountlines entries of the logged in user. On members/members.pl, let's say you have 74 entries in the accountlines tables, the page will execute 115 SELECT instead of 35 if you don't have any accountlines entries. With this patch, the number of SELECT is always 31. To test this patch you should have technical skills to know what to do. Note that USER_INFO was an array of... 1 element. Now it's a hashref. Not that this patch could be improved and the call to GetMember removed. But to be easily backportable, I'd prefer to keep it light. Created attachment 45560 [details] [review] Bug 15344: Remove some other calls of GetMemberDetails from pl scripts Same as previously. For these files it's a bit less obvious. To make sure these changes won't introduce any regression, check that the variable returned by GetMember is never used to get something else than a borrower fields. The 'flags' should not be get neither. For opac-user.tt it's different, other keys are got but there are defined in the pl script. On the way: - 'showname' is removed (never used) - fix scope var issue in opac-user.tt (BORROWER_INF.OPACPatronDetails vs OPACPatronDetails) Created attachment 45604 [details] [review] Bug 15344: Remove unucessary call to GetMemberDetails This subroutine does a lot a processing and should only be called when necessary. In the get_template_and_user subroutine (so called from any pages of Koha), it is call to pass the branchcode, title, firstname, surname and borrowernumber values for the logged in user. This subroutine calls GetMemberAccountRecords which retrieve the items infos for all accountlines entries of the logged in user. On members/members.pl, let's say you have 74 entries in the accountlines tables, the page will execute 115 SELECT instead of 35 if you don't have any accountlines entries. With this patch, the number of SELECT is always 31. To test this patch you should have technical skills to know what to do. Note that USER_INFO was an array of... 1 element. Now it's a hashref. Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 45605 [details] [review] Bug 15344: Remove some other calls of GetMemberDetails from pl scripts Same as previously. For these files it's a bit less obvious. To make sure these changes won't introduce any regression, check that the variable returned by GetMember is never used to get something else than a borrower fields. The 'flags' should not be get neither. For opac-user.tt it's different, other keys are got but there are defined in the pl script. On the way: - 'showname' is removed (never used) - fix scope var issue in opac-user.tt (BORROWER_INF.OPACPatronDetails vs OPACPatronDetails) Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 45848 [details] [review] Bug 15344: Remove unucessary call to GetMemberDetails This subroutine does a lot a processing and should only be called when necessary. In the get_template_and_user subroutine (so called from any pages of Koha), it is call to pass the branchcode, title, firstname, surname and borrowernumber values for the logged in user. This subroutine calls GetMemberAccountRecords which retrieve the items infos for all accountlines entries of the logged in user. On members/members.pl, let's say you have 74 entries in the accountlines tables, the page will execute 115 SELECT instead of 35 if you don't have any accountlines entries. With this patch, the number of SELECT is always 31. To test this patch you should have technical skills to know what to do. Note that USER_INFO was an array of... 1 element. Now it's a hashref. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 45849 [details] [review] Bug 15344: Remove some other calls of GetMemberDetails from pl scripts Same as previously. For these files it's a bit less obvious. To make sure these changes won't introduce any regression, check that the variable returned by GetMember is never used to get something else than a borrower fields. The 'flags' should not be get neither. For opac-user.tt it's different, other keys are got but there are defined in the pl script. On the way: - 'showname' is removed (never used) - fix scope var issue in opac-user.tt (BORROWER_INF.OPACPatronDetails vs OPACPatronDetails) Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master, thanks Jonathan! Patches pushed to 3.22.x, will be in 3.22.2 Check out explodes with a software error: Can't use string ("1900156") as a HASH ref while "strict refs" in use at /home/koha/src/C4/Circulation.pm line 813. Created attachment 46121 [details] [review] Bug 15344: Remove one occurrence in circ/circulation.pl In C4::Circulation::CanBookBeIssued, $borrower->{flags} is called and should be populated by GetMemberDetails It fixes the following error on checking out: Can't use string ("1900156") as a HASH ref while "strict refs" in use at /home/koha/src/C4/Circulation.pm line 813. Kyle and Julian, Have a quick look at the third patch, it fixes a blocking issue. Created attachment 46129 [details] [review] Bug 15344: Remove one occurrence in circ/circulation.pl In C4::Circulation::CanBookBeIssued, $borrower->{flags} is called and should be populated by GetMemberDetails It fixes the following error on checking out: Can't use string ("1900156") as a HASH ref while "strict refs" in use at /home/koha/src/C4/Circulation.pm line 813. Note: Error occurs with patrons with staff permissions, not with 'normal' patrons. Error is fixed with this patch. Signed-off-by: Marc Véron <veron@veron.ch> Setting to blocker because of software error while checking out to patron with staff permissions. Issue is fixed by Jonathan's latest patch. This patch has been pushed to 3.20.x, will be in 3.20.8. (In reply to Frédéric Demians from comment #15) > This patch has been pushed to 3.20.x, will be in 3.20.8. Was it pushed without the third patch? - That would be a blocker, because I still get the error from comment #13 (on master), and it seems that the third patch that fixes it is not on master. The offending line is #273 in circ/circulation.pl, we need the member details here (for useres with staff permission), otherwise we get the software error: Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/C4/Circulation.pm line 815. Marc and Johnatan, thanks. I confirm 3rd patch is required. I've pushed it directly to 3.20.x branch. It still has to be pushed to master/stable. (In reply to Frédéric Demians from comment #17) > Marc and Johnatan, thanks. I confirm 3rd patch is required. I've pushed it > directly to 3.20.x branch. It still has to be pushed to master/stable. Third patch has been pushed to master! Thanks for the followup Jonathan! (In reply to Kyle M Hall from comment #18) > Third patch has been pushed to master! Pushed to 3.22.x as well |