From 96f386aec5b3e6b9e2e12e65f27e49085122d68e Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 Signed-off-by: Matt Blenkinsop --- 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