From cd8ecbfed0fefedf91372d0f708e5b59620ca4b7 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 Signed-off-by: Marcel de Rooy Signed-off-by: Aleisha Amohia Signed-off-by: Aleisha Amohia --- 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 59eebcc7d4..9da9db12c3 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -197,24 +197,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.39.5