Attachment 50889 Details for Bug 16330 – Bug 16330 - REST API: add routes to add, update and delete patrons

[patch] Bug 16330 - REST API: add routes to add, update and delete patrons

Bug-16330---REST-API-add-routes-to-add-update-and-.patch (text/plain), 17.91 KB, created by Benjamin Rokseth on 2016-04-28 09:56:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Benjamin Rokseth

Created: 2016-04-28 09:56:47 UTC

Size: 17.91 KB

patch

obsolete

>From 0260f4d32880658d5f9c645e0b59beb654988fca Mon Sep 17 00:00:00 2001
>From: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
>Date: Wed, 27 Apr 2016 13:47:04 +0000
>Subject: [PATCH] Bug 16330 - REST API: add routes to add, update and delete
> patrons
>
>This patch adds support for add, edit and delete patrons via REST API.
>
>POST /api/v1/patrons                   Create a new patron
>PUT  /api/v1/patrons/<borrowernumber>  Update data about patron
>DEL  /api/v1/patrons/<borrowernumber>  Delete a patron
>
>Test plan:
>1) Apply this patch (on top of dependent bug 15126)
>2) Run tests perl t/db_dependent/api/v1/patrons.t
>3) Add a user with proper rights to use the REST API
>4) play with your favourite REST client (curl/httpie, etc.):
>   Authenticate with the user created above and get a CGISESSION id.
>   Use the CGISESSION to add, edit and delete patrons via the API.
>
>Please note there is no validation of body input in PUT/POST other
>than branchcode,category,userid,cardnumber.
>---
> Koha/REST/V1/Patron.pm          | 145 ++++++++++++++++++++++++++++++++++++++
> api/v1/swagger.json             | 137 ++++++++++++++++++++++++++++++++++++
> t/db_dependent/api/v1/patrons.t | 149 ++++++++++++++++++++++++++++++++++++----
> 3 files changed, 417 insertions(+), 14 deletions(-)
>
>diff --git a/Koha/REST/V1/Patron.pm b/Koha/REST/V1/Patron.pm
>index 2851308..fe2aa27 100644
>--- a/Koha/REST/V1/Patron.pm
>+++ b/Koha/REST/V1/Patron.pm
>@@ -20,7 +20,10 @@ use Modern::Perl;
> use Mojo::Base 'Mojolicious::Controller';
> 
> use C4::Auth qw( haspermission );
>+use Koha::AuthUtils qw(hash_password);
> use Koha::Patrons;
>+use Koha::Patron::Categories;
>+use Koha::Libraries;
> 
> sub list {
>     my ($c, $args, $cb) = @_;
>@@ -55,4 +58,146 @@ sub get {
>     return $c->$cb($patron->unblessed, 200);
> }
> 
>+sub add {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $user = $c->stash('koha.user');
>+
>+    unless ( $user && haspermission($user->userid, {borrowers => 1}) ) {
>+        return $c->$cb({error => "You don't have the required permission"}, 403);
>+    }
>+
>+    my $body = $c->req->json;
>+
>+    # patron cardnumber and/or userid unique?
>+    if ($body->{cardnumber} || $body->{userid}) {
>+        my $patron = Koha::Patrons->find({cardnumber => $body->{cardnumber}, userid => $body->{userid} });
>+        if ($patron) {
>+            return $c->$cb({
>+                error => "Patron cardnumber and userid must be unique",
>+                conflict => { cardnumber => $patron->cardnumber, userid => $patron->userid }
>+            }, 409);
>+        }
>+    }
>+
>+    my $branch = Koha::Libraries->find({branchcode => $body->{branchcode} });
>+    unless ($branch) {
>+        return $c->$cb({error => "Library with branchcode \"" . $body->{branchcode} . "\" does not exist"}, 404);
>+    }
>+    my $category = Koha::Patron::Categories->find({ categorycode => $body->{categorycode} });
>+    unless ($category) {
>+        return $c->$cb({error => "Patron category \"" . $body->{categorycode} . "\" does not exist"}, 404);
>+    }
>+    # All OK - save new patron
>+
>+    if ($body->{password}) { $body->{password} = hash_password($body->{password}) }; # bcrypt password if given
>+
>+    my $patron = eval {
>+        Koha::Patron->new($body)->store;
>+    };
>+
>+    unless ($patron) {
>+        return $c->$cb({error => "Something went wrong, check Koha logs for details"}, 500);
>+    }
>+
>+    return $c->$cb($patron->unblessed, 201);
>+}
>+
>+sub edit {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $user = $c->stash('koha.user');
>+
>+    unless ( $user && haspermission($user->userid, {borrowers => 1}) ) {
>+        return $c->$cb({error => "You don't have the required permission"}, 403);
>+    }
>+
>+    my $patron = Koha::Patrons->find($args->{borrowernumber});
>+
>+    unless ($patron) {
>+        return $c->$cb({error => "Patron not found"}, 404);
>+    }
>+
>+    my $body = $c->req->json;
>+
>+    # Can we change userid and/or cardnumber? in that case check that they are altered first
>+    if ($body->{cardnumber} || $body->{userid}) {
>+        if ( ($body->{cardnumber} && $body->{cardnumber} ne $patron->cardnumber) || ($body->{userid} && $body->{userid} ne $patron->userid) ) {
>+            my $conflictingPatron = Koha::Patrons->find({cardnumber => $body->{cardnumber}, userid => $body->{userid} });
>+            if ($conflictingPatron) {
>+                return $c->$cb({
>+                    error => "Patron cardnumber and userid must be unique",
>+                    conflict => { cardnumber => $conflictingPatron->cardnumber, userid => $conflictingPatron->userid }
>+                }, 409);
>+            }
>+        }
>+    }
>+
>+    if ($body->{branchcode}) {
>+        my $branch = Koha::Libraries->find({branchcode => $body->{branchcode} });
>+        unless ($branch) {
>+            return $c->$cb({error => "Library with branchcode \"" . $body->{branchcode} . "\" does not exist"}, 404);
>+        }
>+    }
>+
>+    if ($body->{categorycode}) {
>+        my $category = Koha::Patron::Categories->find({ categorycode => $body->{categorycode} });
>+        unless ($category) {
>+            return $c->$cb({error => "Patron category \"" . $body->{categorycode} . "\" does not exist"}, 404);
>+        }
>+    }
>+    # ALL OK - Update patron
>+    # Perhaps limit/validate what should be updated here? flags, et.al.
>+    if ($body->{password}) { $body->{password} = hash_password($body->{password}) }; # bcrypt password if given
>+
>+    my $updatedpatron = eval {
>+        $patron->set($body);
>+    };
>+
>+    if ($updatedpatron) {
>+        if ($updatedpatron->is_changed) {
>+
>+            my $res = eval {
>+                $updatedpatron->store;
>+            };
>+
>+            unless ($res) {
>+                return $c->$cb({error => "Something went wrong, check Koha logs for details"}, 500);
>+            }
>+            return $c->$cb($res->unblessed, 200);
>+
>+        } else {
>+            return $c->$cb({}, 204); # No Content = No changes made
>+        }
>+    } else {
>+        return $c->$cb({error => "Something went wrong, check Koha logs for details"}, 500);
>+    }
>+}
>+
>+sub delete {
>+    my ($c, $args, $cb) = @_;
>+    my $user = $c->stash('koha.user');
>+
>+    unless ( $user && haspermission($user->userid, {borrowers => 1}) ) {
>+        return $c->$cb({error => "You don't have the required permission"}, 403);
>+    }
>+
>+    my $patron = Koha::Patrons->find($args->{borrowernumber});
>+
>+    unless ($patron) {
>+        return $c->$cb({error => "Patron not found"}, 404);
>+    }
>+
>+    # check if loans, reservations, debarrment, etc. before deletion!
>+    my $res = $patron->delete;
>+
>+    if ($res eq '1') {
>+        return $c->$cb({}, 200);
>+    } elsif ($res eq '-1') {
>+        return $c->$cb({}, 404);
>+    } else {
>+        return $c->$cb({}, 400);
>+    }
>+}
>+
> 1;
>diff --git a/api/v1/swagger.json b/api/v1/swagger.json
>index e821c44..48dd229 100644
>--- a/api/v1/swagger.json
>+++ b/api/v1/swagger.json
>@@ -38,6 +38,55 @@
>             }
>           }
>         }
>+      },
>+      "post": {
>+        "operationId": "addPatron",
>+        "tags": ["patrons"],
>+        "parameters": [{
>+          "name": "body",
>+          "in": "body",
>+          "description": "A JSON object containing information about the new patron",
>+          "required": true,
>+          "schema": {
>+            "$ref": "#/definitions/patron"
>+          }
>+        }],
>+        "consumes": ["application/json"],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "201": {
>+            "description": "A successfully created patron",
>+            "schema": {
>+              "items": {
>+                "$ref": "#/definitions/patron"
>+              }
>+            }
>+          },
>+          "403": {
>+            "description": "Access forbidden",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "404": {
>+            "description": "Resource not found",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "409": {
>+            "description": "Conflict in creating resource",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "500": {
>+            "description": "Internal error",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          }
>+        }
>       }
>     },
>     "/patrons/{borrowernumber}": {
>@@ -72,6 +121,94 @@
>             }
>           }
>         }
>+      },
>+      "put": {
>+        "operationId": "editPatron",
>+        "tags": ["patrons"],
>+        "parameters": [
>+          { "$ref": "#/parameters/borrowernumberPathParam" },
>+          {
>+            "name": "body",
>+            "in": "body",
>+            "description": "A JSON object containing new information about existing patron",
>+            "required": true,
>+            "schema": {
>+              "$ref": "#/definitions/patron"
>+            }
>+          }
>+        ],
>+        "consumes": ["application/json"],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "200": {
>+            "description": "A successfully updated patron",
>+            "schema": {
>+              "items": {
>+                "$ref": "#/definitions/patron"
>+              }
>+            }
>+          },
>+          "204": {
>+            "description": "No Content",
>+            "schema": {
>+              "type": "object"
>+            }
>+          },
>+          "403": {
>+            "description": "Access forbidden",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "404": {
>+            "description": "Resource not found",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "409": {
>+            "description": "Conflict in updating resource",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "500": {
>+            "description": "Internal error",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          }
>+        }
>+      },
>+      "delete": {
>+        "operationId": "deletePatron",
>+        "tags": ["patrons"],
>+        "parameters": [
>+          { "$ref": "#/parameters/borrowernumberPathParam" }
>+        ],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "200": {
>+            "description": "Patron deleted successfully",
>+            "schema": {
>+              "type": "object"
>+            }
>+          },
>+          "400": {
>+            "description": "Patron deletion failed",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "403": {
>+            "description": "Access forbidden",
>+            "schema": {
>+              "$ref": "#/definitions/error"
>+            }
>+          },
>+          "404": {
>+            "description": "Patron not found",
>+            "schema": { "$ref": "#/definitions/error" }
>+          }
>+        }
>       }
>     }
>   },
>diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t
>index 6bd4e1a..945ea64 100644
>--- a/t/db_dependent/api/v1/patrons.t
>+++ b/t/db_dependent/api/v1/patrons.t
>@@ -17,28 +17,32 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 10;
>-use Test::Mojo;
> use t::lib::TestBuilder;
> 
>+use Test::More tests => 56;
>+use Test::Mojo;
>+
> use C4::Auth;
> use C4::Context;
>-
> use Koha::Database;
>-use Koha::Patron;
> 
>-my $builder = t::lib::TestBuilder->new();
>+BEGIN {
>+    use_ok('Koha::Object');
>+    use_ok('Koha::Patron');
>+}
> 
>-my $dbh = C4::Context->dbh;
>-$dbh->{AutoCommit} = 0;
>-$dbh->{RaiseError} = 1;
>+my $schema  = Koha::Database->schema;
>+my $dbh     = C4::Context->dbh;
>+my $builder = t::lib::TestBuilder->new;
> 
> $ENV{REMOTE_ADDR} = '127.0.0.1';
> my $t = Test::Mojo->new('Koha::REST::V1');
> 
>+$schema->storage->txn_begin;
>+
> my $categorycode = $builder->build({ source => 'Category' })->{ categorycode };
> my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode };
>-my $borrower = $builder->build({
>+my $patron = $builder->build({
>     source => 'Borrower',
>     value => {
>         branchcode   => $branchcode,
>@@ -49,7 +53,7 @@ my $borrower = $builder->build({
> $t->get_ok('/api/v1/patrons')
>   ->status_is(403);
> 
>-$t->get_ok("/api/v1/patrons/" . $borrower->{ borrowernumber })
>+$t->get_ok("/api/v1/patrons/" . $patron->{ borrowernumber })
>   ->status_is(403);
> 
> my $loggedinuser = $builder->build({
>@@ -74,11 +78,128 @@ $tx->req->env({REMOTE_ADDR => '127.0.0.1'});
> $t->request_ok($tx)
>   ->status_is(200);
> 
>-$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{ borrowernumber });
>+$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $patron->{ borrowernumber });
> $tx->req->cookies({name => 'CGISESSID', value => $session->id});
> $t->request_ok($tx)
>   ->status_is(200)
>-  ->json_is('/borrowernumber' => $borrower->{ borrowernumber })
>-  ->json_is('/surname' => $borrower->{ surname });
>+  ->json_is('/borrowernumber' => $patron->{ borrowernumber })
>+  ->json_is('/surname' => $patron->{ surname });
>+
>+### POST /api/v1/patrons
>+
>+my $newpatron = {
>+  branchcode   => $branchcode,
>+  categorycode => $categorycode,
>+  surname      => "TestUser",
>+  cardnumber => "123456",
>+  userid => "testuser"
>+};
>+
>+$newpatron->{ branchcode } = "nonexistent"; # Test invalid branchcode
>+$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_is('/error' => "Library with branchcode \"nonexistent\" does not exist");
>+
>+$newpatron->{ branchcode } = $branchcode;
>+$newpatron->{ categorycode } = "nonexistent"; # Test invalid patron category
>+$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_is('/error' => "Patron category \"nonexistent\" does not exist");
>+$newpatron->{ categorycode } = $categorycode;
>+
>+$newpatron->{ falseproperty } = "Non existent property";
>+$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(500)
>+  ->json_is('/error' => "Something went wrong, check Koha logs for details");
>+
>+delete $newpatron->{ falseproperty };
>+$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(201, 'Patron created successfully')
>+  ->json_has('/borrowernumber', 'got a borrowernumber')
>+  ->json_is('/cardnumber', $newpatron->{ cardnumber })
>+  ->json_is('/surname' => $newpatron->{ surname })
>+  ->json_is('/firstname' => $newpatron->{ firstname });
>+$newpatron->{borrowernumber} = $tx->res->json->{borrowernumber};
>+
>+$tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(409)
>+  ->json_has('/error', 'Fails when trying to POST duplicate cardnumber or userid')
>+  ->json_has('/conflict', { userid => $newpatron->{ userid }, cardnumber => $newpatron->{ cardnumber } });
>+
>+### PUT /api/v1/patrons
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/0" => json => {});
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_has('/error', 'Fails when trying to PUT nonexistent patron');
>+
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => {categorycode => "nonexistent"});
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_is('/error' => "Patron category \"nonexistent\" does not exist");
>+
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => {branchcode => "nonexistent"});
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404)
>+  ->json_is('/error' => "Library with branchcode \"nonexistent\" does not exist");
>+
>+$newpatron->{ falseproperty } = "Non existent property";
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(500)
>+  ->json_is('/error' => "Something went wrong, check Koha logs for details");
>+delete $newpatron->{ falseproperty };
>+
>+$newpatron->{ cardnumber } = $patron-> { cardnumber };
>+$newpatron->{ userid } = $patron-> { userid };
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(409)
>+  ->json_has('/error' => "Fails when trying to update to an existing cardnumber or userid")
>+  ->json_has('/conflict', { cardnumber => $patron->{ cardnumber }, userid => $patron->{ userid } });
>+
>+$newpatron->{ cardnumber } = "123456";
>+$newpatron->{ userid } = "testuser";
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(204, 'No changes - patron NOT updated');
>+
>+$newpatron->{ cardnumber } = "234567";
>+$newpatron->{ userid } = "updatedtestuser";
>+$newpatron->{ surname } = "UpdatedTestUser";
>+
>+$tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $newpatron->{ borrowernumber } => json => $newpatron);
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(200, 'Patron updated successfully')
>+  ->json_has($newpatron);
>+
>+### DELETE /api/v1/patrons
>+
>+$tx = $t->ua->build_tx(DELETE => "/api/v1/patrons/0");
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(404, 'Patron not found');
>+
>+$tx = $t->ua->build_tx(DELETE => "/api/v1/patrons/" . $newpatron->{ borrowernumber });
>+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
>+$t->request_ok($tx)
>+  ->status_is(200, 'Patron deleted successfully');
>+
>+$schema->storage->txn_rollback;
> 
>-$dbh->rollback;
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 16330: 50803 | 50889 | 51066 | 51131 | 51132 | 53216 | 53217 | 53820 | 54942 | 55147 | 56666 | 56667 | 56668 | 63646 | 63647 | 63649 | 63650 | 69546 | 69547 | 69548 | 69549 | 69550 | 69551 | 69752 | 69762 | 69763 | 69764 | 69765 | 69766 | 69767 | 69768 | 69787 | 70034 | 70035 | 70036 | 70037 | 70038 | 71667 | 71668 | 71669 | 71670 | 71671 | 71920 | 71921 | 71922 | 71923 | 71924 | 71925 | 71933 | 71934 | 71935 | 71936 | 71937 | 71938 | 73434