Bug 27995 - Koha::Account::Line->apply should return the update Koha::Account::Line object
Summary: Koha::Account::Line->apply should return the update Koha::Account::Line object
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 27971 28156
Blocks: 27636
  Show dependency treegraph
 
Reported: 2021-03-19 11:23 UTC by Martin Renvoize
Modified: 2021-12-13 21:12 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00


Attachments
Bug 27995: Unit tests (4.79 KB, patch)
2021-03-19 11:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.21 KB, patch)
2021-03-19 11:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Unit tests (4.79 KB, patch)
2021-03-22 15:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.21 KB, patch)
2021-03-22 15:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Unit tests (4.85 KB, patch)
2021-04-05 15:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.27 KB, patch)
2021-04-05 15:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27995: Unit tests (5.34 KB, patch)
2021-04-06 11:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.36 KB, patch)
2021-04-06 11:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Unit tests (5.34 KB, patch)
2021-04-22 15:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.36 KB, patch)
2021-04-22 15:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27995: Unit tests (5.35 KB, patch)
2021-04-23 12:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.38 KB, patch)
2021-04-23 12:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27995: (QA follow-up) Update POD (816 bytes, patch)
2021-04-23 12:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27995: Unit tests (5.41 KB, patch)
2021-04-23 12:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27995: Unit tests (5.41 KB, patch)
2021-04-23 12:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27995: Update return for Koha::Account::Line->apply (3.43 KB, patch)
2021-04-23 12:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27995: (QA follow-up) Update POD (871 bytes, patch)
2021-04-23 12:45 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 2021-04-23 12:11:08 UTC
Created attachment 120080 [details] [review]
Bug 27995: (QA follow-up) Update POD
Comment 17 Kyle M Hall 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 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 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 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