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.
Created attachment 76240 [details] [review] Bug 20978: Unit tests This patch adds unit tests for Koha::Account::add_credit.
Created attachment 76241 [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
Created attachment 76242 [details] [review] Bug 20978: (follow-up) Another use case from C4::Circulation
Created attachment 76243 [details] [review] Bug 20978: Unit tests This patch adds unit tests for Koha::Account::add_credit.
Created attachment 76244 [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
Created attachment 76245 [details] [review] Bug 20978: (follow-up) Another use case from C4::Circulation
Created attachment 76278 [details] [review] Bug 20978: Unit tests This patch adds unit tests for Koha::Account::add_credit.
Created attachment 76279 [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
Created attachment 76280 [details] [review] Bug 20978: (follow-up) Another use case from C4::Circulation
Created attachment 76319 [details] [review] Bug 20978: Unit tests This patch adds unit tests for Koha::Account::add_credit.
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
Created attachment 76321 [details] [review] Bug 20978: (follow-up) Another use case from C4::Circulation
I rebased on top of bug 20946, which is more urgent as it affects the stable branches.
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>
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>
Created attachment 76398 [details] [review] Bug 20978: (follow-up) Another use case from C4::Circulation Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
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>
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>
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>
Created attachment 76515 [details] [review] Bug 20978: (follow-up) POD completeness Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
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>
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>
Created attachment 76636 [details] [review] Bug 20978: (follow-up) POD completeness Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
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>
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>
Awesome work all! Pushed to master for 18.11
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.