[Bug 11960] New: GetMemberDetails is unnecessarily called in CanBookBeRenewed
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Bug ID: 11960 Summary: GetMemberDetails is unnecessarily called in CanBookBeRenewed Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: jonathan.druart@biblibre.com QA Contact: testopia@bugs.koha-community.org C4::Circulation::CanBookBeRenewed calls C4::Members::GetMemberDetails to retrieve categorycode and branchcode. - categorycode is used to retrieve the issuing rule - the borrower information is passed to C4::Circulation::_GetCircControlBranch. Which only uses the branchcode parameter. GetMemberDetails does a lot of useless calls/queries (patronflags, account, etc.) This call could be replaced with a call to C4::Members::GetMember -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26456 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26456&action=edit Bug 11960: GetMemberDetails is unnecessarily called in CanBookBeRenewed C4::Circulation::CanBookBeRenewed calls C4::Members::GetMemberDetails to retrieve categorycode and branchcode. - categorycode is used to retrieve the issuing rule - the borrower information is passed to C4::Circulation::_GetCircControlBranch. Which only uses the branchcode parameter. GetMemberDetails does a lot of useless calls/queries (patronflags, account, etc.). This call could be replaced with a call to C4::Members::GetMember. Note: I presented this small optimisation during a quick introduction to NYTProf (hackfest 14 in Marseille). Test plan: - launch member unit tests - check the code -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26456|0 |1 is obsolete| | --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26459 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26459&action=edit Bug 11960: GetMemberDetails is unnecessarily called in CanBookBeRenewed C4::Circulation::CanBookBeRenewed calls C4::Members::GetMemberDetails to retrieve categorycode and branchcode. - categorycode is used to retrieve the issuing rule - the borrower information is passed to C4::Circulation::_GetCircControlBranch. Which only uses the branchcode parameter. GetMemberDetails does a lot of useless calls/queries (patronflags, account, etc.). This call could be replaced with a call to C4::Members::GetMember. Note: I presented this small optimisation during a quick introduction to NYTProf (hackfest 14 in Marseille). Test plan: - launch member unit tests - check the code -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26459|0 |1 is obsolete| | --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26479 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26479&action=edit [SIGNED OFF] Bug 11960: GetMemberDetails is unnecessarily called in CanBookBeRenewed C4::Circulation::CanBookBeRenewed calls C4::Members::GetMemberDetails to retrieve categorycode and branchcode. - categorycode is used to retrieve the issuing rule - the borrower information is passed to C4::Circulation::_GetCircControlBranch. Which only uses the branchcode parameter. GetMemberDetails does a lot of useless calls/queries (patronflags, account, etc.). This call could be replaced with a call to C4::Members::GetMember. Note: I presented this small optimisation during a quick introduction to NYTProf (hackfest 14 in Marseille). Test plan: - launch member unit tests - check the code Checking the code resulted in the following: CanBookBeRenewed builds a hash reference from the borrowernumber (2482). Note it is only used in this function and not passed in. _GetCircControlBranch (2485) requires that hashreference to have a branchcode key. As stated above. The following line (2486) requires it have a categorycode key. As such, C4::Members::GetMemberDetails is confirmed to be overkill, and C4::Members::GetMember is sufficient. Testing Done ------------ 0) Back up DB 1) Make sure MPL is in the list of libraries. 2) Apply the patch. 3) run the koha qa test tool 4) prove -v t/db_dependent/Circulation.t Patch applies cleanly. QA Test tool was all OK. All tests ran successfully. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26479|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27026 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27026&action=edit Bug 11960: GetMemberDetails is unnecessarily called in CanBookBeRenewed C4::Circulation::CanBookBeRenewed calls C4::Members::GetMemberDetails to retrieve categorycode and branchcode. - categorycode is used to retrieve the issuing rule - the borrower information is passed to C4::Circulation::_GetCircControlBranch. Which only uses the branchcode parameter. GetMemberDetails does a lot of useless calls/queries (patronflags, account, etc.). This call could be replaced with a call to C4::Members::GetMember. Note: I presented this small optimisation during a quick introduction to NYTProf (hackfest 14 in Marseille). Test plan: - launch member unit tests - check the code Checking the code resulted in the following: CanBookBeRenewed builds a hash reference from the borrowernumber (2482). Note it is only used in this function and not passed in. _GetCircControlBranch (2485) requires that hashreference to have a branchcode key. As stated above. The following line (2486) requires it have a categorycode key. As such, C4::Members::GetMemberDetails is confirmed to be overkill, and C4::Members::GetMember is sufficient. Testing Done ------------ 0) Back up DB 1) Make sure MPL is in the list of libraries. 2) Apply the patch. 3) run the koha qa test tool 4) prove -v t/db_dependent/Circulation.t Patch applies cleanly. QA Test tool was all OK. All tests ran successfully. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #5 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11960 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #6 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Pushed to 3.14.x, will be in 3.14.10 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org