Bug 29105 - Add effective_item_type_id to the API items responses
Summary: Add effective_item_type_id to the API items responses
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 31084 31128
  Show dependency treegraph
 
Reported: 2021-09-24 09:57 UTC by Martin Renvoize
Modified: 2023-06-08 22:26 UTC (History)
9 users (show)

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


Attachments
Bug 29105: Add effective_itemtype handling for Item API (926 bytes, patch)
2021-09-24 11:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29105: Unit test (1.58 KB, patch)
2021-09-24 14:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29105: Add effective_itemtype handling to items api (1.17 KB, patch)
2021-09-24 14:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29105: Unit test (1.99 KB, patch)
2022-07-11 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29105: Add effective_itemtype handling to items api (1.67 KB, patch)
2022-07-11 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29105: Unit test (2.04 KB, patch)
2022-07-11 16:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29105: Add effective_itemtype handling to items api (1.72 KB, patch)
2022-07-11 16:09 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2021-09-24 09:57:22 UTC
The item objects we return from various items API routes do not respect the effective itype offerings.

Example, itype is mapped directly from the item itype field to item_type.. it should check for effective itemtype and return the result of that.

Similarly notforloan is mapped directly, but can depend on the effective itemtype.

There are possibly others.
Comment 1 Martin Renvoize 2021-09-24 09:58:04 UTC
I'm not sure how to tackle this, I think it will need a to_api or TO_JSON overload or something.?
Comment 2 Martin Renvoize 2021-09-24 11:19:50 UTC
Created attachment 125235 [details] [review]
Bug 29105: Add effective_itemtype handling for Item API
Comment 3 Martin Renvoize 2021-09-24 14:16:48 UTC
Created attachment 125251 [details] [review]
Bug 29105: Unit test
Comment 4 Martin Renvoize 2021-09-24 14:16:51 UTC
Created attachment 125252 [details] [review]
Bug 29105: Add effective_itemtype handling to items api

This patch adds effective_itemtype handling to the items api.

We overload the to_api method to replace the itype column value with
that of the effective itemtype code.

We also add in the notforloan fallback that is apparent in
catalogue/details.tt status templates.
Comment 5 Martin Renvoize 2021-09-24 14:53:46 UTC
So this patch does one direction (i.e. results), but doesn't cater for searches using q_param yet.
Comment 6 Tomás Cohen Arazi 2021-11-08 13:05:48 UTC
When dealing with an 'item' object it is legit to set item_type to null. Returning the effective (calculated if absent on the item) item type might confuse the consumer to think it has an item_type set. I'd prefer we add the option to embed the effective item_type like in:

x-koha-embed: effective_item_type
Comment 7 Martin Renvoize 2021-11-09 09:38:24 UTC
Hmm.. difficult. I can see where you're coming from... but I'd be tempted to not require an embed to enable it's return and rather return it as a standard field.. so itype + effective_itype fields both always present in the result?
Comment 8 Tomás Cohen Arazi 2021-11-09 09:40:57 UTC
(In reply to Martin Renvoize from comment #7)
> Hmm.. difficult. I can see where you're coming from... but I'd be tempted to
> not require an embed to enable it's return and rather return it as a
> standard field.. so itype + effective_itype fields both always present in
> the result?

Yes.
Comment 9 Fridolin Somers 2022-02-25 06:33:05 UTC
+    $self->notforloan( $self->itemtype->notforloan ) unless $self->notforloan;
$self->notforloan may be 0 so we should use defined no ?
Comment 10 David Nind 2022-07-03 18:43:18 UTC
Changed status to 'In discussion', feel free to change the status back to NSO when it is ready for testing (including a test plan).
Comment 11 Martin Renvoize 2022-07-11 12:11:14 UTC
Created attachment 137504 [details] [review]
Bug 29105: Unit test

This patch adds a unit test for the addition of effective_item_type_id
to the items responses.

Test plan
1) Run the unit test prior to applying the second patch and confirm it
   fails
2) Apply the second patch and confirm the unit test now passes
Comment 12 Martin Renvoize 2022-07-11 12:11:19 UTC
Created attachment 137505 [details] [review]
Bug 29105: Add effective_itemtype handling to items api

This patch adds effective_item_type_id to the items api.

We overload the to_api method to append the effective_item_type_id field
with that of the effective itemtype code.
Comment 13 Martin Renvoize 2022-07-11 12:12:40 UTC
Patches updated as requested and not handling notforloan status as I'll submit that as another bug

We now return both item_type_id and the new effective_item_type_id field in the same response.
Comment 14 Martin Renvoize 2022-07-11 12:15:36 UTC
I'm still somewhat torn here.. it feels wrong somehow to require intimate knowledge of how Koha works with regards to overloading features to be able to use this API route..  an average consumer won't necessarily understand the difference between these fields.

I wondered about taking an alternate approach of requiring a header to revert from automatically replacing item_type_id with the effective equivalent instead to allow for Koha staff client use on the editing of item pages.

Difficult!
Comment 15 Nick Clemens 2022-07-11 16:09:14 UTC
Created attachment 137572 [details] [review]
Bug 29105: Unit test

This patch adds a unit test for the addition of effective_item_type_id
to the items responses.

Test plan
1) Run the unit test prior to applying the second patch and confirm it
   fails
2) Apply the second patch and confirm the unit test now passes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens 2022-07-11 16:09:18 UTC
Created attachment 137573 [details] [review]
Bug 29105: Add effective_itemtype handling to items api

This patch adds effective_item_type_id to the items api.

We overload the to_api method to append the effective_item_type_id field
with that of the effective itemtype code.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2022-07-11 16:20:42 UTC
I think this is fine, as long as this feature (item-level_itpyes set to 'biblio') exists - libraries using biblio itemtypes seem to be in the minority according to:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29106

If we remove the feature later, we remove the extra field

small patch, straight to PQA to let RM weigh in
Comment 18 Tomás Cohen Arazi 2022-07-18 17:42:31 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 19 Lucas Gass 2022-08-23 19:20:29 UTC
Backported to 22.05.x for 22.05.05
Comment 20 Arthur Suzuki 2022-08-30 09:21:48 UTC
thx pushed to 21.11.x for 21.11.12
Comment 21 Victor Grousset/tuxayo 2022-09-14 19:06:27 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

API doc is generated, nothing to document, marking resolved.