To recreate: 1 - Add some debits and credits to accounts 2 - Browse to Reports->Cash register 3 - Select transaction type 'All transactions' 4 - Output to screen 5 - Run report 6 - Note transaction types column is empty 7 - Output to a file 8 - Run report 9 - Open file and note: amount column is out of place extra line before total
Created attachment 126498 [details] [review] Bug 29271: Fix cash register report results This patch changes accounts.inc to use the credit/debit type codes directly rather than needlessly fetching the credit_type/debit_type object jsut to get the code. This allows the BLOCK to work on non-object lines We also only pass a debit or credit type to csv (accountline cannot be both) empty-line.inc is also removed in favor of correcting the markup one more column added to total line To test: 1 - Add some debits and credits to accounts 2 - Browse to Reports->Cash register 3 - Select transaction type 'All transactions' 4 - Output to screen 5 - Run report 6 - Note transaction types column is empty 7 - Output to a file 8 - Run report 9 - Open file and note: amount column is out of place extra line before total 10 - Apply patch 11 - Repeat and note display and export are correct
Note, this affects stables too, but they made need patches for their own version as the issues are slightly different (in 20.05 at least)
Created attachment 126505 [details] [review] Bug 29271: Fix cash register report results This patch changes accounts.inc to use the credit/debit type codes directly rather than needlessly fetching the credit_type/debit_type object jsut to get the code. This allows the BLOCK to work on non-object lines We also only pass a debit or credit type to csv (accountline cannot be both) empty-line.inc is also removed in favor of correcting the markup one more column added to total line To test: 1 - Add some debits and credits to accounts 2 - Browse to Reports->Cash register 3 - Select transaction type 'All transactions' 4 - Output to screen 5 - Run report 6 - Note transaction types column is empty 7 - Output to a file 8 - Run report 9 - Open file and note: amount column is out of place extra line before total 10 - Apply patch 11 - Repeat and note display and export are correct Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 126548 [details] [review] Bug 29271: (QA follow-up) Add missing filter
This is definitely a bug fix and improvement. I notice that the table displays the descriptions of the transaction types, but the export only lists the codes. Could we switch the export to descriptions too please?
Created attachment 126590 [details] [review] Bug 29271: (QA follow-up) Fetch and send descriptions to template
(In reply to Katrin Fischer from comment #5) > This is definitely a bug fix and improvement. I notice that the table > displays the descriptions of the transaction types, but the export only > lists the codes. Could we switch the export to descriptions too please? Done, I was wrong before, we use the objects for the case where the type is not a system defined one. This report should eventually be updated to use object, for now I just fetch the description during the SQL call for simplicity/backportability
Created attachment 126611 [details] [review] Bug 29271: Fix cash register report results This patch changes accounts.inc to use the credit/debit type codes directly rather than needlessly fetching the credit_type/debit_type object jsut to get the code. This allows the BLOCK to work on non-object lines We also only pass a debit or credit type to csv (accountline cannot be both) empty-line.inc is also removed in favor of correcting the markup one more column added to total line To test: 1 - Add some debits and credits to accounts 2 - Browse to Reports->Cash register 3 - Select transaction type 'All transactions' 4 - Output to screen 5 - Run report 6 - Note transaction types column is empty 7 - Output to a file 8 - Run report 9 - Open file and note: amount column is out of place extra line before total 10 - Apply patch 11 - Repeat and note display and export are correct Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 126612 [details] [review] Bug 29271: (QA follow-up) Add missing filter Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 126613 [details] [review] Bug 29271: (QA follow-up) Fetch and send descriptions to template Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
This code must be moved to a module..
Pushed to master for 21.11, thanks to everybody involved!
Caused an unexpected failure, please fix ASAP: t/db_dependent/Letters/TemplateToolkit.t .. 25/28 # Failed test 'Include used in notice' # at t/db_dependent/Letters/TemplateToolkit.t line 1120. # got: ' <span><span> (Cancelled)</span> </span>' # expected: ' <span>Payment<span> (Cancelled)</span> </span>' # Looks like you failed 1 test of 1.
Created attachment 126655 [details] [review] Bug 29271: (follow-up) Remove changes to accounts.inc
(In reply to Jonathan Druart from comment #13) > Caused an unexpected failure, please fix ASAP: > > t/db_dependent/Letters/TemplateToolkit.t .. 25/28 > # Failed test 'Include used in notice' > # at t/db_dependent/Letters/TemplateToolkit.t line 1120. > # got: ' <span><span> (Cancelled)</span> </span>' > # expected: ' <span>Payment<span> (Cancelled)</span> </span>' > # Looks like you failed 1 test of 1. I missed a variable when undoing my changes
(In reply to Nick Clemens from comment #14) > Created attachment 126655 [details] [review] [review] > Bug 29271: (follow-up) Remove changes to accounts.inc Pushed to master, thanks!
Pushed to 21.05.x for 21.05.05
Conflics on 20.11.x, mainly because of Bug 16486
Created attachment 133084 [details] [review] [20.11] Bug 29271: Fix cash register report results This patch changes accounts.inc to use the credit/debit type codes directly rather than needlessly fetching the credit_type/debit_type object jsut to get the code. This allows the BLOCK to work on non-object lines We also only pass a debit or credit type to csv (accountline cannot be both) empty-line.inc is also removed in favor of correcting the markup one more column added to total line To test: 1 - Add some debits and credits to accounts 2 - Browse to Reports->Cash register 3 - Select transaction type 'All transactions' 4 - Output to screen 5 - Run report 6 - Note transaction types column is empty 7 - Output to a file 8 - Run report 9 - Open file and note: amount column is out of place extra line before total 10 - Apply patch 11 - Repeat and note display and export are correct Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133085 [details] [review] [20.11] Bug 29271: (QA follow-up) Add missing filter Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133086 [details] [review] [20.11] Bug 29271: (QA follow-up) Fetch and send descriptions to template Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133087 [details] [review] [20.11] Bug 29271: (follow-up) Remove changes to accounts.inc
Tried my hand at a rebase, with it things seem to be ok in the latest 20.11.17.
Thanks Katrin. Question also in regard other branches: Is that expected that the patches add spaces? Before patch: Manager name;Patron cardnumber;Patron name;Transaction library;Transaction date;Transaction type;Notes;Amount;Title;Barcode;Item type koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;;LOST;;1.00;;; koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;;LOST;;3.00;;; koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;CREDIT;;;-343.00;;; koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;;LOST;;112.00;;; TOTAL ;;;;;;-227.00 After patch: Manager name;Patron cardnumber;Patron name;Transaction library;Transaction date;Transaction type;Notes;Amount;Title;Barcode;Item type koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;Lost item;;1.00;;; koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;Lost item;;3.00;;; koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;Credit;;-343.00;;; koha;eda;Eda Eda;Centerville;2022-04-07T00:00:00;Lost item;;112.00;;; TOTAL;;;;;;;-227.00
I didn't touch that file so I'd say no. Could some '-' in the variables be missing?
> Could some '-' in the variables be missing? Is that about template toolkit whitespace chomping? http://www.template-toolkit.org/docs/manual/Syntax.html#section_Chomping_Whitespace Anyway, is that an issue? Can that cause issues to consume the export?
> Anyway, is that an issue? Assuming it's not. Backported: Pushed to 20.11.x branch for 20.11.19
(In reply to Victor Grousset/tuxayo from comment #26) > > Could some '-' in the variables be missing? > > Is that about template toolkit whitespace chomping? > http://www.template-toolkit.org/docs/manual/Syntax. > html#section_Chomping_Whitespace > > Anyway, is that an issue? Can that cause issues to consume the export? Hi Victor, sorry for not getting back to you about this earlier. I assume since this also happens on the other stable branches, it would be best to fix it on a separate bug. Thx for pushing!
Does this need to be backported to 19.11.x?
It looks like we might be missing the line breaks when using the exports in another language - I feel like we already fixed this before?
(In reply to Katrin Fischer from comment #30) > It looks like we might be missing the line breaks when using the exports in > another language - I feel like we already fixed this before? bug 30704?
(In reply to Jonathan Druart from comment #31) > (In reply to Katrin Fischer from comment #30) > > It looks like we might be missing the line breaks when using the exports in > > another language - I feel like we already fixed this before? > > bug 30704? Now we overlapped, actually this one is more 29968. The whitespace within the fields is another issue.