|
Description
Martin Renvoize (ashimema)
2021-02-05 12:58:58 UTC
Created attachment 116393 [details] [review] Bug 27636: Add offset_amount to Koha::Accounts This is a work in progress.. just putting the patch here for early thoughts and to display progress Created attachment 118471 [details] [review] Bug 27636: Add payin_amount to Koha::Account This patch adds a `payin_amount` method to the Koha::Account as a modern replacement to the `pay` method. The new method uses the `add_credit` and `Koha::Account::Line->apply` methods internally for consistency and returns the created credit Koha::Account::Line result. Test plan 1/ Run t/db_dependent/Koha/Account.t Created attachment 118515 [details] [review] Bug 27636: Add payin_amount to Koha::Account This patch adds a `payin_amount` method to the Koha::Account as a modern replacement to the `pay` method. The new method uses the `add_credit` and `Koha::Account::Line->apply` methods internally for consistency and returns the created credit Koha::Account::Line result. Test plan 1/ Run t/db_dependent/Koha/Account.t Created attachment 118516 [details] [review] Bug 27636: Update ->pay to use ->payin_amount internally This patch is an interim patch to update Koha::Account->pay to use the new Koha::Account->payin_amount method internally. It simply utilises that method for the logic and acts as an input/output shim for compatability and to allow us to use the existing tests. Test plan 1/ Run t/db_dependent/Koha/Account.t Created attachment 118522 [details] [review] Bug 27636: Update failing test It would appear that ->pay didn't fully respect AccountAutoReconcile! Created attachment 118523 [details] [review] Bug 27636: Update failing test It would appear that ->pay didn't fully respect AccountAutoReconcile! Setting NSO for feedback.. there's still work to be done here.. but I think these patches prove the way in that the new payin_amount routine can be used to replace the pay method. Next steps I envisage * Migrate pay tests into payin_amount tests * Migrate pay calls to payin_amount Created attachment 119205 [details] [review] Bug 27636: Add payin_amount to Koha::Account This patch adds a `payin_amount` method to the Koha::Account as a modern replacement to the `pay` method. The new method uses the `add_credit` and `Koha::Account::Line->apply` methods internally for consistency and returns the created credit Koha::Account::Line result. Test plan 1/ Run t/db_dependent/Koha/Account.t Created attachment 119206 [details] [review] Bug 27636: Update ->pay to use ->payin_amount internally This patch is an interim patch to update Koha::Account->pay to use the new Koha::Account->payin_amount method internally. It simply utilises that method for the logic and acts as an input/output shim for compatability and to allow us to use the existing tests. Test plan 1/ Run t/db_dependent/Koha/Account.t Created attachment 119207 [details] [review] Bug 27636: Update failing test It would appear that ->pay didn't fully respect AccountAutoReconcile! Created attachment 119242 [details] [review] Bug 27636: Add payin_amount to Koha::Account This patch adds a `payin_amount` method to the Koha::Account as a modern replacement to the `pay` method. The new method uses the `add_credit` and `Koha::Account::Line->apply` methods internally for consistency and returns the created credit Koha::Account::Line result. Test plan 1/ Run t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Created attachment 119243 [details] [review] Bug 27636: Update ->pay to use ->payin_amount internally This patch is an interim patch to update Koha::Account->pay to use the new Koha::Account->payin_amount method internally. It simply utilises that method for the logic and acts as an input/output shim for compatability and to allow us to use the existing tests. Test plan 1/ Run t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Created attachment 119244 [details] [review] Bug 27636: Update failing test It would appear that ->pay didn't fully respect AccountAutoReconcile! Signed-off-by: David Nind <david@davidnind.com> Created attachment 120022 [details] [review] Bug 27636: Add payin_amount to Koha::Account This patch adds a `payin_amount` method to the Koha::Account as a modern replacement to the `pay` method. The new method uses the `add_credit` and `Koha::Account::Line->apply` methods internally for consistency and returns the created credit Koha::Account::Line result. Test plan 1/ Run t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Created attachment 120023 [details] [review] Bug 27636: Update ->pay to use ->payin_amount internally This patch is an interim patch to update Koha::Account->pay to use the new Koha::Account->payin_amount method internally. It simply utilises that method for the logic and acts as an input/output shim for compatability and to allow us to use the existing tests. Test plan 1/ Run t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Created attachment 120024 [details] [review] Bug 27636: Update failing test It would appear that ->pay didn't fully respect AccountAutoReconcile! Signed-off-by: David Nind <david@davidnind.com> Rebased. Created attachment 120089 [details] [review] Bug 27636: Add payin_amount to Koha::Account This patch adds a `payin_amount` method to the Koha::Account as a modern replacement to the `pay` method. The new method uses the `add_credit` and `Koha::Account::Line->apply` methods internally for consistency and returns the created credit Koha::Account::Line result. Test plan 1/ Run t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120090 [details] [review] Bug 27636: Update ->pay to use ->payin_amount internally This patch is an interim patch to update Koha::Account->pay to use the new Koha::Account->payin_amount method internally. It simply utilises that method for the logic and acts as an input/output shim for compatability and to allow us to use the existing tests. Test plan 1/ Run t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120091 [details] [review] Bug 27636: Update failing test It would appear that ->pay didn't fully respect AccountAutoReconcile! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Created attachment 120257 [details] [review] Bug 27636: (QA follow-up) Fix tests and validate This patch fixes the tests for when a negative amount is passed to the pay method. Prior to now, a negative amount would have been passed through and recorded. This was inconsistent with all other accounts methods and has been deprecated to ensure consistent amounts handling. This patch also introduces basic validation to prevent negatives being entered into the UI. Created attachment 120258 [details] [review] Bug 27636: (QA follow-up) Fix tests and validate This patch fixes the tests for when a negative amount is passed to the pay method. Prior to now, a negative amount would have been passed through and recorded. This was inconsistent with all other accounts methods and has been deprecated to ensure consistent amounts handling. This patch also introduces basic validation to prevent negatives being entered into the UI. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 120259 [details] [review] Bug 27636: Be explicit on the exception we expect Created attachment 120262 [details] [review] Bug 27636: (QA follow-up) Fix tests and validate This patch fixes the tests for when a negative amount is passed to the pay method. Prior to now, a negative amount would have been passed through and recorded. This was inconsistent with all other accounts methods and has been deprecated to ensure consistent amounts handling. This patch also introduces basic validation to prevent negatives being entered into the UI. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120263 [details] [review] Bug 27636: Be explicit on the exception we expect Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120270 [details] [review] Bug 27636: (QA follow-up) Restore AutoReconcile assumption This patch restores the behaviour of paycollect which relied on Koha::Account->pay ignoring the value of the AutoReconcile system preference. We now explicitly fall back to selecting all outstanding debit lines if a selection of lines is not passed into the controller. Created attachment 120271 [details] [review] Bug 27636: (QA follow-up) Restore AutoReconcile assumption This patch restores the behaviour of paycollect which relied on Koha::Account->pay ignoring the value of the AutoReconcile system preference. We now explicitly fall back to selecting all outstanding debit lines if a selection of lines is not passed into the controller. Created attachment 120272 [details] [review] Bug 27636: (QA follow-up) Restore AutoReconcile assumption This patch restores the behaviour of paycollect which relied on Koha::Account->pay ignoring the value of the AutoReconcile system preference. We now explicitly fall back to selecting all outstanding debit lines if a selection of lines is not passed into the controller. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 120276 [details] [review] Bug 27636: (QA follow-up) Restore force AutoReconcile This patch restores the behaviour ->pay always acting as though `AccountAutoReconcile` was enabled. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> That final follow-up needs some tests writing.. they're on my radar and in progress.. but I wanted to put that patch here to get the dashboard green again should you want to push these followups Created attachment 120294 [details] [review] Bug 27636: (QA follow-up) Unit tests for AutoReconcile I get:
t/db_dependent/Accounts.t .. 1/27 Global symbol "$payment" requires explicit package name (did you forget to declare "my $payment"?) at t/db_dependent/Accounts.t line 240.
Then, when the $payment=>$accountline change is done I get:
t/db_dependent/Accounts.t .. 6/27 Odd number of elements in anonymous hash at /kohadevbox/koha/Koha/Account.pm line 109.
# Looks like you planned 17 tests but ran 3.
# Failed test 'Koha::Account::pay - always AutoReconcile + notes tests'
# at t/db_dependent/Accounts.t line 241.
{UNKNOWN}: Not an ARRAY reference at /kohadevbox/koha/Koha/Account/Line.pm line 626. at /kohadevbox/koha/Koha/Account/Line.pm line 690
Enhancement not pushed to 20.11.x Created attachment 120343 [details] [review] Bug 27636: (QA follow-up) Unit tests for AutoReconcile Created attachment 120344 [details] [review] Bug 27636: (QA follow-up) Restore force AutoReconcile This patch restores the behaviour ->pay always acting as though `AccountAutoReconcile` was enabled. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 120345 [details] [review] Bug 27636: (QA follow-up) Unit tests for AutoReconcile Last patches pushed to master. Enhancement not pushed to 20.11.x |