Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

invitations: Improve experience around reactivating users. 

Previously, if you tried to invite a user whose account had been
deactivated, we didn't provide a clear path forward for reactivating
the users, which was confusing.

We fix this by plumbing through to the frontend the information that
there is an existing user account with that email address in this
organization, but that it's deactivated.  For administrators, we
provide a link for how to reactivate the user.

Fixes #8144.
This commit is contained in:
Tlazypanda 2019-11-14 15:51:08 +05:30 committed by Tim Abbott
parent 0dc7542363
commit 30ee0c2a49
6 changed files with 105 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
frontend_tests/node_tests/templates.js
View File

@ -1737,3 +1737,65 @@ run_test('recipient_row', () => {
assert(html.indexOf('<script>alert("Hello")</script>') === -1);
assert(html.indexOf('&lt;script&gt;alert(&quot;Hello&quot;)&lt;/script&gt;') !== -1);
});
run_test('invitation_failed_error', () => {
let err_list = [];
err_list.push("hamlet@zulip.com: Account has been deactivated.");
let data = {
error_message: "We weren't able to invite anyone.",
error_list: err_list,
is_invitee_deactivated: true,
is_admin: true,
};
let html = '<div>';
html += render('invitation_failed_error', data);
html += '</div>';
let p = $(html).find('p#invitation_error_message');
assert.equal(p.text().trim(), "We weren't able to invite anyone.");
let li = $(html).find("li").first();
assert.equal(li.text().trim(), "hamlet@zulip.com: Account has been deactivated.");
let p_admin = $(html).find("p#invitation_admin_message");
assert.equal(p_admin.text().trim(), 'translated: You can reactivate deactivated users from organization settings.');
err_list = [];
err_list.push("hamlet@zulip.com: Account has been deactivated.");
data = {
error_message: "We weren't able to invite anyone.",
error_list: err_list,
is_invitee_deactivated: true,
is_admin: false,
};
html = '<div>';
html += render('invitation_failed_error', data);
html += '</div>';
p = $(html).find("p#invitation_error_message");
assert.equal(p.text().trim(), "We weren't able to invite anyone.");
li = $(html).find("li").first();
assert.equal(li.text().trim(), "hamlet@zulip.com: Account has been deactivated.");
const msg = $(html).find("p#invitation_non_admin_message");
assert.equal(msg.text().trim(), 'translated: Organization administrators can reactivate deactivated users.');
p_admin = $(html).find("p#invitation_admin_message");
assert.equal(p_admin.length, 0);
err_list = [];
err_list.push("othello@zulip.com: Already has an account.");
data = {
error_message: "We weren't able to invite anyone.",
error_list: err_list,
is_invitee_deactivated: false,
is_admin: false,
};
html = '<div>';
html += render('invitation_failed_error', data);
html += '</div>';
p = $(html).find("p#invitation_error_message");
assert.equal(p.text().trim(), "We weren't able to invite anyone.");
li = $(html).find("li").first();
assert.equal(li.text().trim(), "othello@zulip.com: Already has an account.");
p_admin = $(html).find("p#invitation_admin_message");
assert.equal(p_admin.length, 0);
});

9
static/js/invite.js
View File

