Bug 27995

Summary: Koha::Account::Line->apply should return the update Koha::Account::Line object
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: CLOSED FIXED QA Contact: Kyle M Hall (khall) <kyle>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, kyle, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00
Circulation function:
Bug Depends on: 27971, 28156    
Bug Blocks: 27636    
Attachments: Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: (QA follow-up) Update POD
Bug 27995: Unit tests
Bug 27995: Unit tests
Bug 27995: Update return for Koha::Account::Line->apply
Bug 27995: (QA follow-up) Update POD

Description Martin Renvoize (ashimema) 2021-03-19 11:23:19 UTC
We should return the updated Koha::Account::Line object for the credit we are applying.

This would allow subsequent chaining and allow for Koha::Object::Message sets to be embedded in the result and passed back to the caller.
Comment 1 Martin Renvoize (ashimema) 2021-03-19 11:28:42 UTC
Created attachment 118511 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.
Comment 2 Martin Renvoize (ashimema) 2021-03-19 11:28:45 UTC
Created attachment 118512 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff
Comment 3 Martin Renvoize (ashimema) 2021-03-22 15:59:54 UTC
Created attachment 118615 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.
Comment 4 Martin Renvoize (ashimema) 2021-03-22 15:59:57 UTC
Created attachment 118616 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff
Comment 5 Tomás Cohen Arazi (tcohen) 2021-04-05 15:20:46 UTC
Created attachment 119182 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi (tcohen) 2021-04-05 15:20:50 UTC
Created attachment 119183 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi (tcohen) 2021-04-05 15:27:02 UTC
Ok, I like the change. QA scripts are happy. I did a

$ git grep '\>apply'

just in case, so I ran:

k$ prove t/db_dependent/Koha/Account/Line.t \ 
         t/db_dependent/api/v1/patrons_accounts.t \
         t/db_dependent/Circulation.t \
         t/db_dependent/Koha/Account.t \
         t/db_dependent/Koha/Items.t

=> SUCCESS: Tests pass!
Comment 8 Martin Renvoize (ashimema) 2021-04-06 11:11:36 UTC
Created attachment 119203 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Martin Renvoize (ashimema) 2021-04-06 11:11:40 UTC
Created attachment 119204 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Martin Renvoize (ashimema) 2021-04-06 11:12:24 UTC
Tree re-arranged slightly to put bugs that need backporting before these enhancements that don't.

Rebased to accommodate the above.
Comment 11 Tomás Cohen Arazi (tcohen) 2021-04-13 12:45:55 UTC
(In reply to Martin Renvoize from comment #10)
> Tree re-arranged slightly to put bugs that need backporting before these
> enhancements that don't.
> 
> Rebased to accommodate the above.

+1
Comment 12 Martin Renvoize (ashimema) 2021-04-22 15:53:09 UTC
Created attachment 120010 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Martin Renvoize (ashimema) 2021-04-22 15:53:13 UTC
Created attachment 120011 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Kyle M Hall (khall) 2021-04-23 12:10:45 UTC
Created attachment 120078 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Kyle M Hall (khall) 2021-04-23 12:11:01 UTC
Created attachment 120079 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Kyle M Hall (khall) 2021-04-23 12:11:08 UTC
Created attachment 120080 [details] [review]
Bug 27995: (QA follow-up) Update POD
Comment 17 Kyle M Hall (khall) 2021-04-23 12:39:15 UTC
Created attachment 120083 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall (khall) 2021-04-23 12:45:15 UTC
Created attachment 120085 [details] [review]
Bug 27995: Unit tests

This patch updates the unit tests for Koha::Account::Line->apply to
reflect the change we wish to make to the return value.

Instead of returning the scalar amount of available credit left on the
credit line after calling apply, we want to return the full, updated,
Koha::Account::Line object including Koha::Object::Messages for the
result of renewals that may have taken place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall (khall) 2021-04-23 12:45:34 UTC
Created attachment 120086 [details] [review]
Bug 27995: Update return for Koha::Account::Line->apply

This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Kyle M Hall (khall) 2021-04-23 12:45:38 UTC
Created attachment 120087 [details] [review]
Bug 27995: (QA follow-up) Update POD

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Jonathan Druart 2021-04-27 14:25:56 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 22 Fridolin Somers 2021-04-30 09:00:58 UTC
Enhancement not pushed to 20.11.x