Each time a fine is incremented, the date column is updated. When that fine is forgiven ( often via the "Forgive fines" option on the checkin page ), it does not update the column. It makes sense that if updating a fine sets the date column, forgiving it should do so as well.
Created attachment 84339 [details] [review] Bug 22200 - Forgiving a fine (FFOR) does not update the accountline date Each time a fine is incremented, the date column is updated. When that fine is forgiven ( often via the "Forgive fines" option on the checkin page ), it does not update the column. It makes sense that if updating a fine sets the date column, forgiving it should do so as well. Test Plan: 1) Find a checkout that is overdue with fines, where the date is not today 2) Note the date in the accountlines table 3) Check it in, note the date is not updated 4) Apply this patch 5) Repeat steps 1-2 6) Note the date is updated!
I created a fine and changed it so its date is not today. I went to Pay fines and paid, but the date did not update. Am I doing something wrong?
Sorry, I didn't follow the steps precisely. I didn't test on the checking in page, but I did by the Create manual credit way and it wasn't still working.
(In reply to Pierre-Marc Thibault from comment #2) > I created a fine and changed it so its date is not today. I went to Pay > fines and paid, but the date did not update. Am I doing something wrong? That is not the action I meant. You'll need to check in the item with the "forgive fine" option from the checkins page.
(In reply to Pierre-Marc Thibault from comment #3) > Sorry, I didn't follow the steps precisely. I didn't test on the checking in > page, but I did by the Create manual credit way and it wasn't still working. To test, you'll need a checked out overdue item with fines. Nothing else will work.
patch working
Created attachment 85219 [details] [review] Bug 22200: Forgiving a fine (FFOR) does not update the accountline date Each time a fine is incremented, the date column is updated. When that fine is forgiven ( often via the "Forgive fines" option on the checkin page ), it does not update the column. It makes sense that if updating a fine sets the date column, forgiving it should do so as well. Test Plan: 1) Find a checkout that is overdue with fines, where the date is not today 2) Note the date in the accountlines table 3) Check it in, note the date is not updated 4) Apply this patch 5) Repeat steps 1-2 6) Note the date is updated! Signed-off-by: Genevieve Beaudry <genevieve.beaudry@inlibro.com>
I've fixed the commit subject and added Genevieve's sign-off line manually. But I am a little confused here :( I started my tests with verifying the current behavior and noticed that accountlines.date is already updated to the current date when the entry is changed to FFOR in my testing. Can you please take a look?
I think we're barking up the wrong tree here. At this time, I think updating the date column is a *bad* idea. It is our 'created on' date, and timestamp is our 'last updated on' date. Really, we shouldn't be changing the fee type to FFOR, we should be generating a FFOR payment and applying it to the fee. I'm going to ask Tomas to chime in.
Funny.. I was just thinking "Why are we not creating an offsetting writeoff/forgiven whatever credit here instead of updating the overdue amounts directly?" I'm going to adopt this bug and push it into the appropriate spot in the accountlines status tree.
Created attachment 88269 [details] [review] Bug 22200: Add credit when forgiving overdue Prior to this patch the exemptfine option for_FixOverduesOnReturn would directly set the overdue amount to 0 rather than creating an appropriate credit of the type forgiven and offseting it against the debt.
Created attachment 88270 [details] [review] Bug 22200: Add Tests for change
Test Plan: 1) Find a checkout that is overdue with fines 2) Check the item in with the forgive fines option checked 3) Note that the users account details now shows the overdue as forgiven and a forgiven credit is added which matches the overdue amount (and is applied against it, i.e. both overdue and forgiven lines have 0 amountoutstanding).
Created attachment 88271 [details] [review] Bug 22200: Add Tests for change Unit tests to check for the addition of a credit of type 'FOR' when _FixOverduesOnReturn is called with $exemptfine set to true. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88273 [details] [review] Bug 22200: Add credit when forgiving overdue Prior to this patch the exemptfine option for_FixOverduesOnReturn would directly set the overdue amount to 0 rather than creating an appropriate credit of the type forgiven and offseting it against the debt. Test Plan: 1) Find a checkout that is overdue with fines 2) Check the item in with the forgive fines option checked 3) Note that the users account details now shows the overdue as forgiven and a forgiven credit is added which matches the overdue amount (and is applied against it, i.e. both overdue and forgiven lines have 0 amountoutstanding). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88304 [details] [review] Bug 22200: Add Tests for change Unit tests to check for the addition of a credit of type 'FOR' when _FixOverduesOnReturn is called with $exemptfine set to true. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88305 [details] [review] Bug 22200: Add credit when forgiving overdue Prior to this patch the exemptfine option for_FixOverduesOnReturn would directly set the overdue amount to 0 rather than creating an appropriate credit of the type forgiven and offseting it against the debt. Test Plan: 1) Find a checkout that is overdue with fines 2) Check the item in with the forgive fines option checked 3) Note that the users account details now shows the overdue as forgiven and a forgiven credit is added which matches the overdue amount (and is applied against it, i.e. both overdue and forgiven lines have 0 amountoutstanding). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88718 [details] [review] Bug 22200: Add Tests for change Unit tests to check for the addition of a credit of type 'FOR' when _FixOverduesOnReturn is called with $exemptfine set to true. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88719 [details] [review] Bug 22200: Add credit when forgiving overdue Prior to this patch the exemptfine option for_FixOverduesOnReturn would directly set the overdue amount to 0 rather than creating an appropriate credit of the type forgiven and offseting it against the debt. Test Plan: 1) Find a checkout that is overdue with fines 2) Check the item in with the forgive fines option checked 3) Note that the users account details now shows the overdue as forgiven and a forgiven credit is added which matches the overdue amount (and is applied against it, i.e. both overdue and forgiven lines have 0 amountoutstanding). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88750 [details] [review] Bug 22200: (follow-up) Wrap accountline creation in a transaction Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88759 [details] [review] Bug 22200: Add Tests for change Unit tests to check for the addition of a credit of type 'FOR' when _FixOverduesOnReturn is called with $exemptfine set to true. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 88760 [details] [review] Bug 22200: Add credit when forgiving overdue Prior to this patch the exemptfine option for_FixOverduesOnReturn would directly set the overdue amount to 0 rather than creating an appropriate credit of the type forgiven and offseting it against the debt. Test Plan: 1) Find a checkout that is overdue with fines 2) Check the item in with the forgive fines option checked 3) Note that the users account details now shows the overdue as forgiven and a forgiven credit is added which matches the overdue amount (and is applied against it, i.e. both overdue and forgiven lines have 0 amountoutstanding). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 88761 [details] [review] Bug 22200: (follow-up) Wrap accountline creation in a transaction Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 88779 [details] [review] Bug 22200: Add Tests for change Unit tests to check for the addition of a credit of type 'FOR' when _FixOverduesOnReturn is called with $exemptfine set to true. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 88780 [details] [review] Bug 22200: Add credit when forgiving overdue Prior to this patch the exemptfine option for_FixOverduesOnReturn would directly set the overdue amount to 0 rather than creating an appropriate credit of the type forgiven and offseting it against the debt. Test Plan: 1) Find a checkout that is overdue with fines 2) Check the item in with the forgive fines option checked 3) Note that the users account details now shows the overdue as forgiven and a forgiven credit is added which matches the overdue amount (and is applied against it, i.e. both overdue and forgiven lines have 0 amountoutstanding). Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 88781 [details] [review] Bug 22200: (follow-up) Wrap accountline creation in a transaction Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 19.05
Depends on enhancements not in 18.11.x series.