Bug 21849 - Offsets not stored correctly in _FixOverduesOnReturn
Summary: Offsets not stored correctly in _FixOverduesOnReturn
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-16 10:10 UTC by Marcel de Rooy
Modified: 2018-12-12 10:53 UTC (History)
3 users (show)

See Also:
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:


Attachments
Bug 21849: Two useless Koha::Account::Offset->new calls need attention (1.64 KB, patch)
2018-11-16 12:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21849: Add unit tests (2.20 KB, patch)
2018-11-16 14:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21849: Add unit tests (2.23 KB, patch)
2018-12-06 16:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21849: Two useless Koha::Account::Offset->new calls need attention (1.68 KB, patch)
2018-12-06 16:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21849: Add unit tests (2.32 KB, patch)
2018-12-07 06:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21849: Two useless Koha::Account::Offset->new calls need attention (1.77 KB, patch)
2018-12-07 06:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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