Bug 27855 - Allow embedding extended_attributes on /patrons routes
Summary: Allow embedding extended_attributes on /patrons routes
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: Martin Renvoize
URL:
Keywords:
Depends on: 26636 27854
Blocks: 27853 28002
  Show dependency treegraph
 
Reported: 2021-03-03 19:48 UTC by Tomás Cohen Arazi
Modified: 2021-12-13 21:13 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement allows patron extended attributes to be embedded into the patron object responses on the restful api.
Version(s) released in:
21.05.00


Attachments
Bug 27855: Add extended_attribute OpenAPI spec (1.12 KB, patch)
2021-03-04 09:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping (1.22 KB, patch)
2021-03-04 09:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27855: Make GET /patrons/:patron_id use objects.find (1.55 KB, patch)
2021-03-04 09:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27855: Add 'extended_attributes' embedding to the spec (2.96 KB, patch)
2021-03-04 09:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27855: Add extended_attribute OpenAPI spec (1.18 KB, patch)
2021-03-04 11:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping (1.29 KB, patch)
2021-03-04 11:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27855: Make GET /patrons/:patron_id use objects.find (1.62 KB, patch)
2021-03-04 11:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27855: Add 'extended_attributes' embedding to the spec (3.03 KB, patch)
2021-03-04 11:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27855: Add extended_attribute OpenAPI spec (1.18 KB, patch)
2021-03-04 11:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping (1.28 KB, patch)
2021-03-04 11:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Make GET /patrons/:patron_id use objects.find (1.62 KB, patch)
2021-03-04 11:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Add 'extended_attributes' embedding to the spec (3.02 KB, patch)
2021-03-04 11:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: (QA follow-up) Merge extended_attribute.json into patron.json (2.44 KB, patch)
2021-03-04 11:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Add extended_attribute OpenAPI spec (1.23 KB, patch)
2021-03-04 11:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping (1.33 KB, patch)
2021-03-04 11:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Make GET /patrons/:patron_id use objects.find (1.67 KB, patch)
2021-03-04 11:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: Add 'extended_attributes' embedding to the spec (3.08 KB, patch)
2021-03-04 11:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: (QA follow-up) Merge extended_attribute.json into patron.json (2.44 KB, patch)
2021-03-04 11:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27855: (QA follow-up) Rename extended_attribute to patron_extended_attribute (1.52 KB, patch)
2021-03-04 12:00 UTC, Martin Renvoize
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 2021-03-03 19:48:56 UTC

    
Comment 1 Tomás Cohen Arazi 2021-03-04 09:55:49 UTC
Created attachment 117660 [details] [review]
Bug 27855: Add extended_attribute OpenAPI spec
Comment 2 Tomás Cohen Arazi 2021-03-04 09:55:52 UTC
Created attachment 117661 [details] [review]
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping

This patch introduces a mapping to render extended attributes on the
API. As they have an ID, and they will generally be used on the
/patrons umbrella, I removed the borrowernumber on the mapping.

Another option would be to add the field, but make it optional. It
really feels redundant, so I take it out for now.
Comment 3 Tomás Cohen Arazi 2021-03-04 09:55:56 UTC
Created attachment 117662 [details] [review]
Bug 27855: Make GET /patrons/:patron_id use objects.find

This patch makes the route for fetching a patron use the objects.find
helper instead of a plain Koha::Patrons->find. This gives the controller
embedding superpowers.

To test, we just need to check nothing broke:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 4 Tomás Cohen Arazi 2021-03-04 09:55:59 UTC
Created attachment 117663 [details] [review]
Bug 27855: Add 'extended_attributes' embedding to the spec

This patch just adds:
- The optional 'extended_attribute' attribute to the patron object
  OpenAPI definition. It consists of an array of extended_attribute
  objects.
- Add the x-koha-embed: [ 'extended_attributes' ] definition on the GET
  routes for patrons, allowing immedite availability of the
  'extended_attributes' embedding feature.

