Bug 16497: (follow-up) Adapt to existing guidelines and RFC
authorTomas Cohen Arazi <tomascohen@theke.io>
Fri, 11 Jan 2019 16:23:14 +0000 (13:23 -0300)
committerroot <root@f1ebe1bec408>
Tue, 19 Feb 2019 13:52:14 +0000 (13:52 +0000)
This patch makes the original implementation match what is specified on
the RFC [1].

The controller is updated, and so the tests.

To test:
- Apply this patches:
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/libraries.t
=> SUCCESS: Tests pass!

[1] https://wiki.koha-community.org/wiki/Libraries_endpoint_RFC

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Koha/REST/V1/Library.pm
api/v1/swagger/definitions/library.json
api/v1/swagger/parameters.json
api/v1/swagger/parameters/library.json
api/v1/swagger/paths.json
api/v1/swagger/paths/libraries.json
api/v1/swagger/x-primitives.json
t/db_dependent/api/v1/libraries.t

index c0b92b2..0bf16ee 100644 (file)
@@ -43,27 +43,22 @@ Controller function that handles listing Koha::Library objects
 sub list {
     my $c = shift->openapi->valid_input or return;
 
-    my $libraries;
-    my $filter;
-    my $args = $c->req->params->to_hash;
-
-    for my $filter_param ( keys %$args ) {
-        $filter->{$filter_param} = { LIKE => $args->{$filter_param} . "%" };
-    }
-
     return try {
-        my $libraries = Koha::Libraries->search($filter);
+        my $libraries_set = Koha::Libraries->new;
+        my $libraries     = $c->objects->search( $libraries_set, \&_to_model, \&_to_api );
         return $c->render( status => 200, openapi => $libraries );
     }
     catch {
-        if ( $_->isa('DBIx::Class::Exception') ) {
-            return $c->render( status  => 500,
-                               openapi => { error => $_->{msg} } );
-        }
-        else {
-            return $c->render( status => 500,
-                openapi => { error => "Something went wrong, check the logs."} );
+        unless ( blessed $_ && $_->can('rethrow') ) {
+            return $c->render(
+                status  => 500,
+                openapi => { error => "Something went wrong, check Koha logs for details." }
+            );
         }
+        return $c->render(
+            status  => 500,
+            openapi => { error => "$_" }
+        );
     };
 }
 
@@ -76,14 +71,15 @@ Controller function that handles retrieving a single Koha::Library
 sub get {
     my $c = shift->openapi->valid_input or return;
 
-    my $branchcode = $c->validation->param('branchcode');
-    my $library = Koha::Libraries->find({ branchcode => $branchcode });
+    my $library_id = $c->validation->param('library_id');
+    my $library = Koha::Libraries->find( $library_id );
+
     unless ($library) {
         return $c->render( status  => 404,
                            openapi => { error => "Library not found" } );
     }
 
-    return $c->render( status => 200, openapi => $library );
+    return $c->render( status => 200, openapi => _to_api( $library->TO_JSON ) );
 }
 
 =head3 add
@@ -96,23 +92,29 @@ sub add {
     my $c = shift->openapi->valid_input or return;
 
     return try {
-        if (Koha::Libraries->find($c->req->json->{branchcode})) {
-            return $c->render( status => 400,
-                openapi => { error => 'Library already exists' } );
-        }
-        my $library = Koha::Library->new($c->validation->param('body'))->store;
-        my $branchcode = $library->branchcode;
-        $c->res->headers->location($c->req->url->to_string.'/'.$branchcode);
-        return $c->render( status => 201, openapi => $library);
+        my $library = Koha::Library->new( _to_model( $c->validation->param('body') ) );
+        $library->store;
+        $c->res->headers->location( $c->req->url->to_string . '/' . $library->branchcode );
+        return $c->render( status => 201, openapi => _to_api( $library->TO_JSON ) );
     }
     catch {
-        if ( $_->isa('DBIx::Class::Exception') ) {
-            return $c->render( status  => 500,
-                               openapi => { error => $_->{msg} } );
+        unless ( blessed $_ && $_->can('rethrow') ) {
+            return $c->render(
+                status  => 500,
+                openapi => { error => "Something went wrong, check Koha logs for details." }
+            );
+        }
+        if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) {
+            return $c->render(
+                status  => 409,
+                openapi => { error => $_->error, conflict => $_->duplicate_id }
+            );
         }
         else {
-            return $c->render( status => 500,
-                openapi => { error => "Something went wrong, check the logs."} );
+            return $c->render(
+                status  => 500,
+                openapi => { error => "$_" }
+            );
         }
     };
 }
@@ -126,25 +128,33 @@ Controller function that handles updating a Koha::Library object
 sub update {
     my $c = shift->openapi->valid_input or return;
 
-    my $library;
+    my $library = Koha::Libraries->find( $c->validation->param('library_id') );
+
+    if ( not defined $library ) {
+        return $c->render(
+            status  => 404,
+            openapi => { error => "Library not found" }
+        );
+    }
+
     return try {
-        $library = Koha::Libraries->find($c->validation->param('branchcode'));
-        $library->set($c->validation->param('body'))->store;
-        return $c->render( status => 200, openapi => $library );
+        my $params = $c->req->json;
+        $library->set( _to_model($params) );
+        $library->store();
+        return $c->render( status => 200, openapi => _to_api($library->TO_JSON) );
     }
     catch {
-        if ( not defined $library ) {
-            return $c->render( status => 404,
-                               openapi => { error => "Object not found" });
-        }
-        elsif ( $_->isa('DBIx::Class::Exception') ) {
-            return $c->render( status  => 500,
-                               openapi => { error => $_->{msg} } );
-        }
-        else {
-            return $c->render( status => 500,
-                openapi => { error => "Something went wrong, check the logs."} );
+        unless ( blessed $_ && $_->can('rethrow') ) {
+            return $c->render(
+                status  => 500,
+                openapi => { error => "Something went wrong, check Koha logs for details." }
+            );
         }
+
+        return $c->render(
+            status  => 500,
+            openapi => { error => "$_" }
+        );
     };
 }
 
@@ -155,27 +165,150 @@ Controller function that handles deleting a Koha::Library object
 =cut
 
 sub delete {
+
     my $c = shift->openapi->valid_input or return;
 
-    my $library;
+    my $library = Koha::Libraries->find( $c->validation->param( 'library_id' ) );
+
+    if ( not defined $library ) {
+        return $c->render( status => 404, openapi => { error => "Library not found" } );
+    }
+
     return try {
-        $library = Koha::Libraries->find($c->validation->param('branchcode'));
         $library->delete;
         return $c->render( status => 204, openapi => '');
     }
     catch {
-        if ( not defined $library ) {
-            return $c->render( status => 404, openapi => { error => "Object not found" } );
+        unless ( blessed $_ && $_->can('rethrow') ) {
+            return $c->render(
+                status  => 500,
+                openapi => { error => "Something went wrong, check Koha logs for details." }
+            );
         }
-        elsif ( $_->isa('DBIx::Class::Exception') ) {
-            return $c->render( status  => 500,
-                               openapi => { error => $_->{msg} } );
+
+        return $c->render(
+            status  => 500,
+            openapi => { error => "$_" }
+        );
+    };
+}
+
+=head3 _to_api
+
+Helper function that maps a hashref of Koha::Library attributes into REST api
+attribute names.
+
+=cut
+
+sub _to_api {
+    my $library = shift;
+
+    # Rename attributes
+    foreach my $column ( keys %{ $Koha::REST::V1::Library::to_api_mapping } ) {
+        my $mapped_column = $Koha::REST::V1::Library::to_api_mapping->{$column};
+        if (    exists $library->{ $column }
+             && defined $mapped_column )
+        {
+            # key /= undef
+            $library->{ $mapped_column } = delete $library->{ $column };
         }
-        else {
-            return $c->render( status => 500,
-                openapi => { error => "Something went wrong, check the logs."} );
+        elsif (    exists $library->{ $column }
+                && !defined $mapped_column )
+        {
+            # key == undef => to be deleted
+            delete $library->{ $column };
         }
-    };
+    }
+
+    return $library;
+}
+
+=head3 _to_model
+
+Helper function that maps REST api objects into Koha::Library
+attribute names.
+
+=cut
+
+sub _to_model {
+    my $library = shift;
+
+    foreach my $attribute ( keys %{ $Koha::REST::V1::Library::to_model_mapping } ) {
+        my $mapped_attribute = $Koha::REST::V1::Library::to_model_mapping->{$attribute};
+        if (    exists $library->{ $attribute }
+             && defined $mapped_attribute )
+        {
+            # key /= undef
+            $library->{ $mapped_attribute } = delete $library->{ $attribute };
+        }
+        elsif (    exists $library->{ $attribute }
+                && !defined $mapped_attribute )
+        {
+            # key == undef => to be deleted
+            delete $library->{ $attribute };
+        }
+    }
+
+    if ( exists $library->{pickup_location} ) {
+        $library->{pickup_location} = ( $library->{pickup_location} ) ? 1 : 0;
+    }
+
+    return $library;
 }
 
+
+=head2 Global variables
+
+=head3 $to_api_mapping
+
+=cut
+
+our $to_api_mapping = {
+    branchcode       => 'library_id',
+    branchname       => 'name',
+    branchaddress1   => 'address1',
+    branchaddress2   => 'address2',
+    branchaddress3   => 'address3',
+    branchzip        => 'postal_code',
+    branchcity       => 'city',
+    branchstate      => 'state',
+    branchcountry    => 'country',
+    branchphone      => 'phone',
+    branchfax        => 'fax',
+    branchemail      => 'email',
+    branchreplyto    => 'reply_to_email',
+    branchreturnpath => 'return_path_email',
+    branchurl        => 'url',
+    issuing          => undef,
+    branchip         => 'ip',
+    branchprinter    => undef,
+    branchnotes      => 'notes',
+    marcorgcode      => 'marc_org_code',
+};
+
+=head3 $to_model_mapping
+
+=cut
+
+our $to_model_mapping = {
+    library_id        => 'branchcode',
+    name              => 'branchname',
+    address1          => 'branchaddress1',
+    address2          => 'branchaddress2',
+    address3          => 'branchaddress3',
+    postal_code       => 'branchzip',
+    city              => 'branchcity',
+    state             => 'branchstate',
+    country           => 'branchcountry',
+    phone             => 'branchphone',
+    fax               => 'branchfax',
+    email             => 'branchemail',
+    reply_to_email    => 'branchreplyto',
+    return_path_email => 'branchreturnpath',
+    url               => 'branchurl',
+    ip                => 'branchip',
+    notes             => 'branchnotes',
+    marc_org_code     => 'marcorgcode',
+};
+
 1;
index b7f8aa1..d31a83f 100644 (file)
@@ -1,78 +1,70 @@
 {
   "type": "object",
   "properties": {
-    "branchcode": {
-      "$ref": "../x-primitives.json#/branchcode"
+    "library_id": {
+      "$ref": "../x-primitives.json#/library_id"
     },
-    "branchname": {
+    "name": {
       "type": "string",
       "description": "Printable name of library"
     },
-    "branchaddress1": {
+    "address1": {
       "type": ["string", "null"],
       "description": "the first address line of the library"
     },
-    "branchaddress2": {
+    "address2": {
       "type": ["string", "null"],
       "description": "the second address line of the library"
     },
-    "branchaddress3": {
+    "address3": {
       "type": ["string", "null"],
       "description": "the third address line of the library"
     },
-    "branchzip": {
+    "postal_code": {
       "type": ["string", "null"],
-      "description": "the zip or postal code of the library"
+      "description": "the postal code of the library"
     },
-    "branchcity": {
+    "city": {
       "type": ["string", "null"],
       "description": "the city or province of the library"
     },
-    "branchstate": {
+    "state": {
       "type": ["string", "null"],
       "description": "the reqional state of the library"
     },
-    "branchcountry": {
+    "country": {
       "type": ["string", "null"],
       "description": "the county of the library"
     },
-    "branchphone": {
+    "phone": {
       "type": ["string", "null"],
       "description": "the primary phone of the library"
     },
-    "branchfax": {
+    "fax": {
       "type": ["string", "null"],
       "description": "the fax number of the library"
     },
-    "branchemail": {
+    "email": {
       "type": ["string", "null"],
       "description": "the primary email address of the library"
     },
-    "branchreplyto": {
+    "reply_to_email": {
       "type": ["string", "null"],
       "description": "the email to be used as a Reply-To"
     },
-    "branchreturnpath": {
+    "return_path_email": {
       "type": ["string", "null"],
       "description": "the email to be used as Return-Path"
     },
-    "branchurl": {
+    "url": {
       "type": ["string", "null"],
       "description": "the URL for your library or branch's website"
     },
-    "issuing": {
-      "type": ["integer", "null"],
-      "description": "unused in Koha"
-    },
-    "branchip": {
+    "ip": {
       "type": ["string", "null"],
       "description": "the IP address for your library or branch"
     },
-    "branchprinter": {
-      "type": ["string", "null"],
-      "description": "unused in Koha"
-    },
-    "branchnotes": {
+    "notes": {
       "type": ["string", "null"],
       "description": "notes related to your library or branch"
     },
       "type": ["string", "null"],
       "description": "geolocation of your library"
     },
-    "marcorgcode": {
+    "marc_org_code": {
         "type": [ "string", "null" ],
         "description": "MARC Organization Code, see http://www.loc.gov/marc/organizations/orgshome.html, when empty defaults to syspref MARCOrgCode"
+    },
+    "pickup_location": {
+        "type": "boolean",
+        "description": "If the library can act as a pickup location"
     }
   },
   "additionalProperties": false,
-  "required": ["branchcode", "branchname"]
+  "required": ["library_id", "name"]
 }
index 1d5feba..1e9bcbd 100644 (file)
@@ -8,8 +8,8 @@
   "city_id_pp": {
     "$ref": "parameters/city.json#/city_id_pp"
   },
-  "branchcodePathParam": {
-    "$ref": "parameters/library.json#/branchcodePathParam"
+  "library_id_pp": {
+    "$ref": "parameters/library.json#/library_id_pp"
   },
   "holdIdPathParam": {
     "$ref": "parameters/hold.json#/holdIdPathParam"
index 366581e..cb0da0b 100644 (file)
@@ -1,8 +1,8 @@
 {
-  "branchcodePathParam": {
-    "name": "branchcode",
+  "library_id_pp": {
+    "name": "library_id",
     "in": "path",
-    "description": "Branch identifier code",
+    "description": "Internal library identifier",
     "required": true,
     "type": "string"
   }
index 077ebdd..c4104cb 100644 (file)
@@ -23,8 +23,8 @@
   "/libraries": {
     "$ref": "paths/libraries.json#/~1libraries"
   },
-  "/libraries/{branchcode}": {
-    "$ref": "paths/libraries.json#/~1libraries~1{branchcode}"
+  "/libraries/{library_id}": {
+    "$ref": "paths/libraries.json#/~1libraries~1{library_id}"
   },
   "/patrons": {
     "$ref": "paths/patrons.json#/~1patrons"
index 1457788..70249ed 100644 (file)
     "get": {
       "x-mojo-to": "Library#list",
       "operationId": "listLibrary",
-      "tags": ["library"],
-      "parameters": [{
-        "name": "branchname",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on name",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchaddress1",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on address1",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchaddress2",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on address2",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchaddress3",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on address3",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchzip",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on zipcode",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchcity",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on city",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchstate",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on state",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchcountry",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on country",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchphone",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on phone number",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchfax",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on fax number",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchemail",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on email address",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchreplyto",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on Reply-To email address",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchreturnpath",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on Return-Path email address",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchurl",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on website URL",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "issuing",
-        "in": "query",
-        "description": "Unused in Koha",
-        "required": false,
-        "type": "integer"
-      }, {
-        "name": "branchip",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on IP address",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchprinter",
-        "in": "query",
-        "description": "Unused in Koha",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "branchnotes",
-        "in": "query",
-        "description": "Case insensitive 'starts_with' search on notes",
-        "required": false,
-        "type": "string"
-      }, {
-        "name": "opac_info",
-        "in": "query",
-        "description": "Case insensitive 'starts-with' search on OPAC info",
-        "required": false,
-        "type": "string"
-      }],
+      "tags": [
+        "library"
+      ],
+      "parameters": [
+        {
+          "name": "name",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on name",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "address1",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on address1",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "address2",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on address2",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "address3",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on address3",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "postal_code",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on postal code",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "city",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on city",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "state",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on state",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "country",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on country",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "phone",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on phone number",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "fax",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on fax number",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "email",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on email address",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "reply_to_email",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on Reply-To email address",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "return_path_email",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on Return-Path email address",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "url",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on website URL",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "ip",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on IP address",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "notes",
+          "in": "query",
+          "description": "Case insensitive 'starts_with' search on notes",
+          "required": false,
+          "type": "string"
+        },
+        {
+          "name": "opac_info",
+          "in": "query",
+          "description": "Case insensitive 'starts-with' search on OPAC info",
+          "required": false,
+          "type": "string"
+        }
+      ],
       "produces": [
         "application/json"
       ],
     "post": {
       "x-mojo-to": "Library#add",
       "operationId": "addLibrary",
-      "tags": ["library"],
-      "parameters": [{
-        "name": "body",
-        "in": "body",
-        "description": "A JSON object containing informations about the new library",
-        "required": true,
-        "schema": {
-          "$ref": "../definitions.json#/library"
+      "tags": [
+        "library"
+      ],
+      "parameters": [
+        {
+          "name": "body",
+          "in": "body",
+          "description": "A JSON object containing informations about the new library",
+          "required": true,
+          "schema": {
+            "$ref": "../definitions.json#/library"
+          }
         }
-      }],
+      ],
       "produces": [
         "application/json"
       ],
             "$ref": "../definitions.json#/error"
           }
         },
+        "409": {
+          "description": "Conflict in creating resource",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
         "500": {
           "description": "Internal error",
           "schema": {
       },
       "x-koha-authorization": {
         "permissions": {
-          "parameters": "parameters_remaining_permissions"
+          "parameters": "manage_libraries"
         }
       }
     }
   },
-  "/libraries/{branchcode}": {
+  "/libraries/{library_id}": {
     "get": {
       "x-mojo-to": "Library#get",
       "operationId": "getLibrary",
-      "tags": ["library"],
+      "tags": [
+        "library"
+      ],
       "parameters": [
         {
-          "$ref": "../parameters.json#/branchcodePathParam"
+          "$ref": "../parameters.json#/library_id_pp"
         }
       ],
       "produces": [
     "put": {
       "x-mojo-to": "Library#update",
       "operationId": "updateLibrary",
-      "tags": ["library"],
-      "parameters": [{
-        "$ref": "../parameters.json#/branchcodePathParam"
-      }, {
-        "name": "body",
-        "in": "body",
-        "description": "A JSON object containing information on the library",
-        "required": true,
-        "schema": {
-          "$ref": "../definitions.json#/library"
+      "tags": [
+        "library"
+      ],
+      "parameters": [
+        {
+          "$ref": "../parameters.json#/library_id_pp"
+        },
+        {
+          "name": "body",
+          "in": "body",
+          "description": "A JSON object containing information on the library",
+          "required": true,
+          "schema": {
+            "$ref": "../definitions.json#/library"
+          }
         }
-      }],
+      ],
       "consumes": [
         "application/json"
       ],
       },
       "x-koha-authorization": {
         "permissions": {
-          "parameters": "parameters_remaining_permissions"
+          "parameters": "manage_libraries"
         }
       }
     },
     "delete": {
       "x-mojo-to": "Library#delete",
       "operationId": "deleteLibrary",
-      "tags": ["library"],
-      "parameters": [{
-        "$ref": "../parameters.json#/branchcodePathParam"
-      }],
+      "tags": [
+        "library"
+      ],
+      "parameters": [
+        {
+          "$ref": "../parameters.json#/library_id_pp"
+        }
+      ],
       "produces": [
         "application/json"
       ],
       "responses": {
         "204": {
           "description": "Library deleted",
-          "schema": { "type": "string" }
+          "schema": {
+            "type": "string"
+          }
         },
         "401": {
           "description": "Authentication required",
       },
       "x-koha-authorization": {
         "permissions": {
-          "parameters": "parameters_remaining_permissions"
+          "parameters": "manage_libraries"
         }
       }
     }
index 9b077d6..b68632e 100644 (file)
@@ -7,7 +7,7 @@
     "type": "integer",
     "description": "Internal patron identifier"
   },
-  "branchcode": {
+  "library_id": {
     "type": "string",
     "description": "internally assigned library identifier",
     "maxLength": 10,
index 7b59610..db76b6d 100644 (file)
@@ -44,13 +44,11 @@ subtest 'list() tests' => sub {
     $schema->storage->txn_begin;
 
     # Create test context
-    my $library = $builder->build( { source => 'Branch' } );
-    my $another_library = { %$library };   # create a copy of $library but make
-    delete $another_library->{branchcode}; # sure branchcode will be regenerated
-    $another_library = $builder->build(
-        { source => 'Branch', value => $another_library } );
-    my ( $borrowernumber, $session_id ) =
-      create_user_and_session( { authorized => 0 } );
+    my $library = $builder->build_object({ class => 'Koha::Libraries' });
+    my $another_library = $library->unblessed; # create a copy of $library but make
+    delete $another_library->{branchcode};     # sure branchcode will be regenerated
+    $another_library = $builder->build_object({ class => 'Koha::Libraries', value => $another_library });
+    my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 0 } );
 
     ## Authorized user tests
     my $count_of_libraries = Koha::Libraries->search->count;
@@ -59,23 +57,40 @@ subtest 'list() tests' => sub {
     $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
     $t->request_ok($tx)
-      ->status_is(200)
-      ->json_has('/'.($count_of_libraries-1).'/branchcode')
-      ->json_hasnt('/'.($count_of_libraries).'/branchcode');
+      ->status_is( 200, 'SWAGGER3.2.2' )
+      ->json_has('/'.($count_of_libraries-1).'/library_id')
+      ->json_hasnt('/'.($count_of_libraries).'/library_id');
 
     subtest 'query parameters' => sub {
-        my @fields = qw(
-        branchname       branchaddress1 branchaddress2 branchaddress3
-        branchzip        branchcity     branchstate    branchcountry
-        branchphone      branchfax      branchemail    branchreplyto
-        branchreturnpath branchurl      issuing        branchip
-        branchprinter    branchnotes    opac_info
-        );
-        plan tests => scalar(@fields)*3;
-
-        foreach my $field (@fields) {
+
+        my $fields = {
+            name              => 'branchname',
+            address1          => 'branchaddress1',
+            address2          => 'branchaddress2',
+            address3          => 'branchaddress3',
+            postal_code       => 'branchzip',
+            city              => 'branchcity',
+            state             => 'branchstate',
+            country           => 'branchcountry',
+            phone             => 'branchphone',
+            fax               => 'branchfax',
+            email             => 'branchemail',
+            reply_to_email    => 'branchreplyto',
+            return_path_email => 'branchreturnpath',
+            url               => 'branchurl',
+            ip                => 'branchip',
+            notes             => 'branchnotes',
+            opac_info         => 'opac_info',
+        };
+
+        my $size = keys %{$fields};
+
+        plan tests => $size * 3;
+
+        foreach my $field ( keys %{$fields} ) {
+            my $model_field = $fields->{ $field };
             $tx = $t->ua->build_tx( GET =>
-                         "/api/v1/libraries?$field=$library->{$field}" );
+                         "/api/v1/libraries?$field=" . $library->$model_field );
             $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
             $tx->req->env( { REMOTE_ADDR => $remote_address } );
             my $result =
@@ -102,18 +117,20 @@ subtest 'get() tests' => sub {
 
     $schema->storage->txn_begin;
 
-    my $library = $builder->build( { source => 'Branch' } );
+    my $library = $builder->build_object( { class => 'Koha::Libraries' } );
     my ( $borrowernumber, $session_id ) =
       create_user_and_session( { authorized => 0 } );
 
-    my $tx = $t->ua->build_tx( GET => "/api/v1/libraries/" . $library->{branchcode} );
+    my $tx = $t->ua->build_tx( GET => "/api/v1/libraries/" . $library->branchcode );
     $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
     $t->request_ok($tx)
-      ->status_is(200)
-      ->json_is($library);
+      ->status_is( 200, 'SWAGGER3.2.2' )
+      ->json_is( '' => Koha::REST::V1::Library::_to_api( $library->TO_JSON ), 'SWAGGER3.3.2' );
+
+    my $non_existent_code = $library->branchcode;
+    $library->delete;
 
-    my $non_existent_code = 'non_existent'.int(rand(10000));
     $tx = $t->ua->build_tx( GET => "/api/v1/libraries/" . $non_existent_code );
     $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
@@ -125,7 +142,8 @@ subtest 'get() tests' => sub {
 };
 
 subtest 'add() tests' => sub {
-    plan tests => 31;
+
+    plan tests => 17;
 
     $schema->storage->txn_begin;
 
@@ -133,28 +151,10 @@ subtest 'add() tests' => sub {
       create_user_and_session( { authorized => 0 } );
     my ( $authorized_borrowernumber, $authorized_session_id ) =
       create_user_and_session( { authorized => 1 } );
-    my $library = {
-        branchcode       => "LIBRARYBR1",
-        branchname       => "Library Name",
-        branchaddress1   => "Library Address1",
-        branchaddress2   => "Library Address2",
-        branchaddress3   => "Library Address3",
-        branchzip        => "Library Zipcode",
-        branchcity       => "Library City",
-        branchstate      => "Library State",
-        branchcountry    => "Library Country",
-        branchphone      => "Library Phone",
-        branchfax        => "Library Fax",
-        branchemail      => "Library Email",
-        branchreplyto    => "Library Reply-To",
-        branchreturnpath => "Library Return-Path",
-        branchurl        => "http://library.url",
-        issuing          => undef,                  # unused in Koha
-        branchip         => "127.0.0.1",
-        branchprinter    => "Library Printer",      # unused in Koha
-        branchnotes      => "Library Notes",
-        opac_info        => "<p>Library OPAC info</p>",
-    };
+
+    my $library_obj = $builder->build_object({ class => 'Koha::Libraries' });
+    my $library     = Koha::REST::V1::Library::_to_api( $library_obj->TO_JSON );
+    $library_obj->delete;
 
     # Unauthorized attempt to write
     my $tx = $t->ua->build_tx( POST => "/api/v1/libraries" => json => $library );
@@ -189,30 +189,15 @@ subtest 'add() tests' => sub {
     $tx->req->cookies(
         { name => 'CGISESSID', value => $authorized_session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
-    my $branchcode = $t->request_ok($tx)
-      ->status_is(201)
-      ->json_is( '/branchname'       => $library->{branchname} )
-      ->json_is( '/branchaddress1'   => $library->{branchaddress1} )
-      ->json_is( '/branchaddress2'   => $library->{branchaddress2} )
-      ->json_is( '/branchaddress3'   => $library->{branchaddress3} )
-      ->json_is( '/branchzip'        => $library->{branchzip} )
-      ->json_is( '/branchcity'       => $library->{branchcity} )
-      ->json_is( '/branchstate'      => $library->{branchstate} )
-      ->json_is( '/branchcountry'    => $library->{branchcountry} )
-      ->json_is( '/branchphone'      => $library->{branchphone} )
-      ->json_is( '/branchfax'        => $library->{branchfax} )
-      ->json_is( '/branchemail'      => $library->{branchemail} )
-      ->json_is( '/branchreplyto'    => $library->{branchreplyto} )
-      ->json_is( '/branchreturnpath' => $library->{branchreturnpath} )
-      ->json_is( '/branchurl'        => $library->{branchurl} )
-      ->json_is( '/branchip'        => $library->{branchip} )
-      ->json_is( '/branchnotes'      => $library->{branchnotes} )
-      ->json_is( '/opac_info'        => $library->{opac_info} )
-      ->header_is(Location => "/api/v1/libraries/$library->{branchcode}")
-      ->tx->res->json->{branchcode};
+    $t->request_ok($tx)
+      ->status_is( 201, 'SWAGGER3.2.1' )
+      ->json_is( '' => $library, 'SWAGGER3.3.1' )
+      ->header_is( Location => '/api/v1/libraries/' . $library->{library_id}, 'SWAGGER3.4.1' );
 
+    # save the library_id
+    my $library_id = $library->{library_id};
     # Authorized attempt to create with null id
-    $library->{branchcode} = undef;
+    $library->{library_id} = undef;
     $tx = $t->ua->build_tx(
         POST => "/api/v1/libraries" => json => $library );
     $tx->req->cookies(
@@ -223,15 +208,19 @@ subtest 'add() tests' => sub {
       ->json_has('/errors');
 
     # Authorized attempt to create with existing id
-    $library->{branchcode} = $branchcode;
+    $library->{library_id} = $library_id;
     $tx = $t->ua->build_tx(
         POST => "/api/v1/libraries" => json => $library );
     $tx->req->cookies(
         { name => 'CGISESSID', value => $authorized_session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
-    $t->request_ok($tx)
-      ->status_is(400)
-      ->json_is('/error' => 'Library already exists');
+
+    warning_like {
+        $t->request_ok($tx)
+          ->status_is(409)
+          ->json_has( '/error' => "Fails when trying to add an existing library_id")
+          ->json_is(  '/conflict', 'PRIMARY' ); } # WTF
+        qr/^DBD::mysql::st execute failed: Duplicate entry '(.*)' for key 'PRIMARY'/;
 
     $schema->storage->txn_rollback;
 };
@@ -246,10 +235,11 @@ subtest 'update() tests' => sub {
     my ( $authorized_borrowernumber, $authorized_session_id ) =
       create_user_and_session( { authorized => 1 } );
 
-    my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
+    my $library    = $builder->build_object({ class => 'Koha::Libraries' });
+    my $library_id = $library->branchcode;
 
     # Unauthorized attempt to update
-    my $tx = $t->ua->build_tx( PUT => "/api/v1/libraries/$branchcode"
+    my $tx = $t->ua->build_tx( PUT => "/api/v1/libraries/$library_id"
         => json => { branchname => 'New unauthorized name change' } );
     $tx->req->cookies(
         { name => 'CGISESSID', value => $unauthorized_session_id } );
@@ -259,10 +249,10 @@ subtest 'update() tests' => sub {
 
     # Attempt partial update on a PUT
     my $library_with_missing_field = {
-        branchaddress1 => "New library address",
+        address1 => "New library address",
     };
 
-    $tx = $t->ua->build_tx( PUT => "/api/v1/libraries/$branchcode" =>
+    $tx = $t->ua->build_tx( PUT => "/api/v1/libraries/$library_id" =>
                             json => $library_with_missing_field );
     $tx->req->cookies(
         { name => 'CGISESSID', value => $authorized_session_id } );
@@ -270,48 +260,27 @@ subtest 'update() tests' => sub {
     $t->request_ok($tx)
       ->status_is(400)
       ->json_has( "/errors" =>
-          [ { message => "Missing property.", path => "/body/branchaddress2" } ]
+          [ { message => "Missing property.", path => "/body/address2" } ]
       );
 
-    # Full object update on PUT
-    my $library_with_updated_field = {
-        branchcode       => "LIBRARYBR2",
-        branchname       => "Library Name",
-        branchaddress1   => "Library Address1",
-        branchaddress2   => "Library Address2",
-        branchaddress3   => "Library Address3",
-        branchzip        => "Library Zipcode",
-        branchcity       => "Library City",
-        branchstate      => "Library State",
-        branchcountry    => "Library Country",
-        branchphone      => "Library Phone",
-        branchfax        => "Library Fax",
-        branchemail      => "Library Email",
-        branchreplyto    => "Library Reply-To",
-        branchreturnpath => "Library Return-Path",
-        branchurl        => "http://library.url",
-        issuing          => undef,                  # unused in Koha
-        branchip         => "127.0.0.1",
-        branchprinter    => "Library Printer",      # unused in Koha
-        branchnotes      => "Library Notes",
-        opac_info        => "<p>Library OPAC info</p>",
-    };
+    my $deleted_library = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $library_with_updated_field = Koha::REST::V1::Library::_to_api( $deleted_library->TO_JSON );
+    $library_with_updated_field->{library_id} = $library_id;
+    $deleted_library->delete;
 
-    $tx = $t->ua->build_tx(
-        PUT => "/api/v1/libraries/$branchcode" => json => $library_with_updated_field );
-    $tx->req->cookies(
-        { name => 'CGISESSID', value => $authorized_session_id } );
+    $tx = $t->ua->build_tx( PUT => "/api/v1/libraries/$library_id" => json => $library_with_updated_field );
+    $tx->req->cookies( { name => 'CGISESSID', value => $authorized_session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
     $t->request_ok($tx)
-      ->status_is(200)
-      ->json_is( '/branchname' => 'Library Name' );
+      ->status_is(200, 'SWAGGER3.2.1')
+      ->json_is( '' => $library_with_updated_field, 'SWAGGER3.3.3' );
 
     # Authorized attempt to write invalid data
     my $library_with_invalid_field = { %$library_with_updated_field };
     $library_with_invalid_field->{'branchinvalid'} = 'Library invalid';
 
     $tx = $t->ua->build_tx(
-        PUT => "/api/v1/libraries/$branchcode" => json => $library_with_invalid_field );
+        PUT => "/api/v1/libraries/$library_id" => json => $library_with_invalid_field );
     $tx->req->cookies(
         { name => 'CGISESSID', value => $authorized_session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
@@ -364,8 +333,8 @@ subtest 'delete() tests' => sub {
         { name => 'CGISESSID', value => $authorized_session_id } );
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
     $t->request_ok($tx)
-      ->status_is(204)
-      ->content_is('');
+      ->status_is(204, 'SWAGGER3.2.4')
+      ->content_is('', 'SWAGGER3.3.4');
 
     $tx = $t->ua->build_tx( DELETE => "/api/v1/libraries/$branchcode" );
     $tx->req->cookies(