From 70b42ed39ded4657d17bf41a93aac8642fe71fdc Mon Sep 17 00:00:00 2001 From: AntonyAntonio Date: Tue, 19 Jul 2016 02:35:37 -0300 Subject: [PATCH 1/5] (Guillermo) code review changes --- server/controllers/user/signup.php | 17 ++++++++++++++++- server/models/ERRORS.php | 4 ++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/server/controllers/user/signup.php b/server/controllers/user/signup.php index 0088858a..353c5e94 100644 --- a/server/controllers/user/signup.php +++ b/server/controllers/user/signup.php @@ -1,13 +1,28 @@ 'any', - 'requestData' => [] + 'requestData' => [ + 'name' => [ + 'validation' => DataValidator::length(2, 50), + 'error' => ERRORS::INVALID_NAME + ], + 'email' => [ + 'validation' => DataValidator::contains('@'), + 'error' => ERRORS::INVALID_EMAIL + ], + 'password' => [ + 'validation' => DataValidator::length(5, 20), + 'error' => ERRORS::INVALID_PASSWORD + ] + ] ]; } diff --git a/server/models/ERRORS.php b/server/models/ERRORS.php index a8dbe39c..c6dd0e05 100644 --- a/server/models/ERRORS.php +++ b/server/models/ERRORS.php @@ -3,6 +3,10 @@ class ERRORS { const INVALID_CREDENTIALS = 'User or password is not defined'; const SESSION_EXISTS = 'User is already logged in'; const NO_PERMISSION = 'You have no permission to access'; + const INVALID_NAME = 'Invalid name'; + const INVALID_EMAIL = 'Invalid email'; + const INVALID_PASSWORD = 'Invalid password'; const INVALID_TITLE = 'Invalid title'; const INVALID_CONTENT = 'Invalid content'; + } From 8fc5ed0e57ca998567dd221f4e861b23c24c2601 Mon Sep 17 00:00:00 2001 From: AntonyAntonio Date: Tue, 19 Jul 2016 02:38:43 -0300 Subject: [PATCH 2/5] (Guillermo) code review changes --- server/controllers/user/signup.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/controllers/user/signup.php b/server/controllers/user/signup.php index 353c5e94..f9b28273 100644 --- a/server/controllers/user/signup.php +++ b/server/controllers/user/signup.php @@ -11,7 +11,7 @@ class SignUpController extends Controller { 'permission' => 'any', 'requestData' => [ 'name' => [ - 'validation' => DataValidator::length(2, 50), + 'validation' => DataValidator::length(2, 55), 'error' => ERRORS::INVALID_NAME ], 'email' => [ From 5fd68fdc6ea255eec12a3d52818020f9ce985671 Mon Sep 17 00:00:00 2001 From: AntonyAntonio Date: Tue, 19 Jul 2016 16:41:04 -0300 Subject: [PATCH 3/5] (Guillermo) code review changes --- server/controllers/user/signup.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/controllers/user/signup.php b/server/controllers/user/signup.php index f9b28273..db5c9630 100644 --- a/server/controllers/user/signup.php +++ b/server/controllers/user/signup.php @@ -11,15 +11,15 @@ class SignUpController extends Controller { 'permission' => 'any', 'requestData' => [ 'name' => [ - 'validation' => DataValidator::length(2, 55), + 'validation' => DataValidator::length(2, 55)->alpha(), 'error' => ERRORS::INVALID_NAME ], 'email' => [ - 'validation' => DataValidator::contains('@'), + 'validation' => DataValidator::email(), 'error' => ERRORS::INVALID_EMAIL ], 'password' => [ - 'validation' => DataValidator::length(5, 20), + 'validation' => DataValidator::length(5, 200), 'error' => ERRORS::INVALID_PASSWORD ] ] From 76ecd4ebc33fdb06c5770cb70bdfe1ea926f9087 Mon Sep 17 00:00:00 2001 From: AntonyAntonio Date: Tue, 19 Jul 2016 17:39:26 -0300 Subject: [PATCH 4/5] (Guillermo) add tests for signup validation --- tests/scripts.rb | 3 +- tests/user/signup.rb | 75 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/tests/scripts.rb b/tests/scripts.rb index 52956341..f248029b 100644 --- a/tests/scripts.rb +++ b/tests/scripts.rb @@ -1,6 +1,7 @@ class Scripts - def self.createUser(email = 'steve@jobs.com', password = 'custompassword') + def self.createUser(email = 'steve@jobs.com', password = 'custompassword', name = 'steve jobs') response = request('/user/signup', { + 'name' => name, 'email' => email, 'password' => password }) diff --git a/tests/user/signup.rb b/tests/user/signup.rb index c22af4c5..a5d48391 100644 --- a/tests/user/signup.rb +++ b/tests/user/signup.rb @@ -1,6 +1,7 @@ describe '/user/signup' do it 'should create user in database' do response = request('/user/signup', { + 'name' => 'Steve Jobs', 'email' => 'steve@jobs.com', 'password' => 'custom' }) @@ -9,4 +10,78 @@ describe '/user/signup' do (userRow['email']).should.equal('steve@jobs.com') end + + it 'should fail if name is invalid' do + long_text = '' + 100.times {long_text << 'a'} + + result = request('/user/signup', { + name: 't', + email: 'tyrion@outlook.com', + password: 'Lannister' + }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid name') + + result = request('/user/signup', { + name: long_text, + email: 'tyrion@outlook.com', + password: 'Lannister' + }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid name') + + result = request('/user/signup', { + name: 'tyri0n', + email: 'tyrion@outlook.com', + password: 'Lannister' + }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid name') + end + + it 'should fail if email is invalid' do + result = request('/user/signup', { + name: 'tyrion', + email: 'tyrionoutlook.com', + password: 'Lannister' + }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid email') + + result = request('/user/signup', { + name: 'tyrion', + email: 'tyrion@outlookcom', + password: 'Lannister' + }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid email') + + end + + it 'should fail if password is invalid' do + result = request('/user/signup', { + name: 'tyrion', + email: 'tyrion@outlook.com', + password: 'Lann' + }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid password') + + long_text = '' + 250.times {long_text << 'a'} + + result = request('/user/signup', { + name: 'tyrion', + email: 'tyrion@outlook.com', + password: long_text + }) + end + end From 7c1ee7d6fc621a4d04d4a4e40be9c57204881200 Mon Sep 17 00:00:00 2001 From: AntonyAntonio Date: Thu, 21 Jul 2016 15:27:57 -0300 Subject: [PATCH 5/5] guillermo - code review changes [skip ci] --- server/models/ERRORS.php | 1 - tests/user/signup.rb | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/server/models/ERRORS.php b/server/models/ERRORS.php index c6dd0e05..442f9b54 100644 --- a/server/models/ERRORS.php +++ b/server/models/ERRORS.php @@ -8,5 +8,4 @@ class ERRORS { const INVALID_PASSWORD = 'Invalid password'; const INVALID_TITLE = 'Invalid title'; const INVALID_CONTENT = 'Invalid content'; - } diff --git a/tests/user/signup.rb b/tests/user/signup.rb index a5d48391..8344e998 100644 --- a/tests/user/signup.rb +++ b/tests/user/signup.rb @@ -61,7 +61,6 @@ describe '/user/signup' do (result['status']).should.equal('fail') (result['message']).should.equal('Invalid email') - end it 'should fail if password is invalid' do @@ -82,6 +81,9 @@ describe '/user/signup' do email: 'tyrion@outlook.com', password: long_text }) + + (result['status']).should.equal('fail') + (result['message']).should.equal('Invalid password') end end