Bugzilla – Attachment 71651 Details for
Bug 20004
Adapt /v1/cities to new naming guidelines
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20004: Adapt /v1/cities to new naming guidelines
Bug-20004-Adapt-v1cities-to-new-naming-guidelines.patch (text/plain), 13.21 KB, created by
Tomás Cohen Arazi (tcohen)
on 2018-02-15 12:11:21 UTC
(
hide
)
Description:
Bug 20004: Adapt /v1/cities to new naming guidelines
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2018-02-15 12:11:21 UTC
Size:
13.21 KB
patch
obsolete
>From 1420be78af799e6c6cea198f9f1b36337144563b Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Wed, 17 Jan 2018 14:03:28 -0300 >Subject: [PATCH] Bug 20004: Adapt /v1/cities to new naming guidelines > >This patch implements the changes required by the cities endpoint RFC >[1]. > >It uses the objects.search helper, and relies on bug 19686. > >To test: >- Apply the patches >- Compare the spec with the RFC (api/v1/swagger/definitions/city.json) >=> SUCCESS: It makes sense >- Run: > $ kshell > k$ prove t/db_dependent/api/v1/cities.t >=> Tests pass! >- Sign off :-D >--- > Koha/REST/V1/Cities.pm | 149 ++++++++++++++++++++++++++--------- > api/v1/swagger/definitions/city.json | 16 ++-- > api/v1/swagger/parameters.json | 4 +- > api/v1/swagger/parameters/city.json | 6 +- > api/v1/swagger/paths.json | 4 +- > api/v1/swagger/paths/cities.json | 26 +++--- > api/v1/swagger/x-primitives.json | 2 +- > 7 files changed, 142 insertions(+), 65 deletions(-) > >diff --git a/Koha/REST/V1/Cities.pm b/Koha/REST/V1/Cities.pm >index 3003e3aefe..3c7c5e7c4c 100644 >--- a/Koha/REST/V1/Cities.pm >+++ b/Koha/REST/V1/Cities.pm >@@ -19,7 +19,6 @@ use Modern::Perl; > > use Mojo::Base 'Mojolicious::Controller'; > >-use Koha::City; > use Koha::Cities; > > use Try::Tiny; >@@ -27,16 +26,9 @@ use Try::Tiny; > sub list { > my $c = shift->openapi->valid_input or return; > >- my $cities; >- my $filter; >- my $args = $c->req->params->to_hash; >- >- for my $filter_param ( keys %$args ) { >- $filter->{$filter_param} = { LIKE => $args->{$filter_param} . "%" }; >- } >- > return try { >- $cities = Koha::Cities->search($filter); >+ my $cities_set = Koha::Cities->new; >+ my $cities = $c->objects->search( $cities_set, \&_to_model, \&_to_api ); > return $c->render( status => 200, openapi => $cities ); > } > catch { >@@ -55,32 +47,35 @@ sub list { > sub get { > my $c = shift->openapi->valid_input or return; > >- my $city = Koha::Cities->find( $c->validation->param('cityid') ); >+ my $city = Koha::Cities->find( $c->validation->param('city_id') ); > unless ($city) { > return $c->render( status => 404, > openapi => { error => "City not found" } ); > } > >- return $c->render( status => 200, openapi => $city ); >+ return $c->render( status => 200, openapi => _to_api($city->TO_JSON) ); > } > > sub add { > my $c = shift->openapi->valid_input or return; > >- my $city = Koha::City->new( $c->validation->param('body') ); >- > return try { >+ my $city = Koha::City->new( _to_model( $c->validation->param('body') ) ); > $city->store; >- return $c->render( status => 200, openapi => $city ); >+ return $c->render( status => 200, openapi => _to_api($city->TO_JSON) ); > } > catch { > if ( $_->isa('DBIx::Class::Exception') ) { >- return $c->render( status => 500, >- openapi => { error => $_->{msg} } ); >+ return $c->render( >+ status => 500, >+ openapi => { error => $_->{msg} } >+ ); > } > else { >- return $c->render( status => 500, >- openapi => { error => "Something went wrong, check the logs."} ); >+ return $c->render( >+ status => 500, >+ openapi => { error => "Something went wrong, check the logs." } >+ ); > } > }; > } >@@ -88,21 +83,21 @@ sub add { > sub update { > my $c = shift->openapi->valid_input or return; > >- my $city; >+ my $city = Koha::Cities->find( $c->validation->param('city_id') ); >+ >+ if ( not defined $city ) { >+ return $c->render( status => 404, >+ openapi => { error => "Object not found" } ); >+ } > > return try { >- $city = Koha::Cities->find( $c->validation->param('cityid') ); > my $params = $c->req->json; >- $city->set( $params ); >+ $city->set( _to_model($params) ); > $city->store(); >- return $c->render( status => 200, openapi => $city ); >+ return $c->render( status => 200, openapi => _to_api($city->TO_JSON) ); > } > catch { >- if ( not defined $city ) { >- return $c->render( status => 404, >- openapi => { error => "Object not found" } ); >- } >- elsif ( $_->isa('Koha::Exceptions::Object') ) { >+ if ( $_->isa('Koha::Exceptions::Object') ) { > return $c->render( status => 500, > openapi => { error => $_->message } ); > } >@@ -111,25 +106,23 @@ sub update { > openapi => { error => "Something went wrong, check the logs."} ); > } > }; >- > } > > sub delete { > my $c = shift->openapi->valid_input or return; > >- my $city; >+ my $city = Koha::Cities->find( $c->validation->param('city_id') ); >+ if ( not defined $city ) { >+ return $c->render( status => 404, >+ openapi => { error => "Object not found" } ); >+ } > > return try { >- $city = Koha::Cities->find( $c->validation->param('cityid') ); > $city->delete; > return $c->render( status => 200, openapi => "" ); > } > catch { >- if ( not defined $city ) { >- return $c->render( status => 404, >- openapi => { error => "Object not found" } ); >- } >- elsif ( $_->isa('DBIx::Class::Exception') ) { >+ if ( $_->isa('DBIx::Class::Exception') ) { > return $c->render( status => 500, > openapi => { error => $_->{msg} } ); > } >@@ -138,7 +131,91 @@ sub delete { > openapi => { error => "Something went wrong, check the logs."} ); > } > }; >+} >+ >+=head3 _to_api >+ >+Helper function that maps a hashref of Koha::City attributes into REST api >+attribute names. >+ >+=cut >+ >+sub _to_api { >+ my $city = shift; > >+ # Rename attributes >+ foreach my $column ( keys %{ $Koha::REST::V1::Cities::to_api_mapping } ) { >+ my $mapped_column = $Koha::REST::V1::Cities::to_api_mapping->{$column}; >+ if ( exists $city->{ $column } >+ && defined $mapped_column ) >+ { >+ # key /= undef >+ $city->{ $mapped_column } = delete $city->{ $column }; >+ } >+ elsif ( exists $city->{ $column } >+ && !defined $mapped_column ) >+ { >+ # key == undef => to be deleted >+ delete $city->{ $column }; >+ } >+ } >+ >+ return $city; > } > >+=head3 _to_model >+ >+Helper function that maps REST api objects into Koha::Cities >+attribute names. >+ >+=cut >+ >+sub _to_model { >+ my $city = shift; >+ >+ foreach my $attribute ( keys %{ $Koha::REST::V1::Cities::to_model_mapping } ) { >+ my $mapped_attribute = $Koha::REST::V1::Cities::to_model_mapping->{$attribute}; >+ if ( exists $city->{ $attribute } >+ && defined $mapped_attribute ) >+ { >+ # key /= undef >+ $city->{ $mapped_attribute } = delete $city->{ $attribute }; >+ } >+ elsif ( exists $city->{ $attribute } >+ && !defined $mapped_attribute ) >+ { >+ # key == undef => to be deleted >+ delete $city->{ $attribute }; >+ } >+ } >+ >+ return $city; >+} >+ >+=head2 Global variables >+ >+=head3 $to_api_mapping >+ >+=cut >+ >+our $to_api_mapping = { >+ cityid => 'city_id', >+ city_country => 'country', >+ city_name => 'name', >+ city_state => 'state', >+ city_zipcode => 'postal_code' >+}; >+ >+=head3 $to_model_mapping >+ >+=cut >+ >+our $to_model_mapping = { >+ city_id => 'cityid', >+ country => 'city_country', >+ name => 'city_name', >+ postal_code => 'city_zipcode', >+ state => 'city_state' >+}; >+ > 1; >diff --git a/api/v1/swagger/definitions/city.json b/api/v1/swagger/definitions/city.json >index f322136fd9..8643e98c14 100644 >--- a/api/v1/swagger/definitions/city.json >+++ b/api/v1/swagger/definitions/city.json >@@ -1,26 +1,26 @@ > { > "type": "object", > "properties": { >- "cityid": { >- "$ref": "../x-primitives.json#/cityid" >+ "city_id": { >+ "$ref": "../x-primitives.json#/city_id" > }, >- "city_name": { >+ "name": { > "description": "city name", > "type": "string" > }, >- "city_state": { >+ "state": { > "description": "city state", > "type": ["string", "null"] > }, >- "city_zipcode": { >- "description": "city zipcode", >+ "postal_code": { >+ "description": "city postal code", > "type": ["string", "null"] > }, >- "city_country": { >+ "country": { > "description": "city country", > "type": ["string", "null"] > } > }, > "additionalProperties": false, >- "required": ["city_name", "city_state", "city_zipcode", "city_country"] >+ "required": ["name", "state", "postal_code", "country"] > } >diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json >index 1da158c3f4..584a0c2112 100644 >--- a/api/v1/swagger/parameters.json >+++ b/api/v1/swagger/parameters.json >@@ -5,8 +5,8 @@ > "borrowernumberQueryParam": { > "$ref": "parameters/patron.json#/borrowernumberQueryParam" > }, >- "cityidPathParam": { >- "$ref": "parameters/city.json#/cityidPathParam" >+ "city_id_pp": { >+ "$ref": "parameters/city.json#/city_id_pp" > }, > "holdIdPathParam": { > "$ref": "parameters/hold.json#/holdIdPathParam" >diff --git a/api/v1/swagger/parameters/city.json b/api/v1/swagger/parameters/city.json >index c35df74b9f..e971f90160 100644 >--- a/api/v1/swagger/parameters/city.json >+++ b/api/v1/swagger/parameters/city.json >@@ -1,8 +1,8 @@ > { >- "cityidPathParam": { >- "name": "cityid", >+ "city_id_pp": { >+ "name": "city_id", > "in": "path", >- "description": "City id", >+ "description": "City internal identifier", > "required": true, > "type": "integer" > } >diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json >index 9c3a6d2efd..df4f55a999 100644 >--- a/api/v1/swagger/paths.json >+++ b/api/v1/swagger/paths.json >@@ -8,8 +8,8 @@ > "/cities": { > "$ref": "paths/cities.json#/~1cities" > }, >- "/cities/{cityid}": { >- "$ref": "paths/cities.json#/~1cities~1{cityid}" >+ "/cities/{city_id}": { >+ "$ref": "paths/cities.json#/~1cities~1{city_id}" > }, > "/holds": { > "$ref": "paths/holds.json#/~1holds" >diff --git a/api/v1/swagger/paths/cities.json b/api/v1/swagger/paths/cities.json >index 2fe9f49666..cb51dcba2a 100644 >--- a/api/v1/swagger/paths/cities.json >+++ b/api/v1/swagger/paths/cities.json >@@ -8,27 +8,27 @@ > "application/json" > ], > "parameters": [{ >- "name": "city_name", >+ "name": "name", > "in": "query", >- "description": "Case insensative 'starts-with' search on city name", >+ "description": "Case insensative search on city name", > "required": false, > "type": "string" > }, { >- "name": "city_state", >+ "name": "state", > "in": "query", >- "description": "Case insensative 'starts-with' search on city state", >+ "description": "Case insensative search on city state", > "required": false, > "type": "string" > }, { >- "name": "city_country", >+ "name": "country", > "in": "query", >- "description": "Case insensative 'starts_with' search on city country", >+ "description": "Case insensative search on city country", > "required": false, > "type": "string" > }, { >- "name": "city_zipcode", >+ "name": "postal_code", > "in": "query", >- "description": "Case Insensative 'starts-with' search on zipcode", >+ "description": "Case Insensative search on city postal code", > "required": false, > "type": "string" > }], >@@ -117,13 +117,13 @@ > } > } > }, >- "/cities/{cityid}": { >+ "/cities/{city_id}": { > "get": { > "x-mojo-to": "Cities#get", > "operationId": "getCity", > "tags": ["cities"], > "parameters": [{ >- "$ref": "../parameters.json#/cityidPathParam" >+ "$ref": "../parameters.json#/city_id_pp" > }], > "produces": [ > "application/json" >@@ -160,11 +160,11 @@ > "operationId": "updateCity", > "tags": ["cities"], > "parameters": [{ >- "$ref": "../parameters.json#/cityidPathParam" >+ "$ref": "../parameters.json#/city_id_pp" > }, { > "name": "body", > "in": "body", >- "description": "A JSON object containing informations about the new hold", >+ "description": "A city object", > "required": true, > "schema": { > "$ref": "../definitions.json#/city" >@@ -222,7 +222,7 @@ > "operationId": "deleteCity", > "tags": ["cities"], > "parameters": [{ >- "$ref": "../parameters.json#/cityidPathParam" >+ "$ref": "../parameters.json#/city_id_pp" > }], > "produces": [ > "application/json" >diff --git a/api/v1/swagger/x-primitives.json b/api/v1/swagger/x-primitives.json >index 12e5e9de39..5fa58badaa 100644 >--- a/api/v1/swagger/x-primitives.json >+++ b/api/v1/swagger/x-primitives.json >@@ -11,7 +11,7 @@ > "type": ["string", "null"], > "description": "library assigned user identifier" > }, >- "cityid": { >+ "city_id": { > "type": "integer", > "description": "internally assigned city identifier", > "readOnly": true >-- >2.16.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=71651">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20004
:
70627
|
70628
|
71649
|
71651
|
71653
|
71665
|
71666
|
71823
|
71824
|
71825
|
71826