From fe871f6483450fb528045324c88186df0ddd6b2e Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 13 Jul 2020 12:25:13 -0300 Subject: [PATCH] Bug 23634: (QA follow-up) Our PUT is really a PATCH This patch makes the controller not expect that there will always be all the email fields. So it now checks if an email field was passed, and changed, and renders the error if that stands. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Tests written by Nick highlight a problem 2. Apply this patch 3. Repeat 1 => SUCCESS: Problems solved 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Patrons.pm | 46 +++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index a2fc2a6c8e2..04e7a31f640 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -208,24 +208,34 @@ sub update { my $body = $c->validation->param('body'); my $user = $c->stash('koha.user'); - if ( $patron->is_superlibrarian and !$user->is_superlibrarian ) { - my $put_email = $body->{email} // qw{}; - my $db_email = $patron->email // qw{}; - my $put_email_pro = $body->{secondary_email} // qw{}; - my $db_email_pro = $patron->emailpro // qw{}; - my $put_email_B = $body->{altaddress_email} // qw{}; - my $db_email_B = $patron->B_email // qw{}; - - return $c->render( - status => 403, - openapi => { - error => - "Not enough privileges to change a superlibrarian's email" - } - ) - if ($put_email ne $db_email) - || ($put_email_pro ne $db_email_pro) - || ($put_email_B ne $db_email_B); + if ( + $patron->is_superlibrarian + and !$user->is_superlibrarian + and ( exists $body->{email} + or exists $body->{secondary_email} + or exists $body->{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}; + # 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'; + my $db_email = $patron->$db_email_field; + + 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 + and defined $db_email + and $put_email eq $db_email ); + } } $patron->set_from_api($c->validation->param('body'))->store; -- 2.27.0