Skip to content

Commit 992ea18

Browse files
krulis-martinkrulis-martin
committed
Adding proper UI error code for email-already-used situation.
1 parent b4557cd commit 992ea18

File tree

3 files changed

+160
-61
lines changed

3 files changed

+160
-61
lines changed

‎app/V1Module/presenters/RegistrationPresenter.php‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,11 @@ public function actionCreateAccount()
169169
// check if the email is free
170170
$email = trim($req->getPost("email"));
171171
// username is name of column which holds login identifier represented by email
172-
if ($this->logins->getByUsername($email) !== null) {
173-
throw new BadRequestException("This email address is already taken.");
172+
if ($this->logins->getByUsername($email) !== null || $this->users->getByEmail($email) !== null) {
173+
throw new BadRequestException(
174+
"This email address is already taken.",
175+
FrontendErrorMappings::E400_110__USER_EMAIL_ALREADY_EXISTS
176+
);
174177
}
175178

176179
$instanceId = $req->getPost("instanceId");
@@ -272,15 +275,18 @@ public function actionCreateInvitation()
272275
// check if the email is free
273276
$email = trim($format->email);
274277
// username is name of column which holds login identifier represented by email
275-
if ($this->users->getByEmail($email) !== null) {
276-
throw new BadRequestException("This email address is already taken.");
278+
if ($this->logins->getByUsername($email) !== null || $this->users->getByEmail($email) !== null) {
279+
throw new BadRequestException(
280+
"This email address is already taken.",
281+
FrontendErrorMappings::E400_110__USER_EMAIL_ALREADY_EXISTS
282+
);
277283
}
278284

279285
$groupsIds = $format->groups ?? [];
280286
foreach ($groupsIds as $id) {
281287
$group = $this->groups->get($id);
282288
if (!$group || $group->isOrganizational() || !$this->groupAcl->canInviteStudents($group)) {
283-
throw new BadRequestException("Current user cannot invite people in group '$id'");
289+
throw new ForbiddenRequestException("Current user cannot invite people in group '$id'");
284290
}
285291
}
286292

‎app/exceptions/FrontendErrorMappings.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ class FrontendErrorMappings
3434
public const E400_104__EXTERNAL_AUTH_FAILED_USER_NOT_FOUND = "400-104";
3535
/** External authentication failed - unable to register new user because no role was provided. */
3636
public const E400_105__EXTERNAL_AUTH_FAILED_MISSING_ROLE = "400-105";
37+
/** User email already exists, new user cannot be created/invited/registered/.... */
38+
public const E400_110__USER_EMAIL_ALREADY_EXISTS = "400-110";
3739

3840
/** General job config error */
3941
public const E400_200__JOB_CONFIG = "400-200";

‎tests/Presenters/RegistrationPresenter.phpt‎

Lines changed: 147 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ class TestRegistrationPresenter extends Tester\TestCase
7979
$this->presenter = PresenterTestHelper::createPresenter($this->container, RegistrationPresenter::class);
8080
$this->presenter->registrationConfig = new RegistrationConfig(
8181
[
82-
'enabled' => true,
83-
'implicitGroupsIds' => []
82+
'enabled' => true,
83+
'implicitGroupsIds' => []
8484
]
8585
);
8686

@@ -118,14 +118,14 @@ class TestRegistrationPresenter extends Tester\TestCase
118118
'POST',
119119
['action' => 'createAccount'],
120120
[
121-
'email' => $email,
122-
'firstName' => $firstName,
123-
'lastName' => $lastName,
124-
'password' => $password,
125-
'passwordConfirm' => $password,
126-
'instanceId' => $instanceId,
127-
'titlesBeforeName' => $titlesBeforeName,
128-
'titlesAfterName' => $titlesAfterName
121+
'email' => $email,
122+
'firstName' => $firstName,
123+
'lastName' => $lastName,
124+
'password' => $password,
125+
'passwordConfirm' => $password,
126+
'instanceId' => $instanceId,
127+
'titlesBeforeName' => $titlesBeforeName,
128+
'titlesAfterName' => $titlesAfterName
129129
]
130130
);
131131
$response = $this->presenter->run($request);
@@ -170,20 +170,20 @@ class TestRegistrationPresenter extends Tester\TestCase
170170
'POST',
171171
['action' => 'createAccount'],
172172
[
173-
'email' => $email,
174-
'firstName' => $firstName,
175-
'lastName' => $lastName,
176-
'password' => $password,
177-
'passwordConfirm' => $password,
178-
'instanceId' => $instanceId,
179-
'titlesBeforeName' => $titlesBeforeName,
180-
'titlesAfterName' => $titlesAfterName
173+
'email' => $email,
174+
'firstName' => $firstName,
175+
'lastName' => $lastName,
176+
'password' => $password,
177+
'passwordConfirm' => $password,
178+
'instanceId' => $instanceId,
179+
'titlesBeforeName' => $titlesBeforeName,
180+
'titlesAfterName' => $titlesAfterName
181181
]
182182
);
183183
$this->presenter->registrationConfig = new RegistrationConfig(
184184
[
185-
'enabled' => true,
186-
'implicitGroupsIds' => [$groupId]
185+
'enabled' => true,
186+
'implicitGroupsIds' => [$groupId]
187187
]
188188
);
189189
$response = $this->presenter->run($request);
@@ -226,19 +226,19 @@ class TestRegistrationPresenter extends Tester\TestCase
226226
'POST',
227227
['action' => 'createAccount'],
228228
[
229-
'email' => $email,
230-
'firstName' => $firstName,
231-
'lastName' => $lastName,
232-
'password' => $password,
233-
'passwordConfirm' => $password,
234-
'instanceId' => $instanceId,
235-
'titlesBeforeName' => $titlesBeforeName,
236-
'titlesAfterName' => $titlesAfterName
229+
'email' => $email,
230+
'firstName' => $firstName,
231+
'lastName' => $lastName,
232+
'password' => $password,
233+
'passwordConfirm' => $password,
234+
'instanceId' => $instanceId,
235+
'titlesBeforeName' => $titlesBeforeName,
236+
'titlesAfterName' => $titlesAfterName
237237
]
238238
);
239239
$this->presenter->registrationConfig = new RegistrationConfig(
240240
[
241-
'enabled' => false,
241+
'enabled' => false,
242242
]
243243
);
244244

