From bc97867dfc9a9553be700403746ec09864234366 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: Jonathan Druart --- 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 a2fc2a6c8e..04e7a31f64 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.39.5