https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17585 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17585 [Bug 17585] Add the Koha::Patron->get_account_lines method -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17588 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17588 [Bug 17588] Move GetMemberIssuesAndFines to Koha::Patron -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |BLOCKED --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Mika from comment #3)

Tests failed

t/db_dependent/Accounts.t .. 1/20 # No tests run!

# Failed test 'No tests run for subtest "get_balance"' # at t/db_dependent/Accounts.t line 392. The method get_account_lines is not covered by tests!# Looks like your test exited with 255 just after 20. t/db_dependent/Accounts.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 1/20 subtests

Test Summary Report ------------------- t/db_dependent/Accounts.t (Wstat: 65280 Tests: 20 Failed: 1) Failed test: 20 Non-zero exit status: 255 Files=1, Tests=20, 2 wallclock secs ( 0.03 usr 0.00 sys + 1.97 cusr 0.06 csys = 2.06 CPU) Result: FAIL

This patch depends on bug 17585. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57338|0 |1 is obsolete| | --- Comment #5 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 57906 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57906&action=edit [SIGNED-OFF] Bug 17586: Add the Koha::Account::Lines->get_balance method Test plan: prove t/db_dependent/Accounts.t should return green Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Failed QA --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- I don't want to be *too* much of a pedant, but wouldn't this method be better in Koha::Account? I see there are a good number of patches that depend on this, so if that change will require many further changes, I'd be happy with just a final bug at the end of the chain to move it to Koha::Account. Let me know what you think and reset the status accordingly. Kyle -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #9)

(In reply to Kyle M Hall from comment #8)

...

I don't want to be *too* much of a pedant, but wouldn't this method be better in Koha::Account? I see there are a good number of patches that depend on this, so if that change will require many further changes, I'd be happy with just a final bug at the end of the chain to move it to Koha::Account.

How would you write this line in that case: my $balance = $patron->get_account_lines->get_balance; (from the patch on bug 17588).

Maybe just $patron->get_account_balance then?

I would do $patron->account()->balance() with account() returning the Koha::Account object, and balance() returning the account balance. Of course, I'm a fan of being succinct ; ) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57929|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57990&action=edit Bug 17586: Add the Koha::Account::Lines->get_balance method Test plan: prove t/db_dependent/Accounts.t should return green Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Kyle M Hall from comment #10)

I would do $patron->account()->balance() with account() returning the Koha::Account object, and balance() returning the account balance. Of course, I'm a fan of being succinct ; )

I am not 100% this change is the right thing to do but we could go like that to start. For now, the Koha::Account module's responsibilities are not clearly defined and it's not easy to guess what it will become. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Kyle M Hall from comment #14)

(In reply to Jonathan Druart from comment #13)

...

(In reply to Kyle M Hall from comment #10)

...

I would do $patron->account()->balance() with account() returning the Koha::Account object, and balance() returning the account balance. Of course, I'm a fan of being succinct ; )

I am not 100% this change is the right thing to do but we could go like that to start. For now, the Koha::Account module's responsibilities are not clearly defined and it's not easy to guess what it will become.

That's fine. I just wanted to put that thought out. We may have a better notion of where such a method would go after the fact and we can make that change at the end rather then have to modify all the dependent patches.

I have modified the dependent patches, so let's go with it :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Bug 17586 depends on bug 17585, which changed state. Bug 17585 Summary: Add the Koha::Patron->get_account_lines method https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17585 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17586 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add the |Add the |Koha::Account::Lines->get_b |Koha::Account->balance |alance method |method -- You are receiving this mail because: You are watching all bug changes.