Bug 19661

Summary: Add route for fund retrieval
Product: Koha Reporter: Matthias Meusburger <matthias.meusburger>
Component: REST APIAssignee: Matthias Meusburger <matthias.meusburger>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: new feature    
Priority: P5 - low CC: black23, david, katrin.fischer, lucas, martin.renvoize, nick, tomascohen
Version: master   
Hardware: All   
OS: All   
URL: https://wiki.koha-community.org/wiki/Acquisitions_funds_endpoint_RFC
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This new feature adds a REST API for working with acquisition funds, including listing, adding and deleting funds and fund users.
Version(s) released in:
Bug Depends on: 19826    
Bug Blocks:    
Attachments: REST API: Funds Endpoint
Bug 19661 - REST API: Funds Endpoint
Bug 19661 - Use Koha::Acquisition::Funds instead of C4::Budgets
Bug 19661 - Add and change properties and parameters to conform to the funds RFC.
Bug 19661 - Add and change properties and parameters to conform to the funds RFC.
Bug 19661: REST API - Funds Endpoint
Bug 19661: Use Koha::Acquisition::Funds instead of C4::Budgets
Bug 19661: Add and change properties and parameters to conform to the funds RFC.
Bug 19661: (follow-up) Use Basic auth in tests
Bug 19661: REST API - Funds Endpoint
Bug 19661: Use Koha::Acquisition::Funds instead of C4::Budgets
Bug 19661: Add and change properties and parameters to conform to the funds RFC.
Bug 19661: (follow-up) Use Basic auth in tests

Description Matthias Meusburger 2017-11-21 07:52:11 UTC
This patch adds a new route to the REST api:

api/v1/acquisitions/funds/

It allows to retrieve funds informations, possibly using filters.
Comment 1 Matthias Meusburger 2017-11-21 07:53:08 UTC
Created attachment 69243 [details] [review]
REST API: Funds Endpoint

This patch adds a new route to the REST api:

api/v1/acquisitions/funds/
Comment 2 David Bourgault 2017-12-15 20:59:23 UTC
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>
Comment 3 Tomás Cohen Arazi 2017-12-15 21:31:09 UTC
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?
Comment 4 Katrin Fischer 2017-12-26 16:03:22 UTC
Should we move this out of the queue until 19826 is ready and this can be redone on top?
Comment 5 Matthias Meusburger 2018-01-17 10:51:06 UTC
Created attachment 70603 [details] [review]
Bug 19661 - Use Koha::Acquisition::Funds instead of C4::Budgets
Comment 6 Matthias Meusburger 2018-01-17 11:16:36 UTC
Documentation updated here: https://wiki.koha-community.org/wiki/Acquisitions_funds_endpoint_RFC
Comment 7 Tomás Cohen Arazi 2018-01-18 13:07:06 UTC
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
Comment 8 Tomás Cohen Arazi 2018-02-27 13:56:49 UTC
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.
Comment 9 Matthias Meusburger 2019-01-15 09:48:48 UTC
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.
Comment 10 Tomás Cohen Arazi 2019-01-15 10:43:45 UTC
(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 11 Martin Renvoize 2019-01-15 12:27:05 UTC
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?
Comment 12 Matthias Meusburger 2019-01-15 13:46:48 UTC
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.
Comment 13 Matthias Meusburger 2019-01-15 13:50:49 UTC
> 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.
Comment 14 Josef Moravec 2019-02-26 09:20:35 UTC
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>
Comment 15 Josef Moravec 2019-02-26 09:20:38 UTC
Created attachment 85694 [details] [review]
Bug 19661: Use Koha::Acquisition::Funds instead of C4::Budgets

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Josef Moravec 2019-02-26 09:20:42 UTC
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>
Comment 17 Josef Moravec 2019-02-26 09:20:45 UTC
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>
Comment 18 Martin Renvoize 2019-03-12 13:33:38 UTC
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>
Comment 19 Martin Renvoize 2019-03-12 13:33:42 UTC
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>
Comment 20 Martin Renvoize 2019-03-12 13:33:45 UTC
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>
Comment 21 Martin Renvoize 2019-03-12 13:33:48 UTC
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>
Comment 22 Martin Renvoize 2019-03-12 13:34:47 UTC
Thanks Matthias, 

All works well, no QA Script failures and as a new route there's no regressions..


Passing QA
Comment 23 Nick Clemens 2019-03-22 20:36:28 UTC
Awesome work all!

Pushed to master for 19.05
Comment 24 Martin Renvoize 2019-04-01 10:29:46 UTC
As per my commitment to backporting new API endpoints wherever reasonable to do so... Pushed to 18.11.x for 18.11.05
Comment 25 Lucas Gass 2019-04-01 15:58:52 UTC
cant prove t/db_dependent/api/v1/acquisitions_funds.t correctly in 18.05.x, not backporting new feature