From 2210f627a528c11e31eeee06ba52c713d2f3fca6 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Sun, 26 Nov 2017 15:58:56 -0800 Subject: [PATCH] signup: Switch active mirror-dummy users to an AssertionError. Previously, this was a ValidationError, but that doesn't really make sense, since this condition reflects an actual bug in the code. Because this happened to be our only test coverage the ValidationError catch on line 84 of registration.py, we add nocoverage there for now. --- zerver/lib/actions.py | 2 +- zerver/tests/test_signup.py | 29 ++++++++++++++--------------- zerver/views/registration.py | 2 +- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index bba06597e5..612c6f4622 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -3914,7 +3914,7 @@ def validate_email_for_realm(target_realm, email): if existing_user_profile is not None and existing_user_profile.is_mirror_dummy: # Mirror dummy users to be activated must be inactive if existing_user_profile.is_active: - raise ValidationError('%s already has an account' % (email,)) + raise AssertionError("Mirror dummy user is already active!") elif existing_user_profile: # Other users should not already exist at all. raise ValidationError('%s already has an account' % (email,)) diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index a3ba35bc50..211fadf1e9 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -2114,18 +2114,19 @@ class UserSignUpTest(ZulipTestCase): result = self.client_get(confirmation_url, subdomain="zephyr") self.assertEqual(result.status_code, 200) - # If the mirror dummy user is already active, attempting to submit the - # registration form should just redirect to a login page. + # If the mirror dummy user is already active, attempting to + # submit the registration form should raise an AssertionError + # (this is an invalid state, so it's a bug we got here): user_profile.is_active = True user_profile.save() - result = self.submit_reg_form_for_user(email, - password, - from_confirmation='1', - # Pass HTTP_HOST for the target subdomain - HTTP_HOST=subdomain + ".testserver") + with self.assertRaisesRegex(AssertionError, "Mirror dummy user is already active!"): + result = self.submit_reg_form_for_user( + email, + password, + from_confirmation='1', + # Pass HTTP_HOST for the target subdomain + HTTP_HOST=subdomain + ".testserver") - self.assertEqual(result.status_code, 302) - self.assertIn('login', result['Location']) user_profile.is_active = False user_profile.save() @@ -2144,8 +2145,8 @@ class UserSignUpTest(ZulipTestCase): def test_registration_of_active_mirror_dummy_user(self: Any) -> None: """ - Trying to activate an already-active mirror dummy user should just - redirect to a login page. + Trying to activate an already-active mirror dummy user should + raise an AssertionError. """ user_profile = self.mit_user("sipbtest") email = user_profile.email @@ -2153,10 +2154,8 @@ class UserSignUpTest(ZulipTestCase): user_profile.is_active = True user_profile.save() - result = self.client_post('/register/', {'email': email}) - - self.assertEqual(result.status_code, 302) - self.assertIn('login', result['Location']) + with self.assertRaisesRegex(AssertionError, "Mirror dummy user is already active!"): + self.client_post('/register/', {'email': email}, subdomain="zephyr") class DeactivateUserTest(ZulipTestCase): diff --git a/zerver/views/registration.py b/zerver/views/registration.py index d9ca875f07..f4274e356b 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -80,7 +80,7 @@ def accounts_register(request): try: validate_email_for_realm(realm, email) - except ValidationError: + except ValidationError: # nocoverage # We need to add a test for this. return HttpResponseRedirect(reverse('django.contrib.auth.views.login') + '?email=' + urllib.parse.quote_plus(email))