Bugzilla – Attachment 96842 Details for
Bug 24321
Make objects.search use mappings from Koha::Object(s)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24321: Make objects.search use mappings from Koha::Object(s)
Bug-24321-Make-objectssearch-use-mappings-from-Koh.patch (text/plain), 11.23 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-01-06 15:32:38 UTC
(
hide
)
Description:
Bug 24321: Make objects.search use mappings from Koha::Object(s)
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-01-06 15:32:38 UTC
Size:
11.23 KB
patch
obsolete
>From bf60de2187bc2fedf47c277ab0b014508bc78a08 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Tue, 31 Dec 2019 10:23:44 -0300 >Subject: [PATCH] Bug 24321: Make objects.search use mappings from > Koha::Object(s) > >This patch simplifies the objects.search helper so it relies entirely on >the result set object for the attribute mappings. > >The result is no more to_api or to_model mappings are passed. The >controllers need to be cleaned up after this patch. > >To test: >1. Apply this patch >2. Run: > $ kshell > k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t >=> SUCCESS: Tests pass! >3. Sign off :-D > >Note: the original version of this helpers accepted arbitrary mappings >and are now constrianed to real mappings on the Koha::Object level. As >such, the number of tests got reduced. >--- > Koha/REST/Plugin/Objects.pm | 27 ++-- > t/db_dependent/Koha/REST/Plugin/Objects.t | 160 +++------------------- > 2 files changed, 29 insertions(+), 158 deletions(-) > >diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm >index 3d314a784e..e8f0616f94 100644 >--- a/Koha/REST/Plugin/Objects.pm >+++ b/Koha/REST/Plugin/Objects.pm >@@ -29,17 +29,13 @@ Koha::REST::Plugin::Objects > > =head3 objects.search > >- my $patrons_set = Koha::Patrons->new; >- my $patrons = $c->objects->search( $patrons_set, [\&to_model, \&to_api] ); >+ my $patrons_rs = Koha::Patrons->new; >+ my $patrons = $c->objects->search( $patrons_rs ); > > Performs a database search using given Koha::Objects object and query parameters. >-It (optionally) applies the I<$to_model> function reference before building the >-query itself, and (optionally) applies I<$to_api> to the result. > > Returns an arrayref of the hashrefs representing the resulting objects >-for JSON rendering. >- >-Note: Make sure I<$to_model> and I<$to_api> don't autovivify keys. >+for API rendering. > > =cut > >@@ -48,7 +44,7 @@ sub register { > > $app->helper( > 'objects.search' => sub { >- my ( $c, $objects_set, $to_model, $to_api ) = @_; >+ my ( $c, $result_set ) = @_; > > my $args = $c->validation->output; > my $attributes = {}; >@@ -61,7 +57,7 @@ sub register { > { > attributes => $attributes, > params => $reserved_params, >- to_model => $to_model >+ result_set => $result_set > } > ); > >@@ -77,13 +73,12 @@ sub register { > if ( defined $filtered_params ) { > > # Apply the mapping function to the passed params >- $filtered_params = $to_model->($filtered_params) >- if defined $to_model; >+ $filtered_params = $result_set->attributes_from_api($filtered_params); > $filtered_params = $c->build_query_params( $filtered_params, $reserved_params ); > } > > # Perform search >- my $objects = $objects_set->search( $filtered_params, $attributes ); >+ my $objects = $result_set->search( $filtered_params, $attributes ); > > if ($objects->is_paged) { > $c->add_pagination_headers({ >@@ -92,13 +87,7 @@ sub register { > }); > } > >- my @objects_list = map { >- ( defined $to_api ) >- ? $to_api->( $_->TO_JSON ) >- : $_->TO_JSON >- } $objects->as_list; >- >- return \@objects_list; >+ return $objects->to_api; > } > ); > } >diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t >index a65d949fc6..c1a76de564 100644 >--- a/t/db_dependent/Koha/REST/Plugin/Objects.t >+++ b/t/db_dependent/Koha/REST/Plugin/Objects.t >@@ -34,50 +34,6 @@ get '/cities' => sub { > $c->render( status => 200, json => $cities ); > }; > >-get '/cities_to_model' => sub { >- my $c = shift; >- $c->validation->output($c->req->params->to_hash); >- my $cities_set = Koha::Cities->new; >- my $cities = $c->objects->search( $cities_set, \&to_model ); >- $c->render( status => 200, json => $cities ); >-}; >- >-get '/cities_to_model_to_api' => sub { >- my $c = shift; >- $c->validation->output($c->req->params->to_hash); >- my $cities_set = Koha::Cities->new; >- my $cities = $c->objects->search( $cities_set, \&to_model, \&to_api ); >- $c->render( status => 200, json => $cities ); >-}; >- >-get '/cities_sorted' => sub { >- my $c = shift; >- $c->validation->output($c->req->params->to_hash); >- my $cities_set = Koha::Cities->new; >- my $cities = $c->objects->search( $cities_set, \&to_model, \&to_api ); >- $c->render( status => 200, json => $cities ); >-}; >- >-sub to_model { >- my $params = shift; >- >- if ( exists $params->{nombre} ) { >- $params->{city_name} = delete $params->{nombre}; >- } >- >- return $params; >-} >- >-sub to_api { >- my $params = shift; >- >- if ( exists $params->{city_name} ) { >- $params->{nombre} = delete $params->{city_name}; >- } >- >- return $params; >-} >- > # The tests > use Test::More tests => 2; > use Test::Mojo; >@@ -92,7 +48,7 @@ my $builder = t::lib::TestBuilder->new; > > subtest 'objects.search helper' => sub { > >- plan tests => 90; >+ plan tests => 34; > > my $t = Test::Mojo->new; > >@@ -115,12 +71,12 @@ subtest 'objects.search helper' => sub { > } > }); > >- $t->get_ok('/cities?city_name=manuel&_per_page=1&_page=1') >+ $t->get_ok('/cities?name=manuel&_per_page=1&_page=1') > ->status_is(200) > ->header_like( 'Link' => qr/<http:\/\/.*\?.*&_page=2.*>; rel="next",/ ) > ->json_has('/0') > ->json_hasnt('/1') >- ->json_is('/0/city_name' => 'Manuel'); >+ ->json_is('/0/name' => 'Manuel'); > > $builder->build_object({ > class => 'Koha::Cities', >@@ -130,114 +86,40 @@ subtest 'objects.search helper' => sub { > }); > > # _match=starts_with >- $t->get_ok('/cities?city_name=manuel&_per_page=3&_page=1&_match=starts_with') >- ->status_is(200) >- ->json_has('/0') >- ->json_has('/1') >- ->json_hasnt('/2') >- ->json_is('/0/city_name' => 'Manuel') >- ->json_is('/1/city_name' => 'Manuela'); >- >- # _match=ends_with >- $t->get_ok('/cities?city_name=manuel&_per_page=3&_page=1&_match=ends_with') >- ->status_is(200) >- ->json_has('/0') >- ->json_has('/1') >- ->json_hasnt('/2') >- ->json_is('/0/city_name' => 'Manuel') >- ->json_is('/1/city_name' => 'Emanuel'); >- >- # _match=exact >- $t->get_ok('/cities?city_name=manuel&_per_page=3&_page=1&_match=exact') >- ->status_is(200) >- ->json_has('/0') >- ->json_hasnt('/1') >- ->json_is('/0/city_name' => 'Manuel'); >- >- # _match=contains >- $t->get_ok('/cities?city_name=manuel&_per_page=3&_page=1&_match=contains') >- ->status_is(200) >- ->json_has('/0') >- ->json_has('/1') >- ->json_has('/2') >- ->json_hasnt('/3') >- ->json_is('/0/city_name' => 'Manuel') >- ->json_is('/1/city_name' => 'Manuela') >- ->json_is('/2/city_name' => 'Emanuel'); >- >- ## _to_model tests >- # _match=starts_with >- $t->get_ok('/cities_to_model?nombre=manuel&_per_page=3&_page=1&_match=starts_with') >- ->status_is(200) >- ->json_has('/0') >- ->json_has('/1') >- ->json_hasnt('/2') >- ->json_is('/0/city_name' => 'Manuel') >- ->json_is('/1/city_name' => 'Manuela'); >- >- # _match=ends_with >- $t->get_ok('/cities_to_model?nombre=manuel&_per_page=3&_page=1&_match=ends_with') >- ->status_is(200) >- ->json_has('/0') >- ->json_has('/1') >- ->json_hasnt('/2') >- ->json_is('/0/city_name' => 'Manuel') >- ->json_is('/1/city_name' => 'Emanuel'); >- >- # _match=exact >- $t->get_ok('/cities_to_model?nombre=manuel&_per_page=3&_page=1&_match=exact') >- ->status_is(200) >- ->json_has('/0') >- ->json_hasnt('/1') >- ->json_is('/0/city_name' => 'Manuel'); >- >- # _match=contains >- $t->get_ok('/cities_to_model?nombre=manuel&_per_page=3&_page=1&_match=contains') >- ->status_is(200) >- ->json_has('/0') >- ->json_has('/1') >- ->json_has('/2') >- ->json_hasnt('/3') >- ->json_is('/0/city_name' => 'Manuel') >- ->json_is('/1/city_name' => 'Manuela') >- ->json_is('/2/city_name' => 'Emanuel'); >- >- ## _to_model && _to_api tests >- # _match=starts_with >- $t->get_ok('/cities_to_model_to_api?nombre=manuel&_per_page=3&_page=1&_match=starts_with') >+ $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=starts_with') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') > ->json_hasnt('/2') >- ->json_is('/0/nombre' => 'Manuel') >- ->json_is('/1/nombre' => 'Manuela'); >+ ->json_is('/0/name' => 'Manuel') >+ ->json_is('/1/name' => 'Manuela'); > > # _match=ends_with >- $t->get_ok('/cities_to_model_to_api?nombre=manuel&_per_page=3&_page=1&_match=ends_with') >+ $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=ends_with') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') > ->json_hasnt('/2') >- ->json_is('/0/nombre' => 'Manuel') >- ->json_is('/1/nombre' => 'Emanuel'); >+ ->json_is('/0/name' => 'Manuel') >+ ->json_is('/1/name' => 'Emanuel'); > > # _match=exact >- $t->get_ok('/cities_to_model_to_api?nombre=manuel&_per_page=3&_page=1&_match=exact') >+ $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=exact') > ->status_is(200) > ->json_has('/0') > ->json_hasnt('/1') >- ->json_is('/0/nombre' => 'Manuel'); >+ ->json_is('/0/name' => 'Manuel'); > > # _match=contains >- $t->get_ok('/cities_to_model_to_api?nombre=manuel&_per_page=3&_page=1&_match=contains') >+ $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=contains') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') > ->json_has('/2') > ->json_hasnt('/3') >- ->json_is('/0/nombre' => 'Manuel') >- ->json_is('/1/nombre' => 'Manuela') >- ->json_is('/2/nombre' => 'Emanuel'); >+ ->json_is('/0/name' => 'Manuel') >+ ->json_is('/1/name' => 'Manuela') >+ ->json_is('/2/name' => 'Emanuel'); > > $schema->storage->txn_rollback; > }; >@@ -256,21 +138,21 @@ subtest 'objects.search helper, sorting on mapped column' => sub { > $builder->build_object({ class => 'Koha::Cities', value => { city_name => 'A', city_country => 'Argentina' } }); > $builder->build_object({ class => 'Koha::Cities', value => { city_name => 'B', city_country => 'Argentina' } }); > >- $t->get_ok('/cities_sorted?_order_by=%2Bnombre&_order_by=+city_country') >+ $t->get_ok('/cities?_order_by=%2Bname&_order_by=+country') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') > ->json_hasnt('/2') >- ->json_is('/0/nombre' => 'A') >- ->json_is('/1/nombre' => 'B'); >+ ->json_is('/0/name' => 'A') >+ ->json_is('/1/name' => 'B'); > >- $t->get_ok('/cities_sorted?_order_by=-nombre') >+ $t->get_ok('/cities?_order_by=-name') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') > ->json_hasnt('/2') >- ->json_is('/0/nombre' => 'B') >- ->json_is('/1/nombre' => 'A'); >+ ->json_is('/0/name' => 'B') >+ ->json_is('/1/name' => 'A'); > > $schema->storage->txn_rollback; > } >-- >2.24.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 24321
:
96703
|
96704
|
96705
|
96706
|
96707
|
96708
|
96709
|
96710
|
96711
|
96712
|
96713
|
96714
|
96715
|
96716
|
96717
|
96718
|
96719
|
96720
|
96721
|
96722
|
96744
|
96840
|
96841
|
96842
|
96843
|
96844
|
96845
|
96846
|
96847
|
96848
|
96849
|
96850
|
96851
|
96852
|
96874
|
96875
|
96876
|
96877
|
96878
|
96879
|
96880
|
96881
|
96882
|
96883
|
96884
|
96885
|
96886
|
96887
|
96939
|
96940
|
96941
|
96942
|
96943
|
96944
|
96945
|
96946
|
96947
|
96948
|
96949
|
96950
|
96951
|
96952