From ca13a2e18acbae630790ab2b7b63f75202d678a8 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Mon, 8 Apr 2024 15:23:17 +0200
Subject: [PATCH] Bug 36505: Add support for `extended_attributes` in PUT
 /patrons/:patron_id

This patch does what the title says. With it, you will be able to PUT on
the already existing endpoint, but also pass:

```json
[ { "type": "THE_TYPE", "value": "a" }, ... ]
```

Bonus: to ease testing I added `x-koha-embed: extended_attributes`
support.

To test:
1. Apply the unit tests
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: This is not implemented!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Exhaustive testes pass!
5. Sign off :-D

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/REST/V1/Patrons.pm           | 89 +++++++++++++++++++------------
 api/v1/swagger/paths/patrons.yaml | 18 ++++++-
 2 files changed, 73 insertions(+), 34 deletions(-)

diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm
index 16bfca58c3d..81aca249fdc 100644
--- a/Koha/REST/V1/Patrons.pm
+++ b/Koha/REST/V1/Patrons.pm
@@ -273,14 +273,15 @@ sub update {
             and (  exists $body->{email}
                 or exists $body->{secondary_email}
                 or exists $body->{altaddress_email} )
-          )
+            )
         {
-            foreach my $email_field ( qw(email secondary_email altaddress_email) ) {
+            foreach my $email_field (qw(email secondary_email altaddress_email)) {
                 my $exists_email = exists $body->{$email_field};
                 next unless $exists_email;
 
                 # exists, verify if we are asked to change it
-                my $put_email      = $body->{$email_field};
+                my $put_email = $body->{$email_field};
+
                 # As of writing this patch, 'email' is the only unmapped field
                 # (i.e. it preserves its name, hence this fallback)
                 my $db_email_field = $patron->to_api_mapping->{$email_field} // 'email';
@@ -289,28 +290,38 @@ sub update {
                 return $c->render(
                     status  => 403,
                     openapi => { error => "Not enough privileges to change a superlibrarian's email" }
-                  )
-                  unless ( !defined $put_email and !defined $db_email )
-                  or (  defined $put_email
+                    )
+                    unless ( !defined $put_email and !defined $db_email )
+                    or (defined $put_email
                     and defined $db_email
                     and $put_email eq $db_email );
             }
         }
 
-        $patron->set_from_api($body)->store;
-        $patron->discard_changes;
-        return $c->render(
-            status  => 200,
-            openapi => $c->objects->to_api($patron),
+        $patron->_result->result_source->schema->txn_do(
+            sub {
+                # Remove `extended_attributes` before storing
+                my $extended_attributes = delete $body->{extended_attributes};
+
+                if ($extended_attributes) {
+                    my $rs = $patron->extended_attributes(
+                        [ map { { code => $_->{type}, attribute => $_->{value} } } @{$extended_attributes} ] );
+                }
+
+                $patron->set_from_api($body)->store;
+                $patron->discard_changes;
+
+                return $c->render(
+                    status  => 200,
+                    openapi => $c->objects->to_api($patron),
+                );
+            }
         );
-    }
-    catch {
+    } catch {
         unless ( blessed $_ && $_->can('rethrow') ) {
             return $c->render(
                 status  => 500,
-                openapi => {
-                    error => "Something went wrong, check Koha logs for details."
-                }
+                openapi => { error => "Something went wrong, check Koha logs for details." }
             );
         }
         if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) {
@@ -318,22 +329,17 @@ sub update {
                 status  => 409,
                 openapi => { error => $_->error, conflict => $_->duplicate_id }
             );
-        }
-        elsif ( $_->isa('Koha::Exceptions::Patron::InvalidUserid') ) {
+        } elsif ( $_->isa('Koha::Exceptions::Patron::InvalidUserid') ) {
             return $c->render(
                 status  => 400,
-                openapi => { error => "Problem with ". $_->userid }
+                openapi => { error => "Problem with " . $_->userid }
             );
-        }
-        elsif ( $_->isa('Koha::Exceptions::Object::FKConstraint') ) {
+        } elsif ( $_->isa('Koha::Exceptions::Object::FKConstraint') ) {
             return $c->render(
                 status  => 400,
-                openapi => { error => "Given " .
-                            $patron->to_api_mapping->{$_->broken_fk}
-                            . " does not exist" }
+                openapi => { error => "Given " . $patron->to_api_mapping->{ $_->broken_fk } . " does not exist" }
             );
-        }
-        elsif ( $_->isa('Koha::Exceptions::MissingParameter') ) {
+        } elsif ( $_->isa('Koha::Exceptions::MissingParameter') ) {
             return $c->render(
                 status  => 400,
                 openapi => {
@@ -341,8 +347,7 @@ sub update {
                     parameters => $_->parameter
                 }
             );
-        }
-        elsif ( $_->isa('Koha::Exceptions::BadParameter') ) {
+        } elsif ( $_->isa('Koha::Exceptions::BadParameter') ) {
             return $c->render(
                 status  => 400,
                 openapi => {
@@ -350,16 +355,34 @@ sub update {
                     parameters => $_->parameter
                 }
             );
-        }
-        elsif ( $_->isa('Koha::Exceptions::NoChanges') ) {
+        } elsif ( $_->isa('Koha::Exceptions::NoChanges') ) {
             return $c->render(
                 status  => 204,
                 openapi => { error => "No changes have been made" }
             );
+        } elsif ( $_->isa('Koha::Exceptions::Patron::Attribute::InvalidType') ) {
+            return $c->render(
+                status  => 400,
+                openapi => { error => "$_", error_code => 'invalid_attribute_type' }
+            );
+        } elsif ( $_->isa('Koha::Exceptions::Patron::Attribute::UniqueIDConstraint') ) {
+            return $c->render(
+                status  => 400,
+                openapi => { error => "$_", error_code => 'attribute_not_unique' }
+            );
+        } elsif ( $_->isa('Koha::Exceptions::Patron::Attribute::NonRepeatable') ) {
+            return $c->render(
+                status  => 400,
+                openapi => { error => "$_", error_code => 'non_repeatable_attribute' }
+            );
+        } elsif ( $_->isa('Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute') ) {
+            return $c->render(
+                status  => 400,
+                openapi => { error => "$_", error_code => 'missing_mandatory_attribute' }
+            );
         }
-        else {
-            $c->unhandled_exception($_);
-        }
+
+        $c->unhandled_exception($_);
     };
 }
 
diff --git a/api/v1/swagger/paths/patrons.yaml b/api/v1/swagger/paths/patrons.yaml
index 9c3e5ccaa86..68649d10bd5 100644
--- a/api/v1/swagger/paths/patrons.yaml
+++ b/api/v1/swagger/paths/patrons.yaml
@@ -535,6 +535,16 @@
         required: true
         schema:
           $ref: "../swagger.yaml#/definitions/patron"
+      - name: x-koha-embed
+        in: header
+        required: false
+        description: Embed list sent as a request header
+        type: array
+        items:
+          type: string
+          enum:
+            - extended_attributes
+        collectionFormat: csv
     consumes:
       - application/json
     produces:
@@ -546,7 +556,13 @@
           items:
             $ref: "../swagger.yaml#/definitions/patron"
       "400":
-        description: Bad parameter
+        description: |
+          Bad parameter. Possible `error_code` attribute values:
+
+            * `invalid_attribute_type`
+            * `attribute_not_unique`
+            * `non_repeatable_attribute`
+            * `missing_mandatory_attribute`
         schema:
           $ref: "../swagger.yaml#/definitions/error"
       "403":
-- 
2.44.0