Skip to content

Commit e82129f

Browse files
lcharettelcharette
authored
Merge pull request #955 from userfrosting/hotfix-issue#953
Fix #953
2 parents 2eb889d + fb6da3a commit e82129f

4 files changed

Lines changed: 83 additions & 61 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
77

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

1112
## [v4.2.0]
1213
### Changed Requirements
@@ -708,6 +709,7 @@ See [http://learn.userfrosting.com/upgrading/40-to-41](Upgrading 4.0.x to 4.1.x
708709
[#919]: https://github.com/userfrosting/UserFrosting/issues/919
709710
[#940]: https://github.com/userfrosting/UserFrosting/issues/940
710711
[#950]: https://github.com/userfrosting/UserFrosting/issues/950
712+
[#953]: https://github.com/userfrosting/UserFrosting/issues/953
711713

712714
[v4.2.0]: https://github.com/userfrosting/UserFrosting/compare/v4.1.22...v4.2.0
713715
[v4.2.1]: https://github.com/userfrosting/UserFrosting/compare/v4.2.0...v4.2.1

‎app/sprinkles/account/src/Account/Registration.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,15 @@ public function validate()
161161

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

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

‎app/sprinkles/account/src/Controller/AccountController.php‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -913,12 +913,9 @@ public function register(Request $request, Response $response, $args)
913913
// Now that we check the form, we can register the actual user
914914
$registration = new Registration($this->ci, $data);
915915

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

923920
// Success message
924921
if ($config['site.registration.require_email_verification']) {

‎app/sprinkles/account/tests/Unit/RegistrationTest.php‎

Lines changed: 74 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use UserFrosting\Sprinkle\Core\Tests\RefreshDatabase;
1616
use UserFrosting\Sprinkle\Account\Account\Registration;
1717
use UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface;
18-
use UserFrosting\Support\Exception\HttpException;
18+
use UserFrosting\Sprinkle\Account\Database\Models\User;
1919

2020
/**
2121
* RegistrationTest Class
@@ -26,6 +26,17 @@ class RegistrationTest extends TestCase
2626
use TestDatabase;
2727
use RefreshDatabase;
2828

29+
/**
30+
* @var array Test user data
31+
*/
32+
protected $fakeUserData = [
33+
'user_name' => 'FooBar',
34+
'first_name' => 'Foo',
35+
'last_name' => 'Bar',
36+
'email' => 'Foo@Bar.com',
37+
'password' => 'FooBarFooBar123'
38+
];
39+
2940
public function tearDown()
3041
{
3142
parent::tearDown();
@@ -45,7 +56,43 @@ public function setUp()
4556
}
4657

4758
/**
48-
* testNormalRegistration
59+
* Test validation works
60+
*/
61+
public function testValidation()
62+
{
63+
$registration = new Registration($this->ci, [
64+
'user_name' => 'OwlFancy',
65+
'first_name' => 'Owl',
66+
'last_name' => 'Fancy',
67+
'email' => 'owl@fancy.com',
68+
'password' => 'owlFancy1234'
69+
]);
70+
71+
$validation = $registration->validate();
72+
$this->assertTrue($validation);
73+
}
74+
75+
/**
76+
* Test the $requiredProperties property
77+
* @depends testValidation
78+
* @expectedException UserFrosting\Support\Exception\HttpException
79+
* @expectedExceptionMessage Account can't be registrated as 'first_name' is required to create a new user.
80+
*/
81+
public function testMissingFields()
82+
{
83+
$registration = new Registration($this->ci, [
84+
'user_name' => 'OwlFancy',
85+
//'first_name' => 'Owl',
86+
'last_name' => 'Fancy',
87+
'email' => 'owl@fancy.com',
88+
'password' => 'owlFancy1234'
89+
]);
90+
91+
$validation = $registration->validate();
92+
}
93+
94+
/**
95+
* @depends testValidation
4996
*/
5097
public function testNormalRegistration()
5198
{
@@ -56,77 +103,53 @@ public function testNormalRegistration()
56103
// Tests can't mail properly
57104
$this->ci->config['site.registration.require_email_verification'] = false;
58105

59-
// Genereate user data
60-
$fakeUserData = [
61-
'user_name' => 'FooBar',
62-
'first_name' => 'Foo',
63-
'last_name' => 'Bar',
64-
'email' => 'Foo@Bar.com',
65-
'password' => 'FooBarFooBar123'
66-
];
67-
68106
// Get class
69-
$registration = new Registration($this->ci, $fakeUserData);
107+
$registration = new Registration($this->ci, $this->fakeUserData);
70108
$this->assertInstanceOf(Registration::class, $registration);
71109

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

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

79-
// We try to register the same user again. Should throw an error
80-
$registration = new Registration($this->ci, $fakeUserData);
81-
$this->expectException(HttpException::class);
82-
$validation = $registration->validate();
83-
84-
// Should throw email error if we change the username
85-
$fakeUserData['user_name'] = 'BarFoo';
86-
$registration = new Registration($this->ci, $fakeUserData);
87-
$this->expectException(HttpException::class);
88-
$validation = $registration->validate();
118+
// Make sure the user is added to the db by querying it
119+
$users = User::where('email', 'Foo@Bar.com')->get();
120+
$this->assertCount(1, $users);
121+
$this->assertSame('FooBar', $users->first()['user_name']);
89122
}
90123

91124
/**
92-
* Test validation works
125+
* @depends testNormalRegistration
126+
* @expectedException UserFrosting\Support\Exception\HttpException
127+
* @expectedExceptionMessage Username is already in use.
93128
*/
94-
public function testValidation()
129+
public function testValidationWithDuplicateUsername()
95130
{
96-
// Reset database
97-
$this->refreshDatabase();
98-
99-
$registration = new Registration($this->ci, [
100-
'user_name' => 'FooBar',
101-
'first_name' => 'Foo',
102-
'last_name' => 'Bar',
103-
'email' => 'Foo@Bar.com',
104-
'password' => 'FooBarFooBar123'
105-
]);
131+
// Create the first user to test against
132+
$this->testNormalRegistration();
106133

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

112139
/**
113-
* Test the $requiredProperties property
140+
* @depends testNormalRegistration
141+
* @expectedException UserFrosting\Support\Exception\HttpException
142+
* @expectedExceptionMessage Email is already in use.
114143
*/
115-
public function testMissingFields()
144+
public function testValidationWithDuplicateEmail()
116145
{
117-
// Reset database
118-
$this->refreshDatabase();
146+
// Create the first user to test against
147+
$this->testNormalRegistration();
119148

120-
$registration = new Registration($this->ci, [
121-
'user_name' => 'FooBar',
122-
//'first_name' => 'Foo',
123-
'last_name' => 'Bar',
124-
'email' => 'Foo@Bar.com',
125-
'password' => 'FooBarFooBar123'
126-
]);
127-
128-
// Validate user. Shouldn't tell us the username is already in use since we reset the database
129-
$this->expectException(HttpException::class);
149+
// Should throw email error if we change the username
150+
$fakeUserData = $this->fakeUserData;
151+
$fakeUserData['user_name'] = 'BarFoo';
152+
$registration = new Registration($this->ci, $fakeUserData);
130153
$validation = $registration->validate();
131154
}
132155
}

0 commit comments

Comments
 (0)