Description
Tomás Cohen Arazi
2023-03-07 19:41:48 UTC
Created attachment 147888 [details] [review] Bug 33161: Add +strings support to GET /items and /items/:item_id [WIP] 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 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! 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> 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> 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. 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. 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> 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> 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> Created attachment 148013 [details] [review] Bug 33161: (follow-up) Remove stray variables and fix POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 148014 [details] [review] Bug 33161: Make stub method throw an exception Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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> 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> 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> Created attachment 148042 [details] [review] Bug 33161: (follow-up) Remove stray variables and fix POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 148043 [details] [review] Bug 33161: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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> 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> 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> 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> 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> Excellent work, I think this is really nice and clean now! Passing QA 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> Pushed to master for 23.05. Nice work everyone, thanks! Created attachment 148075 [details] [review] Bug 33161: (QA follow-up) Fix api_strings_mapping inconsistency Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Enhancement - not backporting to 22.11.x Nice work everyone! (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) I think this makes t/db_dependent/Koha/REST/Plugin/Objects.t fail. (In reply to Jonathan Druart from comment #29) > I think this makes t/db_dependent/Koha/REST/Plugin/Objects.t fail. It is 33de3ef9dcc8d2c0b021b2da80a2c6673cd06e82 As agreed with Tomas, I'm backporting this one :) Will be in 22.11.04 + _strings: + type: + - object + - "null" description: A return claims object if one exists that's unresolved Missing description in the api spec file. (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... |