Attachment 91426 Details for Bug 23285 – Bug 23285: Add patrons PATCH route

[patch] Bug 23285: Add patrons PATCH route

0001-Bug-23285-Add-patrons-PATCH-route.patch (text/plain), 17.03 KB, created by Johanna Räisä on 2019-07-09 09:02:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Johanna Räisä

Created: 2019-07-09 09:02:48 UTC

Size: 17.03 KB

patch

obsolete

>From ccf0bfeb1096ce27786e3a5d755c3c6382ff866b Mon Sep 17 00:00:00 2001
>From: Johanna Raisa <johanna.raisa@gmail.com>
>Date: Tue, 9 Jul 2019 11:53:07 +0300
>Subject: [PATCH] Bug 23285: Add patrons PATCH route
>
>Implementation for patrons PATCH route.
>
>Test plan
>- Apply this patch
>- Run prove t/db_dependent/api/v1/patrons.t
>- Create user with borrowers permissions
>- Authenticate with the user created above and get a CGISESSION id.
>- Use the CGISESSION to patch patrons via the API
>- PATCH /patrons/{patron_id} with some parameters in the body
>
>Sponsored-by: Koha-Suomi Oy
>---
> Koha/REST/V1/Patrons.pm                      |   2 +-
> api/v1/swagger/definitions.json              |   3 +
> api/v1/swagger/definitions/patron_patch.json | 283 +++++++++++++++++++
> api/v1/swagger/paths/patrons.json            |  78 +++++
> t/db_dependent/api/v1/patrons.t              |  66 ++++-
> 5 files changed, 430 insertions(+), 2 deletions(-)
> create mode 100644 api/v1/swagger/definitions/patron_patch.json
>
>diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm
>index 5e8e398eac..71b67171a7 100644
>--- a/Koha/REST/V1/Patrons.pm
>+++ b/Koha/REST/V1/Patrons.pm
>@@ -198,7 +198,7 @@ sub update {
> 
>         $patron->set($body)->store;
>         $patron->discard_changes;
>-        return $c->render( status => 200, openapi => $patron );
>+        return $c->render( status => 200, openapi => _to_api( $patron->TO_JSON ) );
>     }
>     catch {
>         unless ( blessed $_ && $_->can('rethrow') ) {
>diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json
>index e3001b5e60..4a7b6a8ab8 100644
>--- a/api/v1/swagger/definitions.json
>+++ b/api/v1/swagger/definitions.json
>@@ -32,6 +32,9 @@
>   "patron_balance": {
>     "$ref": "definitions/patron_balance.json"
>   },
>+  "patron_patch": {
>+    "$ref": "definitions/patron_patch.json"
>+  },
>   "allows_renewal": {
>     "$ref": "definitions/allows_renewal.json"
>   },
>diff --git a/api/v1/swagger/definitions/patron_patch.json b/api/v1/swagger/definitions/patron_patch.json
>new file mode 100644
>index 0000000000..2f55493d12
>--- /dev/null
>+++ b/api/v1/swagger/definitions/patron_patch.json
>@@ -0,0 +1,283 @@
>+{
>+  "type": "object",
>+  "properties": {
>+    "patron_id": {
>+      "$ref": "../x-primitives.json#/patron_id"
>+    },
>+    "cardnumber": {
>+      "$ref": "../x-primitives.json#/cardnumber"
>+    },
>+    "surname": {
>+      "$ref": "../x-primitives.json#/surname"
>+    },
>+    "firstname": {
>+      "$ref": "../x-primitives.json#/firstname"
>+    },
>+    "title": {
>+      "type": ["string", "null"],
>+      "description": "patron's title"
>+    },
>+    "other_name": {
>+      "type": ["string", "null"],
>+      "description": "any other names associated with the patron"
>+    },
>+    "initials": {
>+      "type": ["string", "null"],
>+      "description": "initials of the patron"
>+    },
>+    "street_number": {
>+      "type": ["string", "null"],
>+      "description": "street number of patron's primary address"
>+    },
>+    "street_type": {
>+      "type": ["string", "null"],
>+      "description": "street type of patron's primary address"
>+    },
>+    "address": {
>+      "type": ["string", "null"],
>+      "description": "first address line of patron's primary address"
>+    },
>+    "address2": {
>+      "type": ["string", "null"],
>+      "description": "second address line of patron's primary address"
>+    },
>+    "city": {
>+      "type": ["string", "null"],
>+      "description": "city or town of patron's primary address"
>+    },
>+    "state": {
>+      "type": ["string", "null"],
>+      "description": "state or province of patron's primary address"
>+    },
>+    "postal_code": {
>+      "type": ["string", "null"],
>+      "description": "zip or postal code of patron's primary address"
>+    },
>+    "country": {
>+      "type": ["string", "null"],
>+      "description": "country of patron's primary address"
>+    },
>+    "email": {
>+      "$ref": "../x-primitives.json#/email"
>+    },
>+    "phone": {
>+      "$ref": "../x-primitives.json#/phone"
>+    },
>+    "mobile": {
>+      "type": ["string", "null"],
>+      "description": "the other phone number for patron's primary address"
>+    },
>+    "fax": {
>+      "type": ["string", "null"],
>+      "description": "fax number for patron's primary address"
>+    },
>+    "secondary_email": {
>+      "type": ["string", "null"],
>+      "description": "secondary email address for patron's primary address"
>+    },
>+    "secondary_phone": {
>+      "type": ["string", "null"],
>+      "description": "secondary phone number for patron's primary address"
>+    },
>+    "altaddress_street_number": {
>+      "type": ["string", "null"],
>+      "description": "street number of patron's alternate address"
>+    },
>+    "altaddress_street_type": {
>+      "type": ["string", "null"],
>+      "description": "street type of patron's alternate address"
>+    },
>+    "altaddress_address": {
>+      "type": ["string", "null"],
>+      "description": "first address line of patron's alternate address"
>+    },
>+    "altaddress_address2": {
>+      "type": ["string", "null"],
>+      "description": "second address line of patron's alternate address"
>+    },
>+    "altaddress_city": {
>+      "type": ["string", "null"],
>+      "description": "city or town of patron's alternate address"
>+    },
>+    "altaddress_state": {
>+      "type": ["string", "null"],
>+      "description": "state or province of patron's alternate address"
>+    },
>+    "altaddress_postal_code": {
>+      "type": ["string", "null"],
>+      "description": "zip or postal code of patron's alternate address"
>+    },
>+    "altaddress_country": {
>+      "type": ["string", "null"],
>+      "description": "country of patron's alternate address"
>+    },
>+    "altaddress_email": {
>+      "type": ["string", "null"],
>+      "description": "email address for patron's alternate address"
>+    },
>+    "altaddress_phone": {
>+      "type": ["string", "null"],
>+      "description": "phone number for patron's alternate address"
>+    },
>+    "date_of_birth": {
>+      "type": ["string", "null"],
>+      "format": "date",
>+      "description": "patron's date of birth"
>+    },
>+    "library_id": {
>+      "type": "string",
>+      "description": "Internal identifier for the patron's home library"
>+    },
>+    "category_id": {
>+      "type": "string",
>+      "description": "Internal identifier for the patron's category"
>+    },
>+    "date_enrolled": {
>+      "type": ["string", "null"],
>+      "format": "date",
>+      "description": "date the patron was added to Koha"
>+    },
>+    "expiry_date": {
>+      "type": ["string", "null"],
>+      "format": "date",
>+      "description": "date the patron's card is set to expire"
>+    },
>+    "date_renewed": {
>+      "type": ["string", "null"],
>+      "description": "date the patron's card was last renewed"
>+    },
>+    "incorrect_address": {
>+      "type": ["boolean", "null"],
>+      "description": "set to 1 if library marked this patron as having an unconfirmed address"
>+    },
>+    "patron_card_lost": {
>+      "type": ["boolean", "null"],
>+      "description": "set to 1 if library marked this patron as having lost his card"
>+    },
>+    "restricted": {
>+      "type": "boolean",
>+      "readOnly": true,
>+      "description": "If any restriction applies to the patron"
>+    },
>+    "guarantor_id": {
>+      "type": ["integer", "null"],
>+      "description": "patron_id used for children or professionals to link them to guarantor or organizations"
>+    },
>+    "staff_notes": {
>+      "type": ["string", "null"],
>+      "description": "a note on the patron's account"
>+    },
>+    "relationship_type": {
>+      "type": ["string", "null"],
>+      "description": "used for children to include the relationship to their guarantor"
>+    },
>+    "gender": {
>+      "type": ["string", "null"],
>+      "description": "patron's gender"
>+    },
>+    "userid": {
>+      "type": ["string", "null"],
>+      "description": "patron's login"
>+    },
>+    "opac_notes": {
>+      "type": ["string", "null"],
>+      "description": "a note on the patron's account visible in OPAC and staff client"
>+    },
>+    "altaddress_notes": {
>+      "type": ["string", "null"],
>+      "description": "a note related to patron's alternate address"
>+    },
>+    "statistics_1": {
>+      "type": ["string", "null"],
>+      "description": "a field that can be used for any information unique to the library"
>+    },
>+    "statistics_2": {
>+      "type": ["string", "null"],
>+      "description": "a field that can be used for any information unique to the library"
>+    },
>+    "altcontact_firstname": {
>+      "type": ["string", "null"],
>+      "description": "first name of alternate contact for the patron"
>+    },
>+    "altcontact_surname": {
>+      "type": ["string", "null"],
>+      "description": "surname or last name of the alternate contact for the patron"
>+    },
>+    "altcontact_address": {
>+      "type": ["string", "null"],
>+      "description": "the first address line for the alternate contact for the patron"
>+    },
>+    "altcontact_address2": {
>+      "type": ["string", "null"],
>+      "description": "the second address line for the alternate contact for the patron"
>+    },
>+    "altcontact_city": {
>+      "type": ["string", "null"],
>+      "description": "the city for the alternate contact for the patron"
>+    },
>+    "altcontact_state": {
>+      "type": ["string", "null"],
>+      "description": "the state for the alternate contact for the patron"
>+    },
>+    "altcontact_postal_code": {
>+      "type": ["string", "null"],
>+      "description": "the zipcode for the alternate contact for the patron"
>+    },
>+    "altcontact_country": {
>+      "type": ["string", "null"],
>+      "description": "the country for the alternate contact for the patron"
>+    },
>+    "altcontact_phone": {
>+      "type": ["string", "null"],
>+      "description": "the phone number for the alternate contact for the patron"
>+    },
>+    "sms_number": {
>+      "type": ["string", "null"],
>+      "description": "the mobile phone number where the patron would like to receive notices (if SMS turned on)"
>+    },
>+    "sms_provider_id": {
>+      "type": ["integer", "null"],
>+      "description": "the provider of the mobile phone number defined in smsalertnumber"
>+    },
>+    "privacy": {
>+      "type": "integer",
>+      "description": "patron's privacy settings related to their reading history"
>+    },
>+    "privacy_guarantor_checkouts": {
>+      "type": "integer",
>+      "description": "controls if relatives can see this patron's checkouts"
>+    },
>+    "check_previous_checkout": {
>+      "type": "string",
>+      "description": "produce a warning for this patron if this item has previously been checked out to this patron if 'yes', not if 'no', defer to category setting if 'inherit'"
>+    },
>+    "updated_on": {
>+      "type": "string",
>+      "format": "date-time",
>+      "description": "time of last change could be useful for synchronization with external systems (among others)"
>+    },
>+    "last_seen": {
>+      "type": ["string", "null"],
>+      "format": "date-time",
>+      "description": "last time a patron has been seen (connected at the OPAC or staff interface)"
>+    },
>+    "lang": {
>+      "type": "string",
>+      "description": "lang to use to send notices to this patron"
>+    },
>+    "login_attempts": {
>+      "type": ["integer", "null"],
>+      "description": "number of failed login attemps"
>+    },
>+    "overdrive_auth_token": {
>+      "type": ["string", "null"],
>+      "description": "persist OverDrive auth token"
>+    },
>+    "anonymized": {
>+      "type": "boolean",
>+      "readOnly": true,
>+      "description": "If the patron has been anonymized"
>+    }
>+  },
>+  "additionalProperties": false
>+}
>diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json
>index 27bd08f69f..ed522eec66 100644
>--- a/api/v1/swagger/paths/patrons.json
>+++ b/api/v1/swagger/paths/patrons.json
>@@ -654,6 +654,84 @@
>         }
>       }
>     },
>+    "patch": {
>+      "x-mojo-to": "Patrons#update",
>+      "operationId": "patchPatron",
>+      "tags": ["patrons"],
>+      "parameters": [
>+        {
>+          "$ref": "../parameters.json#/patron_id_pp"
>+        },
>+        {
>+          "name": "body",
>+          "in": "body",
>+          "description": "A JSON object containing new information about existing patron",
>+          "required": true,
>+          "schema": {
>+            "$ref": "../definitions.json#/patron_patch"
>+          }
>+        }
>+      ],
>+      "consumes": ["application/json"],
>+      "produces": ["application/json"],
>+      "responses": {
>+        "200": {
>+          "description": "A successfully updated patron",
>+          "schema": {
>+            "items": {
>+              "$ref": "../definitions.json#/patron"
>+            }
>+          }
>+        },
>+        "202": {
>+          "description": "Accepted and waiting for librarian verification",
>+          "schema": {
>+            "type": "object"
>+          }
>+        },
>+        "204": {
>+          "description": "No Content",
>+          "schema": {
>+            "type": "object"
>+          }
>+        },
>+        "400": {
>+          "description": "Bad parameter",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "403": {
>+          "description": "Access forbidden",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "404": {
>+          "description": "Resource not found",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "409": {
>+          "description": "Conflict in updating resource",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        },
>+        "500": {
>+          "description": "Internal server error",
>+          "schema": {
>+            "$ref": "../definitions.json#/error"
>+          }
>+        }
>+      },
>+      "x-koha-authorization": {
>+        "permissions": {
>+          "borrowers": "1"
>+        }
>+      }
>+    },
>     "delete": {
>       "x-mojo-to": "Patrons#delete",
>       "operationId": "deletePatron",
>diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t
>index 4f96a12430..5e66783bae 100644
>--- a/t/db_dependent/api/v1/patrons.t
>+++ b/t/db_dependent/api/v1/patrons.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 5;
>+use Test::More tests => 6;
> use Test::Mojo;
> use Test::Warn;
> 
>@@ -313,6 +313,70 @@ subtest 'update() tests' => sub {
>     };
> };
> 
>+subtest 'patch() tests' => sub {
>+    plan tests => 2;
>+
>+    $schema->storage->txn_begin;
>+    unauthorized_access_tests('PATCH', 123, {email => 'nobody@example.com'});
>+    $schema->storage->txn_rollback;
>+
>+    subtest 'librarian access tests' => sub {
>+        plan tests => 15;
>+
>+        $schema->storage->txn_begin;
>+
>+        t::lib::Mocks::mock_preference('minPasswordLength', 1);
>+        my ( $patron_id_1, $session_id ) = create_user_and_session({ authorized => 1 });
>+        my ( $patron_id_2, undef )       = create_user_and_session({ authorized => 0 });
>+
>+        my $patron_1  = Koha::Patrons->find($patron_id_1);
>+        my $patron_2  = Koha::Patrons->find($patron_id_2);
>+        my $newpatron = {firstname => "myfirstname"};
>+
>+        my $tx = $t->ua->build_tx(PATCH => "/api/v1/patrons/-1" => json => $newpatron );
>+        $tx->req->cookies({name => 'CGISESSID', value => $session_id});
>+        $t->request_ok($tx)
>+          ->status_is(404)
>+          ->json_has('/error', 'Fails when trying to PATCH nonexistent patron');
>+
>+        $newpatron->{falseproperty} = "Non existent property";
>+        $tx = $t->ua->build_tx(PATCH => "/api/v1/patrons/" . $patron_2->id => json => $newpatron );
>+        $tx->req->cookies({name => 'CGISESSID', value => $session_id});
>+        $t->request_ok($tx)
>+          ->status_is(400)
>+          ->json_is('/errors/0/message' =>
>+                    'Properties not allowed: falseproperty.');
>+        delete $newpatron->{falseproperty};
>+
>+        # Set both cardnumber and userid to already existing values
>+        $newpatron->{cardnumber} = $patron_1->cardnumber;
>+        $newpatron->{userid}     = $patron_1->userid;
>+
>+        $tx = $t->ua->build_tx( PATCH => "/api/v1/patrons/" . $patron_2->id => json => $newpatron );
>+        $tx->req->cookies({ name => 'CGISESSID', value => $session_id });
>+        warning_like {
>+            $t->request_ok($tx)
>+              ->status_is(409)
>+              ->json_has( '/error' => "Fails when trying to update to an existing cardnumber or userid")
>+              ->json_is(  '/conflict', 'cardnumber' ); }
>+            qr/^DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key 'cardnumber'/;
>+
>+        delete $newpatron->{cardnumber};
>+        delete $newpatron->{userid};
>+
>+        #Update only firstname field
>+        $tx = $t->ua->build_tx(PATCH => "/api/v1/patrons/" . $patron_2->id => json => $newpatron);
>+        $tx->req->cookies({name => 'CGISESSID', value => $session_id});
>+        $t->request_ok($tx)
>+          ->status_is(200, 'Patron updated successfully')
>+          ->json_has($newpatron);
>+        is(Koha::Patrons->find( $patron_2->id )->firstname,
>+           $newpatron->{ firstname }, 'Patron is really updated!');
>+
>+        $schema->storage->txn_rollback;
>+    };
>+};
>+
> subtest 'delete() tests' => sub {
>     plan tests => 2;
> 
>-- 
>2.17.1
>

Actions: View | Diff | Splinter Review

Attachments on bug 23285: 91426