Bug 29271 - Cash register report not displaying or exporting correctly
Summary: Cash register report not displaying or exporting correctly
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-19 13:02 UTC by Nick Clemens
Modified: 2021-10-23 02:10 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.05


Attachments
Bug 29271: Fix cash register report results (7.67 KB, patch)
2021-10-19 13:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29271: Fix cash register report results (7.73 KB, patch)
2021-10-19 14:12 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29271: (QA follow-up) Add missing filter (1.04 KB, patch)
2021-10-19 21:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29271: (QA follow-up) Fetch and send descriptions to template (5.05 KB, patch)
2021-10-20 14:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29271: Fix cash register report results (7.83 KB, patch)
2021-10-20 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29271: (QA follow-up) Add missing filter (1.10 KB, patch)
2021-10-20 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29271: (QA follow-up) Fetch and send descriptions to template (5.13 KB, patch)
2021-10-20 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29271: (follow-up) Remove changes to accounts.inc (1.19 KB, patch)
2021-10-21 10:03 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-10-19 13:02:47 UTC
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
Comment 1 Nick Clemens 2021-10-19 13:06:33 UTC
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
Comment 2 Nick Clemens 2021-10-19 13:07:40 UTC
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)
Comment 3 Andrew Fuerste-Henry 2021-10-19 14:12:59 UTC
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>
Comment 4 Katrin Fischer 2021-10-19 21:11:34 UTC
Created attachment 126548 [details] [review]
Bug 29271: (QA follow-up) Add missing filter
Comment 5 Katrin Fischer 2021-10-19 21:12:26 UTC
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?
Comment 6 Nick Clemens 2021-10-20 14:16:16 UTC
Created attachment 126590 [details] [review]
Bug 29271: (QA follow-up) Fetch and send descriptions to template
Comment 7 Nick Clemens 2021-10-20 14:17:22 UTC
(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
Comment 8 Katrin Fischer 2021-10-20 20:58:07 UTC
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>
Comment 9 Katrin Fischer 2021-10-20 20:58:11 UTC
Created attachment 126612 [details] [review]
Bug 29271: (QA follow-up) Add missing filter

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2021-10-20 20:58:15 UTC
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>
Comment 11 Jonathan Druart 2021-10-21 08:14:54 UTC
This code must be moved to a module..
Comment 12 Jonathan Druart 2021-10-21 08:33:21 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 13 Jonathan Druart 2021-10-21 09:52:16 UTC
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.
Comment 14 Nick Clemens 2021-10-21 10:03:51 UTC
Created attachment 126655 [details] [review]
Bug 29271: (follow-up) Remove changes to accounts.inc
Comment 15 Nick Clemens 2021-10-21 10:04:29 UTC
(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
Comment 16 Jonathan Druart 2021-10-21 10:27:06 UTC
(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!
Comment 17 Kyle M Hall 2021-10-22 15:27:55 UTC
Pushed to 21.05.x for 21.05.05
Comment 18 Fridolin Somers 2021-10-23 02:10:30 UTC
Conflics on 20.11.x, mainly because of Bug 16486