Bug 23827 - [19.05] Cash register statistics uses old accounttype values
Summary: [19.05] Cash register statistics uses old accounttype values
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: 19.05
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 22521
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-16 14:29 UTC by Andrew Fuerste-Henry
Modified: 2019-11-22 10:49 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23827: Correction for missed 'F' and 'FU' case (3.55 KB, patch)
2019-10-16 15:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23827: Correction for missed 'F' and 'FU' case [19.05] (3.63 KB, patch)
2019-10-16 15:22 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23827: Correction for missed 'F' and 'FU' case (3.61 KB, patch)
2019-11-07 08:13 UTC, Martin Renvoize
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 2019-10-16 14:29:11 UTC
Bug 22521 changes the accounttype values for fines and adds a new status field. cash_register_stats.pl does not use these new values.

To test:
- use the fines cron to generate new accounttype OVERDUE
- go to cash_register_stats.pl and run a report for transaction type Fine
- observe that none of your fines show in the report, as it is looking for accounttype=F/FU

This also impacts stable.
Comment 1 Martin Renvoize 2019-10-16 15:02:43 UTC
Created attachment 94299 [details] [review]
Bug 23827: Correction for missed 'F' and 'FU' case

When replacing F and FU with 'OVERDUE' + status in bug 22521 I missed
the cases inside the cash_register_stats template. This patch should
correct that mistake.
Comment 2 Martin Renvoize 2019-10-16 15:03:31 UTC
That should do the trick.
Comment 3 ByWater Sandboxes 2019-10-16 15:22:49 UTC
Created attachment 94305 [details] [review]
Bug 23827: Correction for missed 'F' and 'FU' case [19.05]

When replacing F and FU with 'OVERDUE' + status in bug 22521 I missed
the cases inside the cash_register_stats template. This patch should
correct that mistake.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 4 Andrew Fuerste-Henry 2019-10-16 15:24:13 UTC
- created a circ rule that includes fines
- checked out an item with a due date in the past
- checked that item back in
- verified patron had a fine
- verified that fine shows in cash register stats when filtering for Fine
Comment 5 Andrew Fuerste-Henry 2019-10-16 15:24:32 UTC
Thanks for jumping right on this, Martin!
Comment 6 Jonathan Druart 2019-10-24 10:41:45 UTC
"OVERDUEA"? I suspect a typo here.
Comment 7 Nick Clemens 2019-10-25 12:19:53 UTC
(In reply to Jonathan Druart from comment #6)
> "OVERDUEA"? I suspect a typo here.

OVERDUEA = OVERDUE ACCRUING (UNRETURNED)
Comment 8 Jonathan Druart 2019-10-30 11:54:47 UTC
(In reply to Nick Clemens from comment #7)
> (In reply to Jonathan Druart from comment #6)
> > "OVERDUEA"? I suspect a typo here.
> 
> OVERDUEA = OVERDUE ACCRUING (UNRETURNED)

Where did you find that?

git grep 'OVERDUEA'
git grep 'OVERDUE ACCRUING'
=> No result.
Comment 9 Nick Clemens 2019-10-30 16:37:00 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to Nick Clemens from comment #7)
> > (In reply to Jonathan Druart from comment #6)
> > > "OVERDUEA"? I suspect a typo here.
> > 
> > OVERDUEA = OVERDUE ACCRUING (UNRETURNED)
> 
> Where did you find that?
> 
> git grep 'OVERDUEA'
> git grep 'OVERDUE ACCRUING'
> => No result.

It was in the code later, that is how I understood it:
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94305&action=diff#a/reports/cash_register_stats.pl_sec1
Comment 10 Jonathan Druart 2019-10-31 12:24:12 UTC
We are testing if $transaction_type eq 'OVERDUEA', ok.

But where is it set to this value?
Comment 11 Jonathan Druart 2019-10-31 12:29:58 UTC
(In reply to Jonathan Druart from comment #10)
> We are testing if $transaction_type eq 'OVERDUEA', ok.
> 
> But where is it set to this value?

ok ok, forget that. I get it now, thanks Nick!
Comment 12 Jonathan Druart 2019-10-31 12:30:31 UTC
Can you rebase it please?
Comment 13 Martin Renvoize 2019-10-31 17:39:39 UTC
Setting this bug to 19.05 as in master it's rather different.. I'm going to double-check the logic in current master before creating a new bug to record an issue there if I find one.
Comment 14 Marcel de Rooy 2019-11-01 06:35:15 UTC
QAing
Comment 15 Marcel de Rooy 2019-11-01 06:41:30 UTC
CONFLICT (content): Merge conflict in reports/cash_register_stats.pl
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt
Comment 16 Martin Renvoize 2019-11-07 08:13:51 UTC
Created attachment 95130 [details] [review]
Bug 23827: Correction for missed 'F' and 'FU' case

When replacing F and FU with 'OVERDUE' + status in bug 22521 I missed
the cases inside the cash_register_stats template. This patch should
correct that mistake.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 17 Martin Renvoize 2019-11-07 08:14:57 UTC
It applies to 19.05 fine for me.. this bug is for 19.05.x (and below) only.
Comment 18 Marcel de Rooy 2019-11-08 10:16:44 UTC
QA generally skips patches for older versions. Change status in order to put it on the list of the appropriate RMaint ?