[Bug 24820] New: The cashup workflow should use the static 'date' field rather than the transient 'timestamp' field in accountlines
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Bug ID: 24820 Summary: The cashup workflow should use the static 'date' field rather than the transient 'timestamp' field in accountlines Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Fines and fees Assignee: martin.renvoize@ptfs-europe.com Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org The 'cashup' process introduced in bug 23355 assumes that once set, the timestamp of an accountline is fixed. This assumption is incorrect as the timestamp is a 'last updated' time and as such is transient. We should be using the 'date' field for comparisons instead as this field is a fixed 'created on' record. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24818 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24818 [Bug 24818] Accountline creation dates should be datetimes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 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 watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 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=24820 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100265 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100265&action=edit Bug 24820: Use 'date' instead of 'timestamp' for cashup Prior to this patch we were comparing cash register action timestamp to accountlines date fields to fetch the list of outstanding accountlines. accountlines.date is a transient field used as 'updated_on' and so this was a logical mistake. We should be using the 'date' field which is fixed 'created_on' time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 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=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #2 from David Nind <david@davidnind.com> --- Test plan? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This really just corrects an error in the original bug 23355. Because we were using 'timestamp', which is the 'last updated' date, if a credit action was later modified somehow.. partial application to a second debit perhaps, then the cashup report would display it again. Using the 'created on' date instead, means the data is more consistently displayed in the correct cashup period. I'm not really sure how to prove this in a test plan however... hmm.. perhaps a new unit test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #4 from David Nind <david@davidnind.com> --- Patch no longer applies - was going to just check that the cashup process worked. Applying: Bug 24820: Use 'date' instead of 'timestamp' for cashup Using index info to reconstruct a base tree... M Koha/Cash/Register.pm Falling back to patching base and 3-way merge... Auto-merging Koha/Cash/Register.pm CONFLICT (content): Merge conflict in Koha/Cash/Register.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 24820: Use 'date' instead of 'timestamp' for cashup -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100265|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102080 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102080&action=edit Bug 24820: Use 'date' instead of 'timestamp' for cashup Prior to this patch we were comparing cash register action timestamp to accountlines date fields to fetch the list of outstanding accountlines. accountlines.date is a transient field used as 'updated_on' and so this was a logical mistake. We should be using the 'date' field which is fixed 'created_on' time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Rebased -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #7 from David Nind <david@davidnind.com> --- Thanks Martin! I'm happy to sign off, however I'm getting a database error if I do the cashup from the Point of sale > Branch details screen (any of the buttons), but I don't get it when doing a cashup from the Point of sale > Register details screen. I get this before the patch applies, so it is not related to this bug. I'm also getting an error when I try to display older transactions on the Point of sale > Register details screen as well. Should this be logged as a separate bug(s), or is there a way to fix it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The bug with cashup on the branch details page has now been corrected in master. Well spotted, it was a rebase error. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 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=24820 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102080|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 102465 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102465&action=edit Bug 24820: Use 'date' instead of 'timestamp' for cashup Prior to this patch we were comparing cash register action timestamp to accountlines date fields to fetch the list of outstanding accountlines. accountlines.date is a transient field used as 'updated_on' and so this was a logical mistake. We should be using the 'date' field which is fixed 'created_on' time. To test, make sure the cash register and record cashup features works as expected. Test plan: 1) Enable EnablePointOfSale and UseCashRegisters system preferences. 2) Add a cash register (Koha Administration > Accounting > Cash registers > New cash register). 3) From the Point of sale screen add two new items for purchase (Point of sale > Administration > Configure items for purchase > add new debit types). 4) Add some transactions. 5) Record a cashup (Register details > Record cashup). 6) Repeat steps 4-5 at least once. 7) Everyting should work as expected. 8) Sign off! 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=24820 --- Comment #10 from David Nind <david@davidnind.com> --- Thanks Martin for fixing the cashup error!
I'm also getting an error when I try to display older transactions on the Point of sale > Register details screen as well.
There is still an error occurring in the 'Older transactions' section for the point of sale system on the register details screen - this error occurs when you select a date range for transactions: Template process failed: undef error - The given date (04/06/2020) does not match the date format (iso) at /kohadevbox/koha/Koha/DateUtils.pm line 168. at /kohadevbox/koha/C4/Templates.pm line 122 in C4::Templates::output at /kohadevbox/koha/C4/Templates.pm line 122 119: 120: my $data; 121: binmode( STDOUT, ":utf8" ); 122: $template->process( $self->filename, $vars, \$data ) 123: || die "Template process failed: ", $template->error(); 124: return $data; 125: } Show function arguments in (eval) at /kohadevbox/koha/pos/register.pl line 146 143: } 144: } 145: 146: output_html_with_http_headers( $input, $cookie, $template->output ); in CGI::Compile::ROOT::kohadevbox_koha_pos_register_2epl::__ANON__ at /kohadevbox/koha/pos/register.pl line 2 1: #!/usr/bin/perl ..... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25139 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to David Nind from comment #10)
Thanks Martin for fixing the cashup error!
I'm also getting an error when I try to display older transactions on the Point of sale > Register details screen as well.
There is still an error occurring in the 'Older transactions' section for the point of sale system on the register details screen - this error occurs when you select a date range for transactions:
Template process failed: undef error - The given date (04/06/2020) does not match the date format (iso) at /kohadevbox/koha/Koha/DateUtils.pm line 168. at /kohadevbox/koha/C4/Templates.pm line 122 in C4::Templates::output at /kohadevbox/koha/C4/Templates.pm line 122 119: 120: my $data; 121: binmode( STDOUT, ":utf8" ); 122: $template->process( $self->filename, $vars, \$data ) 123: || die "Template process failed: ", $template->error(); 124: return $data; 125: } Show function arguments
in (eval) at /kohadevbox/koha/pos/register.pl line 146 143: } 144: } 145: 146: output_html_with_http_headers( $input, $cookie, $template->output ); in CGI::Compile::ROOT::kohadevbox_koha_pos_register_2epl::__ANON__ at /kohadevbox/koha/pos/register.pl line 2 1: #!/usr/bin/perl .....
I believe this is a separate bug - I have the same broken behaviour without the patch. I filed a new bug report: new Bug 25139 - POS explodes in error when trying to display older transactions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Ok, it might be late, but why only change one timestamp to date? - ? { 'timestamp' => { '>' => $since->get_column('timestamp')->as_query } } + ? { 'date' => { '>' => $since->get_column('timestamp')->as_query } } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #12)
Ok, it might be late, but why only change one timestamp to date?
- ? { 'timestamp' => { '>' => $since->get_column('timestamp')->as_query } } + ? { 'date' => { '>' => $since->get_column('timestamp')->as_query } }
Sorry Katrin, not sure I understand your question there? 'date' is the field in the accountlines table and 'timestamp' is the field in the cash_register_actions table. Are you suggesting I update the cash_register_actions table to have a field name of 'date' to mate the accountlines 'date' field name? If we're thinking of doing that I'd actually prefer to change both tables to 'created' or similar to denote exactly their use but I felt that's outside of scope for this bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Patch now attached to bug 25139 for the aforementioned previous transactions issue. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Martin Renvoize from comment #13)
(In reply to Katrin Fischer from comment #12)
Ok, it might be late, but why only change one timestamp to date?
- ? { 'timestamp' => { '>' => $since->get_column('timestamp')->as_query } } + ? { 'date' => { '>' => $since->get_column('timestamp')->as_query } }
Sorry Katrin, not sure I understand your question there?
'date' is the field in the accountlines table and 'timestamp' is the field in the cash_register_actions table.
Are you suggesting I update the cash_register_actions table to have a field name of 'date' to mate the accountlines 'date' field name? If we're thinking of doing that I'd actually prefer to change both tables to 'created' or similar to denote exactly their use but I felt that's outside of scope for this bug
Just me being confused - it didn't occur to me we were looking at values from 2 different tables here... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102465|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102950 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102950&action=edit Bug 24820: Use 'date' instead of 'timestamp' for cashup Prior to this patch we were comparing cash register action timestamp to accountlines date fields to fetch the list of outstanding accountlines. accountlines.date is a transient field used as 'updated_on' and so this was a logical mistake. We should be using the 'date' field which is fixed 'created_on' time. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 --- Comment #17 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=24820 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102986 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102986&action=edit Bug 24820: (RM follow-up) Fix test after switch to 'date' field 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=24820 --- Comment #19 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=24820 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #20 from Joy Nelson <joy@bywatersolutions.com> --- missing dependencies - not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26172 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26172 [Bug 26172] Add a cashup summary view -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|26172 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26172 [Bug 26172] Add a cashup summary view -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org