Bugzilla – Attachment 190922 Details for
Bug 41530
item_type vs item_type_id inconsistencies in the API
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41530: Rename item_type => item_type_id in transfer_limits API spec
cff89b6.patch (text/plain), 11.83 KB, created by
Martin Renvoize (ashimema)
on 2026-01-06 10:47:53 UTC
(
hide
)
Description:
Bug 41530: Rename item_type => item_type_id in transfer_limits API spec
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-01-06 10:47:53 UTC
Size:
11.83 KB
patch
obsolete
>From cff89b6c9e80add9410109994752eaa463af225f Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Tue, 6 Jan 2026 10:38:47 +0000 >Subject: [PATCH] Bug 41530: Rename item_type => item_type_id in > transfer_limits API spec > >Following the API consistency work in bug 41456, this patch renames >the item_type field to item_type_id in the transfer_limits API >endpoints and definitions. > >The field should only be named item_type when it contains an embedded >object (type: object/null). When it contains a foreign key ID >(type: string/null), it should be named item_type_id for consistency. > >Changes: >- Updated api/v1/swagger/definitions/transfer_limit.yaml >- Updated api/v1/swagger/paths/transfer_limits.yaml (including documentation) >- Updated Koha::Item::Transfer::Limit->to_api_mapping >- Updated Koha::REST::V1::TransferLimits error messages >- Updated t/db_dependent/api/v1/transfer_limits.t >- Updated koha-tmpl/intranet-tmpl/prog/en/modules/admin/transfer_limits.tt > >Test plan: >1. Run API tests: prove t/db_dependent/api/v1/transfer_limits.t >2. Visit /cgi-bin/koha/admin/transfer_limits.pl >3. Select an item type from the dropdown >4. Check/uncheck transfer limits - verify they work correctly >5. Confirm the API requests use item_type_id field >--- > Koha/Item/Transfer/Limit.pm | 2 +- > Koha/REST/V1/TransferLimits.pm | 12 ++++---- > .../swagger/definitions/transfer_limit.yaml | 4 +-- > api/v1/swagger/paths/transfer_limits.yaml | 28 +++++++++---------- > .../prog/en/modules/admin/transfer_limits.tt | 2 +- > t/db_dependent/api/v1/transfer_limits.t | 14 ++++++---- > 6 files changed, 32 insertions(+), 30 deletions(-) > >diff --git a/Koha/Item/Transfer/Limit.pm b/Koha/Item/Transfer/Limit.pm >index 7bb7fbc1500..46309e72638 100644 >--- a/Koha/Item/Transfer/Limit.pm >+++ b/Koha/Item/Transfer/Limit.pm >@@ -44,7 +44,7 @@ sub to_api_mapping { > limitId => 'limit_id', > toBranch => 'to_library_id', > fromBranch => 'from_library_id', >- itemtype => 'item_type', >+ itemtype => 'item_type_id', > ccode => 'collection_code', > }; > } >diff --git a/Koha/REST/V1/TransferLimits.pm b/Koha/REST/V1/TransferLimits.pm >index 93af1d846af..63cbcef0740 100644 >--- a/Koha/REST/V1/TransferLimits.pm >+++ b/Koha/REST/V1/TransferLimits.pm >@@ -126,24 +126,24 @@ sub batch_add { > return try { > my $params = $c->req->json; > >- if ( $params->{item_type} && $params->{collection_code} ) { >+ if ( $params->{item_type_id} && $params->{collection_code} ) { > return $c->render( > status => 400, > openapi => { >- error => "You can only pass 'item_type' or 'collection_code' at a time", >+ error => "You can only pass 'item_type_id' or 'collection_code' at a time", > } > ); > } > > if ( ( C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' && $params->{collection_code} ) >- || ( C4::Context->preference("BranchTransferLimitsType") eq 'ccode' && $params->{item_type} ) ) >+ || ( C4::Context->preference("BranchTransferLimitsType") eq 'ccode' && $params->{item_type_id} ) ) > { > return $c->render( > status => 409, > openapi => { > error => $params->{collection_code} >- ? "You passed 'collection_code' but configuration expects 'item_type'" >- : "You passed 'item_type' but configuration expects 'collection_code'" >+ ? "You passed 'collection_code' but configuration expects 'item_type_id'" >+ : "You passed 'item_type_id' but configuration expects 'collection_code'" > } > ); > } >@@ -178,7 +178,7 @@ sub batch_add { > > my $key = sprintf( > "%s:%s:%s:%s", $limit_params->{from_branch_id} || q{}, >- $limit_params->{to_branch_id} || q{}, $limit_params->{item_type} || q{}, >+ $limit_params->{to_branch_id} || q{}, $limit_params->{item_type_id} || q{}, > $limit_params->{collection_code} || q{} > ); > next if exists $existing_limits{$key}; >diff --git a/api/v1/swagger/definitions/transfer_limit.yaml b/api/v1/swagger/definitions/transfer_limit.yaml >index cb0f1b925be..3a53c3d5f03 100644 >--- a/api/v1/swagger/definitions/transfer_limit.yaml >+++ b/api/v1/swagger/definitions/transfer_limit.yaml >@@ -10,11 +10,11 @@ properties: > from_library_id: > type: string > description: Internal library id for which library the item is coming from >- item_type: >+ item_type_id: > type: > - string > - "null" >- description: Itemtype defining the type for this limi >+ description: Itemtype defining the type for this limit > collection_code: > type: > - string >diff --git a/api/v1/swagger/paths/transfer_limits.yaml b/api/v1/swagger/paths/transfer_limits.yaml >index 74ad2c8b6a8..fdc5956f2ee 100644 >--- a/api/v1/swagger/paths/transfer_limits.yaml >+++ b/api/v1/swagger/paths/transfer_limits.yaml >@@ -17,9 +17,9 @@ > description: Search on from_library_id > required: false > type: string >- - name: item_type >+ - name: item_type_id > in: query >- description: Search on item_type >+ description: Search on item_type_id > required: false > type: string > - name: collection_code >@@ -179,17 +179,17 @@ > > This resource batch creates new transfer limits based on the given data. > >- Transfer limits can be defined based on `collection_code` or `item_type`. The endpoint >+ Transfer limits can be defined based on `collection_code` or `item_type_id`. The endpoint > only accepts one at a time, and which one is used will depend on the value of the > `BranchTransferLimitsType` system preference. > >- For example, if the paramters `to_library_id: 'BranchA'` and `item_type: 'BOOK'` are passed in >+ For example, if the paramters `to_library_id: 'BranchA'` and `item_type_id: 'BOOK'` are passed in > new transfer limits for `BOOK`s will be created, with one transfer limit each for all the > branches defined in Koha. Given 4 branches, 3 limits would be created: > >- * to_library_id: 'BranchA', from_library_id: 'BranchB', item_type: 'BOOK' >- * to_library_id: 'BranchA', from_library_id: 'BranchC', item_type: 'BOOK' >- * to_library_id: 'BranchA', from_library_id: 'BranchD', item_type: 'BOOK' >+ * to_library_id: 'BranchA', from_library_id: 'BranchB', item_type_id: 'BOOK' >+ * to_library_id: 'BranchA', from_library_id: 'BranchC', item_type_id: 'BOOK' >+ * to_library_id: 'BranchA', from_library_id: 'BranchD', item_type_id: 'BOOK' > > > The body of the query would look like >@@ -197,19 +197,19 @@ > ``` > { > 'to_library_id': 'BranchA', >- 'item_type': 'BOOK' >+ 'item_type_id': 'BOOK' > > } > ``` > > As another example, imagine we have an itemtype that is never supposed to be transferred to another library. >- If we call that itemtype `LOCAL_ONLY` we would need to just pass `item_type: 'LOCAL_ONLY'`, which would create transfer >+ If we call that itemtype `LOCAL_ONLY` we would need to just pass `item_type_id: 'LOCAL_ONLY'`, which would create transfer > limits for all libraries, meaning those items would never be allowed to transfer to another > library. The body of the query would look like > > ``` > { >- 'item_type': 'LOCAL_ONLY' >+ 'item_type_id': 'LOCAL_ONLY' > } > ``` > tags: >@@ -229,11 +229,11 @@ > from_library_id: > type: string > description: Internal library id for which library the item is coming from >- item_type: >+ item_type_id: > type: > - string > - "null" >- description: Itemtype defining the type for this limi >+ description: Itemtype defining the type for this limit > collection_code: > type: > - string >@@ -303,11 +303,11 @@ > from_library_id: > type: string > description: Internal library id for which library the item is coming from >- item_type: >+ item_type_id: > type: > - string > - "null" >- description: Itemtype defining the type for this limi >+ description: Itemtype defining the type for this limit > collection_code: > type: > - string >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/transfer_limits.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/transfer_limits.tt >index 0fc767c6d4b..d99762c1dbc 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/transfer_limits.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/transfer_limits.tt >@@ -148,7 +148,7 @@ > [% INCLUDE 'datatables.inc' %] > <script> > const branchTransferLimitsType = "[% BranchTransferLimitsType | html %]"; >- const val_type = branchTransferLimitsType == "itemtype" ? "item_type" : "collection_code"; >+ const val_type = branchTransferLimitsType == "itemtype" ? "item_type_id" : "collection_code"; > const branches = [% To.json(branches) | $raw %]; > > $('#loading_limits').hide(); >diff --git a/t/db_dependent/api/v1/transfer_limits.t b/t/db_dependent/api/v1/transfer_limits.t >index e1c65102f6d..5906d882591 100755 >--- a/t/db_dependent/api/v1/transfer_limits.t >+++ b/t/db_dependent/api/v1/transfer_limits.t >@@ -196,7 +196,7 @@ subtest 'batch_add() and batch_delete() tests' => sub { > $unauthorized_patron->set_password( { password => $password, skip_validation => 1 } ); > my $unauth_userid = $unauthorized_patron->userid; > >- my $limit_hashref = { item_type => $itemtype->id }; >+ my $limit_hashref = { item_type_id => $itemtype->id }; > > # Unauthorized attempt to write > $t->post_ok( "//$unauth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref )->status_is(403); >@@ -216,20 +216,22 @@ subtest 'batch_add() and batch_delete() tests' => sub { > > # Create all combinations of to/from libraries > $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => >- { item_type => 'X', collection_code => 'Y' } )->status_is(400) >- ->json_is( '/error' => "You can only pass 'item_type' or 'collection_code' at a time" ); >+ { item_type_id => 'X', collection_code => 'Y' } )->status_is(400) >+ ->json_is( '/error' => "You can only pass 'item_type_id' or 'collection_code' at a time" ); > > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'ccode' ); > > # Create all combinations of to/from libraries >- $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => { item_type => 'X' } ) >- ->status_is(409)->json_is( '/error' => "You passed 'item_type' but configuration expects 'collection_code'" ); >+ $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => { item_type_id => 'X' } ) >+ ->status_is(409) >+ ->json_is( '/error' => "You passed 'item_type_id' but configuration expects 'collection_code'" ); > > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); > > # Create all combinations of to/from libraries > $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => { collection_code => 'X' } ) >- ->status_is(409)->json_is( '/error' => "You passed 'collection_code' but configuration expects 'item_type'" ); >+ ->status_is(409) >+ ->json_is( '/error' => "You passed 'collection_code' but configuration expects 'item_type_id'" ); > > # Create all combinations of to/from libraries > $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41530
: 190922 |
190923
|
190924