Bug 8112 - Overdues with Fines report, giving duplicates
Summary: Overdues with Fines report, giving duplicates
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Kyle M Hall
QA Contact: Mason James
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-17 14:40 UTC by Kyle M Hall
Modified: 2017-09-11 14:28 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 8112 - Overdues with Fines giving duplicates (1.32 KB, patch)
2012-05-17 14:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8112 - Overdues with Fines giving duplicates - Followup - Add items.itemnumber to GROUP BY (1.06 KB, patch)
2012-08-13 14:46 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
0001-SIGNED-OFF-Bug-8112-Overdues-with-Fines-giving-dupli (1.38 KB, patch)
2012-08-23 22:55 UTC, wajasu
Details | Diff | Splinter Review
0002-SIGNED-OFF-Bug-8112-Overdues-with-Fines-dup-Followup. (1.17 KB, patch)
2012-08-23 22:56 UTC, wajasu
Details | Diff | Splinter Review
circulation_overdues_report_dupefine.png (31.79 KB, image/png)
2012-08-23 22:57 UTC, wajasu
Details
circulation_overdues_report_dupefine.png (31.79 KB, image/png)
2012-08-23 22:58 UTC, wajasu
Details
circulation_overdues_report_createdupefine_same_item.png (43.98 KB, image/png)
2012-08-23 22:59 UTC, wajasu
Details
circulation_overdues_report_dupefine1_nodupeshowing.png (22.53 KB, image/png)
2012-08-23 23:00 UTC, wajasu
Details
Bug 8112 - Overdues with Fines dup Followup 2 - Add items.itemnumber to GROUP BY when using $location (1.11 KB, patch)
2012-09-18 13:32 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
overdues duplicate (10.48 KB, image/png)
2013-02-21 10:56 UTC, Koha Team University Lyon 3
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-05-17 14:40:17 UTC
If a borrower has multiple fines of type FU for a given item, that row will appear in the Overdues with Fines report for each fine on record.
Comment 1 Kyle M Hall 2012-05-17 14:41:45 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2012-07-12 10:06:49 UTC
Hi Kyle,
What is a fine of type FU ?
The only occurrence I find into the code is in the routine C4::Overdues::UpdateFine. It contains 7 FIXME and it is not easy at all to understand what it does :-/

How can I add this type of fine ?
Perhaps can you add a test plan for this patch please ?
Comment 3 Chris Cormack 2012-07-12 10:34:16 UTC
misc/cronjobs/fines.pl calls UpdateFine

UpdateFine either updates a fine, or adds a new one, both of type FU

Its a fine on an item that is still on issue. 

Hope this helps
Comment 4 Julian Maurice 2012-08-09 14:18:43 UTC
GROUP BY borrowernumber will show only 1 entry by borrowernumber, even if he has fines on several items. Is this what we want?

I would suggest to GROUP BY borrowers.borrowernumber, items.itemnumber
Comment 5 Kyle M Hall (khall) 2012-08-13 14:46:28 UTC Comment hidden (obsolete)
Comment 6 wajasu 2012-08-23 22:55:05 UTC
Created attachment 11813 [details] [review]
0001-SIGNED-OFF-Bug-8112-Overdues-with-Fines-giving-dupli


This patch deals with fixing the overdues with fines report only.

Handling of reconciling those dups must be addressed somewhere else.

I only tested the behavior of the report with the above two patches.

Test:
1) configured a patron with overdue items with fines.
2) i ran the overdue with fines report
i saw two rows, one for each patron ( normal scenario)
3) to emulate a dupe FU fine update in accountlines, I insert the row
refer to attachment above ... circulation_overdues_report_createdupefine_same_item.png
for mysql work
4) i ran the overdue with fines report
I saw three rows, but two for the same patron and same item (multiple FU case)
refer to attachment above ... circulation_overdues_report_dupefine.png
5) applied 0001 and 0002 patches, and ran report gain
I saw only two rows, even tough dups exist in the database.
refer to  circulation_overdues_report_dupefine1_nodupeshowing.png

This reports behaviour will work wether dups exist or not.  But when in place,
it hides the dups.

Note: Folks on IRC mentioned this was the result of an upgrade from 3.6.x to 3.8.x
In leiu of performing an upgrade, I took this route.

