This patch adds a new route to the REST api: api/v1/acquisitions/funds/ It allows to retrieve funds informations, possibly using filters.
Created attachment 69243 [details] [review] REST API: Funds Endpoint This patch adds a new route to the REST api: api/v1/acquisitions/funds/
Created attachment 69833 [details] [review] Bug 19661 - REST API: Funds Endpoint This patch adds a new route to the REST api: api/v1/acquisitions/funds/ https://bugs.koha-community.org/show_bug.cgi?id=19661 Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Matthias, everything looks ok. Could you please describe the enpdoint here: https://wiki.koha-community.org/wiki/REST_api_RFCs (You can copy and paste from https://wiki.koha-community.org/wiki/Patrons_endpoint_RFC and adapt it). Also, would you consider implementing it using the objects.search implementation from Julian?
Should we move this out of the queue until 19826 is ready and this can be redone on top?
Created attachment 70603 [details] [review] Bug 19661 - Use Koha::Acquisition::Funds instead of C4::Budgets
Documentation updated here: https://wiki.koha-community.org/wiki/Acquisitions_funds_endpoint_RFC
I think this work needs a review, as some (relevant) attributes are missing on the fund definition: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=Koha/Schema/Result/Aqbudget.pm;h=12a8c8d730afe8a1557e836742a7cafd33255388;hb=HEAD
I've added the ''needs_rfc'' keyword to this bug. The community process requires a vote on the RFC, so the dev and interested parties should take care of adding the RFC to the agenda on a dev meeting, and gather support for it.
Created attachment 83941 [details] [review] Bug 19661 - Add and change properties and parameters to conform to the funds RFC. Also, perform an exact search when searching on the fund owner id.
(In reply to Matthias Meusburger from comment #9) > Created attachment 83941 [details] [review] [review] > Bug 19661 - Add and change properties and parameters to conform to the funds > RFC. > > Also, perform an exact search when searching on the fund owner id. Matthias, great work. I would like to mention I'm proposing a change here: https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API basically standardize return codes and add the Location header in POST (see bug 16497 for an example).
Comment on attachment 69833 [details] [review] Bug 19661 - REST API: Funds Endpoint Review of attachment 69833 [details] [review]: ----------------------------------------------------------------- ::: Koha/REST/V1/Acquisitions/Funds.pm @@ +19,5 @@ > + > +use Mojo::Base 'Mojolicious::Controller'; > + > +use C4::Budgets; > +use JSON qw(to_json); Unused import here @@ +83,5 @@ > + > + return $returnfund; > +} > + > +=head3 _to_model Should this private routine be introduced before any corresponding write functions are added to the controller or should it be added when those routines are?
Created attachment 83970 [details] [review] Bug 19661 - Add and change properties and parameters to conform to the funds RFC. Also, perform an exact search when searching on the fund owner id.
> Unused import here Unused import removed. > Should this private routine be introduced before any corresponding write functions are added to the controller or should it be added when those routines are? I just wanted to make future work easier :) Besides, since the RFC has already been voted, it's unlikely to change before new features are added. But any option is fine by me.
Created attachment 85693 [details] [review] Bug 19661: REST API - Funds Endpoint This patch adds a new route to the REST api: /api/v1/acquisitions/funds/ Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85694 [details] [review] Bug 19661: Use Koha::Acquisition::Funds instead of C4::Budgets Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85695 [details] [review] Bug 19661: Add and change properties and parameters to conform to the funds RFC. Also, perform an exact search when searching on the fund owner id. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85696 [details] [review] Bug 19661: (follow-up) Use Basic auth in tests Test plan: prove t/db_dependent/api/v1/acquisitions_funds.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 86537 [details] [review] Bug 19661: REST API - Funds Endpoint This patch adds a new route to the REST api: /api/v1/acquisitions/funds/ Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 86538 [details] [review] Bug 19661: Use Koha::Acquisition::Funds instead of C4::Budgets Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 86539 [details] [review] Bug 19661: Add and change properties and parameters to conform to the funds RFC. Also, perform an exact search when searching on the fund owner id. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 86540 [details] [review] Bug 19661: (follow-up) Use Basic auth in tests Test plan: prove t/db_dependent/api/v1/acquisitions_funds.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks Matthias, All works well, no QA Script failures and as a new route there's no regressions.. Passing QA
Awesome work all! Pushed to master for 19.05
As per my commitment to backporting new API endpoints wherever reasonable to do so... Pushed to 18.11.x for 18.11.05
cant prove t/db_dependent/api/v1/acquisitions_funds.t correctly in 18.05.x, not backporting new feature