Bug 29664 - Do not show voided payments in cash register statistics wizard
Summary: Do not show voided payments in cash register statistics wizard
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal with 10 votes (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 34552
  Show dependency treegraph
 
Reported: 2021-12-08 17:52 UTC by Andrew Fuerste-Henry
Modified: 2023-12-28 20:47 UTC (History)
6 users (show)

See Also:
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


Attachments
Bug 29664: Removing void payments from cash_register_stats (875 bytes, patch)
2023-03-24 18:07 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: Removing void payments from the cash register report (819 bytes, patch)
2023-03-24 18:25 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: Removing void payments from the cash register report (880 bytes, patch)
2023-03-24 18:44 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: Removing void payments from the cash register report (1010 bytes, patch)
2023-03-24 18:54 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: Removing void payments from the cash register report (926 bytes, patch)
2023-03-24 19:08 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: Removing void payments from the cash register report (928 bytes, patch)
2023-03-24 19:20 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: WIP Test Patch (945 bytes, patch)
2023-04-03 16:19 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 29664: Do not show voided payments in cash_register_stats.pl (1.16 KB, patch)
2023-05-01 13:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 29664: Do not show voided payments in cash_register_stats.pl (1.21 KB, patch)
2023-05-03 12:07 UTC, David Nind
Details | Diff | Splinter Review
Bug 29664: Do not show voided payments in cash_register_stats.pl (1.30 KB, patch)
2023-06-30 07:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29664: (QA follow-up) Typo transacton (1011 bytes, patch)
2023-06-30 07:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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 2023-07-03 17:50:49 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 18 Martin Renvoize 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