Bug 23666: Add PATCH /patron/:patron_id/extended_attributes/:extended_attribute_id

This patch adds the described route. It is designed to use the
underlying libraries' methods to update an existing attribute.

The tests cover the use cases.

Note: I added handling for two exceptions that can only occur on bad
data (i.e. not by using our codebase). This are:
- Koha::Exceptions::Patron::Attribute::InvalidType
- Koha::Exceptions::Patron::Attribute::NonRepeatable

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons_extended_attributes.t
=> SUCCESS: Tests pass!
3. PLay with the route
=> SUCCESS: Expected behavior!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

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

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

Koha/REST/V1/Patrons/Attributes.pm

@@ -186,7 +186,6 @@ sub overwrite {
                     status  => 400,
                     openapi => { error => "$_" }
                 );
-
             }
             elsif (
                 $_->isa(
@@ -222,9 +221,84 @@ sub overwrite {
     };
 }
 
+
+=head3 update
+
+Controller method that handles updating a single extended patron attribute.
+
+=cut
+
+sub update {
+    my $c = shift->openapi->valid_input or return;
+
+    my $patron = Koha::Patrons->find( $c->validation->param('patron_id') );
+
+    unless ($patron) {
+        return $c->render(
+            status  => 404,
+            openapi => {
+                error => 'Patron not found'
+            }
+        );
+    }
+
+    return try {
+        my $attribute = $patron->extended_attributes->find(
+            $c->validation->param('extended_attribute_id') );
+
+        unless ($attribute) {
+            return $c->render(
+                status  => 404,
+                openapi => {
+                    error => 'Attribute not found'
+                }
+            );
+        }
+
+        $attribute->set_from_api( $c->validation->param('body') )->store;
+        $attribute->discard_changes;
+
+        return $c->render(
+            status  => 200,
+            openapi => $attribute->to_api
+        );
+    }
+    catch {
+        if ( blessed $_ ) {
+            if ( $_->isa('Koha::Exceptions::Patron::Attribute::InvalidType') ) {
+                return $c->render(
+                    status  => 400,
+                    openapi => { error => "$_" }
+                );
+            }
+            elsif (
+                $_->isa(
+                    'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint')
+              )
+            {
+                return $c->render(
+                    status  => 409,
+                    openapi => { error => "$_" }
+                );
+            }
+            elsif (
+                $_->isa('Koha::Exceptions::Patron::Attribute::NonRepeatable') )
+            {
+                return $c->render(
+                    status  => 409,
+                    openapi => { error => "$_" }
+                );
+            }
+        }
+
+        $c->unhandled_exception($_);
+    };
+
+}
+
 =head3 delete
 
-Controller method that handling removing an extended patron attribute
+Controller method that handles removing an extended patron attribute.
 
 =cut
 

api/v1/swagger/paths/patrons_extended_attributes.json

@@ -251,6 +251,99 @@
     }
   },
   "/patrons/{patron_id}/extended_attributes/{extended_attribute_id}": {
+    "patch": {
+      "x-mojo-to": "Patrons::Attributes#update",
+      "operationId": "updatePatronAttribute",
+      "tags": [
+        "patrons",
+        "extended_attributes"
+      ],
+      "parameters": [
+        {
+          "$ref": "../parameters.json#/patron_id_pp"
+        },
+        {
+          "name": "extended_attribute_id",
+          "in": "path",
+          "description": "Internal patron extended attribute identifier",
+          "type": "integer",
+          "required": true
+        },
+        {
+          "name": "body",
+          "in": "body",
+          "description": "An object containing the updated values for the patron extended attribute",
+          "required": true,
+          "schema": {
+            "type": "object",
+            "properties": {
+              "value": {
+                "description": "Extended attribute value",
+                "type": "string"
+              }
+            }
+          }
+        }
+      ],
+      "produces": [
+        "application/json"
+      ],
+      "responses": {
+        "200": {
+          "description": "A successfully updated patron extended attribute",
+          "schema": {
+            "$ref": "../definitions.json#/patron_extended_attribute"
+          }
+        },
+        "400": {
+          "description": "Bad parameter",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "401": {
+          "description": "Authentication required",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "403": {
+          "description": "Access forbidden",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "404": {
+          "description": "Object 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"
+          }
+        },
+        "503": {
+          "description": "Under maintenance",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      },
+      "x-koha-authorization": {
+        "permissions": {
+          "borrowers": "edit_borrowers"
+        }
+      }
+    },
     "delete": {
       "x-mojo-to": "Patrons::Attributes#delete",
       "operationId": "deletePatronAttribute",

t/db_dependent/api/v1/patrons_extended_attributes.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 4;
+use Test::More tests => 5;
 use Test::Mojo;
 
 use t::lib::TestBuilder;
@@ -394,3 +394,117 @@ subtest 'delete() tests' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'update() tests' => sub {
+
+    plan tests => 12;
+
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { flags => 2**4 }    # 'borrowers' flag == 4
+        }
+    );
+    my $password = 'thePassword123';
+    $patron->set_password( { password => $password, skip_validation => 1 } );
+    my $userid = $patron->userid;
+
+    my $repeatable_attr_type = $builder->build_object(
+        {
+            class => 'Koha::Patron::Attribute::Types',
+            value => {
+                mandatory     => 0,
+                repeatable    => 1,
+                unique_id     => 0,
+                category_code => undef
+            }
+        }
+    );
+    my $unique_attr_type = $builder->build_object(
+        {
+            class => 'Koha::Patron::Attribute::Types',
+            value => {
+                mandatory     => 0,
+                repeatable    => 0,
+                unique_id     => 1,
+                category_code => undef
+            }
+        }
+    );
+
+    # Add a unique attribute to our patron
+    my $unique_attribute = $patron->add_extended_attribute(
+        {
+            code      => $unique_attr_type->code,
+            attribute => 'WOW'
+        }
+    );
+
+    # Let's have an attribute ID we are sure doesn't exist on the DB
+    my $non_existent_attribute = $patron->add_extended_attribute(
+        {
+            code      => $repeatable_attr_type->code,
+            attribute => 'BOO'
+        }
+    );
+    my $non_existent_attribute_id = $non_existent_attribute->id;
+    $non_existent_attribute->delete;
+
+    my $non_existent_patron =
+      $builder->build_object( { class => 'Koha::Patrons' } );
+    my $non_existent_patron_id = $non_existent_patron->id;
+
+    # get rid of the patron
+    $non_existent_patron->delete;
+
+    $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
+          . $non_existent_patron_id
+          . '/extended_attributes/'
+          . 123 => json => { value => 'something' } )->status_is(404)
+      ->json_is( '/error' => 'Patron not found' );
+
+    $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
+          . $patron->id
+          . '/extended_attributes/'
+          . $non_existent_attribute_id => json => { value => 'something' } )
+      ->status_is(404)->json_is( '/error' => 'Attribute not found' );
+
+    my $response =
+      $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
+          . $patron->id
+          . '/extended_attributes/'
+          . $unique_attribute->id => json => { value => 'HEY' } )
+      ->status_is(200)->tx->res->json;
+
+    is_deeply(
+        Koha::Patron::Attributes->find( $response->{extended_attribute_id} )
+          ->to_api,
+        $response,
+        "The returned object is on the DB"
+    );
+
+    my $unique_value = 'HEHE';
+
+    # Add a patron with the unique attribute to test changing to it
+    $builder->build_object( { class => 'Koha::Patrons' } )
+      ->add_extended_attribute(
+        {
+            code      => $unique_attr_type->code,
+            attribute => $unique_value
+        }
+      );
+
+    $t->patch_ok( "//$userid:$password@/api/v1/patrons/"
+          . $patron->id
+          . '/extended_attributes/'
+          . $unique_attribute->id => json => { value => $unique_value } )
+      ->status_is(409)
+      ->json_is( '/error' =>
+            "Your action breaks a unique constraint on the attribute. type="
+          . $unique_attr_type->code
+          . " value=$unique_value" );
+
+    $schema->storage->txn_rollback;
+};