Bug 20978

Summary: Add Koha::Account::add_credit
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: josef.moravec, kyle.m.hall, kyle, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21002
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 20946    
Bug Blocks: 20944, 20980, 20997, 21584    
Attachments: Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: (follow-up) POD completeness
Bug 20978: Unit tests
Bug 20978: Add Koha::Account::add_credit
Bug 20978: (follow-up) Another use case from C4::Circulation
Bug 20978: (follow-up) POD completeness
Bug 20978: (QA follow-up) Do not delete existing data on tests
Bug 20978: (QA follow-up) Do not delete existing data on tests
Bug 20978: (QA follow-up) Do not delete existing data on tests

Description Tomás Cohen Arazi 2018-06-21 15:27:36 UTC
There's code spread in several controller scripts that directly inserts account lines on the DB, with hardcoded values that only exist on the forms data comes from (mancredit.pl, maninvoice, etc).

There should be a high-level function to deal with credits with a simple set of valid values.
Comment 1 Tomás Cohen Arazi 2018-06-21 16:07:41 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2018-06-21 16:07:49 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2018-06-21 16:36:03 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2018-06-21 17:16:38 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2018-06-21 17:16:45 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2018-06-21 17:16:51 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2018-06-22 13:27:35 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2018-06-22 13:27:58 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2018-06-22 13:28:38 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 2018-06-23 08:55:36 UTC
Created attachment 76319 [details] [review]
Bug 20978: Unit tests

This patch adds unit tests for Koha::Account::add_credit.
Comment 11 Tomás Cohen Arazi 2018-06-23 08:55:44 UTC
Created attachment 76320 [details] [review]
Bug 20978: Add Koha::Account::add_credit

This patch adds:
- Koha::Account::add_credit
- A single source of truth for accounttype and offset_type values

To test:
- Apply the first patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> FAIL: Tests fail!
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 12 Tomás Cohen Arazi 2018-06-23 08:55:52 UTC
Created attachment 76321 [details] [review]
Bug 20978: (follow-up) Another use case from C4::Circulation
Comment 13 Tomás Cohen Arazi 2018-06-23 08:57:16 UTC
I rebased on top of bug 20946, which is more urgent as it affects the stable branches.
Comment 14 Josef Moravec 2018-06-25 18:37:04 UTC
Created attachment 76396 [details] [review]
Bug 20978: Unit tests

This patch adds unit tests for Koha::Account::add_credit.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 15 Josef Moravec 2018-06-25 18:37:09 UTC
Created attachment 76397 [details] [review]
Bug 20978: Add Koha::Account::add_credit

This patch adds:
- Koha::Account::add_credit
- A single source of truth for accounttype and offset_type values

To test:
- Apply the first patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> FAIL: Tests fail!
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Josef Moravec 2018-06-25 18:37:15 UTC
Created attachment 76398 [details] [review]
Bug 20978: (follow-up) Another use case from C4::Circulation

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Kyle M Hall 2018-06-26 13:59:48 UTC
Created attachment 76439 [details] [review]
Bug 20978: Unit tests

This patch adds unit tests for Koha::Account::add_credit.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall 2018-06-26 13:59:59 UTC
Created attachment 76440 [details] [review]
Bug 20978: Add Koha::Account::add_credit

This patch adds:
- Koha::Account::add_credit
- A single source of truth for accounttype and offset_type values

To test:
- Apply the first patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> FAIL: Tests fail!
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall 2018-06-26 14:00:03 UTC
Created attachment 76441 [details] [review]
Bug 20978: (follow-up) Another use case from C4::Circulation

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Tomás Cohen Arazi 2018-06-27 14:02:33 UTC
Created attachment 76515 [details] [review]
Bug 20978: (follow-up) POD completeness

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Tomás Cohen Arazi 2018-07-02 15:30:33 UTC
Created attachment 76633 [details] [review]
Bug 20978: Unit tests

This patch adds unit tests for Koha::Account::add_credit.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Tomás Cohen Arazi 2018-07-02 15:30:39 UTC
Created attachment 76634 [details] [review]
Bug 20978: Add Koha::Account::add_credit

This patch adds:
- Koha::Account::add_credit
- A single source of truth for accounttype and offset_type values

To test:
- Apply the first patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> FAIL: Tests fail!
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Tomás Cohen Arazi 2018-07-02 15:30:46 UTC
Created attachment 76635 [details] [review]
Bug 20978: (follow-up) Another use case from C4::Circulation

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Tomás Cohen Arazi 2018-07-02 15:30:55 UTC
Created attachment 76636 [details] [review]
Bug 20978: (follow-up) POD completeness

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 25 Tomás Cohen Arazi 2018-07-06 12:11:53 UTC
Created attachment 76726 [details] [review]
Bug 20978: (QA follow-up) Do not delete existing data on tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 26 Tomás Cohen Arazi 2018-07-06 12:19:31 UTC
Created attachment 76727 [details] [review]
Bug 20978: (QA follow-up) Do not delete existing data on tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 27 Tomás Cohen Arazi 2018-07-06 12:37:12 UTC
Created attachment 76733 [details] [review]
Bug 20978: (QA follow-up) Do not delete existing data on tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 28 Nick Clemens 2018-07-06 14:44:06 UTC
Awesome work all!

Pushed to master for 18.11
Comment 29 Martin Renvoize 2018-07-09 17:05:57 UTC
Awesome to see the code cleanup going on here!

Not backporting to 18.05.x series at the moment as it's an enhancement/refactoring piece.