To test:
1. Apply this patchset
2. Restart Plack
3. Have some known patron_id/borrowernumber that has some extended
   attributes ('Patron attributes' on the UI).
4. Enable Basic authentication
5. Assuming the known patron_id is 1, point your favourite REST tool to http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1
   e.g.: if your user/pass is koha/koha
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron object correctly, no extended_attributes
6. Repeat 5, adding the x-koha-embed header like this:
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'x-koha-embed: extended_attributes' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron, with the extended attributes inside!
7. Sign off :-D
Comment 5 Kyle M Hall 2021-03-04 11:41:11 UTC
Created attachment 117681 [details] [review]
Bug 27855: Add extended_attribute OpenAPI spec

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2021-03-04 11:41:21 UTC
Created attachment 117682 [details] [review]
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping

This patch introduces a mapping to render extended attributes on the
API. As they have an ID, and they will generally be used on the
/patrons umbrella, I removed the borrowernumber on the mapping.

Another option would be to add the field, but make it optional. It
really feels redundant, so I take it out for now.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2021-03-04 11:41:24 UTC
Created attachment 117683 [details] [review]
Bug 27855: Make GET /patrons/:patron_id use objects.find

This patch makes the route for fetching a patron use the objects.find
helper instead of a plain Koha::Patrons->find. This gives the controller
embedding superpowers.

To test, we just need to check nothing broke:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2021-03-04 11:41:27 UTC
Created attachment 117684 [details] [review]
Bug 27855: Add 'extended_attributes' embedding to the spec

This patch just adds:
- The optional 'extended_attribute' attribute to the patron object
  OpenAPI definition. It consists of an array of extended_attribute
  objects.
- Add the x-koha-embed: [ 'extended_attributes' ] definition on the GET
  routes for patrons, allowing immedite availability of the
  'extended_attributes' embedding feature.

To test:
1. Apply this patchset
2. Restart Plack
3. Have some known patron_id/borrowernumber that has some extended
   attributes ('Patron attributes' on the UI).
4. Enable Basic authentication
5. Assuming the known patron_id is 1, point your favourite REST tool to http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1
   e.g.: if your user/pass is koha/koha
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron object correctly, no extended_attributes
6. Repeat 5, adding the x-koha-embed header like this:
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'x-koha-embed: extended_attributes' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron, with the extended attributes inside!
7. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Martin Renvoize 2021-03-04 11:41:36 UTC
Created attachment 117685 [details] [review]
Bug 27855: Add extended_attribute OpenAPI spec

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-03-04 11:41:40 UTC
Created attachment 117686 [details] [review]
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping

This patch introduces a mapping to render extended attributes on the
API. As they have an ID, and they will generally be used on the
/patrons umbrella, I removed the borrowernumber on the mapping.

Another option would be to add the field, but make it optional. It
really feels redundant, so I take it out for now.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2021-03-04 11:41:43 UTC
Created attachment 117687 [details] [review]
Bug 27855: Make GET /patrons/:patron_id use objects.find

This patch makes the route for fetching a patron use the objects.find
helper instead of a plain Koha::Patrons->find. This gives the controller
embedding superpowers.

To test, we just need to check nothing broke:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-03-04 11:41:47 UTC
Created attachment 117688 [details] [review]
Bug 27855: Add 'extended_attributes' embedding to the spec

This patch just adds:
- The optional 'extended_attribute' attribute to the patron object
  OpenAPI definition. It consists of an array of extended_attribute
  objects.
- Add the x-koha-embed: [ 'extended_attributes' ] definition on the GET
  routes for patrons, allowing immedite availability of the
  'extended_attributes' embedding feature.

To test:
1. Apply this patchset
2. Restart Plack
3. Have some known patron_id/borrowernumber that has some extended
   attributes ('Patron attributes' on the UI).
