Bugzilla – Attachment 189158 Details for
Bug 38446
Permission error for additional fields
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38446: Add a dedicated route to retrieve ERM-related additional fields
Bug-38446-Add-a-dedicated-route-to-retrieve-ERM-re.patch (text/plain), 20.81 KB, created by
Matt Blenkinsop
on 2025-11-06 11:40:24 UTC
(
hide
)
Description:
Bug 38446: Add a dedicated route to retrieve ERM-related additional fields
Filename:
MIME Type:
Creator:
Matt Blenkinsop
Created:
2025-11-06 11:40:24 UTC
Size:
20.81 KB
patch
obsolete
>From 96f386aec5b3e6b9e2e12e65f27e49085122d68e Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 4 Nov 2025 13:51:42 +0100 >Subject: [PATCH] Bug 38446: Add a dedicated route to retrieve ERM-related > additional fields > >In order to prevent permission errors to user without the 'parameters' >permission we are introducing a separate endpoint for the ERM module to >retrieve additional fields for this specific module. > >* Adds a new endpoint /erm/extended_attribute_types >* Adjusts Cypress and REST API tests >* Adds REST API swagger specs > >I wanted to keep the code as simple as possible and decided to create a >wrapper for the additional fields API client. The magic is in >koha-tmpl/intranet-tmpl/prog/js/fetch/additional-fields-api-client.js >Ideally we should provide a test for this but I failed to write them. >If required by QA I would suggest to deal with them on a separate bug to >not delay this bugfix. The code has been kept designed to be used in >other contexts but the api client is not used outside of Vue and >outside of the ERM module. > >Test plan: >Confirm that user without the 'parameters' permission can use the ERM >module with additional fields. > >Sponsored-by: Karlsruhe Institute of Technology (KIT) >Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> >Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> >--- > Koha/REST/V1/ExtendedAttributeTypes.pm | 59 +++++++--- > .../paths/erm_extended_attribute_types.yaml | 57 ++++++++++ > api/v1/swagger/swagger.yaml | 2 + > .../js/fetch/additional-fields-api-client.js | 36 ++++++- > .../prog/js/fetch/erm-api-client.js | 10 ++ > .../integration/AdditionalFields_spec.ts | 36 +++---- > .../api/v1/extended_attribute_types.t | 102 +++++++++++++++++- > 7 files changed, 270 insertions(+), 32 deletions(-) > create mode 100644 api/v1/swagger/paths/erm_extended_attribute_types.yaml > >diff --git a/Koha/REST/V1/ExtendedAttributeTypes.pm b/Koha/REST/V1/ExtendedAttributeTypes.pm >index 18ac38a1e9c..5fe84da2f29 100644 >--- a/Koha/REST/V1/ExtendedAttributeTypes.pm >+++ b/Koha/REST/V1/ExtendedAttributeTypes.pm >@@ -35,14 +35,12 @@ use Try::Tiny qw( catch try ); > > =head2 Methods > >-=head3 list >+=head3 _list > > =cut > >-sub list { >- my $c = shift->openapi->valid_input or return; >- >- my $resource_type = $c->param('resource_type'); >+sub _list { >+ my ( $self, @resource_types ) = @_; > > # FIXME: Maybe not the best place for this mapping > my $resource_to_table = { >@@ -56,17 +54,54 @@ sub list { > order => 'aqorders', > }; > >- return try { >- my $additional_fields_set = Koha::AdditionalFields->new; >+ my @tables; >+ for my $resource_type (@resource_types) { > if ( $resource_type && $resource_to_table->{$resource_type} ) { >- $additional_fields_set = >- $additional_fields_set->search( { tablename => $resource_to_table->{$resource_type} } ); >+ push @tables, $resource_to_table->{$resource_type}; > } elsif ($resource_type) { >- $additional_fields_set = $additional_fields_set->search( { tablename => $resource_type } ); >- } else { >- $additional_fields_set = $additional_fields_set->search(); >+ push @tables, $resource_type; > } >+ } >+ return Koha::AdditionalFields->new->search( ( @tables ? { tablename => \@tables } : () ) ); >+} >+ >+=head3 list >+ >+List all additional fields, can be filtered using the resource_type parameter. >+ >+=cut >+ >+sub list { >+ my ($self) = @_; >+ >+ my $c = $self->openapi->valid_input or return; >+ >+ my $resource_type = $c->param('resource_type'); > >+ return try { >+ my $additional_fields_set = $self->_list($resource_type); >+ return $c->render( >+ status => 200, >+ openapi => $c->objects->search($additional_fields_set) >+ ); >+ } catch { >+ $c->unhandled_exception($_); >+ }; >+} >+ >+=head3 list_erm >+ >+List the ERM-related additional fields, can be filtered using the resource_type parameter. >+ >+=cut >+ >+sub list_erm { >+ my ($self) = @_; >+ my $c = shift->openapi->valid_input or return; >+ my @resource_types = qw(erm_licenses erm_agreements erm_packages); >+ >+ return try { >+ my $additional_fields_set = $self->_list(@resource_types); > return $c->render( > status => 200, > openapi => $c->objects->search($additional_fields_set) >diff --git a/api/v1/swagger/paths/erm_extended_attribute_types.yaml b/api/v1/swagger/paths/erm_extended_attribute_types.yaml >new file mode 100644 >index 00000000000..662252d1d73 >--- /dev/null >+++ b/api/v1/swagger/paths/erm_extended_attribute_types.yaml >@@ -0,0 +1,57 @@ >+--- >+/erm/extended_attribute_types: >+ get: >+ x-mojo-to: ExtendedAttributeTypes#list_erm >+ operationId: listErmAdditionalFields >+ tags: >+ - additional_fields >+ summary: List ERM extended attribute types >+ produces: >+ - application/json >+ parameters: >+ - description: filter by resource type >+ in: query >+ name: resource_type >+ type: string >+ enum: >+ - license >+ - agreement >+ - package >+ - $ref: "../swagger.yaml#/parameters/match" >+ - $ref: "../swagger.yaml#/parameters/order_by" >+ - $ref: "../swagger.yaml#/parameters/page" >+ - $ref: "../swagger.yaml#/parameters/per_page" >+ - $ref: "../swagger.yaml#/parameters/q_param" >+ - $ref: "../swagger.yaml#/parameters/q_body" >+ responses: >+ 200: >+ description: A list of extended attribute types for ERM >+ schema: >+ items: >+ $ref: "../swagger.yaml#/definitions/extended_attribute_type" >+ type: array >+ "400": >+ description: | >+ Bad request. Possible `error_code` attribute values: >+ >+ * `invalid_query` >+ schema: >+ $ref: "../swagger.yaml#/definitions/error" >+ 403: >+ description: Access forbidden >+ schema: >+ $ref: "../swagger.yaml#/definitions/error" >+ 500: >+ description: |- >+ Internal server error. Possible `error_code` attribute values: >+ * `internal_server_error` >+ schema: >+ $ref: "../swagger.yaml#/definitions/error" >+ 503: >+ description: Under maintenance >+ schema: >+ $ref: "../swagger.yaml#/definitions/error" >+ x-koha-authorization: >+ permissions: >+ - parameters: manage_additional_fields >+ - erm: 1 >diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml >index a06539dbd1e..cc2f7b359af 100644 >--- a/api/v1/swagger/swagger.yaml >+++ b/api/v1/swagger/swagger.yaml >@@ -383,6 +383,8 @@ paths: > $ref: "./paths/erm_custom_reports.yaml#/~1erm~1eUsage~1metric_types_report~1{data_type}" > "/erm/eUsage/provider_rollup_report/{data_type}": > $ref: "./paths/erm_custom_reports.yaml#/~1erm~1eUsage~1provider_rollup_report~1{data_type}" >+ /erm/extended_attribute_types: >+ $ref: ./paths/erm_extended_attribute_types.yaml#/~1erm~1extended_attribute_types > /erm/licenses: > $ref: ./paths/erm_licenses.yaml#/~1erm~1licenses > "/erm/licenses/{license_id}": >diff --git a/koha-tmpl/intranet-tmpl/prog/js/fetch/additional-fields-api-client.js b/koha-tmpl/intranet-tmpl/prog/js/fetch/additional-fields-api-client.js >index db25c0f0e57..8d359ec0e2b 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/fetch/additional-fields-api-client.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/fetch/additional-fields-api-client.js >@@ -1,3 +1,5 @@ >+import ERMAPIClient from "@fetch/erm-api-client"; >+ > export class AdditionalFieldsAPIClient { > constructor(HttpClient) { > this.httpClient = new HttpClient({ >@@ -16,4 +18,36 @@ export class AdditionalFieldsAPIClient { > } > } > >-export default AdditionalFieldsAPIClient; >+export class AdditionalFieldsAPIClientWrapper { >+ constructor(HttpClient) { >+ this.clients = { >+ admin: new AdditionalFieldsAPIClient(HttpClient), >+ erm: new ERMAPIClient(HttpClient), >+ }; >+ } >+ >+ get additional_fields() { >+ return { >+ getAll: resource_type => { >+ let moduleName = this.getModuleName(resource_type); >+ return this.clients[moduleName].additional_fields.getAll( >+ resource_type >+ ); >+ }, >+ }; >+ } >+ >+ getModuleName(resource_type) { >+ const moduleMappings = { >+ erm: ["agreement", "license", "package"], >+ }; >+ >+ for (const [module, resourceTypes] of Object.entries(moduleMappings)) { >+ if (resourceTypes.includes(resource_type)) return module; >+ } >+ >+ return "admin"; >+ } >+} >+ >+export default AdditionalFieldsAPIClientWrapper; >diff --git a/koha-tmpl/intranet-tmpl/prog/js/fetch/erm-api-client.js b/koha-tmpl/intranet-tmpl/prog/js/fetch/erm-api-client.js >index b618e0d672b..ba64f3a88cf 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/fetch/erm-api-client.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/fetch/erm-api-client.js >@@ -447,6 +447,16 @@ export class ERMAPIClient { > }), > }; > } >+ >+ get additional_fields() { >+ return { >+ getAll: resource_type => >+ this.httpClient.getAll({ >+ endpoint: "extended_attribute_types", >+ params: { resource_type }, >+ }), >+ }; >+ } > } > > export default ERMAPIClient; >diff --git a/t/cypress/integration/AdditionalFields_spec.ts b/t/cypress/integration/AdditionalFields_spec.ts >index 10730b993e0..ff5c367489c 100644 >--- a/t/cypress/integration/AdditionalFields_spec.ts >+++ b/t/cypress/integration/AdditionalFields_spec.ts >@@ -489,7 +489,7 @@ describe("Additional Fields operations", () => { > let license_additional_fields = get_licenses_additional_fields(); > let av_cats = get_av_cats(); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: license_additional_fields, > statusCode: 200, > }); >@@ -551,7 +551,7 @@ describe("Additional Fields operations", () => { > statusCode: 200, > body: vendors, > }); >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: [], > statusCode: 200, > }).as("empty-additional-fields"); >@@ -562,7 +562,7 @@ describe("Additional Fields operations", () => { > cy.wait("@get-empty-license"); > cy.get("#licenses_show #additional_fields").should("not.exist"); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: license_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -657,7 +657,7 @@ describe("Additional Fields operations", () => { > statusCode: 200, > body: vendors, > }); >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: [], > statusCode: 200, > }).as("empty-additional-fields"); >@@ -666,7 +666,7 @@ describe("Additional Fields operations", () => { > cy.visit("/cgi-bin/koha/erm/licenses/add"); > cy.get("#licenses_add form #additional_fields").should("not.exist"); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: license_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -791,7 +791,7 @@ describe("Additional Fields operations", () => { > body: vendors, > }); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: license_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -888,7 +888,7 @@ describe("Additional Fields operations", () => { > let agreement_additional_fields = get_agreements_additional_fields(); > let av_cats = get_av_cats(); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: agreement_additional_fields, > statusCode: 200, > }); >@@ -950,7 +950,7 @@ describe("Additional Fields operations", () => { > statusCode: 200, > body: vendors, > }); >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: [], > statusCode: 200, > }).as("empty-additional-fields"); >@@ -961,7 +961,7 @@ describe("Additional Fields operations", () => { > cy.wait("@get-empty-agreement"); > cy.get("#agreements_show #additional_fields").should("not.exist"); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: agreement_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -1056,7 +1056,7 @@ describe("Additional Fields operations", () => { > statusCode: 200, > body: vendors, > }); >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: [], > statusCode: 200, > }).as("empty-additional-fields"); >@@ -1065,7 +1065,7 @@ describe("Additional Fields operations", () => { > cy.visit("/cgi-bin/koha/erm/agreements/add"); > cy.get("#agreements_add form #additional_fields").should("not.exist"); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: agreement_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -1190,7 +1190,7 @@ describe("Additional Fields operations", () => { > body: vendors, > }); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: agreement_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -1290,7 +1290,7 @@ describe("Additional Fields operations", () => { > get_packages_additional_fields(); > let av_cats = get_av_cats(); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: eholdings_package_additional_fields, > statusCode: 200, > }); >@@ -1359,7 +1359,7 @@ describe("Additional Fields operations", () => { > statusCode: 200, > body: vendors, > }); >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: [], > statusCode: 200, > }).as("empty-additional-fields"); >@@ -1370,7 +1370,7 @@ describe("Additional Fields operations", () => { > cy.wait("@get-empty-eholdings-package"); > cy.get("#packages_list #additional_fields").should("not.exist"); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: eholdings_package_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -1468,7 +1468,7 @@ describe("Additional Fields operations", () => { > statusCode: 200, > body: vendors, > }); >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: [], > statusCode: 200, > }).as("empty-additional-fields"); >@@ -1477,7 +1477,7 @@ describe("Additional Fields operations", () => { > cy.visit("/cgi-bin/koha/erm/eholdings/local/packages/add"); > cy.get("#packages_add form #additional_fields").should("not.exist"); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: eholdings_package_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >@@ -1605,7 +1605,7 @@ describe("Additional Fields operations", () => { > body: vendors, > }); > >- cy.intercept("GET", "/api/v1/extended_attribute_types*", { >+ cy.intercept("GET", "/api/v1/erm/extended_attribute_types*", { > body: eholdings_package_additional_fields, > statusCode: 200, > }).as("existing-additional-fields"); >diff --git a/t/db_dependent/api/v1/extended_attribute_types.t b/t/db_dependent/api/v1/extended_attribute_types.t >index 26728af5377..b7c0dbaf74d 100755 >--- a/t/db_dependent/api/v1/extended_attribute_types.t >+++ b/t/db_dependent/api/v1/extended_attribute_types.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > > use Test::NoWarnings; >-use Test::More tests => 2; >+use Test::More tests => 3; > use Test::Mojo; > > use t::lib::TestBuilder; >@@ -133,3 +133,103 @@ subtest 'list() tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'list_erm() tests' => sub { >+ >+ plan tests => 23; >+ >+ $schema->storage->txn_begin; >+ >+ Koha::AdditionalFields->search->delete; >+ >+ my $librarian = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 2**3 } # parameters >+ } >+ ); >+ my $password = 'thePassword123'; >+ $librarian->set_password( { password => $password, skip_validation => 1 } ); >+ my $userid = $librarian->userid; >+ >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 0 } >+ } >+ ); >+ >+ $patron->set_password( { password => $password, skip_validation => 1 } ); >+ my $unauth_userid = $patron->userid; >+ >+ my $erm_patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 28 } # erm => 1 >+ } >+ ); >+ >+ $patron->set_password( { password => $password, skip_validation => 1 } ); >+ my $erm_userid = $patron->userid; >+ >+ ## Authorized user tests >+ # No additional fields, so empty array should be returned >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types")->status_is(200)->json_is( [] ); >+ >+ my $additional_field = $builder->build_object( >+ { >+ class => 'Koha::AdditionalFields', >+ value => { tablename => 'aqinvoices', name => 'af_name' }, >+ } >+ ); >+ >+ # One aqinvoices additional_field created, none for 'erm' should get returned >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types")->status_is(200)->json_is( [] ); >+ >+ my $erm_agreement_additional_field = $builder->build_object( >+ { >+ class => 'Koha::AdditionalFields', >+ value => { tablename => 'erm_agreements', name => 'agreement_name' }, >+ } >+ ); >+ >+ my $erm_license_additional_field = $builder->build_object( >+ { >+ class => 'Koha::AdditionalFields', >+ value => { tablename => 'erm_licenses', name => 'license_name' }, >+ } >+ ); >+ >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types")->status_is(200)->json_is( >+ [ >+ $erm_agreement_additional_field->to_api, >+ $erm_license_additional_field->to_api, >+ ] >+ ); >+ >+ # Filtering works, two existing additional fields returned for the queried table name >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types?resource_type=license")->status_is(200) >+ ->json_is( [ $erm_license_additional_field->to_api ] ); >+ >+ # Cannot retrieve attributes from other tables >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types?resource_type=order")->status_is(400) >+ ->json_is( >+ "/errors" => [ { message => "Not in enum list: license, agreement, package.", path => "/resource_type" } ] ); >+ >+ # Warn on unsupported query parameter >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types?blah=blah")->status_is(400) >+ ->json_is( [ { path => '/query/blah', message => 'Malformed query string' } ] ); >+ >+ # erm only user >+ $t->get_ok("//$userid:$password@/api/v1/erm/extended_attribute_types")->status_is(200)->json_is( >+ [ >+ $erm_agreement_additional_field->to_api, >+ $erm_license_additional_field->to_api, >+ ] >+ ); >+ >+ # Unauthorized access >+ $t->get_ok("//$unauth_userid:$password@/api/v1/extended_attribute_types")->status_is(403); >+ >+ $schema->storage->txn_rollback; >+}; >-- >2.50.1
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 38446
:
174758
|
176471
|
183133
|
189010
|
189013
| 189158 |
189159