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 <kyle>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, lucas, martin.renvoize, nick, tomascohen
Version: master   
Target Milestone: ---   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20598
Whiteboard:
Change sponsored?: --- Patch complexity: Trivial patch
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
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 2018-11-16 12:04:48 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 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 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 2018-11-16 14:54:57 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 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 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 2018-12-11 20:57:36 UTC
Awesome work all!

Pushed to master for 19.05
Comment 10 Martin Renvoize 2018-12-12 10:53:19 UTC
Pushed to 18.11.x for 18.11.01
Comment 11 Lucas Gass 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