wajasu
Comment 7 wajasu 2012-08-23 22:56:04 UTC
Created attachment 11814 [details] [review]
0002-SIGNED-OFF-Bug-8112-Overdues-with-Fines-dup-Followup.
Comment 8 wajasu 2012-08-23 22:57:09 UTC
Created attachment 11815 [details]
circulation_overdues_report_dupefine.png
Comment 9 wajasu 2012-08-23 22:58:06 UTC Comment hidden (obsolete)
Comment 10 wajasu 2012-08-23 22:59:48 UTC
Created attachment 11817 [details]
circulation_overdues_report_createdupefine_same_item.png
Comment 11 wajasu 2012-08-23 23:00:57 UTC
Created attachment 11818 [details]
circulation_overdues_report_dupefine1_nodupeshowing.png
Comment 12 Jonathan Druart 2012-08-27 12:21:07 UTC
Is it normal the followup changes only 1 query ?
I think the problem exists in the 2 cases. But I have not tested !
Comment 13 Mason James 2012-09-16 13:12:33 UTC
(In reply to comment #6)
> Created attachment 11813 [details] [review]
> 0001-SIGNED-OFF-Bug-8112-Overdues-with-Fines-giving-dupli
> 
> 
> This patch deals with fixing the overdues with fines report only.
> 
> Handling of reconciling those dups must be addressed somewhere else.
> 
> I only tested the behavior of the report with the above two patches.
> 
> Test:
> 1) configured a patron with overdue items with fines.
> 2) i ran the overdue with fines report
> i saw two rows, one for each patron ( normal scenario)
> 3) to emulate a dupe FU fine update in accountlines, I insert the row
> refer to attachment above ...
> circulation_overdues_report_createdupefine_same_item.png
> for mysql work
> 4) i ran the overdue with fines report
> I saw three rows, but two for the same patron and same item (multiple FU
> case)
> refer to attachment above ... circulation_overdues_report_dupefine.png
> 5) applied 0001 and 0002 patches, and ran report gain
> I saw only two rows, even tough dups exist in the database.
> refer to  circulation_overdues_report_dupefine1_nodupeshowing.png
> 
> This reports behaviour will work wether dups exist or not.  But when in
> place,
> it hides the dups.
> 
> Note: Folks on IRC mentioned this was the result of an upgrade from 3.6.x to
> 3.8.x
> In leiu of performing an upgrade, I took this route.
> 
> wajasu
Comment 14 Mason James 2012-09-16 13:19:06 UTC
> 0001-SIGNED-OFF-Bug-8112-Overdues-with-Fines-giving-dupli

we have a test plan here, and a sign-off
i think this is OK to pass-QA on...

$ koha-qa.pl  -c 2

testing 2 commit(s) (applied to commit ce03893)
 * 7ba58d0 Bug 8112 - Overdues with Fines dup Followup - Add items.itemnumbe
      C4/Overdues.pm

 * 16abfcc Bug 8112 - Overdues with Fines giving duplicates
      C4/Overdues.pm

* C4/Overdues.pm                                                           OK
Comment 15 Mason James 2012-09-16 13:22:08 UTC
(In reply to comment #12)
> Is it normal the followup changes only 1 query ?
> I think the problem exists in the 2 cases. But I have not tested !

oops, i missed Jonathans comment

Kyle, any response for only the 1 query change, not 2?
Comment 16 Kyle M Hall (khall) 2012-09-18 13:32:12 UTC
(In reply to comment #15)
> (In reply to comment #12)
> > Is it normal the followup changes only 1 query ?
> > I think the problem exists in the 2 cases. But I have not tested !
> 
> oops, i missed Jonathans comment
> 
> Kyle, any response for only the 1 query change, not 2?

Janathan is right. Good catch!
Comment 17 Kyle M Hall (khall) 2012-09-18 13:32:25 UTC
Created attachment 12309 [details] [review]
Bug 8112 - Overdues with Fines dup Followup 2 - Add items.itemnumber to GROUP BY when using $location
Comment 18 wajasu 2012-10-04 18:18:49 UTC
After signing off bug 8253 (for 3.8.x and suggesting it be applied to master as well), I realized that this older patch is an earlier bandaid for duplicate fines at the reporting level. 

As long as 8253 gets applied to master, this report fix is obsolete.

So after talking with Kyle, we are marking it as a duplicate of 8253.

wajasu

*** This bug has been marked as a duplicate of bug 8253 ***
Comment 19 Koha Team University Lyon 3 2013-02-21 10:56:11 UTC
Created attachment 15582 [details]
overdues duplicate

Hello,

We did an update 2 days ago to 3.8.8 version. We had initially pushed aside this patch thinking that the problem will be resolved by 8253 but it did'nt. (see joined image "overdues duplicate"). Reapplying it had been necessary. So I reopen this bug.


Olivier Crouzet
Comment 20 Katrin Fischer 2015-06-06 23:14:44 UTC
Hm, lookig at this. Patches still apply, but:

Wondering - 2 fines accrueing for the same item at the same time? This sounds like a bug, there should only be 1 F and 1 FU I think. When checking an item in, the FU is set to F in my testing and no double ups happen.

Moving the status to get some feedback.