From 32718a68b684e0df860f7cc1ffff4a45773adf3c Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 28 Oct 2022 19:25:46 -0300 Subject: [PATCH] Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _authorised_values => { attribute_1 => { category => 'some_category_name', description => 'description' } } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- Koha/Object.pm | 4 +- t/db_dependent/Koha/Object.t | 117 ++++++++++++++-------- t/db_dependent/Koha/REST/Plugin/Objects.t | 35 +++++-- 3 files changed, 103 insertions(+), 53 deletions(-) diff --git a/Koha/Object.pm b/Koha/Object.pm index a574bf8fc7..f6601cb40b 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -555,8 +555,8 @@ sub to_api { # coded values handling my $avs = {}; - if ( $params->{av_expand} and $self->can('_fetch_authorised_values') ) { - $avs = $self->_fetch_authorised_values; + if ( $params->{av_expand} and $self->can('api_av_mapping') ) { + $avs = $self->api_av_mapping($params); } # Remove forbidden attributes if required (including their coded values) diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index d1495d3a33..e2e99adefb 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 22; +use Test::More tests => 21; use Test::Exception; use Test::Warn; use DateTime; @@ -226,7 +226,7 @@ subtest 'TO_JSON tests' => sub { subtest "to_api() tests" => sub { - plan tests => 29; + plan tests => 30; $schema->storage->txn_begin; @@ -423,6 +423,80 @@ subtest "to_api() tests" => sub { } }; + subtest 'Authorised values expansion' => sub { + + plan tests => 4; + + $schema->storage->txn_begin; + + # new category + my $category = $builder->build_object({ class => 'Koha::AuthorisedValueCategories' }); + # add two countries + my $argentina = $builder->build_object( + { class => 'Koha::AuthorisedValues', + value => { category => $category->category_name, lib => 'AR (Argentina)', lib_opac => 'Argentina' } + } + ); + my $france = $builder->build_object( + { class => 'Koha::AuthorisedValues', + value => { category => $category->category_name, lib => 'FR (France)', lib_opac => 'France' } + } + ); + + my $city_mock = Test::MockModule->new('Koha::City'); + $city_mock->mock( + 'api_av_mapping', + sub { + my ( $self, $params ) = @_; + + my $av = Koha::AuthorisedValues->find( + { + authorised_value => $self->city_country, + category => $category->category_name, + } + ); + + return { + city_country => { + category => $av->category, + description => ($params->{public}) ? $av->lib_opac : $av->lib, + } + }; + } + ); + $city_mock->mock( 'public_read_list', sub { return [ 'city_id', 'city_country', 'city_name', 'city_state' ] } ); + + my $cordoba = $builder->build_object( + { class => 'Koha::Cities', + value => { city_country => $argentina->authorised_value, city_name => 'Cordoba' } + } + ); + my $marseille = $builder->build_object( + { class => 'Koha::Cities', + value => { city_country => $france->authorised_value, city_name => 'Marseille' } + } + ); + + my $mobj = $marseille->to_api( { av_expand => 1, public => 1 } ); + my $cobj = $cordoba->to_api( { av_expand => 1, public => 0 } ); + + ok( exists $mobj->{_authorised_values}, '_authorised_values exists for Marseille' ); + ok( exists $cobj->{_authorised_values}, '_authorised_values exists for Córdoba' ); + + is_deeply( + $mobj->{_authorised_values}->{country}, + { category => $category->category_name, description => $france->lib_opac }, + 'Authorised value for country expanded' + ); + is_deeply( + $cobj->{_authorised_values}->{country}, + { category => $category->category_name, description => $argentina->lib }, + 'Authorised value for country expanded' + ); + + $schema->storage->txn_rollback; + }; + $schema->storage->txn_rollback; }; @@ -997,44 +1071,5 @@ subtest 'messages() and add_message() tests' => sub { isnt( $patron->object_messages, undef, '->messages initializes the array if required' ); is( scalar @{ $patron->object_messages }, 0, '->messages returns an empty arrayref' ); - $schema->storage->txn_rollback; -}; - -subtest 'Authorised values expansion' => sub { - plan tests => 4; - - $schema->storage->txn_begin; - - Koha::AuthorisedValues->search({category => 'Countries'})->delete; - Koha::AuthorisedValueCategories->search({category_name =>'Countries'})->delete; - - my $cat = $builder->build_object({ class => 'Koha::AuthorisedValueCategories', value => {category_name =>'Countries'} }); - my $fr = $builder->build_object({ class => 'Koha::AuthorisedValues', value => {authorised_value => 'FR', lib=>'France', category=>$cat->category_name} }); - my $us = $builder->build_object({ class => 'Koha::AuthorisedValues', value => {authorised_value => 'US', lib=>'United States of America', category=>$cat->category_name} }); - my $ar = $builder->build_object({ class => 'Koha::AuthorisedValues', value => {authorised_value => 'AR', lib=>'Argentina', category=>$cat->category_name} }); - - my $city_class = Test::MockModule->new('Koha::City'); - $city_class->mock( '_fetch_authorised_values', - sub { - my ($self) = @_; - use Koha::AuthorisedValues; - my $av = Koha::AuthorisedValues->find({authorised_value => $self->city_country, category => 'Countries'}); - return {country => $av->unblessed}; - } - ); - - my $marseille = $builder->build_object({ class => 'Koha::Cities', value => {city_country => 'FR', city_name => 'Marseille'} }); - my $cordoba = $builder->build_object({ class => 'Koha::Cities', value => {city_country => 'AR', city_name => 'Córdoba'} }); - - my $mobj = $marseille->to_api({av_expand => 1}); - my $cobj = $cordoba->to_api({av_expand => 1}); - - isnt($mobj->{_authorised_values}, undef, '_authorised_values exists for Marseille'); - isnt($cobj->{_authorised_values}, undef, '_authorised_values exists for Córdoba'); - - is($mobj->{_authorised_values}->{country}->{lib}, $fr->lib, 'Authorised value for country expanded'); - is($cobj->{_authorised_values}->{country}->{lib}, $ar->lib, 'Authorised value for country expanded'); - - $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index 509d98975b..53923b1d28 100755 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -670,17 +670,24 @@ subtest 'objects.find helper with expanded authorised values' => sub { my $city_class = Test::MockModule->new('Koha::City'); $city_class->mock( - '_fetch_authorised_values', + 'api_av_mapping', sub { - my ($self) = @_; + my ($self, $params) = @_; use Koha::AuthorisedValues; + my $av = Koha::AuthorisedValues->find( { authorised_value => $self->city_country, category => 'Countries' } ); - return { country => $av->unblessed }; + + return { + city_country => { + category => $av->category, + description => ( $params->{public} ) ? $av->lib_opac : $av->lib, + } + }; } ); @@ -706,7 +713,7 @@ subtest 'objects.find helper with expanded authorised values' => sub { $t->get_ok( '/cities/' . $manuel->cityid => { 'x-koha-av-expand' => 1 } ) ->status_is(200)->json_is( '/name' => 'Manuel' ) ->json_has('/_authorised_values') - ->json_is( '/_authorised_values/country/lib' => $ar->lib ); + ->json_is( '/_authorised_values/country/description' => $ar->lib ); $t->get_ok( '/cities/' . $manuel->cityid => { 'x-koha-av-expand' => 0 } ) ->status_is(200)->json_is( '/name' => 'Manuel' ) @@ -715,7 +722,7 @@ subtest 'objects.find helper with expanded authorised values' => sub { $t->get_ok( '/cities/' . $manuela->cityid => { 'x-koha-av-expand' => 1 } ) ->status_is(200)->json_is( '/name' => 'Manuela' ) ->json_has('/_authorised_values') - ->json_is( '/_authorised_values/country/lib' => $us->lib ); + ->json_is( '/_authorised_values/country/description' => $us->lib ); $schema->storage->txn_rollback; }; @@ -771,20 +778,28 @@ subtest 'objects.search helper with expanded authorised values' => sub { my $city_class = Test::MockModule->new('Koha::City'); $city_class->mock( - '_fetch_authorised_values', + 'api_av_mapping', sub { - my ($self) = @_; + my ($self, $params) = @_; use Koha::AuthorisedValues; + my $av = Koha::AuthorisedValues->find( { authorised_value => $self->city_country, category => 'Countries' } ); - return { country => $av->unblessed }; + + return { + city_country => { + category => $av->category, + description => ( $params->{public} ) ? $av->lib_opac : $av->lib, + } + }; } ); + $builder->build_object( { class => 'Koha::Cities', @@ -809,10 +824,10 @@ subtest 'objects.search helper with expanded authorised values' => sub { ->json_has('/0')->json_has('/1')->json_hasnt('/2') ->json_is( '/0/name' => 'Manuel' ) ->json_has('/0/_authorised_values') - ->json_is( '/0/_authorised_values/country/lib' => $ar->lib ) + ->json_is( '/0/_authorised_values/country/description' => $ar->lib ) ->json_is( '/1/name' => 'Manuela' ) ->json_has('/1/_authorised_values') - ->json_is( '/1/_authorised_values/country/lib' => $us->lib ); + ->json_is( '/1/_authorised_values/country/description' => $us->lib ); $t->get_ok( '/cities?name=manuel&_per_page=4&_page=1&_match=starts_with' => { 'x-koha-av-expand' => 0 } )->status_is(200) -- 2.20.1