Bug 20006 - Adapt /v1/holds to new naming guidelines
Summary: Adapt /v1/holds to new naming guidelines
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL: https://wiki.koha-community.org/wiki/...
Keywords:
Depends on: 19816
Blocks: 19774 22206 22218
  Show dependency treegraph
 
Reported: 2018-01-17 17:50 UTC by Tomás Cohen Arazi
Modified: 2020-11-12 15:07 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20006: Spec changes (13.71 KB, patch)
2019-01-24 17:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: Unit tests fixes (10.09 KB, patch)
2019-01-24 17:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
But 20006: Adapt /holds to the RFC (15.75 KB, patch)
2019-01-24 17:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: Adapt /holds to the RFC (15.75 KB, patch)
2019-01-24 18:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: (follow-up) Fix holds.js (1.51 KB, patch)
2019-01-24 18:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: (follow-up) Plural class name (3.46 KB, patch)
2019-01-24 19:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: Spec changes (13.77 KB, patch)
2019-01-25 08:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20006: Unit tests fixes (10.15 KB, patch)
2019-01-25 08:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20006: Adapt /holds to the RFC (15.82 KB, patch)
2019-01-25 08:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20006: (follow-up) Fix holds.js (1.56 KB, patch)
2019-01-25 08:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20006: (follow-up) Plural class name (3.51 KB, patch)
2019-01-25 08:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20006: Spec changes (13.83 KB, patch)
2019-02-13 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20006: Unit tests fixes (10.21 KB, patch)
2019-02-13 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20006: Adapt /holds to the RFC (15.87 KB, patch)
2019-02-13 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20006: (follow-up) Fix holds.js (1.63 KB, patch)
2019-02-13 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20006: (follow-up) Plural class name (3.58 KB, patch)
2019-02-13 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20006: Spec changes (13.92 KB, patch)
2019-02-28 12:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: Unit tests fixes (10.21 KB, patch)
2019-02-28 12:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: Adapt /holds to the RFC (15.87 KB, patch)
2019-02-28 12:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: (follow-up) Fix holds.js (1.63 KB, patch)
2019-02-28 12:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20006: (follow-up) Plural class name (3.58 KB, patch)
2019-02-28 12:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2018-01-17 17:50:22 UTC

    
Comment 1 Tomás Cohen Arazi 2019-01-24 17:57:39 UTC
Created attachment 84374 [details] [review]
Bug 20006: Spec changes

This patch makes the spec match the voted RFC.
Comment 2 Tomás Cohen Arazi 2019-01-24 17:57:43 UTC
Created attachment 84375 [details] [review]
Bug 20006: Unit tests fixes

This patch makes the holds endpoint tests pass and expect data structures that match the voted RFC.

As we voted to introduce a /public namespace for unprivileged access to
endpoints, this endpoint gets the ability for owners and guarantors to
manipulate holds through the API.
Comment 3 Tomás Cohen Arazi 2019-01-24 17:57:46 UTC
Created attachment 84376 [details] [review]
But 20006: Adapt /holds to the RFC

This patch makes the /holds endpoint respect the voted RFC. Some
behaviours are changed as well:

- As we voted to introduce a /public namespace for unprivileged access to
endpoints, this endpoint gets the ability for owners and guarantors to
manipulate holds through the API.

- GET /holds now uses the objects->search helper, so it now has
pagination.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 4 Tomás Cohen Arazi 2019-01-24 17:58:55 UTC
TODO: Still need to adjust the holds page to use the new aPI spec.
Comment 5 Tomás Cohen Arazi 2019-01-24 18:22:25 UTC
Created attachment 84381 [details] [review]
Bug 20006: Adapt /holds to the RFC

This patch makes the /holds endpoint respect the voted RFC. Some
behaviours are changed as well:

- As we voted to introduce a /public namespace for unprivileged access to
endpoints, this endpoint gets the ability for owners and guarantors to
manipulate holds through the API.

- GET /holds now uses the objects->search helper, so it now has
pagination.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 6 Tomás Cohen Arazi 2019-01-24 18:22:30 UTC
Created attachment 84382 [details] [review]
Bug 20006: (follow-up) Fix holds.js

This patch makes holds.js call the endpoint with 'pickup_library_id'
instead of the old 'branchcode' property.

