Bug 7060

Summary: All instances of GetMemberDetails are unnecessary in C4::Circulation
Product: Koha Reporter: Ian Walls <koha.sekjal>
Component: Architecture, internals, and plumbingAssignee: Ian Walls <koha.sekjal>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: blocker    
Priority: PATCH-Sent (DO NOT USE) CC: chris, nengard
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 7058    
Attachments: Proposed patch
[SIGNED-OFF] Bug 7060: GetMemberDetails unnecessary in C4::Circulation
Followup Patch

Description Ian Walls 2011-10-18 23:50:54 UTC
GetMemberDetails is used in the following C4::Circulation subroutines

CanBookBeIssued:  used twice, both times only cardnumber, surname, firstname, and borrowernumber are used, which are all accessible more efficiently from GetMember

AddReturn:  only borrowernumber and cardnumber are used.  the $borrower hashref is returned by AddReturn, but it is only captured in tools/inventory.pl and C4/SIP/ILS/Transaction/Checkin.pm, and in neither place is anything done with it.  GetMember could be used here without any issue

AddRenewal: uses only passes the $borrower object to CalcDateDue, which only uses categorycode and dateexpiry (both covered by GetMember hashref)

AddIssue:  uses an incoming $borrower object, but only the borrowernumber and categorycode fields, so we only need a GetMember object here.

GetRenewalCount:  only uses categorycode

Changing all calls of GetMemberDetails to GetMember in C4/Circulation.pm should be safe, and vastly more efficient
Comment 1 Ian Walls 2011-10-19 00:00:45 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2011-10-19 19:48:40 UTC
Created attachment 5996 [details] [review]
[SIGNED-OFF] Bug 7060: GetMemberDetails unnecessary in C4::Circulation

Replaces all instances of GetMemberDetails with the more efficient GetMember.  Since
no hash values other than the borrowers table's fields are used in this module, the
transition is safe.

To test:

1.  Checkout a material; the page should load without error
2.  Renew a material: the page should load without error
3.  Return a material: the page should load without error

The above test plan invokes all the subroutines affected by this patch; any call to an undefined
value in a hashref should give a warning

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 3 Ian Walls 2011-10-19 22:18:16 UTC
Marking Passed QA, RM please verify
Comment 4 Chris Cormack 2011-10-20 01:10:28 UTC
Pushed, please test
Comment 5 Ian Walls 2011-10-21 14:47:30 UTC
I made a mistake.  $borrower->{flags} are used by AddReturn in circ/returns.pl.  GetMemberDetails must be used, otherwise returns break.  working on fix now...
Comment 6 Ian Walls 2011-10-21 14:54:38 UTC
Created attachment 6017 [details] [review]
Followup Patch

Restores GetMemberDetails usage in AddReturn; since that subroutine returned the borrower object, it was a mistake to change to GetMember, since key things like flags are not returned in that hashref.
Comment 7 Chris Cormack 2011-10-21 15:09:13 UTC
Follow up pushed