Skip to content

Commit

Permalink
Merge branch 'hotfix' into feature-docker-refinements
Browse files Browse the repository at this point in the history
  • Loading branch information
Silic0nS0ldier committed Apr 7, 2019
2 parents 1d674fe + e82129f commit 03191b3
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 80 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ before_install:

before_script:
# install deps and UF
- composer install
- COMPOSER_MEMORY_LIMIT=-1 travis_retry composer install --no-interaction
- php bakery debug
- php bakery build-assets
- php bakery migrate
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [v4.2.1]
- Fix Italian translation ([#950])
- User Registration fails when trying to register two accounts with the same email address ([#953])

## [v4.2.0]
### Changed Requirements
- Changed minimum Node.js version to **v10.12.0**
Expand Down Expand Up @@ -704,5 +708,8 @@ See [http://learn.userfrosting.com/upgrading/40-to-41](Upgrading 4.0.x to 4.1.x
[#888]: https://github.com/userfrosting/UserFrosting/issues/888
[#919]: https://github.com/userfrosting/UserFrosting/issues/919
[#940]: https://github.com/userfrosting/UserFrosting/issues/940
[#950]: https://github.com/userfrosting/UserFrosting/issues/950
[#953]: https://github.com/userfrosting/UserFrosting/issues/953

[v4.2.0]: https://github.com/userfrosting/UserFrosting/compare/v4.1.22...v4.2.0
[v4.2.1]: https://github.com/userfrosting/UserFrosting/compare/v4.2.0...v4.2.1
2 changes: 1 addition & 1 deletion app/sprinkles/account/locale/it_IT/messages.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,5 @@
'USER_OR_EMAIL_INVALID' => "L'indirizzo mail o il nome utente non sono validi",
'USER_OR_PASS_INVALID' => 'Il nome utente o la password non sono validi',

'WELCOME' => 'Bentornato, {{display_name}}'
'WELCOME' => 'Bentornato, {{first_name}}'
];
8 changes: 4 additions & 4 deletions app/sprinkles/account/src/Account/Registration.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@ public function validate()

// Check if username is unique
if (!$this->usernameIsUnique($this->userdata['user_name'])) {
$e = new HttpException();
$e->addUserMessage('USERNAME.IN_USE');
$e = new HttpException('Username is already in use.');
$e->addUserMessage('USERNAME.IN_USE', ['user_name' => $this->userdata['user_name']]);
throw $e;
}

// Check if email is unique
if (!$this->emailIsUnique($this->userdata['email'])) {
$e = new HttpException();
$e->addUserMessage('EMAIL.IN_USE');
$e = new HttpException('Email is already in use.');
$e->addUserMessage('EMAIL.IN_USE', ['email' => $this->userdata['email']]);
throw $e;
}

Expand Down
9 changes: 3 additions & 6 deletions app/sprinkles/account/src/Controller/AccountController.php
Original file line number Diff line number Diff line change
Expand Up @@ -913,12 +913,9 @@ public function register(Request $request, Response $response, $args)
// Now that we check the form, we can register the actual user
$registration = new Registration($this->ci, $data);

try {
$user = $registration->register();
} catch (\Exception $e) {
$ms->addMessageTranslated('danger', $e->getMessage(), $data);
$error = true;
}
// Try registration. An HttpException will be thrown if it fails
// No need to catch, as this kind of exception will automatically returns the addMessageTranslated
$user = $registration->register();

// Success message
if ($config['site.registration.require_email_verification']) {
Expand Down
125 changes: 74 additions & 51 deletions app/sprinkles/account/tests/Unit/RegistrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use UserFrosting\Sprinkle\Core\Tests\RefreshDatabase;
use UserFrosting\Sprinkle\Account\Account\Registration;
use UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface;
use UserFrosting\Support\Exception\HttpException;
use UserFrosting\Sprinkle\Account\Database\Models\User;

/**
* RegistrationTest Class
Expand All @@ -26,6 +26,17 @@ class RegistrationTest extends TestCase
use TestDatabase;
use RefreshDatabase;

/**
* @var array Test user data
*/
protected $fakeUserData = [
'user_name' => 'FooBar',
'first_name' => 'Foo',
'last_name' => 'Bar',
'email' => '[email protected]',
'password' => 'FooBarFooBar123'
];

public function tearDown()
{
parent::tearDown();
Expand All @@ -45,7 +56,43 @@ public function setUp()
}

/**
* testNormalRegistration
* Test validation works
*/
public function testValidation()
{
$registration = new Registration($this->ci, [
'user_name' => 'OwlFancy',
'first_name' => 'Owl',
'last_name' => 'Fancy',
'email' => '[email protected]',
'password' => 'owlFancy1234'
]);

$validation = $registration->validate();
$this->assertTrue($validation);
}

/**
* Test the $requiredProperties property
* @depends testValidation
* @expectedException UserFrosting\Support\Exception\HttpException
* @expectedExceptionMessage Account can't be registrated as 'first_name' is required to create a new user.
*/
public function testMissingFields()
{
$registration = new Registration($this->ci, [
'user_name' => 'OwlFancy',
//'first_name' => 'Owl',
'last_name' => 'Fancy',
'email' => '[email protected]',
'password' => 'owlFancy1234'
]);

$validation = $registration->validate();
}

/**
* @depends testValidation
*/
public function testNormalRegistration()
{
Expand All @@ -56,77 +103,53 @@ public function testNormalRegistration()
// Tests can't mail properly
$this->ci->config['site.registration.require_email_verification'] = false;

// Genereate user data
$fakeUserData = [
'user_name' => 'FooBar',
'first_name' => 'Foo',
'last_name' => 'Bar',
'email' => '[email protected]',
'password' => 'FooBarFooBar123'
];

// Get class
$registration = new Registration($this->ci, $fakeUserData);
$registration = new Registration($this->ci, $this->fakeUserData);
$this->assertInstanceOf(Registration::class, $registration);

// Register user
$user = $registration->register();

// Registration should return a valid user
// Registration should return a valid user, with a new ID
$this->assertInstanceOf(UserInterface::class, $user);
$this->assertEquals('FooBar', $user->user_name);
$this->assertInternalType('int', $user->id);

// We try to register the same user again. Should throw an error
$registration = new Registration($this->ci, $fakeUserData);
$this->expectException(HttpException::class);
$validation = $registration->validate();

// Should throw email error if we change the username
$fakeUserData['user_name'] = 'BarFoo';
$registration = new Registration($this->ci, $fakeUserData);
$this->expectException(HttpException::class);
$validation = $registration->validate();
// Make sure the user is added to the db by querying it
$users = User::where('email', '[email protected]')->get();
$this->assertCount(1, $users);
$this->assertSame('FooBar', $users->first()['user_name']);
}

/**
* Test validation works
* @depends testNormalRegistration
* @expectedException UserFrosting\Support\Exception\HttpException
* @expectedExceptionMessage Username is already in use.
*/
public function testValidation()
public function testValidationWithDuplicateUsername()
{
// Reset database
$this->refreshDatabase();

$registration = new Registration($this->ci, [
'user_name' => 'FooBar',
'first_name' => 'Foo',
'last_name' => 'Bar',
'email' => '[email protected]',
'password' => 'FooBarFooBar123'
]);
// Create the first user to test against
$this->testNormalRegistration();

// Validate user. Shouldn't tell us the username is already in use since we reset the database
// We try to register the same user again. Should throw an error
$registration = new Registration($this->ci, $this->fakeUserData);
$validation = $registration->validate();
$this->assertTrue($validation);
}

/**
* Test the $requiredProperties property
* @depends testNormalRegistration
* @expectedException UserFrosting\Support\Exception\HttpException
* @expectedExceptionMessage Email is already in use.
*/
public function testMissingFields()
public function testValidationWithDuplicateEmail()
{
// Reset database
$this->refreshDatabase();
// Create the first user to test against
$this->testNormalRegistration();

$registration = new Registration($this->ci, [
'user_name' => 'FooBar',
//'first_name' => 'Foo',
'last_name' => 'Bar',
'email' => '[email protected]',
'password' => 'FooBarFooBar123'
]);

// Validate user. Shouldn't tell us the username is already in use since we reset the database
$this->expectException(HttpException::class);
// Should throw email error if we change the username
$fakeUserData = $this->fakeUserData;
$fakeUserData['user_name'] = 'BarFoo';
$registration = new Registration($this->ci, $fakeUserData);
$validation = $registration->validate();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,18 @@

namespace UserFrosting\Sprinkle\Core\Tests\Integration;

use UserFrosting\Sprinkle\Core\Tests\TestDatabase;
use UserFrosting\Sprinkle\Core\Database\Migrator\Migrator;
use UserFrosting\Tests\TestCase;

/**
* Tests for the Migrator Service
* Tests for the Migrator Service
*
* @author Louis Charette
* @author Louis Charette
*/
class DatabaseMigratorServiceTest extends TestCase
{
use TestDatabase;

/**
* {@inheritdoc}
*/
public function setUp()
{
parent::setUp();

// Setup test database
$this->setupTestDatabase();
}

public function testMigratorService()
{
$this->assertInstanceOf('UserFrosting\Sprinkle\Core\Database\Migrator\Migrator', $this->ci->migrator);
$this->assertInstanceOf(Migrator::class, $this->ci->migrator);
}
}

0 comments on commit 03191b3

Please sign in to comment.