To test:
- Place a hold
- Go to the patron's detail page
- Go to the Holds tab
- Change the pickup location
=> SUCCESS: It works!
Comment 7 Tomás Cohen Arazi 2019-01-24 19:13:46 UTC
Created attachment 84386 [details] [review]
Bug 20006: (follow-up) Plural class name
Comment 8 Josef Moravec 2019-01-25 08:39:29 UTC Comment hidden (obsolete)
Comment 9 Josef Moravec 2019-01-25 08:39:32 UTC Comment hidden (obsolete)
Comment 10 Josef Moravec 2019-01-25 08:39:36 UTC Comment hidden (obsolete)
Comment 11 Josef Moravec 2019-01-25 08:39:39 UTC Comment hidden (obsolete)
Comment 12 Josef Moravec 2019-01-25 08:39:42 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize 2019-02-13 10:57:28 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize 2019-02-13 10:57:31 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize 2019-02-13 10:57:34 UTC Comment hidden (obsolete)
Comment 16 Martin Renvoize 2019-02-13 10:57:37 UTC Comment hidden (obsolete)
Comment 17 Martin Renvoize 2019-02-13 10:57:40 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2019-02-13 10:58:24 UTC
I've given this a pretty thorough testing and we're passing the QA scripts.. passing QA.
Comment 19 Martin Renvoize 2019-02-13 11:01:22 UTC
Side Note: We need some clear release notes for this one as it's a breaking change in the api functionality and we've not incremented the api version for it.
Comment 20 Martin Renvoize 2019-02-22 15:02:03 UTC
Tomas and I discussed this and we're confident it's good to push for 19.05 without an api version bump. We believe the v1 api isn't in common use yet as it's still 'stabalising' and as such this is good to go.
Comment 21 Tomás Cohen Arazi 2019-02-28 12:48:28 UTC
Created attachment 85856 [details] [review]
Bug 20006: Spec changes

This patch makes the spec match the voted RFC.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Tomás Cohen Arazi 2019-02-28 12:48:32 UTC
Created attachment 85857 [details] [review]
Bug 20006: Unit tests fixes

This patch makes the holds endpoint tests pass and expect data structures that match the voted RFC.

As we voted to introduce a /public namespace for unprivileged access to
endpoints, this endpoint gets the ability for owners and guarantors to
manipulate holds through the API.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Tomás Cohen Arazi 2019-02-28 12:48:35 UTC
Created attachment 85858 [details] [review]
Bug 20006: Adapt /holds to the RFC

This patch makes the /holds endpoint respect the voted RFC. Some
behaviours are changed as well:

- As we voted to introduce a /public namespace for unprivileged access to
endpoints, this endpoint gets the ability for owners and guarantors to
manipulate holds through the API.

- GET /holds now uses the objects->search helper, so it now has
pagination.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Tomás Cohen Arazi 2019-02-28 12:48:39 UTC
Created attachment 85859 [details] [review]
Bug 20006: (follow-up) Fix holds.js

This patch makes holds.js call the endpoint with 'pickup_library_id'
instead of the old 'branchcode' property.

To test:
- Place a hold
- Go to the patron's detail page
- Go to the Holds tab
- Change the pickup location
=> SUCCESS: It works!

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Tomás Cohen Arazi 2019-02-28 12:48:43 UTC
Created attachment 85860 [details] [review]
Bug 20006: (follow-up) Plural class name

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Tomás Cohen Arazi 2019-02-28 13:05:50 UTC
I've just rebased it to current master (conflict in parameters.json after /libraries got pushed).
Comment 27 Nick Clemens 2019-03-15 19:57:08 UTC
Awesome work all!

Pushed to master for 19.05
Comment 28 Martin Renvoize 2019-03-19 10:47:23 UTC
Enhancement with breaking changes.. will not be backported to 18.11.x series.
Comment 29 Martin Renvoize 2019-03-19 10:54:38 UTC
I decided to backport this in the end.

As per my policy of backporting new/enhanced API endpoints where it's simple to do so.

I believe, although this is a breaking change, it's better to get this change in early and people less people to adapt to it now rather than leave it to be adapted later once more people are using the API's.

Pushed to 18.11.x for 18.11.04
Comment 30 Lucas Gass 2019-03-26 23:36:40 UTC
enhancment not backporting to 18.05.x