Bugzilla – Attachment 143077 Details for
Bug 26635
Expand coded values in REST API call
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26635: Refined data structure and methods
Bug-26635-Refined-data-structure-and-methods.patch (text/plain), 13.44 KB, created by
Martin Renvoize (ashimema)
on 2022-11-03 11:30:06 UTC
(
hide
)
Description:
Bug 26635: Refined data structure and methods
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-11-03 11:30:06 UTC
Size:
13.44 KB
patch
obsolete
>From 80f8d1ce4b2e0bff57a7db28a29c9dd373a84a0a Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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: > >_str => { > attribute_1 => { > category => 'some_category_name', > str => 'description', > type => 'av' > }, > ... >} > >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 <tomascohen@theke.io> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/Object.pm | 6 +- > t/db_dependent/Koha/Object.t | 134 +++++++++++++++------- > t/db_dependent/Koha/REST/Plugin/Objects.t | 70 +++++++---- > 3 files changed, 144 insertions(+), 66 deletions(-) > >diff --git a/Koha/Object.pm b/Koha/Object.pm >index a574bf8fc7..3b2b80407b 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) >@@ -597,7 +597,7 @@ sub to_api { > } > } > >- $json_object->{_authorised_values} = $avs >+ $json_object->{_str} = $avs > if $params->{av_expand}; > > # Make sure we duplicate the $params variable to avoid >diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t >index d1495d3a33..6e7d8a1a9f 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,97 @@ 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, >+ str => ( $params->{public} ) ? $av->lib_opac : $av->lib, >+ type => 'av', >+ } >+ }; >+ } >+ ); >+ $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->{_str}, '_str exists for Marseille' ); >+ ok( exists $cobj->{_str}, '_str exists for Córdoba' ); >+ >+ is_deeply( >+ $mobj->{_str}->{country}, >+ { >+ category => $category->category_name, >+ str => $france->lib_opac, >+ type => 'av', >+ }, >+ 'Authorised value for country expanded' >+ ); >+ is_deeply( >+ $cobj->{_str}->{country}, >+ { >+ category => $category->category_name, >+ str => $argentina->lib, >+ type => 'av' >+ }, >+ 'Authorised value for country expanded' >+ ); >+ >+ $schema->storage->txn_rollback; >+ }; >+ > $schema->storage->txn_rollback; > }; > >@@ -997,44 +1088,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..21f89f0639 100755 >--- a/t/db_dependent/Koha/REST/Plugin/Objects.t >+++ b/t/db_dependent/Koha/REST/Plugin/Objects.t >@@ -621,7 +621,8 @@ subtest 'objects.search helper, search_limited() tests' => sub { > }; > > subtest 'objects.find helper with expanded authorised values' => sub { >- plan tests => 14; >+ >+ plan tests => 18; > > $schema->storage->txn_begin; > >@@ -670,17 +671,25 @@ 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, >+ str => ( $params->{public} ) ? $av->lib_opac : $av->lib, >+ type => 'av', >+ } >+ }; > } > ); > >@@ -703,26 +712,30 @@ subtest 'objects.find helper with expanded authorised values' => sub { > } > ); > >- $t->get_ok( '/cities/' . $manuel->cityid => { 'x-koha-av-expand' => 1 } ) >+ $t->get_ok( '/cities/' . $manuel->id => { '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_has('/_str') >+ ->json_is( '/_str/country/type' => 'av' ) >+ ->json_is( '/_str/country/category' => $cat->category_name ) >+ ->json_is( '/_str/country/str' => $ar->lib ); > >- $t->get_ok( '/cities/' . $manuel->cityid => { 'x-koha-av-expand' => 0 } ) >+ $t->get_ok( '/cities/' . $manuel->id => { 'x-koha-av-expand' => 0 } ) > ->status_is(200)->json_is( '/name' => 'Manuel' ) >- ->json_hasnt('/_authorised_values'); >+ ->json_hasnt('/_str'); > >- $t->get_ok( '/cities/' . $manuela->cityid => { 'x-koha-av-expand' => 1 } ) >+ $t->get_ok( '/cities/' . $manuela->id => { '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_has('/_str') >+ ->json_is( '/_str/country/type' => 'av' ) >+ ->json_is( '/_str/country/category' => $cat->category_name ) >+ ->json_is( '/_str/country/str' => $us->lib ); > > $schema->storage->txn_rollback; > }; > > subtest 'objects.search helper with expanded authorised values' => sub { > >- plan tests => 20; >+ plan tests => 24; > > my $t = Test::Mojo->new; > >@@ -771,20 +784,29 @@ 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, >+ str => ( $params->{public} ) ? $av->lib_opac : $av->lib, >+ type => 'av', >+ } >+ }; > } > ); > >+ > $builder->build_object( > { > class => 'Koha::Cities', >@@ -808,17 +830,21 @@ subtest 'objects.search helper with expanded authorised values' => sub { > { 'x-koha-av-expand' => 1 } )->status_is(200) > ->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_has('/0/_str') >+ ->json_is( '/0/_str/country/str' => $ar->lib ) >+ ->json_is( '/0/_str/country/type' => 'av' ) >+ ->json_is( '/0/_str/country/category' => $cat->category_name ) > ->json_is( '/1/name' => 'Manuela' ) >- ->json_has('/1/_authorised_values') >- ->json_is( '/1/_authorised_values/country/lib' => $us->lib ); >+ ->json_has('/1/_str') >+ ->json_is( '/1/_str/country/str' => $us->lib ) >+ ->json_is( '/1/_str/country/type' => 'av' ) >+ ->json_is( '/1/_str/country/category' => $cat->category_name ); > > $t->get_ok( '/cities?name=manuel&_per_page=4&_page=1&_match=starts_with' => > { 'x-koha-av-expand' => 0 } )->status_is(200) > ->json_has('/0')->json_has('/1')->json_hasnt('/2') >- ->json_is( '/0/name' => 'Manuel' )->json_hasnt('/0/_authorised_values') >- ->json_is( '/1/name' => 'Manuela' )->json_hasnt('/1/_authorised_values'); >+ ->json_is( '/0/name' => 'Manuel' )->json_hasnt('/0/_str') >+ ->json_is( '/1/name' => 'Manuela' )->json_hasnt('/1/_str'); > > > $schema->storage->txn_rollback; >-- >2.20.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 26635
:
111400
|
111401
|
111402
|
111411
|
111412
|
111413
|
111414
|
112538
|
112539
|
116232
|
116233
|
116234
|
116235
|
116592
|
116593
|
120698
|
120699
|
120700
|
124268
|
142761
|
142762
|
142763
|
142776
|
142792
|
142793
|
142794
|
142795
|
142796
|
142801
|
142802
|
142803
|
142804
|
142805
|
143039
|
143040
|
143041
|
143042
|
143043
|
143044
|
143073
|
143074
|
143075
|
143076
|
143077
|
143078
|
143079
|
143080
|
143127
|
143128
|
143129
|
143130
|
143131
|
143132
|
143133
|
143134
|
143276
|
143354
|
143355
|
143356
|
143357
|
143358
|
143359
|
143360
|
143361
|
143362