@@ -265,14 +265,14 @@ class TestRegistrationPresenter extends Tester\TestCase
265265
'POST',
266266
['action' => 'createAccount'],
267267
[
268-
'email' => $email,
269-
'firstName' => $firstName,
270-
'lastName' => $lastName,
271-
'password' => $password,
272-
'passwordConfirm' => $password,
273-
'instanceId' => $instanceId,
274-
'titlesBeforeName' => $titlesBeforeName,
275-
'titlesAfterName' => $titlesAfterName
268+
'email' => $email,
269+
'firstName' => $firstName,
270+
'lastName' => $lastName,
271+
'password' => $password,
272+
'passwordConfirm' => $password,
273+
'instanceId' => $instanceId,
274+
'titlesBeforeName' => $titlesBeforeName,
275+
'titlesAfterName' => $titlesAfterName
276276
]
277277
);
278278

@@ -302,14 +302,14 @@ class TestRegistrationPresenter extends Tester\TestCase
302302
'POST',
303303
['action' => 'createAccount'],
304304
[
305-
'email' => $email,
306-
'firstName' => $firstName,
307-
'lastName' => $lastName,
308-
'password' => $password,
309-
'passwordConfirm' => $passwordConfirm,
310-
'instanceId' => $instanceId,
311-
'titlesBeforeName' => $titlesBeforeName,
312-
'titlesAfterName' => $titlesAfterName
305+
'email' => $email,
306+
'firstName' => $firstName,
307+
'lastName' => $lastName,
308+
'password' => $password,
309+
'passwordConfirm' => $passwordConfirm,
310+
'instanceId' => $instanceId,
311+
'titlesBeforeName' => $titlesBeforeName,
312+
'titlesAfterName' => $titlesAfterName
313313
]
314314
);
315315

@@ -336,14 +336,14 @@ class TestRegistrationPresenter extends Tester\TestCase
336336
'POST',
337337
['action' => 'createAccount'],
338338
[
339-
'email' => $email,
340-
'firstName' => $firstName,
341-
'lastName' => $lastName,
342-
'password' => $password,
343-
'passwordConfirm' => $password,
344-
'instanceId' => $instanceId,
345-
'titlesBeforeName' => $titlesBeforeName,
346-
'titlesAfterName' => $titlesAfterName
339+
'email' => $email,
340+
'firstName' => $firstName,
341+
'lastName' => $lastName,
342+
'password' => $password,
343+
'passwordConfirm' => $password,
344+
'instanceId' => $instanceId,
345+
'titlesBeforeName' => $titlesBeforeName,
346+
'titlesAfterName' => $titlesAfterName
347347
]
348348
);
349349