4. Enable Basic authentication
5. Assuming the known patron_id is 1, point your favourite REST tool to http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1
   e.g.: if your user/pass is koha/koha
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron object correctly, no extended_attributes
6. Repeat 5, adding the x-koha-embed header like this:
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'x-koha-embed: extended_attributes' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron, with the extended attributes inside!
7. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2021-03-04 11:41:50 UTC
Created attachment 117689 [details] [review]
Bug 27855: (QA follow-up) Merge extended_attribute.json into patron.json

As the extended_attribute object is only ever returned inline with
patrons,this patch merges the specification into the patron definition
to clarify it usage.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2021-03-04 11:43:07 UTC
Created attachment 117690 [details] [review]
Bug 27855: Add extended_attribute OpenAPI spec

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2021-03-04 11:43:10 UTC
Created attachment 117691 [details] [review]
Bug 27855: Implement Koha::Patron::Attribute->to_api_mapping

This patch introduces a mapping to render extended attributes on the
API. As they have an ID, and they will generally be used on the
/patrons umbrella, I removed the borrowernumber on the mapping.

Another option would be to add the field, but make it optional. It
really feels redundant, so I take it out for now.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2021-03-04 11:43:13 UTC
Created attachment 117692 [details] [review]
Bug 27855: Make GET /patrons/:patron_id use objects.find

This patch makes the route for fetching a patron use the objects.find
helper instead of a plain Koha::Patrons->find. This gives the controller
embedding superpowers.

To test, we just need to check nothing broke:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2021-03-04 11:43:16 UTC
Created attachment 117693 [details] [review]
Bug 27855: Add 'extended_attributes' embedding to the spec

This patch just adds:
- The optional 'extended_attribute' attribute to the patron object
  OpenAPI definition. It consists of an array of extended_attribute
  objects.
- Add the x-koha-embed: [ 'extended_attributes' ] definition on the GET
  routes for patrons, allowing immedite availability of the
  'extended_attributes' embedding feature.

To test:
1. Apply this patchset
2. Restart Plack
3. Have some known patron_id/borrowernumber that has some extended
   attributes ('Patron attributes' on the UI).
4. Enable Basic authentication
5. Assuming the known patron_id is 1, point your favourite REST tool to http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1
   e.g.: if your user/pass is koha/koha
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron object correctly, no extended_attributes
6. Repeat 5, adding the x-koha-embed header like this:
   curl --location --request GET 'http://kohadev-intra.myDNSname.org:8081/api/v1/patrons/1' \
        --header 'Content-Type: application/json' \
        --header 'x-koha-embed: extended_attributes' \
        --header 'Authorization: Basic a29oYTprb2hh'
=> SUCCESS: You get the patron, with the extended attributes inside!
7. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2021-03-04 11:43:20 UTC
Created attachment 117694 [details] [review]
Bug 27855: (QA follow-up) Merge extended_attribute.json into patron.json

As the extended_attribute object is only ever returned inline with
patrons,this patch merges the specification into the patron definition
to clarify it usage.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2021-03-04 12:00:56 UTC
Created attachment 117695 [details] [review]
Bug 27855: (QA follow-up) Rename extended_attribute to patron_extended_attribute

This path renames the extended_attribute definition to
patron_extended_attribute to clarify that these attributes are always
attached to a patron and not a more generic class of attributes.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2021-03-04 12:03:14 UTC
Works great.. this is almost too easy with the nice foundations we have in the API helpers now!

Passing QA
Comment 21 Jonathan Druart 2021-03-09 09:55:31 UTC
Is there a plan for those routes to be used from Koha?
moremember and memberentry would benefit a lot from this.
Comment 22 Jonathan Druart 2021-03-09 10:02:49 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 23 Martin Renvoize 2021-03-09 10:34:09 UTC
(In reply to Jonathan Druart from comment #21)
> Is there a plan for those routes to be used from Koha?
> moremember and memberentry would benefit a lot from this.

No plan yet.. we were very keen to have it for customers who want to start managing background patron imports using a modern API.

However.. I do love the idea of moving moremember and memberentry forward using these routes :)
Comment 24 Fridolin Somers 2021-03-12 08:52:05 UTC
Enhancement not pushed to 20.11.x