Created attachment 76053 [details] [review] Bug 20942: OpenAPI spec for /patrons/{patron_id}/account This patch adds the OpenAPI spec for the following paths: - /patrons/{patron_id}/account It also adds object definitions for: - balance - account line Account line is to be used on both /account/lines (when implemented) and for embeding the outstanding lines in the balance endpoint (/patrons/{patron_id}/account).
Created attachment 76054 [details] [review] Bug 20942: Unit tests for /patrons/{patron_id}/account This patch adds tests for the /patrons/{patron_id}/account endpoint. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => FAIL: Tests should fail because the endpoint is not implemented.
Created attachment 76055 [details] [review] Bug 20942: Add route to get patron's account balance This patch implements the /patrons/{patron_id}/account endpoint. It can be used to get the patron's balance information, including outstanding lines. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: tests pass! - Make your favourite REST testing tool (RESTer on Firefox?) do: GET /api/v1/patrons/{patron_id}/account to a valid patron_id. Play with Koha's UI to add credits/payments and notice the endpoint returns the right info following the voted RFC. [1] - Sign off :-D [1] https://wiki.koha-community.org/wiki/Patrons_account_lines_endpoint_RFC
Created attachment 76393 [details] [review] Bug 20942: Split debit and credit lines This patch splits the balance to match this object schema: { balance => #, outstanding_credits => { total => #, lines => [ credit_line_1, ..., credit_line_n ] }, outstanding_debits => { total => #, lines => [ debit_line_1, ..., debit_line_m ] } } This change is made to ease usage from the UI. Also because the outstanding credits need to be applied to outstanding debits in order to the balance value to make sense. So we still need to have each total. Tests are added for this change, and the schema files are adjusted as well. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests pass! - Sign off :-D staff_id is changed into user_id as voted on the dev meeting the RFC got approved. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 76409 [details] [review] Bug 20942: OpenAPI spec for /patrons/{patron_id}/account This patch adds the OpenAPI spec for the following paths: - /patrons/{patron_id}/account It also adds object definitions for: - balance - account line Account line is to be used on both /account/lines (when implemented) and for embeding the outstanding lines in the balance endpoint (/patrons/{patron_id}/account). Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 76410 [details] [review] Bug 20942: Unit tests for /patrons/{patron_id}/account This patch adds tests for the /patrons/{patron_id}/account endpoint. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => FAIL: Tests should fail because the endpoint is not implemented. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 76411 [details] [review] Bug 20942: Add route to get patron's account balance This patch implements the /patrons/{patron_id}/account endpoint. It can be used to get the patron's balance information, including outstanding lines. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: tests pass! - Make your favourite REST testing tool (RESTer on Firefox?) do: GET /api/v1/patrons/{patron_id}/account to a valid patron_id. Play with Koha's UI to add credits/payments and notice the endpoint returns the right info following the voted RFC. [1] - Sign off :-D [1] https://wiki.koha-community.org/wiki/Patrons_account_lines_endpoint_RFC Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 76412 [details] [review] Bug 20942: Split debit and credit lines This patch splits the balance to match this object schema: { balance => #, outstanding_credits => { total => #, lines => [ credit_line_1, ..., credit_line_n ] }, outstanding_debits => { total => #, lines => [ debit_line_1, ..., debit_line_m ] } } This change is made to ease usage from the UI. Also because the outstanding credits need to be applied to outstanding debits in order to the balance value to make sense. So we still need to have each total. Tests are added for this change, and the schema files are adjusted as well. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests pass! - Sign off :-D staff_id is changed into user_id as voted on the dev meeting the RFC got approved. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
General question: cannot we have only one of $to_api_mapping and $to_model_mapping, and generate the other one? It seems that we are duplicating the information.
Created attachment 76641 [details] [review] Bug 20942: (QA follow-up) Use $lines->total_outstanding This patch makes the controller code use $lines->total_outstanding instead of expecting ->outstanding_debits and ->outstanding_credits return the total. Tests should pass as before. No behaviour change is expected. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests still pass, no behaviour change. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Jonathan Druart from comment #9) > General question: cannot we have only one of $to_api_mapping and > $to_model_mapping, and generate the other one? It seems that we are > duplicating the information. I don't think this is urgent, but yes, we could have a hash of hashes, one of them generated by reverse.
Created attachment 76808 [details] [review] Bug 20942: OpenAPI spec for /patrons/{patron_id}/account This patch adds the OpenAPI spec for the following paths: - /patrons/{patron_id}/account It also adds object definitions for: - balance - account line Account line is to be used on both /account/lines (when implemented) and for embeding the outstanding lines in the balance endpoint (/patrons/{patron_id}/account). Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 76809 [details] [review] Bug 20942: Unit tests for /patrons/{patron_id}/account This patch adds tests for the /patrons/{patron_id}/account endpoint. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => FAIL: Tests should fail because the endpoint is not implemented. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 76810 [details] [review] Bug 20942: Add route to get patron's account balance This patch implements the /patrons/{patron_id}/account endpoint. It can be used to get the patron's balance information, including outstanding lines. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: tests pass! - Make your favourite REST testing tool (RESTer on Firefox?) do: GET /api/v1/patrons/{patron_id}/account to a valid patron_id. Play with Koha's UI to add credits/payments and notice the endpoint returns the right info following the voted RFC. [1] - Sign off :-D [1] https://wiki.koha-community.org/wiki/Patrons_account_lines_endpoint_RFC Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 76811 [details] [review] Bug 20942: Split debit and credit lines This patch splits the balance to match this object schema: { balance => #, outstanding_credits => { total => #, lines => [ credit_line_1, ..., credit_line_n ] }, outstanding_debits => { total => #, lines => [ debit_line_1, ..., debit_line_m ] } } This change is made to ease usage from the UI. Also because the outstanding credits need to be applied to outstanding debits in order to the balance value to make sense. So we still need to have each total. Tests are added for this change, and the schema files are adjusted as well. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests pass! - Sign off :-D staff_id is changed into user_id as voted on the dev meeting the RFC got approved. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 76812 [details] [review] Bug 20942: (QA follow-up) Use $lines->total_outstanding This patch makes the controller code use $lines->total_outstanding instead of expecting ->outstanding_debits and ->outstanding_credits return the total. Tests should pass as before. No behaviour change is expected. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests still pass, no behaviour change. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x series for 18.05.03 I've made the decision to backport new api routes (one's that are a simple backport at least - where simple means there's not a lot of unported dependancies or the api route bug itself doesn't include lots of related refactoring work). My justification here is that an API route isn't immediately user facing and as such most users will not see the change. However, it does allow for third-party integrations to be developed against a more stable code base earlier and allow for a wider range of Koha versions to be supported by third-party applications.. that can only be seen as a good thing in my opinion.
(In reply to Martin Renvoize from comment #18) > Pushed to 18.05.x series for 18.05.03 > > I've made the decision to backport new api routes (one's that are a simple > backport at least - where simple means there's not a lot of unported > dependancies or the api route bug itself doesn't include lots of related > refactoring work). > > My justification here is that an API route isn't immediately user facing and > as such most users will not see the change. However, it does allow for > third-party integrations to be developed against a more stable code base > earlier and allow for a wider range of Koha versions to be supported by > third-party applications.. that can only be seen as a good thing in my > opinion. I agree! Thanks!
I prefer not to push to 17.11.x to focus on major bugs.