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.
I'm not sure how to tackle this, I think it will need a to_api or TO_JSON overload or something.?
Created attachment 125235 [details] [review] Bug 29105: Add effective_itemtype handling for Item API
Created attachment 125251 [details] [review] Bug 29105: Unit test
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.
So this patch does one direction (i.e. results), but doesn't cater for searches using q_param yet.
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
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?
(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.
+ $self->notforloan( $self->itemtype->notforloan ) unless $self->notforloan; $self->notforloan may be 0 so we should use defined no ?
Changed status to 'In discussion', feel free to change the status back to NSO when it is ready for testing (including a test plan).
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
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.
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.
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!
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>
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>
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
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.05
thx pushed to 21.11.x for 21.11.12
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. API doc is generated, nothing to document, marking resolved.