From 711ca8801a3c0a21429ebc354462b725e5ee85a8 Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sun, 21 Jul 2024 23:30:06 +0400 Subject: [PATCH 1/5] feat: address review comments --- app/controllers/api/projects.php | 20 ++++++++++ src/Appwrite/Auth/Validator/MockNumber.php | 4 +- .../Projects/ProjectsConsoleClientTest.php | 38 ++++++++++++++++++- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index af588fffc4..1569b4685e 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -882,6 +882,26 @@ App::patch('/v1/projects/:projectId/auth/mock-numbers') throw new Exception(Exception::PROJECT_NOT_FOUND); } + /* Ensure there are no duplicate numbers . Numbers have a structure. Throw an exception if there are duplicate numbers + [ + { + "number": "+1234567890", + "code": "123456" + }, + { + "number": "+1234567891", + "code": "123456" + } + ] + */ + $uniqueNumbers = []; + foreach ($numbers as $number) { + if (isset($uniqueNumbers[$number['number']])) { + throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Duplicate numbers are not allowed.'); + } + $uniqueNumbers[$number['number']] = $number['code']; + } + $auths = $project->getAttribute('auths', []); $auths['mockNumbers'] = $numbers; diff --git a/src/Appwrite/Auth/Validator/MockNumber.php b/src/Appwrite/Auth/Validator/MockNumber.php index 2c1c81f863..6f1cf81cae 100644 --- a/src/Appwrite/Auth/Validator/MockNumber.php +++ b/src/Appwrite/Auth/Validator/MockNumber.php @@ -47,8 +47,8 @@ class MockNumber extends Validator } $otp = new Text(6, 6); - if (!$otp->isValid($value['otp'])) { - $this->message = 'OTP must be a valid string and exactly 6 characters.'; + if (!$otp->isValid($value['otp']) || !\ctype_digit($value['otp'])) { + $this->message = 'Invalid OTP. Please make sure the OTP is a 6 digit number'; return false; } diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 753322f168..7be97e2136 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -1592,7 +1592,7 @@ class ProjectsConsoleClientTest extends Scope ] ]); $this->assertEquals(400, $response['headers']['status-code']); - $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and OTP must be a valid string and exactly 6 characters.', $response['body']['message']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Invalid OTP. Please make sure the OTP is a 6 digit number', $response['body']['message']); /** Trying to pass an OTP shorter than 6 characters*/ $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ @@ -1607,7 +1607,22 @@ class ProjectsConsoleClientTest extends Scope ] ]); $this->assertEquals(400, $response['headers']['status-code']); - $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and OTP must be a valid string and exactly 6 characters.', $response['body']['message']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Invalid OTP. Please make sure the OTP is a 6 digit number', $response['body']['message']); + + /** Trying to pass an OTP with non numeric characters */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'numbers' => [ + [ + 'phone' => '+1655513432', + 'otp' => '123re2' + ] + ] + ]); + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Invalid OTP. Please make sure the OTP is a 6 digit number', $response['body']['message']); /** Trying to pass an invalid phone number */ $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ @@ -1639,6 +1654,25 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Phone number must start with a \'+\' can have a maximum of fifteen digits.', $response['body']['message']); + /** Trying to pass duplicate numbers */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'numbers' => [ + [ + 'phone' => '+1655513432', + 'otp' => '123456' + ], + [ + 'phone' => '+1655513432', + 'otp' => '123456' + ] + ] + ]); + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Phone number must start with a \'+\' can have a maximum of fifteen digits.', $response['body']['message']); + $numbers = []; for ($i = 0; $i < 11; $i++) { $numbers[] = [ From 98b8e13add3c6e42fafd8685106738058621398d Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sun, 21 Jul 2024 23:45:39 +0400 Subject: [PATCH 2/5] feat: address review comments --- app/controllers/api/projects.php | 18 +++--------------- .../Projects/ProjectsConsoleClientTest.php | 2 +- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 1569b4685e..e934a2799c 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -882,24 +882,12 @@ App::patch('/v1/projects/:projectId/auth/mock-numbers') throw new Exception(Exception::PROJECT_NOT_FOUND); } - /* Ensure there are no duplicate numbers . Numbers have a structure. Throw an exception if there are duplicate numbers - [ - { - "number": "+1234567890", - "code": "123456" - }, - { - "number": "+1234567891", - "code": "123456" - } - ] - */ $uniqueNumbers = []; foreach ($numbers as $number) { - if (isset($uniqueNumbers[$number['number']])) { - throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Duplicate numbers are not allowed.'); + if (isset($uniqueNumbers[$number['phone']])) { + throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Duplicate phone numbers are not allowed.'); } - $uniqueNumbers[$number['number']] = $number['code']; + $uniqueNumbers[$number['phone']] = $number['otp']; } $auths = $project->getAttribute('auths', []); diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 7be97e2136..dd0c8420d5 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -1671,7 +1671,7 @@ class ProjectsConsoleClientTest extends Scope ] ]); $this->assertEquals(400, $response['headers']['status-code']); - $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Phone number must start with a \'+\' can have a maximum of fifteen digits.', $response['body']['message']); + $this->assertEquals('Duplicate phone numbers are not allowed.', $response['body']['message']); $numbers = []; for ($i = 0; $i < 11; $i++) { From 7614a356a2fa7628812841d0c79f827596c39fbc Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Mon, 22 Jul 2024 12:55:15 +0400 Subject: [PATCH 3/5] feat: address review comments --- app/controllers/api/projects.php | 12 ++++++------ src/Appwrite/Auth/Validator/MockNumber.php | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index e934a2799c..f9b2753eb6 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -876,12 +876,6 @@ App::patch('/v1/projects/:projectId/auth/mock-numbers') ->inject('dbForConsole') ->action(function (string $projectId, array $numbers, Response $response, Database $dbForConsole) { - $project = $dbForConsole->getDocument('projects', $projectId); - - if ($project->isEmpty()) { - throw new Exception(Exception::PROJECT_NOT_FOUND); - } - $uniqueNumbers = []; foreach ($numbers as $number) { if (isset($uniqueNumbers[$number['phone']])) { @@ -889,6 +883,12 @@ App::patch('/v1/projects/:projectId/auth/mock-numbers') } $uniqueNumbers[$number['phone']] = $number['otp']; } + + $project = $dbForConsole->getDocument('projects', $projectId); + + if ($project->isEmpty()) { + throw new Exception(Exception::PROJECT_NOT_FOUND); + } $auths = $project->getAttribute('auths', []); diff --git a/src/Appwrite/Auth/Validator/MockNumber.php b/src/Appwrite/Auth/Validator/MockNumber.php index 6f1cf81cae..ac5ba89fc5 100644 --- a/src/Appwrite/Auth/Validator/MockNumber.php +++ b/src/Appwrite/Auth/Validator/MockNumber.php @@ -46,8 +46,8 @@ class MockNumber extends Validator return false; } - $otp = new Text(6, 6); - if (!$otp->isValid($value['otp']) || !\ctype_digit($value['otp'])) { + $otp = new Text(6, 6, Text::NUMBERS); + if (!$otp->isValid($value['otp'])) { $this->message = 'Invalid OTP. Please make sure the OTP is a 6 digit number'; return false; } From eb4e259be5d0c277457b4a51a5e0441cdc83751c Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Mon, 22 Jul 2024 13:08:02 +0400 Subject: [PATCH 4/5] chore: linter --- app/controllers/api/projects.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index f9b2753eb6..cdb1575d76 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -883,7 +883,7 @@ App::patch('/v1/projects/:projectId/auth/mock-numbers') } $uniqueNumbers[$number['phone']] = $number['otp']; } - + $project = $dbForConsole->getDocument('projects', $projectId); if ($project->isEmpty()) { From 5eacde84ae29ad5a10e37f8859cff64626fef032 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Wed, 7 Aug 2024 00:01:54 +0530 Subject: [PATCH 5/5] Add value to variables template --- .../Response/Model/TemplateFunction.php | 3 +- .../Utopia/Response/Model/TemplateRuntime.php | 3 +- .../Response/Model/TemplateVariable.php | 67 ++++++++++--------- 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/Appwrite/Utopia/Response/Model/TemplateFunction.php b/src/Appwrite/Utopia/Response/Model/TemplateFunction.php index 7a3a4cfbd5..f5df10986f 100644 --- a/src/Appwrite/Utopia/Response/Model/TemplateFunction.php +++ b/src/Appwrite/Utopia/Response/Model/TemplateFunction.php @@ -110,8 +110,7 @@ class TemplateFunction extends Model 'default' => [], 'example' => [], 'array' => true - ]) - ; + ]); } /** diff --git a/src/Appwrite/Utopia/Response/Model/TemplateRuntime.php b/src/Appwrite/Utopia/Response/Model/TemplateRuntime.php index c08ea9b32a..c98a59789d 100644 --- a/src/Appwrite/Utopia/Response/Model/TemplateRuntime.php +++ b/src/Appwrite/Utopia/Response/Model/TemplateRuntime.php @@ -33,8 +33,7 @@ class TemplateRuntime extends Model 'description' => 'Path to function in VCS (Version Control System) repository', 'default' => '', 'example' => 'node/starter', - ]) - ; + ]); } /** diff --git a/src/Appwrite/Utopia/Response/Model/TemplateVariable.php b/src/Appwrite/Utopia/Response/Model/TemplateVariable.php index b0fd919dbf..c992083a87 100644 --- a/src/Appwrite/Utopia/Response/Model/TemplateVariable.php +++ b/src/Appwrite/Utopia/Response/Model/TemplateVariable.php @@ -10,37 +10,42 @@ class TemplateVariable extends Model public function __construct() { $this - ->addRule('name', [ - 'type' => self::TYPE_STRING, - 'description' => 'Variable Name.', - 'default' => '', - 'example' => 'APPWRITE_DATABASE_ID', - ]) - ->addRule('description', [ - 'type' => self::TYPE_STRING, - 'description' => 'Variable Description.', - 'default' => '', - 'example' => 'The ID of the Appwrite database that contains the collection to sync.', - ]) - ->addRule('placeholder', [ - 'type' => self::TYPE_STRING, - 'description' => 'Variable Placeholder.', - 'default' => '', - 'example' => '64a55...7b912', - ]) - ->addRule('required', [ - 'type' => self::TYPE_BOOLEAN, - 'description' => 'Is the variable required?', - 'default' => false, - 'example' => false, - ]) - ->addRule('type', [ - 'type' => self::TYPE_STRING, - 'description' => 'Variable Type.', - 'default' => '', - 'example' => 'password', - ]) - ; + ->addRule('name', [ + 'type' => self::TYPE_STRING, + 'description' => 'Variable Name.', + 'default' => '', + 'example' => 'APPWRITE_DATABASE_ID', + ]) + ->addRule('description', [ + 'type' => self::TYPE_STRING, + 'description' => 'Variable Description.', + 'default' => '', + 'example' => 'The ID of the Appwrite database that contains the collection to sync.', + ]) + ->addRule('value', [ + 'type' => self::TYPE_STRING, + 'description' => 'Variable Value.', + 'default' => '', + 'example' => '512', + ]) + ->addRule('placeholder', [ + 'type' => self::TYPE_STRING, + 'description' => 'Variable Placeholder.', + 'default' => '', + 'example' => '64a55...7b912', + ]) + ->addRule('required', [ + 'type' => self::TYPE_BOOLEAN, + 'description' => 'Is the variable required?', + 'default' => false, + 'example' => false, + ]) + ->addRule('type', [ + 'type' => self::TYPE_STRING, + 'description' => 'Variable Type.', + 'default' => '', + 'example' => 'password', + ]); } /**