From 47d86aab9bce62e24071713a4bd3662a0e3c67a5 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 7 Oct 2015 16:53:24 +0100 Subject: [PATCH] Bug 14974: Joubu's try to use the REST API for cities Here's my try. The naive way is to do AJAX calls and redirect to the list after delete/update/add. The main issue is that it does not simplify the code so far... Question: How to display notif to the user after actions (delete/update/add)? How to simplify the swagger file? How to present a search interface? See other comments in the code. --- Koha/REST/V1/Cities.pm | 98 +++++++++++ admin/cities.pl | 53 +----- api/v1/swagger.json | 192 +++++++++++++++++++++ .../intranet-tmpl/prog/en/modules/admin/cities.tt | 176 ++++++++++++------- 4 files changed, 406 insertions(+), 113 deletions(-) create mode 100644 Koha/REST/V1/Cities.pm diff --git a/Koha/REST/V1/Cities.pm b/Koha/REST/V1/Cities.pm new file mode 100644 index 0000000..944eb75 --- /dev/null +++ b/Koha/REST/V1/Cities.pm @@ -0,0 +1,98 @@ +package Koha::REST::V1::Cities; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use Mojo::Base 'Mojolicious::Controller'; + +use C4::Auth qw( haspermission ); +use Koha::City; +use Koha::Cities; + +sub list_cities { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + # Check permissions here + + # Catch possible errors here + my $cities = Koha::Cities->search( + { + map { $args->{$_} ? ( $_ => $args->{$_} ): () } keys %$args + } + ); + + return $c->$cb($cities->unblessed, 200); +} + +sub get_city { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + # Check permissions here + + # Catch possible errors here + my $city = Koha::Cities->find($args->{cityid}); + unless ($city) { + return $c->$cb({error => "City not found"}, 404); + } + + return $c->$cb($city->unblessed, 200); +} + +sub add_city { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + # Check permissions here + + # Catch possible errors here + my $city = Koha::City->new($args->{city}); + $city->store; + + return $c->$cb($city->unblessed, 200); +} + +sub update_city { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + # Check permissions here + + # Catch possible errors here + my $city = Koha::Cities->find($args->{city}{cityid}); + while ( my ( $k, $v ) = each %{$args->{city}} ) { + $city->$k($v); + } + $city->store; + + return $c->$cb($city->unblessed, 200); +} + +sub delete_city { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + # Check permissions here + + # Catch possible errors here + my $city = Koha::Cities->find($args->{cityid})->delete; + + return $c->$cb("", 200); +} + +1; diff --git a/admin/cities.pl b/admin/cities.pl index 4a6247c..e8abeec 100755 --- a/admin/cities.pl +++ b/admin/cities.pl @@ -30,7 +30,7 @@ my $input = new CGI; my $searchfield = $input->param('city_name') // q||; my $cityid = $input->param('cityid'); my $op = $input->param('op') || 'list'; -my @messages; +my $message = $input->param('message'); my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { template_name => "admin/cities.tt", @@ -50,55 +50,6 @@ if ( $op eq 'add_form' ) { } $template->param( city => $city, ); -} elsif ( $op eq 'add_validate' ) { - my $cityid = $input->param('cityid'); - my $city_name = $input->param('city_name'); - my $city_state = $input->param('city_state'); - my $city_zipcode = $input->param('city_zipcode'); - my $city_country = $input->param('city_country'); - - if ($cityid) { - my $city = Koha::Cities->find($cityid); - $city->city_name($city_name); - $city->city_state($city_state); - $city->city_zipcode($city_zipcode); - $city->city_country($city_country); - eval { $city->store; }; - if ($@) { - push @messages, { type => 'error', code => 'error_on_update' }; - } else { - push @messages, { type => 'message', code => 'success_on_update' }; - } - } else { - my $city = Koha::City->new( - { city_name => $city_name, - city_state => $city_state, - city_zipcode => $city_zipcode, - city_country => $city_country, - } - ); - eval { $city->store; }; - if ($@) { - push @messages, { type => 'error', code => 'error_on_insert' }; - } else { - push @messages, { type => 'message', code => 'success_on_insert' }; - } - } - $searchfield = q||; - $op = 'list'; -} elsif ( $op eq 'delete_confirm' ) { - my $city = Koha::Cities->find($cityid); - $template->param( city => $city, ); -} elsif ( $op eq 'delete_confirmed' ) { - my $city = Koha::Cities->find($cityid); - my $deleted = eval { $city->delete; }; - - if ( $@ or not $deleted ) { - push @messages, { type => 'error', code => 'error_on_delete' }; - } else { - push @messages, { type => 'message', code => 'success_on_delete' }; - } - $op = 'list'; } if ( $op eq 'list' ) { @@ -109,7 +60,7 @@ if ( $op eq 'list' ) { $template->param( cityid => $cityid, searchfield => $searchfield, - messages => \@messages, + message => $message, op => $op, ); diff --git a/api/v1/swagger.json b/api/v1/swagger.json index a20cb20..4290852 100644 --- a/api/v1/swagger.json +++ b/api/v1/swagger.json @@ -75,6 +75,161 @@ } } } + }, + "/cities": { + "get": { + "x-mojo-controller": "Koha::REST::V1::Cities", + "operationId": "listCities", + "tags": ["cities"], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A list of cities", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/city" + } + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + } + } + }, + "put": { + "x-mojo-controller": "Koha::REST::V1::Cities", + "operationId": "AddCity", + "tags": ["cities"], + "parameters": [ + { + "$ref": "#/parameters/cityBodyParam" + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "City added", + "schema": { + "$ref": "#/definitions/city" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, + "/cities/{cityid}": { + "get": { + "x-mojo-controller": "Koha::REST::V1::Cities", + "operationId": "getCity", + "tags": ["cities"], + "parameters": [ + { + "$ref": "#/parameters/cityidPathParam" + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A city", + "schema": { + "$ref": "#/definitions/city" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "City not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + }, + "put": { + "x-mojo-controller": "Koha::REST::V1::Cities", + "operationId": "updateCity", + "tags": ["cities"], + "parameters": [ + { + "$ref": "#/parameters/cityBodyParam" + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A city", + "schema": { + "$ref": "#/definitions/city" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "City not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + }, + "delete": { + "x-mojo-controller": "Koha::REST::V1::Cities", + "operationId": "deleteCity", + "tags": ["cities"], + "parameters": [ + { + "$ref": "#/parameters/cityidPathParam" + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "City deleted", + "schema": { + "type": "string" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "City not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } } }, "definitions": { @@ -106,6 +261,29 @@ "type": "string" } } + }, + "city": { + "type": "object", + "properties": { + "cityid": { + "$ref": "#/definitions/cityid" + }, + "city_name": { + "description": "city name" + }, + "city_state": { + "description": "city state" + }, + "city_zipcode": { + "description": "city zipcode" + }, + "city_country": { + "description": "city country" + } + } + }, + "cityid": { + "description": "city id" } }, "parameters": { @@ -115,6 +293,20 @@ "description": "Internal borrower identifier", "required": "true", "type": "integer" + }, + "cityidPathParam": { + "name": "cityid", + "in": "path", + "description": "City id", + "required": "true", + "type": "integer" + }, + "cityBodyParam": { + "name": "city", + "in": "body", + "description": "A JSON object representing a city", + "required": "true", + "schema": { "$ref": "#/definitions/city" } } } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt index facb119..2ffab05 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt @@ -5,16 +5,112 @@ [% INCLUDE 'datatables.inc' %] @@ -39,21 +135,19 @@
-[% FOR m IN messages %] -
- [% SWITCH m.code %] - [% CASE 'error_on_update' %] - An error occurred when updating this city. Perhaps it already exists. +[% IF message %] +
+ [% SWITCH message %] + [% CASE 'updated' %] + City updated successfully. + [% CASE 'added' %] + City added successfully. + [% CASE 'deleted' %] + City deleted successfully. [% CASE 'error_on_insert' %] An error occurred when adding this city. The city id might already exist. [% CASE 'error_on_delete' %] An error occurred when deleting this city. Check the logs. - [% CASE 'success_on_update' %] - City updated successfully. - [% CASE 'success_on_insert' %] - City added successfully. - [% CASE 'success_on_delete' %] - City deleted successfully. [% CASE 'already_exists' %] This city already exists. [% CASE %] @@ -69,9 +163,9 @@

New city

[% END %] -
+ - +
    @@ -104,37 +198,6 @@ [% END %] -[% IF op == 'delete_confirm' %] -
    -

    Delete City "[% city.city_name %]?"

    - - - - - - - - - - - - - - - - -
    City id[% city.cityid %]
    City[% city.city_name %]
    State[% city.city_state %]
    Zip/Postal code[% city.city_zipcode %]
    Country[% city.city_country %]
    -
    - - - -
    -
    - -
    -
    -[% END %] - [% IF op == 'list' %]
    @@ -158,17 +221,6 @@   - [% FOREACH city IN cities %] - - [% city.cityid %] - [% city.city_name %] - [% city.city_state %] - [% city.city_zipcode %] - [% city.city_country %] - Edit - Delete - - [% END %] [% ELSE %] -- 2.1.0