[Bug 12868] New: GetMemberDetails calls GetMemberAccountRecords incorrectly
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Bug ID: 12868 Summary: GetMemberDetails calls GetMemberAccountRecords incorrectly Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com GetMemberDetails takes two parameters: borrowernumber and cardnumber. Between the two of them, a borrower hash is created. This borrower hash has a borrower number in it as well as a card number. However, the outstanding amount is based on the borrowernumber parameter and not the discovered borrowernumber based on cardnumber when borrowernumber is passed as undef. It should use the borrower hash's borrower number instead. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 31354 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31354&action=edit Bug 12868: Improving t/db_dependent/Member.t The mock function of GetMemberAccountRecord did not properly account for the undef case. This was corrected. Then all 4 combinations of borrower number and card number being defined or not were called to GetMemberDetail. The problematic test case is where the borrower number is undefined and the cardnumber is defined. TEST PLAN --------- 1) Apply just this first patch. 2) prove -v t/db_dependent/Member.t -- This should fail! 3) Run koha QA test tools. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 31355 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31355&action=edit Bug 12868: Wrong variable used for borrower number When only the card number is passed to GetMemberDetail, the value of $borrowernumber is undefined. Even after finding the correct borrower and providing a nice hash ($borrower), the GetMemberAccountRecords is called with the wrong borrower number, even though it is in the hash ($borrower). This was fixed by changing $borrowernumber to $borrower->{borrowernumber}, so that the hash's value will always be used, since it is correct regardless of whether borrowernumber or cardnumber were used to find the borrower. TEST PLAN --------- 1) Apply both patches 2) prove -v t/db_dependent/Member.t -- This time the previously failing test will pass. 3) run koha QA test tools. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #3 from Frédéric Demians <frederic@tamil.fr> --- Now, on HEAD: prove -v t/db_dependent/Members.t fails, without applying the first patch. So, I can't follow the test plan. Otherwise, the second patch, fixing GetMemberDetails, make sense. IMHO it could be signed-off as it is, even without a test case... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Frederic, the tests pass for me on a MARC21 database with the sample database data on current master - maybe it looks for some category codes that are not in your database or similar? Some of the db_dependent tests rely heavily on the sample data. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |mtompset@hotmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Frédéric Demians from comment #3)
Now, on HEAD:
prove -v t/db_dependent/Members.t
fails, without applying the first patch. So, I can't follow the test plan.
Otherwise, the second patch, fixing GetMemberDetails, make sense. IMHO it could be signed-off as it is, even without a test case...
Could you past the screen shot of this failing when you only apply the second patch? I did try that case just now, and this is working for me as expected. no patch: pass as expected. patch 1 only: 1 failure as expected. patch 2 only: pass as expected. patch 1&2: pass as expected. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- (In reply to M. Tompsett from comment #5)
(In reply to Frédéric Demians from comment #3)
Now, on HEAD: prove -v t/db_dependent/Members.t fails ...
Nasty, I booted up my UNIMARC VM, updated to the latest origin/master, and ran the prove. KABOOM! Turns out my categories table only has 'BIB', but line 873 in C4/Members.pm is trying to find 'S'. Which makes that variable undefined and a nasty kaboom! Added S and PT. Retried. KABOOM! Branchcode issues. Added CPL, MPL. Retried. Success. Okay. Nothing applied. Success. Both applies. Success. Only 2 applied. Success. Only 1 applied. Expected failure. So, if you ensure that you have these Branch codes and Categories added, and then attempt the test plans in comment #1 and comment #2, there should be no problem. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- I'm not sure this is really an enhancement, since this is more of a bug fix. The testing is "enhanced", but that's only because it didn't catch the case that was failing. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Mark, Looking at the code, I completely agree with your patch. How did you find this problem? Is there a place where an error/problem is visible on the interface? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Greetings. I can't remember why, but I suspect it was due to some failures I was encountering in t/db_dependent/Members.t attempting to integrate a local modification into Koha, and when I started tracing GetMemberDetails, that problem jumped out at me. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31354|0 |1 is obsolete| | Attachment #31355|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33942 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33942&action=edit Bug 12868: Improving t/db_dependent/Member.t The mock function of GetMemberAccountRecord did not properly account for the undef case. This was corrected. Then all 4 combinations of borrower number and card number being defined or not were called to GetMemberDetail. The problematic test case is where the borrower number is undefined and the cardnumber is defined. TEST PLAN --------- 1) Apply just this first patch. 2) prove -v t/db_dependent/Member.t -- This should fail! 3) Run koha QA test tools. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33943 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33943&action=edit Bug 12868: Wrong variable used for borrower number When only the card number is passed to GetMemberDetail, the value of $borrowernumber is undefined. Even after finding the correct borrower and providing a nice hash ($borrower), the GetMemberAccountRecords is called with the wrong borrower number, even though it is in the hash ($borrower). This was fixed by changing $borrowernumber to $borrower->{borrowernumber}, so that the hash's value will always be used, since it is correct regardless of whether borrowernumber or cardnumber were used to find the borrower. TEST PLAN --------- 1) Apply both patches 2) prove -v t/db_dependent/Member.t -- This time the previously failing test will pass. 3) run koha QA test tools. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Patrons |Architecture, internals, | |and plumbing -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 M. de Rooy <m.de.rooy@rijksmuseum.nl> 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=12868 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33942|0 |1 is obsolete| | Attachment #33943|0 |1 is obsolete| | --- Comment #12 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 34314 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34314&action=edit Bug 12868: Improving t/db_dependent/Member.t The mock function of GetMemberAccountRecord did not properly account for the undef case. This was corrected. Then all 4 combinations of borrower number and card number being defined or not were called to GetMemberDetail. The problematic test case is where the borrower number is undefined and the cardnumber is defined. TEST PLAN --------- 1) Apply just this first patch. 2) prove -v t/db_dependent/Member.t -- This should fail! 3) Run koha QA test tools. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 --- Comment #13 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 34315 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34315&action=edit Bug 12868: Wrong variable used for borrower number When only the card number is passed to GetMemberDetail, the value of $borrowernumber is undefined. Even after finding the correct borrower and providing a nice hash ($borrower), the GetMemberAccountRecords is called with the wrong borrower number, even though it is in the hash ($borrower). This was fixed by changing $borrowernumber to $borrower->{borrowernumber}, so that the hash's value will always be used, since it is correct regardless of whether borrowernumber or cardnumber were used to find the borrower. TEST PLAN --------- 1) Apply both patches 2) prove -v t/db_dependent/Member.t -- This time the previously failing test will pass. 3) run koha QA test tools. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Mark! AGJ345 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Status|Passed QA |Pushed to Stable --- Comment #15 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.18.x will be in 3.18.3 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12868 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #16 from Mason James <mtj@kohaaloha.com> --- Pushed to 3.16.x, will be in 3.16.7 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org