A customer has enquired about whether there is an API endpoint to return all patrons with outstanding charges on their account. Currently the API offers getPatronAccount but this requires a patron id and isn't practical to check all patron account details as you need to loop through all patron ids
You want a filter on /patrons? And an embed?
Martin and I have discussed refactoring the patron accounts API - I've got a patch incoming to add this endpoint until we can get round to doing that work. I think the end goal will be to use /patrons with an embed and query parameters to fetch account data
Created attachment 153477 [details] [review] Bug 34277: Add a patrons with outstanding charges endpoint This patch adds an endpoint to return all patrons with outstanding debits/credits on their account. Currently this can't be achieved through the API without looping through all patrons using /patrons/{patron_id}/account Test plan: 1) Apply patch 2) Add some charges/credits to some patron accounts 3) Call /api/v1/patrons/get_balances 4) An array should be returned with all the patrons that you added charges to
Created attachment 153478 [details] [review] Bug 34277: Add unit test prove t/db_dependent/api/v1/patrons_accounts
Unit tests failed: # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/patrons_accounts.t line 510. # Structures begin differing at: # $got->[0] = HASH(0xaaaae46bfd98) # $expected->[0] = Does not exist # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/patrons_accounts.t line 525. # Structures begin differing at: # $got->[0]{outstanding_debits}{total} = '0.15' # $expected->[0]{outstanding_debits}{total} = '10' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/patrons_accounts.t line 552. # Structures begin differing at: # $got->[0]{outstanding_debits}{lines}[0]{interface} = 'intranet' # $expected->[0]{outstanding_debits}{lines}[0]{interface} = 'test' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/patrons_accounts.t line 590. # Structures begin differing at: # $got->[0]{balance} = '0.15' # $expected->[0]{balance} = '0' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/patrons_accounts.t line 628. # Structures begin differing at: # $got->[0]{outstanding_credits}{total} = '0' # $expected->[0]{outstanding_credits}{total} = '-10' # Looks like you failed 5 tests of 15. t/db_dependent/api/v1/patrons_accounts.t .. 6/6 # Failed test 'get_patron_balances() tests' # at t/db_dependent/api/v1/patrons_accounts.t line 645. # Looks like you failed 1 test of 6. t/db_dependent/api/v1/patrons_accounts.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/6 subtests Test Summary Report ------------------- t/db_dependent/api/v1/patrons_accounts.t (Wstat: 256 Tests: 6 Failed: 1) Failed test: 6 Non-zero exit status: 1 Files=1, Tests=6, 11 wallclock secs ( 0.02 usr 0.02 sys + 4.81 cusr 0.70 csys = 5.55 CPU) Result: FAIL
Hi Laura, The tests are working fine for me and for one of my team as well who applied it from bugzilla - did you restart plack before running the tests?
Tests fail for me when I add a manual credit to a patron accout: # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/patrons_accounts.t line 510. # Structures begin differing at: # $got->[0] = HASH(0x56200b91c9c8) # $expected->[0] = Does not exist ok 4 - GET //aMgjvtk9LM4Tcta89qLDALIsozh9yWEECy4Au6B:thePassword123@/api/v1/patrons/get_balances ok 5 - 200 OK not ok 6 - exact match for JSON Pointer "" # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/patrons_accounts.t line 525. # Structures begin differing at: # $got->[0]{outstanding_credits}{lines}[0] = HASH(0x56200b4754d0) # $expected->[0]{outstanding_credits}{lines}[0] = Does not exist ok 7 - GET //aMgjvtk9LM4Tcta89qLDALIsozh9yWEECy4Au6B:thePassword123@/api/v1/patrons/get_balances ok 8 - 200 OK not ok 9 - exact match for JSON Pointer "" # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/patrons_accounts.t line 552. # Structures begin differing at: # $got->[0]{outstanding_credits}{lines}[0] = HASH(0x56200b9414b8) # $expected->[0]{outstanding_credits}{lines}[0] = Does not exist ok 10 - GET //aMgjvtk9LM4Tcta89qLDALIsozh9yWEECy4Au6B:thePassword123@/api/v1/patrons/get_balances ok 11 - 200 OK not ok 12 - exact match for JSON Pointer "" # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/patrons_accounts.t line 590. # Structures begin differing at: # $got->[0]{outstanding_credits}{lines}[0]{credit_type} = 'CREDIT' # $expected->[0]{outstanding_credits}{lines}[0]{credit_type} = 'PAYMENT' ok 13 - GET //aMgjvtk9LM4Tcta89qLDALIsozh9yWEECy4Au6B:thePassword123@/api/v1/patrons/get_balances ok 14 - 200 OK not ok 15 - exact match for JSON Pointer "" # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/patrons_accounts.t line 628. # Structures begin differing at: # $got->[0]{borrowernumber} = '51' # $expected->[0]{borrowernumber} = '66' # Looks like you failed 5 tests of 15.
Created attachment 154516 [details] [review] Bug 34277: Add unit test prove t/db_dependent/api/v1/patrons_accounts.t
Tests should now be ignoring existing lines to isolate the test environment
Comment on attachment 153477 [details] [review] Bug 34277: Add a patrons with outstanding charges endpoint Review of attachment 153477 [details] [review]: ----------------------------------------------------------------- ::: api/v1/swagger/definitions/patron_balance.yaml @@ +1,4 @@ > --- > type: object > properties: > + borrowernumber: This should be `patron_id`.
Can't we simplify it like GET /patrons?with_outstanding_debts=1 x-koha-embed: balance This way you will benefit from things like pagination, etc. Your current approach just returns the whole resultset, which could still be done with GET /patrons?with_outstanding_debts=1&_per_page=-1 x-koha-embed: balance My two cents.
(In reply to Tomás Cohen Arazi from comment #11) > Can't we simplify it like > > GET /patrons?with_outstanding_debts=1 > x-koha-embed: balance For implementing this, you could: - Use Koha::Patrons->filter_by_amount_owed (or a simplified version, > 0, on top of it) - You would need to implement Koha::Patron->balance, for embedding purposes.
Created attachment 156185 [details] [review] [ALTERNATE] Bug 34277: Add a way to filter patrons by their outstanding balance This patch adds to new `query parameters` to the `GET /patrons` route: * owes_less_than * owes_more_than This parameters will be used to build a query, based on Koha::Patrons->filter_by_amount_owed, to return only patrons matching the criteria. FIXME: Tests missing yet
Created attachment 156186 [details] [review] [ALTERNATE] Bug 34277: Add option to embed balance details on GET /patrons
Created attachment 156187 [details] [review] [ALTERNATE] Bug 34277: (follow-up) Use Koha::Patron->balance_details in GET /patrons/{patron_id}/account This patch makes the existing route reuse the introduced method. To verify no behavior change: 1. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests pass! 2. Sign off :-D
Hi, this are my 2 cents. Another angle for the problem. Keep in mind that: - I haven't written tests - I haven't dug into options for the weird 'outstanding' attribute that gets introduced by ->filter_by_amount_owed
The test doesn't pass. error : Test Summary Report ------------------- t/db_dependent/api/v1/patrons_accounts.t (Wstat: 512 Tests: 1 Failed: 1) Failed test: 1 Non-zero exit status: 2 Parse errors: Bad plan. You planned 5 tests but ran 1. Files=1, Tests=1, 3 wallclock secs ( 0.01 usr 0.00 sys + 3.07 cusr 0.13 csys = 3.21 CPU) Result: FAIL