@@ -363,8 +363,8 @@ class TestRegistrationPresenter extends Tester\TestCase
363363
'POST',
364364
['action' => 'validateRegistrationData'],
365365
[
366-
'email' => "totallyFreeEmail@EmailFreeTotally.freeEmailTotally",
367-
'password' => "totallySecurePasswordWhichIsNot123456"
366+
'email' => "totallyFreeEmail@EmailFreeTotally.freeEmailTotally",
367+
'password' => "totallySecurePasswordWhichIsNot123456"
368368
]
369369
);
370370
$response = $this->presenter->run($request);
@@ -511,8 +511,99 @@ class TestRegistrationPresenter extends Tester\TestCase
511511
]
512512
);
513513
},
514-
BadRequestException::class
514+
ForbiddenRequestException::class
515+
);
516+
}
517+
518+
public function testCreateInvitationWithSameName()
519+
{
520+
$student = $this->presenter->users->getByEmail(PresenterTestHelper::STUDENT_GROUP_MEMBER_LOGIN);
521+
Assert::notNull($student);
522+
523+
$email = "newguy@recodex.com";
524+
$firstName = $student->getFirstName();
525+
$lastName = $student->getLastName();
526+
$instances = $this->instances->findAll();
527+
$instanceId = array_pop($instances)->getId();
528+
$titlesBeforeName = "titlesBeforeName";
529+
$titlesAfterName = "titlesAfterName";
530+
531+
$groups = [];
532+
foreach ($this->presenter->groups->findAll() as $group) {
533+
if (!$group->isArchived() && !$group->isOrganizational()) {
534+
$groups[] = $group->getId();
535+
}
536+
}
537+
Assert::truthy($groups);
538+
539+
PresenterTestHelper::loginDefaultAdmin($this->container);
540+
$payload = PresenterTestHelper::performPresenterRequest(
541+
$this->presenter,
542+
$this->presenterPath,
543+
'POST',
544+
['action' => 'createInvitation'],
545+
[
546+
'email' => $email,
547+
'firstName' => $firstName,
548+
'lastName' => $lastName,
549+
'titlesBeforeName' => $titlesBeforeName,
550+
'titlesAfterName' => $titlesAfterName,
551+
'instanceId' => $instanceId,
552+
'groups' => $groups,
553+
'locale' => 'en',
554+
]
555+
);
556+
557+
Assert::count(1, $payload);
558+
$user = current($payload);
559+
Assert::equal($student->getId(), $user["id"]);
560+
}
561+
562+
public function testCreateInvitationWithSameNameEnabled()
563+
{
564+
$student = $this->presenter->users->getByEmail(PresenterTestHelper::STUDENT_GROUP_MEMBER_LOGIN);
565+
Assert::notNull($student);
566+
567+
$email = "newguy@recodex.com";
568+
$firstName = $student->getFirstName();
569+
$lastName = $student->getLastName();
570+
$instances = $this->instances->findAll();
571+
$instanceId = array_pop($instances)->getId();
572+
$titlesBeforeName = "titlesBeforeName";
573+
$titlesAfterName = "titlesAfterName";
574+
575+
$groups = [];
576+
foreach ($this->presenter->groups->findAll() as $group) {
577+
if (!$group->isArchived() && !$group->isOrganizational()) {
578+
$groups[] = $group->getId();
579+
}
580+
}
581+
Assert::truthy($groups);
582+
583+
$this->emailHelperMock->shouldReceive("send")
584+
->with("noreply@recodex", [$email], "en", 'User Admin Admin has invited you in ReCodEx!', Mockery::any())
585+
->once()->andReturn(true);
586+
587+
PresenterTestHelper::loginDefaultAdmin($this->container);
588+
$payload = PresenterTestHelper::performPresenterRequest(
589+
$this->presenter,
590+
$this->presenterPath,
591+
'POST',
592+
['action' => 'createInvitation'],
593+
[
594+
'email' => $email,
595+
'firstName' => $firstName,
596+
'lastName' => $lastName,
597+
'titlesBeforeName' => $titlesBeforeName,
598+
'titlesAfterName' => $titlesAfterName,
599+
'instanceId' => $instanceId,
600+
'groups' => $groups,
601+
'locale' => 'en',
602+
'ignoreNameCollision' => true,
603+
]
515604
);
605+
606+
Assert::equal("OK", $payload);
516607
}
517608

518609
public function testAcceptInvitation()

0 commit comments

Comments
 (0)