Bug 21722

Summary: Update C4::Accounts::chargelostitem to use Koha::Account->add_debit
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 21747    
Bug Blocks: 21728    
Attachments: Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Tidy up tests and increase coverage
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Tidy up tests and increase coverage
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Tidy up tests and increase coverage
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Tidy up tests and increase coverage
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Tidy up tests and increase coverage
Bug 21722: Use Koha::Account->add_debit in chargelostitem
Bug 21722: Tidy up tests and increase coverage
Bug 21722: Use Koha::Account->add_debit in chargelostitem

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.