Bug 33161 - Implement +strings for GET /items and GET /items/:item_id
Summary: Implement +strings for GET /items and GET /items/:item_id
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 26635
Blocks: 8179
  Show dependency treegraph
 
Reported: 2023-03-07 19:41 UTC by Tomás Cohen Arazi
Modified: 2024-01-09 11:38 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Exposes the `+strings` option on the `/items` endpoints. The allows api consumers to request that string expansions of various coded values from these endpoints are embedded into the response.
Version(s) released in:
23.05.00, 22.11.04


Attachments
Bug 33161: Add +strings support to GET /items and /items/:item_id [WIP] (4.33 KB, patch)
2023-03-08 01:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Add +strings support to GET /items and /items/:item_id (7.37 KB, patch)
2023-03-08 18:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Add +strings support to GET /items and /items/:item_id (7.42 KB, patch)
2023-03-08 19:07 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33161: Add +strings support to GET /items and /items/:item_id (7.88 KB, patch)
2023-03-09 13:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: (follow-up) Consistent use of db fields throughout (4.90 KB, patch)
2023-03-09 13:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: Clarify method names (3.22 KB, patch)
2023-03-09 13:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: Add +strings support to GET /items and /items/:item_id (7.94 KB, patch)
2023-03-09 14:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: (follow-up) Consistent use of db fields throughout (5.04 KB, patch)
2023-03-09 14:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Clarify method names (3.28 KB, patch)
2023-03-09 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: (follow-up) Remove stray variables and fix POD (1.71 KB, patch)
2023-03-09 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Make stub method throw an exception (1.03 KB, patch)
2023-03-09 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Add +strings support to GET /items and /items/:item_id (7.94 KB, patch)
2023-03-09 22:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: (follow-up) Consistent use of db fields throughout (5.04 KB, patch)
2023-03-09 22:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Clarify method names (3.28 KB, patch)
2023-03-09 22:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: (follow-up) Remove stray variables and fix POD (1.71 KB, patch)
2023-03-09 22:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Unit tests (7.29 KB, patch)
2023-03-09 22:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: Add +strings support to GET /items and /items/:item_id (7.99 KB, patch)
2023-03-10 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: (follow-up) Consistent use of db fields throughout (4.96 KB, patch)
2023-03-10 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: Clarify method names (3.62 KB, patch)
2023-03-10 11:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: (follow-up) Remove stray variables and fix POD (1.75 KB, patch)
2023-03-10 11:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: Unit tests (7.35 KB, patch)
2023-03-10 11:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33161: Unit tests (7.69 KB, patch)
2023-03-10 12:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33161: (QA follow-up) Fix api_strings_mapping inconsistency (2.14 KB, patch)
2023-03-10 14:33 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 2023-03-07 19:41:48 UTC
Bug 26635 implemented the barebones, now is time to use it.
Comment 1 Tomás Cohen Arazi 2023-03-08 01:07:50 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2023-03-08 18:43:37 UTC
Created attachment 147973 [details] [review]
Bug 33161: Add +strings support to GET /items and /items/:item_id

This patch introduces the `api_strings_mapping` method to the
*Koha::Item* class, and makes the API spec for the following routes:

* GET /items
* GET /items/:item_id

accept the new `+strings` parameter that can be passed through the
`x-koha-embed` header and was introduced by bug 26635.

The `api_strings_mapping` method has its roots on the cool
`columns_to_str` method already present. The main differences:

* It is aware of the `public_read_list` for attributes so no hidden
  information is exposed.
* Attribute names get mapped for consistency with the API (e.g.
  `homebranch` is converted into `home_library_id`, etc).
