[Bug 11229] New: Payments wrongly saved in Statistics table by C4::Accounts::makepartialpayment
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Bug ID: 11229 Summary: Payments wrongly saved in Statistics table by C4::Accounts::makepartialpayment Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: mathieu.saby@univ-rennes2.fr QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com C4::Accounts::makepartialpayment save the transaction in statistics table, by calling UpdateStats. The fist given param is "$user". It should be "$branch" M. Saby -- 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=11229 --- Comment #1 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Same issue in makepartialpayment. Both subs are called by members/paycollect.pl We don't use fines, so I don't know if the branch to save is the homebranch of the user, or of the librarian. M. Saby -- 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=11229 --- Comment #2 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- I meant "makepayment" ;-) -- 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=11229 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Payments wrongly saved in |Payments wrongly saved in |Statistics table by |Statistics table by |C4::Accounts::makepartialpa |C4::Accounts::makepartialpa |yment |yment and makepayment -- 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=11229 mathieu saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11230 -- 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=11229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 8876 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Severity|minor |normal --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I still see the missing branchcodes in my database: select branch, datetime, type, proccode from statistics where type= 'payment' order by datetime desc; This looks a bit suspicious: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Accounts.pm;hb=... -- 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=11229 Cindy Murdock Ames <cmurdock@ccfls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cmurdock@ccfls.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small 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=11229 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 39971 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39971&action=edit Bug 11229: branch missing from statistics of type 'payment' There are several ways of paying fines, not all of them recorded the branch the payment was made at correctly: 1) Pay button next to an individual fine 2) Checkbox + pay amount 3) Checkbox + pay selected For each of those, check if the payment registered in the table statistics contains the branchcode before and after the patch. The patch should fix 1). 2) and 3) are ok with and without the patch. Important: Don't change the amount, pay individual fines. -- 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=11229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |katrin.fischer@bsz-bw.de |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=11229 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com Status|Needs Signoff |In Discussion --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- This patch is insufficient in my mind. Read the C4::Account code to which you linked. recordpayment sets $branch to logged in user's branch. Function seems to be SIP based on comments, so should be user paying own fine, right? recordpayment_selectaccts sets $branch to logged in user's branch. Function seems to be SIP based on comments, so should be user paying own fine, right? makepayment takes $branch as a parameter, but then uses $user in the UpdateStats call?! This patch fixes that. makepartialpayment takes $branch as a parameter, but then uses $user in the UpdateStats call?! This patch misses this. WriteOffFee takes $branch as a parameter, but if it is 0, '', or undef, it uses the logged in user's branch. It correctly uses $branch for UpdateStats. I think the handling used here should be duplicated in the makepayment and makepartialpayment functions. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 41473 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41473&action=edit Bug 11229: (regression tests) makepayment and makepartialpayment should log correctly This patch introduces tests for makepayment and makepartialpayment to check they are correctly logging to the statistics table. To test: - Run $ prove t/db_dependent/Accounts.t => FAIL: tests fail because statistics are not correctly logged. Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39971|0 |1 is obsolete| | --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 41474 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41474&action=edit Bug 11229: makepayment doesn't log branch in statistics correctly This patch fixes makepayment() so it correctly logs the branch code to the statistics table. To test: - Run (with the regression tests applied) $ prove t/db_dependent/Accounts.t => FAIL: makepayment() tests fail because logging is wrong - Apply the patch - Run $ prove t/db_dependent/Accounts.t => SUCCESS: tests for makepayment now pass (Note: makepartialpayment tests still fail as they need the next patch) Original commit message relying on the UI to test: There are several ways of paying fines, not all of them recorded the branch the payment was made at correctly: 1) Pay button next to an individual fine 2) Checkbox + pay amount 3) Checkbox + pay selected For each of those, check if the payment registered in the table statistics contains the branchcode before and after the patch. The patch should fix 1). 2) and 3) are ok with and without the patch. Important: Don't change the amount, pay individual fines. Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Note: i fixed the commit message, because this is a fix for makepayment -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 41475 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41475&action=edit Bug 11229: (followup) makepartialpayment fix makepartialpayment() wasn't logging the branch either. This patch fixes that too. To test: - Run (with the regression tests applied) $ prove t/db_dependent/Accounts.t => FAIL: tests fail because makepartialpayment is not logging the branch code - Apply the patch - Run $ prove t/db_dependent/Accounts.t => SUCCESS: tests pass - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Summary|Payments wrongly saved in |makepayment and |Statistics table by |makepartialpayment |C4::Accounts::makepartialpa |(C4::Accounts) don't log |yment and makepayment |branch to statistics table Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=11229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41473|0 |1 is obsolete| | Attachment #41474|0 |1 is obsolete| | Attachment #41475|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41619 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41619&action=edit Bug 11229: (regression tests) makepayment and makepartialpayment should log correctly This patch introduces tests for makepayment and makepartialpayment to check they are correctly logging to the statistics table. To test: - Run $ prove t/db_dependent/Accounts.t => FAIL: tests fail because statistics are not correctly logged. Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41620 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41620&action=edit Bug 11229: makepayment doesn't log branch in statistics correctly This patch fixes makepayment() so it correctly logs the branch code to the statistics table. To test: - Run (with the regression tests applied) $ prove t/db_dependent/Accounts.t => FAIL: makepayment() tests fail because logging is wrong - Apply the patch - Run $ prove t/db_dependent/Accounts.t => SUCCESS: tests for makepayment now pass (Note: makepartialpayment tests still fail as they need the next patch) Original commit message relying on the UI to test: There are several ways of paying fines, not all of them recorded the branch the payment was made at correctly: 1) Pay button next to an individual fine 2) Checkbox + pay amount 3) Checkbox + pay selected For each of those, check if the payment registered in the table statistics contains the branchcode before and after the patch. The patch should fix 1). 2) and 3) are ok with and without the patch. Important: Don't change the amount, pay individual fines. Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Note: i fixed the commit message, because this is a fix for makepayment Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41621 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41621&action=edit Bug 11229: (followup) makepartialpayment fix makepartialpayment() wasn't logging the branch either. This patch fixes that too. To test: - Run (with the regression tests applied) $ prove t/db_dependent/Accounts.t => FAIL: tests fail because makepartialpayment is not logging the branch code - Apply the patch - Run $ prove t/db_dependent/Accounts.t => SUCCESS: tests pass - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41622 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41622&action=edit Bug 11229: Make the tests pass even if issues exist in the DB This is not introduced by the previous patches of this patchset. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41623 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41623&action=edit Bug 11229: Remove error rollback ineffective with AutoCommit enabled The full error is: rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1621. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #14)
Created attachment 41623 [details] [review] Bug 11229: Remove error rollback ineffective with AutoCommit enabled
The full error is: rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1621.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
RM: Feel free to skip this one if you disagree. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #15)
(In reply to Jonathan Druart from comment #14)
Created attachment 41623 [details] [review] [review] Bug 11229: Remove error rollback ineffective with AutoCommit enabled
The full error is: rollback ineffective with AutoCommit enabled at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1621.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
RM: Feel free to skip this one if you disagree.
As usual ;) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41623|0 |1 is obsolete| | --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Comment on attachment 41623 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41623 Bug 11229: Remove error rollback ineffective with AutoCommit enabled The problem is generated by how TestBuilder is coded. We should fix it instead. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks everyone! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org