Bug 29664

Summary: Do not show voided payments in cash register statistics wizard
Product: Koha Reporter: Andrew Fuerste-Henry <andrew>
Component: ReportsAssignee: Kyle M Hall (khall) <kyle>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: brittanie.maccarone, Laura.escamilla, m.de.rooy, martin.renvoize, nick, nugged
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08
Circulation function:
Bug Depends on:    
Bug Blocks: 34552    
Attachments: Bug 29664: Removing void payments from cash_register_stats
Bug 29664: Removing void payments from the cash register report
Bug 29664: Removing void payments from the cash register report
Bug 29664: Removing void payments from the cash register report
Bug 29664: Removing void payments from the cash register report
Bug 29664: Removing void payments from the cash register report
Bug 29664: WIP Test Patch
Bug 29664: Do not show voided payments in cash_register_stats.pl
Bug 29664: Do not show voided payments in cash_register_stats.pl
Bug 29664: Do not show voided payments in cash_register_stats.pl
Bug 29664: (QA follow-up) Typo transacton

Description Andrew Fuerste-Henry 2021-12-08 17:52:43 UTC
To recreate:
- take a payment of $1
- void that payment
- go to reports, Cash Register (cash_register_stats.pl) and run a report for all payments taken today, see your voided payment

We should not included voided credits, it throws off the totals.
Comment 1 Brittanie Maccarone 2022-04-27 16:20:02 UTC
We actively have a ticket for this issue, from Nov 2021.  Duplicates being shown in our reports is affecting the information that is available to show the county that controls our funds.  At the moment we are simply not using the function, and making notes elsewhere.  But with multiple branches there is always the risk it gets used and shows our money as incorrect.
Comment 2 Katrin Fischer 2022-11-20 13:46:06 UTC
I have just verified that this is still the beahviour on master. When limiting the search to payments, the voided payments show and the total given is wrong.
Comment 3 Laura Escamilla 2023-03-24 18:07:03 UTC Comment hidden (obsolete)
Comment 4 Laura Escamilla 2023-03-24 18:25:36 UTC Comment hidden (obsolete)
Comment 5 Laura Escamilla 2023-03-24 18:44:04 UTC Comment hidden (obsolete)
Comment 6 Laura Escamilla 2023-03-24 18:54:26 UTC Comment hidden (obsolete)
Comment 7 Laura Escamilla 2023-03-24 19:08:08 UTC Comment hidden (obsolete)
Comment 8 Laura Escamilla 2023-03-24 19:20:39 UTC Comment hidden (obsolete)
Comment 9 Laura Escamilla 2023-04-03 16:19:02 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall (khall) 2023-05-01 13:53:05 UTC
Created attachment 150456 [details] [review]
Bug 29664: Do not show voided payments in cash_register_stats.pl

Test Plan:
1) Set up your cash register
2) Take a payment
3) Void that payment
4) Verify the voided fee shows for the "All payments to the library" and
   "Payment" transaction type filters
5) Apply this patch
6) Restart all the things!
7) Verify the voided fee no longer shows for the "All payments to the library" and
   "Payment" transaction type filters
Comment 11 David Nind 2023-05-03 12:07:17 UTC
Created attachment 150580 [details] [review]
Bug 29664: Do not show voided payments in cash_register_stats.pl

Test Plan:
1) Set up your cash register
2) Take a payment
3) Void that payment
4) Verify the voided fee shows for the "All payments to the library" and
   "Payment" transaction type filters
5) Apply this patch
6) Restart all the things!
7) Verify the voided fee no longer shows for the "All payments to the library" and
   "Payment" transaction type filters

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Katrin Fischer 2023-05-03 21:16:19 UTC
There is an entry in the pull downs "Credit has been voided" that makes me wonder if we should still show the voided payments somehow, but marked as such. Maybe give them their own entry in the list of types?

I understand what you don't want to see them in payments. 

Would love to get another set of eyes on this one. Martin maybe?
Comment 13 Marcel de Rooy 2023-06-30 07:22:07 UTC
QA: Looking here
Comment 14 Marcel de Rooy 2023-06-30 07:39:53 UTC
Created attachment 152887 [details] [review]
Bug 29664: Do not show voided payments in cash_register_stats.pl

Test Plan:
1) Set up your cash register
2) Take a payment
3) Void that payment
4) Verify the voided fee shows for the "All payments to the library" and
   "Payment" transaction type filters
5) Apply this patch
6) Restart all the things!
7) Verify the voided fee no longer shows for the "All payments to the library" and
   "Payment" transaction type filters

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2023-06-30 07:39:55 UTC
Created attachment 152888 [details] [review]
Bug 29664: (QA follow-up) Typo transacton

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2023-06-30 07:40:12 UTC
(In reply to Katrin Fischer from comment #12)
> There is an entry in the pull downs "Credit has been voided" that makes me
> wonder if we should still show the voided payments somehow, but marked as
> such. Maybe give them their own entry in the list of types?
> 
> I understand what you don't want to see them in payments. 
> 
> Would love to get another set of eyes on this one. Martin maybe?

The filter is applied to PAYMENT and ACT. You could still see them with ALL or VOID?

Seems that we overload VOID a bit btw:
Koha/Account/Line.pm:                    debit_type_code   => 'VOID',
Koha/Account/Line.pm:                status => 'VOID'
Koha/Account/Line.pm:                        type      => 'VOID',

No reason to keep this here now. Last word to RM.
Comment 17 Tomás Cohen Arazi (tcohen) 2023-07-03 17:50:49 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 18 Martin Renvoize (ashimema) 2023-07-17 15:00:02 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 19 Matt Blenkinsop 2023-07-18 13:11:58 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 20 Andrii Nugged 2023-08-01 23:15:11 UTC
Follow-up bug note:

        $whereTType .= q{ AND status != 'VOID' };

This makes MariaDB act unexpectedly: statuses with NULL are also skipped!
When I added this to our piloting production:

        $whereTType .= q{ AND (status IS NULL OR status != 'VOID') };

it started to show those accountlines which has NULLs.


Reasoning: https://mariadb.com/kb/en/null-values/

SELECT NULL != 'VOID', 'VOID' != 'VOID', 'TOTAL' != 'VOID';

+----------------+------------------+-------------------+
| NULL != 'VOID' | 'VOID' != 'VOID' | 'TOTAL' != 'VOID' |
+----------------+------------------+-------------------+
|           NULL |                0 |                 1 |
+----------------+------------------+-------------------+
Comment 21 Nick Clemens (kidclamp) 2023-09-05 11:18:16 UTC
(In reply to Andrii Nugged from comment #20)
> Follow-up bug note:
> 
>         $whereTType .= q{ AND status != 'VOID' };
> 
> This makes MariaDB act unexpectedly: statuses with NULL are also skipped!
> When I added this to our piloting production:

Make sure to file a new bug once an issue has been pushed. See bug 34552