Bug 20004: Adapt /v1/cities to new naming guidelines
authorTomas Cohen Arazi <tomascohen@theke.io>
Wed, 17 Jan 2018 17:03:28 +0000 (14:03 -0300)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Fri, 16 Feb 2018 20:53:42 +0000 (17:53 -0300)
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

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Koha/REST/V1/Cities.pm
api/v1/swagger/definitions/city.json
api/v1/swagger/parameters.json
api/v1/swagger/parameters/city.json
api/v1/swagger/paths.json
api/v1/swagger/paths/cities.json
api/v1/swagger/x-primitives.json

index 3003e3a..3c7c5e7 100644 (file)
@@ -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;
index f322136..8643e98 100644 (file)
@@ -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"]
 }
index 1da158c..584a0c2 100644 (file)
@@ -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"
index c35df74..e971f90 100644 (file)
@@ -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"
     }
index 9c3a6d2..df4f55a 100644 (file)
@@ -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"
index 2fe9f49..cb51dcb 100644 (file)
@@ -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"
       }],
       }
     }
   },
-  "/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"
       "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"
       "operationId": "deleteCity",
       "tags": ["cities"],
       "parameters": [{
-        "$ref": "../parameters.json#/cityidPathParam"
+        "$ref": "../parameters.json#/city_id_pp"
       }],
       "produces": [
         "application/json"
index 12e5e9d..5fa58ba 100644 (file)
@@ -11,7 +11,7 @@
     "type": ["string", "null"],
     "description": "library assigned user identifier"
   },
-  "cityid": {
+  "city_id": {
     "type": "integer",
     "description": "internally assigned city identifier",
     "readOnly": true