Bug 21722 - Update C4::Accounts::chargelostitem to use Koha::Account->add_debit
Summary: Update C4::Accounts::chargelostitem to use Koha::Account->add_debit
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 21747
Blocks: 21728
  Show dependency treegraph
 
Reported: 2018-10-30 09:32 UTC by Martin Renvoize
Modified: 2019-06-27 09:24 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.03 KB, patch)
2018-11-02 11:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.73 KB, patch)
2018-12-13 13:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.73 KB, patch)
2018-12-13 14:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Tidy up tests and increase coverage (25.43 KB, patch)
2018-12-13 19:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.76 KB, patch)
2018-12-13 19:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Tidy up tests and increase coverage (28.78 KB, patch)
2018-12-13 20:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.76 KB, patch)
2018-12-13 20:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Tidy up tests and increase coverage (29.94 KB, patch)
2018-12-13 21:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.76 KB, patch)
2018-12-13 21:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Tidy up tests and increase coverage (29.94 KB, patch)
2018-12-14 08:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.76 KB, patch)
2018-12-14 08:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21722: Tidy up tests and increase coverage (29.96 KB, patch)
2019-01-22 09:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.82 KB, patch)
2019-01-22 09:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21722: Tidy up tests and increase coverage (30.01 KB, patch)
2019-02-15 14:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21722: Use Koha::Account->add_debit in chargelostitem (6.87 KB, patch)
2019-02-15 14:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2018-10-30 09:32:21 UTC
All account handling should go via the Koha::Account class.
Comment 1 Martin Renvoize 2018-11-02 11:26:02 UTC
Created attachment 81889 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Comment 2 Martin Renvoize 2018-12-13 11:52:00 UTC
Test plan:

Prior to applying patch, both db_dependent/Circulation/Chargelostitem.t and db_dependent/Accounts.t tests should pass.

After applying patch the above two tests should still pass.
Comment 3 Martin Renvoize 2018-12-13 13:13:51 UTC
Created attachment 83164 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_list_item`.
Comment 4 Martin Renvoize 2018-12-13 14:32:10 UTC
Created attachment 83172 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_list_item`.
Comment 5 Martin Renvoize 2018-12-13 15:36:13 UTC
I will add tests for the new/altered behaviour here.. the existing tests do not cover the effect of the FinesLog at all right now.
Comment 6 Martin Renvoize 2018-12-13 19:33:21 UTC
Created attachment 83181 [details] [review]
Bug 21722: Tidy up tests and increase coverage

This patch moved the chargelostitem 'branchcode' test to within the
'chargelostitem' subtest block and adds tests for additional accountline
fields that should be getting set by this method.

Sponsored-by: PTFS Europe
Comment 7 Martin Renvoize 2018-12-13 19:33:24 UTC
Created attachment 83182 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_list_item`.

Sponsored-by: PTFS Europe
Comment 8 Martin Renvoize 2018-12-13 20:29:01 UTC
Created attachment 83186 [details] [review]
Bug 21722: Tidy up tests and increase coverage

This patch moves the chargelostitem 'branchcode' test to within the
'chargelostitem' subtest block in t/db_dependent/Accounts.t. It also
adds tests for additional accountline fields that should be getting
set by this method and adds tests for ProcessingFeeNote before removing
t/db_dependent/Circulation/Chargelostitem.t which covered this
previously.

Sponsored-by: PTFS Europe
Comment 9 Martin Renvoize 2018-12-13 20:29:04 UTC
Created attachment 83187 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_list_item`.

Sponsored-by: PTFS Europe
Comment 10 Martin Renvoize 2018-12-13 20:33:52 UTC
Test Plan

1) Confirm t/db_dependant/Account.t and t/db_dependant/Circulation/Chargelostitem.t test pass before applying any patches.
2) Confirm that one test fails in t/db_dependant/Account.t after applying the first patch which cleans up the tests and adds additional coverage (The test that's expected to fail is one that tests the value of the 'note' field is empty and it fails due to the difference between q{} and undef)
3) Apply the final patch and note the tests now pass.
Comment 11 Martin Renvoize 2018-12-13 21:25:36 UTC
Created attachment 83189 [details] [review]
Bug 21722: Tidy up tests and increase coverage

This patch moves the chargelostitem 'branchcode' test to within the
'chargelostitem' subtest block in t/db_dependent/Accounts.t. It also
adds tests for additional accountline fields that should be getting
set by this method and adds tests for ProcessingFeeNote before removing
t/db_dependent/Circulation/Chargelostitem.t which covered this
previously.

Sponsored-by: PTFS Europe
Comment 12 Martin Renvoize 2018-12-13 21:25:39 UTC
Created attachment 83190 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_list_item`.

Sponsored-by: PTFS Europe
Comment 13 Martin Renvoize 2018-12-14 08:26:51 UTC
Created attachment 83197 [details] [review]
Bug 21722: Tidy up tests and increase coverage

This patch moves the chargelostitem 'branchcode' test to within the
'chargelostitem' subtest block in t/db_dependent/Accounts.t. It also
adds tests for additional accountline fields that should be getting
set by this method and adds tests for ProcessingFeeNote before removing
t/db_dependent/Circulation/Chargelostitem.t which covered this
previously.

Sponsored-by: PTFS Europe
Comment 14 Martin Renvoize 2018-12-14 08:26:54 UTC
Created attachment 83198 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_lost_item`.

Sponsored-by: PTFS Europe
Comment 15 Josef Moravec 2019-01-22 09:17:16 UTC
Created attachment 84276 [details] [review]
Bug 21722: Tidy up tests and increase coverage

This patch moves the chargelostitem 'branchcode' test to within the
'chargelostitem' subtest block in t/db_dependent/Accounts.t. It also
adds tests for additional accountline fields that should be getting
set by this method and adds tests for ProcessingFeeNote before removing
t/db_dependent/Circulation/Chargelostitem.t which covered this
previously.

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Josef Moravec 2019-01-22 09:17:19 UTC
Created attachment 84277 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_lost_item`.

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Tomás Cohen Arazi 2019-02-15 14:18:46 UTC
Created attachment 85173 [details] [review]
Bug 21722: Tidy up tests and increase coverage

This patch moves the chargelostitem 'branchcode' test to within the
'chargelostitem' subtest block in t/db_dependent/Accounts.t. It also
adds tests for additional accountline fields that should be getting
set by this method and adds tests for ProcessingFeeNote before removing
t/db_dependent/Circulation/Chargelostitem.t which covered this
previously.

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2019-02-15 14:18:49 UTC
Created attachment 85174 [details] [review]
Bug 21722: Use Koha::Account->add_debit in chargelostitem

This patch updates C4::Accounts::chargelostitem to use the newly
available add_debit method from Koha::Account.

Known Side Effects: If the FinesLog is enabled then the 'action'
recorded will change from the generic `create_fee` (for both the lost
charge and any associated processing charge) to the more descriptive
terms `create_processing` and `create_lost_item`.

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Nick Clemens 2019-03-04 19:24:33 UTC
Awesome work all!

Pushed to master for 19.05
Comment 20 Martin Renvoize 2019-03-05 15:17:45 UTC
Enhancement will not be backported to stable branch.