* The data structure it returns includes information about the source
  for the descriptions (e.g. it it is an authorised value, then `type`
  will be `av`, and the related category information is returned so
  dropdowns and such can be built. The same goes for other types as
 `library`, `item_type` and `call_number_source`.

To test:
1. Apply this patch
2. Reload everything
3. Play with your favourite REST tool (e.g. Postman)
4. Try:
GET http://localhost:8081/api/v1/items
x-koha-embed: +strings
=> SUCCESS: You get a list of items, they include the new _strings
structure, and the contents make sense!
5. Repeat with a specific item:
GET http://localhost:8081/api/v1/items/14
x-koha-embed: +strings
=> SUCCESS: It all makes sense!
6. Sign off :-D

Sponsored-by: Virginia Polytechnic Institute and State University
Comment 3 Tomás Cohen Arazi 2023-03-08 19:00:44 UTC
Feedback is welcome. This has been around for a while as part of bug 8179, but we ended up with a more refined solution once we got bug 26635 sorted.

Thanks in advance!
Comment 4 Pedro Amorim 2023-03-08 19:07:32 UTC
Created attachment 147977 [details] [review]
Bug 33161: Add +strings support to GET /items and /items/:item_id

This patch introduces the `api_strings_mapping` method to the
*Koha::Item* class, and makes the API spec for the following routes:

* GET /items
* GET /items/:item_id

accept the new `+strings` parameter that can be passed through the
`x-koha-embed` header and was introduced by bug 26635.

The `api_strings_mapping` method has its roots on the cool
`columns_to_str` method already present. The main differences:

* It is aware of the `public_read_list` for attributes so no hidden
  information is exposed.
* Attribute names get mapped for consistency with the API (e.g.
  `homebranch` is converted into `home_library_id`, etc).
* The data structure it returns includes information about the source
  for the descriptions (e.g. it it is an authorised value, then `type`
  will be `av`, and the related category information is returned so
  dropdowns and such can be built. The same goes for other types as
 `library`, `item_type` and `call_number_source`.

To test:
1. Apply this patch
2. Reload everything
3. Play with your favourite REST tool (e.g. Postman)
4. Try:
GET http://localhost:8081/api/v1/items
x-koha-embed: +strings
=> SUCCESS: You get a list of items, they include the new _strings
structure, and the contents make sense!
5. Repeat with a specific item:
GET http://localhost:8081/api/v1/items/14
x-koha-embed: +strings
=> SUCCESS: It all makes sense!
6. Sign off :-D

Sponsored-by: Virginia Polytechnic Institute and State University
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 5 Tomás Cohen Arazi 2023-03-09 13:00:47 UTC
Created attachment 147995 [details] [review]
Bug 33161: Add +strings support to GET /items and /items/:item_id

This patch introduces the `api_strings_mapping` method to the
*Koha::Item* class, and makes the API spec for the following routes:

* GET /items
* GET /items/:item_id
* GET /acquisitions/orders

accept the new `+strings` parameter that can be passed through the
`x-koha-embed` header and was introduced by bug 26635. In the case of
/acquisitions/orders, you will need to use

x-koha-embed: items+strings

I introduce it here to highlight the flebility we introduced with bug
26635.

The `api_strings_mapping` method has its roots on the cool
`columns_to_str` method already present. The main differences:

* It is aware of the `public_read_list` for attributes so no hidden
  information is exposed.
* Attribute names get mapped for consistency with the API (e.g.
  `homebranch` is converted into `home_library_id`, etc).
* The data structure it returns includes information about the source
  for the descriptions (e.g. it it is an authorised value, then `type`
  will be `av`, and the related category information is returned so
  dropdowns and such can be built. The same goes for other types as
 `library`, `item_type` and `call_number_source`.

To test:
1. Apply this patch
2. Reload everything
3. Play with your favourite REST tool (e.g. Postman)
4. Try:
GET http://localhost:8081/api/v1/items
x-koha-embed: +strings
=> SUCCESS: You get a list of items, they include the new _strings
structure, and the contents make sense!
5. Repeat with a specific item:
GET http://localhost:8081/api/v1/items/14
x-koha-embed: +strings
=> SUCCESS: It all makes sense!
6. Sign off :-D

Sponsored-by: Virginia Polytechnic Institute and State University
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 6 Martin Renvoize 2023-03-09 13:09:16 UTC
Created attachment 147998 [details] [review]
Bug 33161: (follow-up) Consistent use of db fields throughout

We should stick to DB field names as long as possible to allow re-use
and only convert to api field names via to_api_mapping at the last
moment inside the parent to_api function.
Comment 7 Martin Renvoize 2023-03-09 13:15:57 UTC
Created attachment 147999 [details] [review]
Bug 33161: Clarify method names

The api_strings_mapping method isn't really only about api strings.. we
deal with database fields in and out.. we just happen to then use those
in to_api to map in the api.

This patch simply renames the standard method whilst we're still early.
Comment 8 Tomás Cohen Arazi 2023-03-09 14:57:55 UTC
Created attachment 148010 [details] [review]
Bug 33161: Add +strings support to GET /items and /items/:item_id

This patch introduces the `api_strings_mapping` method to the
*Koha::Item* class, and makes the API spec for the following routes:

* GET /items
* GET /items/:item_id
* GET /acquisitions/orders

accept the new `+strings` parameter that can be passed through the
`x-koha-embed` header and was introduced by bug 26635. In the case of
/acquisitions/orders, you will need to use

x-koha-embed: items+strings

I introduce it here to highlight the flebility we introduced with bug
26635.

The `api_strings_mapping` method has its roots on the cool
`columns_to_str` method already present. The main differences:

* It is aware of the `public_read_list` for attributes so no hidden
  information is exposed.
* Attribute names get mapped for consistency with the API (e.g.
  `homebranch` is converted into `home_library_id`, etc).
* The data structure it returns includes information about the source
  for the descriptions (e.g. it it is an authorised value, then `type`
  will be `av`, and the related category information is returned so
  dropdowns and such can be built. The same goes for other types as
 `library`, `item_type` and `call_number_source`.

To test:
1. Apply this patch
2. Reload everything
3. Play with your favourite REST tool (e.g. Postman)
4. Try:
GET http://localhost:8081/api/v1/items
x-koha-embed: +strings
=> SUCCESS: You get a list of items, they include the new _strings
structure, and the contents make sense!
5. Repeat with a specific item:
GET http://localhost:8081/api/v1/items/14
x-koha-embed: +strings
=> SUCCESS: It all makes sense!
6. Sign off :-D

Sponsored-by: Virginia Polytechnic Institute and State University
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2023-03-09 14:57:58 UTC
Created attachment 148011 [details] [review]
Bug 33161: (follow-up) Consistent use of db fields throughout

We should stick to DB field names as long as possible to allow re-use
and only convert to api field names via to_api_mapping at the last
moment inside the parent to_api function.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Tomás Cohen Arazi 2023-03-09 14:58:02 UTC
Created attachment 148012 [details] [review]
Bug 33161: Clarify method names

The api_strings_mapping method isn't really only about api strings.. we
deal with database fields in and out.. we just happen to then use those
in to_api to map in the api.

This patch simply renames the standard method whilst we're still early.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 11 Tomás Cohen Arazi 2023-03-09 14:58:05 UTC
Created attachment 148013 [details] [review]
Bug 33161: (follow-up) Remove stray variables and fix POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2023-03-09 14:58:08 UTC
Created attachment 148014 [details] [review]
Bug 33161: Make stub method throw an exception

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2023-03-09 22:50:44 UTC
Created attachment 148039 [details] [review]
Bug 33161: Add +strings support to GET /items and /items/:item_id

This patch introduces the `api_strings_mapping` method to the
*Koha::Item* class, and makes the API spec for the following routes:

* GET /items
* GET /items/:item_id
* GET /acquisitions/orders

accept the new `+strings` parameter that can be passed through the
`x-koha-embed` header and was introduced by bug 26635. In the case of
/acquisitions/orders, you will need to use

x-koha-embed: items+strings

I introduce it here to highlight the flebility we introduced with bug
26635.

The `api_strings_mapping` method has its roots on the cool
`columns_to_str` method already present. The main differences:

* It is aware of the `public_read_list` for attributes so no hidden
  information is exposed.
* Attribute names get mapped for consistency with the API (e.g.
  `homebranch` is converted into `home_library_id`, etc).
* The data structure it returns includes information about the source
  for the descriptions (e.g. it it is an authorised value, then `type`
  will be `av`, and the related category information is returned so
  dropdowns and such can be built. The same goes for other types as
 `library`, `item_type` and `call_number_source`.

To test:
1. Apply this patch
2. Reload everything
3. Play with your favourite REST tool (e.g. Postman)
4. Try:
GET http://localhost:8081/api/v1/items
x-koha-embed: +strings
=> SUCCESS: You get a list of items, they include the new _strings
structure, and the contents make sense!
5. Repeat with a specific item:
GET http://localhost:8081/api/v1/items/14
x-koha-embed: +strings
=> SUCCESS: It all makes sense!
6. Sign off :-D

Sponsored-by: Virginia Polytechnic Institute and State University
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2023-03-09 22:50:48 UTC
Created attachment 148040 [details] [review]
Bug 33161: (follow-up) Consistent use of db fields throughout

We should stick to DB field names as long as possible to allow re-use
and only convert to api field names via to_api_mapping at the last
moment inside the parent to_api function.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2023-03-09 22:50:51 UTC
Created attachment 148041 [details] [review]
Bug 33161: Clarify method names

The api_strings_mapping method isn't really only about api strings.. we
deal with database fields in and out.. we just happen to then use those
in to_api to map in the api.

This patch simply renames the standard method whilst we're still early.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2023-03-09 22:50:55 UTC
Created attachment 148042 [details] [review]
Bug 33161: (follow-up) Remove stray variables and fix POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2023-03-09 22:50:58 UTC
Created attachment 148043 [details] [review]
Bug 33161: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Martin Renvoize 2023-03-10 11:53:51 UTC
Created attachment 148059 [details] [review]
Bug 33161: Add +strings support to GET /items and /items/:item_id

This patch introduces the `api_strings_mapping` method to the
*Koha::Item* class, and makes the API spec for the following routes:

* GET /items
* GET /items/:item_id
* GET /acquisitions/orders

accept the new `+strings` parameter that can be passed through the
`x-koha-embed` header and was introduced by bug 26635. In the case of
/acquisitions/orders, you will need to use

x-koha-embed: items+strings

I introduce it here to highlight the flebility we introduced with bug
26635.

The `api_strings_mapping` method has its roots on the cool
`columns_to_str` method already present. The main differences:

* It is aware of the `public_read_list` for attributes so no hidden
  information is exposed.
* Attribute names get mapped for consistency with the API (e.g.
  `homebranch` is converted into `home_library_id`, etc).
* The data structure it returns includes information about the source
  for the descriptions (e.g. it it is an authorised value, then `type`
  will be `av`, and the related category information is returned so
  dropdowns and such can be built. The same goes for other types as
 `library`, `item_type` and `call_number_source`.

To test:
1. Apply this patch
2. Reload everything
3. Play with your favourite REST tool (e.g. Postman)
4. Try:
GET http://localhost:8081/api/v1/items
x-koha-embed: +strings
=> SUCCESS: You get a list of items, they include the new _strings
structure, and the contents make sense!
5. Repeat with a specific item:
GET http://localhost:8081/api/v1/items/14
x-koha-embed: +strings
=> SUCCESS: It all makes sense!
6. Sign off :-D

Sponsored-by: Virginia Polytechnic Institute and State University
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2023-03-10 11:53:54 UTC
Created attachment 148060 [details] [review]
Bug 33161: (follow-up) Consistent use of db fields throughout

We should stick to DB field names as long as possible to allow re-use
and only convert to api field names via to_api_mapping at the last
moment inside the parent to_api function.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Martin Renvoize 2023-03-10 11:53:57 UTC
Created attachment 148061 [details] [review]
Bug 33161: Clarify method names

The api_strings_mapping method isn't really only about api strings.. we
deal with database fields in and out.. we just happen to then use those
in to_api to map in the api.

This patch simply renames the standard method whilst we're still early.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Martin Renvoize 2023-03-10 11:54:00 UTC
Created attachment 148062 [details] [review]
Bug 33161: (follow-up) Remove stray variables and fix POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2023-03-10 11:54:04 UTC
Created attachment 148063 [details] [review]
Bug 33161: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2023-03-10 11:54:51 UTC
Excellent work, I think this is really nice and clean now!

Passing QA
Comment 24 Tomás Cohen Arazi 2023-03-10 12:45:38 UTC
Created attachment 148068 [details] [review]
Bug 33161: Unit tests

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Edit: added missing test for unmapped attribute

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Tomás Cohen Arazi 2023-03-10 13:24:13 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 26 Tomás Cohen Arazi 2023-03-10 14:33:21 UTC
Created attachment 148075 [details] [review]
Bug 33161: (QA follow-up) Fix api_strings_mapping inconsistency

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 27 Matt Blenkinsop 2023-03-10 16:46:27 UTC
Enhancement - not backporting to 22.11.x
Nice work everyone!
Comment 28 Tomás Cohen Arazi 2023-03-10 16:54:08 UTC
(In reply to Matt Blenkinsop from comment #27)
> Enhancement - not backporting to 22.11.x
> Nice work everyone!

We might want this backported for other backports (ill_requests)
Comment 29 Jonathan Druart 2023-03-17 10:14:57 UTC
I think this makes t/db_dependent/Koha/REST/Plugin/Objects.t fail.
Comment 30 Tomás Cohen Arazi 2023-03-17 10:32:31 UTC
(In reply to Jonathan Druart from comment #29)
> I think this makes t/db_dependent/Koha/REST/Plugin/Objects.t fail.

It is 33de3ef9dcc8d2c0b021b2da80a2c6673cd06e82
Comment 31 Martin Renvoize 2023-03-20 16:46:56 UTC
As agreed with Tomas, I'm backporting this one :)
Comment 32 Martin Renvoize 2023-03-20 16:56:48 UTC
Will be in 22.11.04
Comment 33 Jonathan Druart 2023-04-12 08:08:38 UTC
+  _strings:
+    type:
+      - object
+      - "null"
     description: A return claims object if one exists that's unresolved


Missing description in the api spec file.
Comment 34 Jonathan Druart 2024-01-09 11:38:57 UTC
(In reply to Jonathan Druart from comment #33)
> +  _strings:
> +    type:
> +      - object
> +      - "null"
>      description: A return claims object if one exists that's unresolved
> 
> 
> Missing description in the api spec file.

still valid...