From 58501b8cc5b0a50871014af5fca4ded7b5d2f5b5 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 17 Jul 2025 14:31:20 +0200 Subject: [PATCH] Bug 40433: Add missing maxLength to item, library and patron The test added in xt/api.t was not fully accurate, we skipped the columns that have the same name as the api attribute (ie. the ones not part of to_api_mapping) Test plan: prove xt/api.t should pass --- api/v1/swagger/definitions/item.yaml | 1 + api/v1/swagger/definitions/library.yaml | 1 + api/v1/swagger/definitions/patron.yaml | 3 +++ xt/api.t | 14 +++++++++----- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/api/v1/swagger/definitions/item.yaml b/api/v1/swagger/definitions/item.yaml index f43c8ef1a88..42dcd36e3e5 100644 --- a/api/v1/swagger/definitions/item.yaml +++ b/api/v1/swagger/definitions/item.yaml @@ -162,6 +162,7 @@ properties: - string - "null" description: Authorized value for the shelving location for this item + maxLength: 80 permanent_location: type: - string diff --git a/api/v1/swagger/definitions/library.yaml b/api/v1/swagger/definitions/library.yaml index c4debe94b5e..9039fbb8475 100644 --- a/api/v1/swagger/definitions/library.yaml +++ b/api/v1/swagger/definitions/library.yaml @@ -96,6 +96,7 @@ properties: - string - "null" description: geolocation of your library + maxLength: 255 marc_org_code: type: - string diff --git a/api/v1/swagger/definitions/patron.yaml b/api/v1/swagger/definitions/patron.yaml index 0895ffdbe8a..72f64fac15b 100644 --- a/api/v1/swagger/definitions/patron.yaml +++ b/api/v1/swagger/definitions/patron.yaml @@ -9,6 +9,7 @@ properties: - string - "null" description: library assigned user identifier + maxLength: 32 surname: type: - string @@ -241,6 +242,7 @@ properties: - string - "null" description: patron's login + maxLength: 75 opac_notes: type: - string @@ -356,6 +358,7 @@ properties: lang: type: string description: lang to use to send notices to this patron + maxLength: 25 login_attempts: type: - integer diff --git a/xt/api.t b/xt/api.t index 2f55b0c027a..3aa2e7f90fc 100755 --- a/xt/api.t +++ b/xt/api.t @@ -220,10 +220,13 @@ subtest 'maxlength + enum' => sub { my $reversed_api_mapping = { reverse map { defined $api_mapping->{$_} ? ( $_ => $api_mapping->{$_} ) : () } keys %$api_mapping }; - while ( my ( $attr, $properties ) = each %{ $definition->{properties} } ) { - my $db_col = $reversed_api_mapping->{$attr}; - next unless $db_col; - my $column_info = $koha_object->_result->column_info($db_col); + my $db_columns = $koha_object->_result->columns_info; + while ( my ( $db_col, $column_info ) = each %{$db_columns} ) { + my $api_attr = $api_mapping->{$db_col} || $db_col; + my $properties = $definition->{properties}->{$api_attr}; + + next unless $properties; + next unless $column_info->{size}; next if ref( $column_info->{size} ) eq 'ARRAY'; # decimal # FIXME Could have a test for this as well @@ -233,7 +236,8 @@ subtest 'maxlength + enum' => sub { ; # FIXME This is not fully correct, we might want to make sure enum is set for both DB and spec. eg. now checkprevcheckout is enum only for the api spec if ( !exists $properties->{maxLength} || $column_info->{size} != $properties->{maxLength} ) { - push @failures, sprintf "%s.%s should have maxLength=%s in api spec", $def, $attr, $column_info->{size}; + push @failures, sprintf "%s.%s should have maxLength=%s in api spec", $def, $api_attr, + $column_info->{size}; } } is( scalar(@failures), 0, "maxLength tests for $def" ) or diag Dumper @failures; -- 2.34.1