[Bug 24815] New: Koha::Cash::Register relations should return sets not undef
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Bug ID: 24815 Summary: Koha::Cash::Register relations should return sets not undef Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org The cash register class contains a bad pattern of returning 'undef' for relations instead of an empty resultset. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23321 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23321 [Bug 23321] Add 'cash registers' to the accounts system -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100203 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100203&action=edit Bug 24815: Correct return values of Koha::Cash::Register relations This patch correct all cases of return undef to instead return an empty resultset. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23355 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23355 [Bug 23355] Add 'cashup' process to the accounts system -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I will double check tomorrow but I think the following one is not correct: @@ -86,7 +84,6 @@ sub last_cashup { { order_by => { '-desc' => [ 'timestamp', 'id' ] }, rows => 1 } )->single; - return unless $rs; return Koha::Cash::Register::Action->_new_from_dbic($rs); } -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #2)
I will double check tomorrow but I think the following one is not correct:
@@ -86,7 +84,6 @@ sub last_cashup { { order_by => { '-desc' => [ 'timestamp', 'id' ] }, rows => 1 } )->single;
- return unless $rs; return Koha::Cash::Register::Action->_new_from_dbic($rs); }
You are right. We return undef on the singular case -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100203|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100219&action=edit Bug 24815: Correct return values of Koha::Cash::Register relations This patch correct all cases of return undef to instead return an empty resultset. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100220&action=edit Bug 24815: Add additional tests -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24817 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24817 [Bug 24817] Add sleep calls to Koha::Cash::Register test -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13985 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13985 [Bug 13985] Cash Management - Koha as 'Point of Sale' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Jon Knight <J.P.Knight@lboro.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |J.P.Knight@lboro.ac.uk --- Comment #6 from Jon Knight <J.P.Knight@lboro.ac.uk> --- Trying it on an up-to-date koha-testing-docker with a clean branch off the current origin/master the tests don't apply: $ git bz apply 24815 Bug 24815 - Koha::Cash::Register relations should return sets not undef 100219 - Bug 24815: Correct return values of Koha::Cash::Register relations 100220 - Bug 24815: Add additional tests Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 24815: Correct return values of Koha::Cash::Register relations Applying: Bug 24815: Add additional tests Using index info to reconstruct a base tree... M t/db_dependent/Koha/Cash/Register.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Koha/Cash/Register.t CONFLICT (content): Merge conflict in t/db_dependent/Koha/Cash/Register.t error: Failed to merge in the changes. Patch failed at 0001 Bug 24815: Add additional tests The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-24815-Add-additional-tests-XWf3eu.patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100219|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 101907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101907&action=edit Bug 24815: Correct return values of Koha::Cash::Register relations This patch correct all cases of return undef to instead return an empty resultset. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100220|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 101908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101908&action=edit Bug 24815: Add additional tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Rebased, thanks for yesting Jon -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_05_target, | |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@bugs.koha-c |y.org |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101907|0 |1 is obsolete| | --- Comment #10 from David Nind <david@davidnind.com> --- Created attachment 103559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103559&action=edit Bug 24815: Correct return values of Koha::Cash::Register relations This patch correct all cases of return undef to instead return an empty resultset. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101908|0 |1 is obsolete| | --- Comment #11 from David Nind <david@davidnind.com> --- Created attachment 103560 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103560&action=edit Bug 24815: Add additional tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #12 from David Nind <david@davidnind.com> --- Tested by: 1) Running tests before and after patch applied (prove t/db_dependent/Koha/Cash/Register.t) and made sure they pass. 2) Setting up a cash register, adding items for purchase, adding some transactions, running a cashup, searching for previous transactions <-- all seemed to work as expected. Hope that this is sufficent for testing! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 103560 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103560 Bug 24815: Add additional tests Review of attachment 103560: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=24815&attachment=103560) ----------------------------------------------------------------- ::: t/db_dependent/Koha/Cash/Register.t @@ +249,5 @@
+ # add_cashup should not happen simultaneously with any other action + # that results in an accountline attached to the same cash register. + # In testing, we need to sleep for a second after each action that + # adds to the database. (We cannot use Time::Fake as timestamps are + # being added at the DB level, not in perl.
Hum, double check this, it has been removed previously. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103559|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103786 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103786&action=edit Bug 24815: Correct return values of Koha::Cash::Register relations This patch correct all cases of return undef to instead return an empty resultset. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103560|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103787&action=edit Bug 24815: Add additional tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 103788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103788&action=edit Bug 24815: (follow-up) Rebase correction -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- You are indeed correct, it came back through a bad rebase.. corrected in the followup -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103786|0 |1 is obsolete| | Attachment #103787|0 |1 is obsolete| | Attachment #103788|0 |1 is obsolete| | --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103790 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103790&action=edit Bug 24815: Correct return values of Koha::Cash::Register relations This patch correct all cases of return undef to instead return an empty resultset. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103791 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103791&action=edit Bug 24815: Add additional tests Signed-off-by: David Nind <david@davidnind.com> Bug 24815: (follow-up) Rebase correction Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_05_candidate, | |RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24815 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #21 from Joy Nelson <joy@bywatersolutions.com> --- not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org