@ -72,14 +72,21 @@ function submit_invitation_form() {
// Some users were not invited.
const invitee_emails_errored = [];
const error_list = [];
let is_invitee_deactivated = false;
arr.errors.forEach(function (value) {
error_list.push(value.join(': '));
const [email, error_message, deactivated] = value;
error_list.push(`${email}: ${error_message}`);
if (deactivated) {
is_invitee_deactivated = true;
}
invitee_emails_errored.push(value[0]);
});
const error_response = render_invitation_failed_error({
error_message: arr.msg,
error_list: error_list,
is_admin: page_params.is_admin,
is_invitee_deactivated: is_invitee_deactivated,
});
ui_report.message(error_response, invite_status, "alert-warning");
invitee_emails_group.addClass('warning');

9
static/templates/invitation_failed_error.hbs
View File

@ -1,6 +1,13 @@
<p>{{ error_message }}</p>
<p id="invitation_error_message">{{ error_message }}</p>
<ul>
{{#each error_list}}
<li>{{this}}</li>
{{/each}}
</ul>
{{#if is_invitee_deactivated}}
{{#if is_admin}}
<p id="invitation_admin_message">{{#tr this}}You can reactivate deactivated users from <a href="#organization/deactivated-users-admin">organization settings.</a>{{/tr}}</p>
{{else}}
<p id="invitation_non_admin_message">{{t "Organization administrators can reactivate deactivated users." }}</p>
{{/if}}
{{/if}}

34
zerver/lib/actions.py
View File

@ -5034,40 +5034,42 @@ def validate_email_for_realm(target_realm: Realm, email: str) -> None:
raise AssertionError("Mirror dummy user is already active!")
# Other users should not already exist at all.
raise ValidationError(_('%s already has an account') %
(email,), code = _("Already has an account."))
(email,), code = _("Already has an account."), params={'deactivated': False})
elif not existing_user_profile.is_mirror_dummy:
raise ValidationError('The account for %s has been deactivated' % (email,),
code = _("Account has been deactivated."))
code = _("Account has been deactivated."), params={'deactivated': True})
def validate_email(user_profile: UserProfile, email: str) -> Tuple[Optional[str], Optional[str]]:
def validate_email(user_profile: UserProfile, email: str) -> Tuple[Optional[str], Optional[str],
bool]:
try:
validators.validate_email(email)
except ValidationError:
return _("Invalid address."), None
return _("Invalid address."), None, False
try:
email_allowed_for_realm(email, user_profile.realm)
except DomainNotAllowedForRealmError:
return _("Outside your domain."), None
return _("Outside your domain."), None, False
except DisposableEmailError:
return _("Please use your real email address."), None
return _("Please use your real email address."), None, False
except EmailContainsPlusError:
return _("Email addresses containing + are not allowed."), None
return _("Email addresses containing + are not allowed."), None, False
try:
validate_email_for_realm(user_profile.realm, email)
except ValidationError as error:
return None, (error.code)
return None, (error.code), (error.params['deactivated'])
return None, None
return None, None, False
class InvitationError(JsonableError):
code = ErrorCode.INVITATION_FAILED
data_fields = ['errors', 'sent_invitations']
def __init__(self, msg: str, errors: List[Tuple[str, str]], sent_invitations: bool) -> None:
def __init__(self, msg: str, errors: List[Tuple[str, str, bool]],
sent_invitations: bool) -> None:
self._msg = msg # type: str
self.errors = errors # type: List[Tuple[str, str]]
self.errors = errors # type: List[Tuple[str, str, bool]]
self.sent_invitations = sent_invitations # type: bool
def estimate_recent_invites(realms: Iterable[Realm], *, days: int) -> int:
@ -5135,18 +5137,18 @@ def do_invite_users(user_profile: UserProfile,
[], sent_invitations=False)
validated_emails = [] # type: List[str]
errors = [] # type: List[Tuple[str, str]]
skipped = [] # type: List[Tuple[str, str]]
errors = [] # type: List[Tuple[str, str, bool]]
skipped = [] # type: List[Tuple[str, str, bool]]
for email in invitee_emails:
if email == '':
continue
email_error, email_skipped = validate_email(user_profile, email)
email_error, email_skipped, deactivated = validate_email(user_profile, email)
if not (email_error or email_skipped):
validated_emails.append(email)
elif email_error:
errors.append((email, email_error))
errors.append((email, email_error, deactivated))
elif email_skipped:
skipped.append((email, email_skipped))
skipped.append((email, email_skipped, deactivated))
if errors:
raise InvitationError(

12
zerver/tests/test_auth_backends.py
View File

@ -3428,19 +3428,23 @@ class EmailValidatorTestCase(ZulipTestCase):
inviter = self.example_user('hamlet')
cordelia = self.example_user('cordelia')
error, _ = validate_email(inviter, 'fred+5555@zulip.com')
error, _, is_deactivated = validate_email(inviter, 'fred+5555@zulip.com')
self.assertEqual(False, is_deactivated)
self.assertIn('containing + are not allowed', error)
_, error = validate_email(inviter, cordelia.email)
_, error, is_deactivated = validate_email(inviter, cordelia.email)
self.assertEqual(False, is_deactivated)
self.assertEqual(error, 'Already has an account.')
cordelia.is_active = False
cordelia.save()
_, error = validate_email(inviter, cordelia.email)
_, error, is_deactivated = validate_email(inviter, cordelia.email)
self.assertEqual(True, is_deactivated)
self.assertEqual(error, 'Account has been deactivated.')
_, error = validate_email(inviter, 'fred-is-fine@zulip.com')
_, error, is_deactivated = validate_email(inviter, 'fred-is-fine@zulip.com')
self.assertEqual(False, is_deactivated)
self.assertEqual(error, None)
class LDAPBackendTest(ZulipTestCase):

2
zerver/views/user_settings.py
View File

@ -94,7 +94,7 @@ def json_change_settings(request: HttpRequest, user_profile: UserProfile,
if user_profile.delivery_email != new_email and new_email != '':
if user_profile.realm.email_changes_disabled and not user_profile.is_realm_admin:
return json_error(_("Email address changes are disabled in this organization."))
error, skipped = validate_email(user_profile, new_email)
error, skipped, deactivated = validate_email(user_profile, new_email)
if error:
return json_error(error)
if skipped: