Bug 21849

Summary: Offsets not stored correctly in _FixOverduesOnReturn
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Fines and feesAssignee: Kyle M Hall (khall) <kyle>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, lucas, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20598
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 21849: Two useless Koha::Account::Offset->new calls need attention
Bug 21849: Add unit tests
Bug 21849: Add unit tests
Bug 21849: Two useless Koha::Account::Offset->new calls need attention
Bug 21849: Add unit tests
Bug 21849: Two useless Koha::Account::Offset->new calls need attention

Description Marcel de Rooy 2018-11-16 10:10:04 UTC
Came across those calls in bug 20598 in _FixOverduesOnReturn

        Koha::Account::Offset->new(
            {
                debit_id => $accountline->id,
                type => 'Forgiven',
                amount => $amountoutstanding * -1,
            }
        );

This does nothing if you don't store data.
Not sure about severity or component.
Comment 1 Kyle M Hall (khall) 2018-11-16 12:04:48 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2018-11-16 13:58:02 UTC
THis is a good opportunity to have tests for this behaviour, in preparation to the replacement of manually adding the accountline and the offset, in favour of Koha::Account methods (add_credit, add_debit and adjust).
Comment 3 Kyle M Hall (khall) 2018-11-16 14:26:38 UTC
(In reply to Tomás Cohen Arazi from comment #2)
> THis is a good opportunity to have tests for this behaviour, in preparation
> to the replacement of manually adding the accountline and the offset, in
> favour of Koha::Account methods (add_credit, add_debit and adjust).

Good idea.
Comment 4 Kyle M Hall (khall) 2018-11-16 14:54:57 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi (tcohen) 2018-12-06 16:03:03 UTC
Created attachment 82919 [details] [review]
Bug 21849: Add unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi (tcohen) 2018-12-06 16:03:08 UTC
Created attachment 82920 [details] [review]
Bug 21849: Two useless Koha::Account::Offset->new calls need attention

Came across those calls in bug 20598 in _FixOverduesOnReturn

        Koha::Account::Offset->new(
            {
                debit_id => $accountline->id,
                type => 'Forgiven',
                amount => $amountoutstanding * -1,
            }
        );

This does nothing if you don't store data.

Test Plan:
1) Apply this patch
2) Set up 2 items with overdue fines
3) Return one with dropbox mode
4) Note the dropbox account offset is created
5) Return one with full fine forgiveness
6) Note the forgiven account offset is created

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Marcel de Rooy 2018-12-07 06:44:06 UTC
Created attachment 82947 [details] [review]
Bug 21849: Add unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2018-12-07 06:44:13 UTC
Created attachment 82948 [details] [review]
Bug 21849: Two useless Koha::Account::Offset->new calls need attention

Came across those calls in bug 20598 in _FixOverduesOnReturn

        Koha::Account::Offset->new(
            {
                debit_id => $accountline->id,
                type => 'Forgiven',
                amount => $amountoutstanding * -1,
            }
        );

This does nothing if you don't store data.

Test Plan:
1) Apply this patch
2) Set up 2 items with overdue fines
3) Return one with dropbox mode
4) Note the dropbox account offset is created
5) Return one with full fine forgiveness
6) Note the forgiven account offset is created

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Nick Clemens (kidclamp) 2018-12-11 20:57:36 UTC
Awesome work all!

Pushed to master for 19.05
Comment 10 Martin Renvoize (ashimema) 2018-12-12 10:53:19 UTC
Pushed to 18.11.x for 18.11.01
Comment 11 Lucas Gass (lukeg) 2018-12-19 15:51:25 UTC
backported to 18.05 for 18.05.07
Comment 12 Fridolin Somers 2019-01-02 09:23:48 UTC
Pushed to 17.11.x for 17.11.14