refactor: Extract email_for_user_settings.

We want to be able to unit test this value,
since it's conditional on several factors:

    - am I an admin?
    - can non-admins view emails?
    - do we have delivery_email for the user?

I'm mocking show_email in the tests, since the
show_email code is in `settings_org` and
kind of hard to unit test.  It's not impossible,
but it's too much for this commit.  (Either
we need to extract it out to a nice file or
deal with mocking jQuery.  That module is
mostly data-oriented, so it would be nice
to have something like `settings_config` that
is actually pure data.)
This commit is contained in:
Steve Howell 2019-12-28 16:41:20 +00:00 committed by Tim Abbott
parent 3a95be2f2f
commit 3e4326afda
4 changed files with 44 additions and 9 deletions

View File

@ -5,6 +5,7 @@ set_global('blueslip', global.make_zblueslip({
error: false, // We check for errors in people_errors.js error: false, // We check for errors in people_errors.js
})); }));
set_global('page_params', {}); set_global('page_params', {});
set_global('settings_org', {});
set_global('md5', function (s) { set_global('md5', function (s) {
return 'md5-' + s; return 'md5-' + s;
}); });
@ -22,6 +23,7 @@ const me = {
const isaac = { const isaac = {
email: 'isaac@example.com', email: 'isaac@example.com',
delivery_email: 'isaac-delivery@example.com',
user_id: 32, user_id: 32,
full_name: 'Isaac Newton', full_name: 'Isaac Newton',
}; };
@ -884,6 +886,31 @@ run_test('matches_user_settings_search', () => {
assert.equal(match({full_name: 'Joe Frederick'}, 're'), true); assert.equal(match({full_name: 'Joe Frederick'}, 're'), true);
}); });
run_test('email_for_user_settings', () => {
const email = people.email_for_user_settings;
settings_org.show_email = () => {
return false;
};
assert.equal(email(isaac), undefined);
settings_org.show_email = () => {
return true;
};
page_params.is_admin = true;
assert.equal(email(isaac), isaac.delivery_email);
// Fall back to email if delivery_email is not there.
assert.equal(
email({email: 'foo@example.com'}),
'foo@example.com');
page_params.is_admin = false;
assert.equal(email(isaac), isaac.email);
});
run_test('emails_strings_to_user_ids_array', function () { run_test('emails_strings_to_user_ids_array', function () {
const steven = { const steven = {
email: 'steven@example.com', email: 'steven@example.com',

View File

@ -995,6 +995,18 @@ exports.matches_user_settings_search = function (person, value) {
); );
}; };
exports.email_for_user_settings = function (person) {
if (!settings_org.show_email()) {
return;
}
if (page_params.is_admin && person.delivery_email) {
return person.delivery_email;
}
return person.email;
};
exports.maybe_incr_recipient_count = function (message) { exports.maybe_incr_recipient_count = function (message) {
if (message.type !== 'private') { if (message.type !== 'private') {
return; return;

View File

@ -167,7 +167,7 @@ function populate_users(realm_people_data) {
return render_admin_user_list({ return render_admin_user_list({
can_modify: page_params.is_admin, can_modify: page_params.is_admin,
// It's always safe to show the fake email addresses for bot users // It's always safe to show the fake email addresses for bot users
show_email: true, display_email: item.email,
user: item, user: item,
}); });
}, },
@ -212,7 +212,7 @@ function populate_users(realm_people_data) {
const $row = $(render_admin_user_list({ const $row = $(render_admin_user_list({
can_modify: page_params.is_admin, can_modify: page_params.is_admin,
is_current_user: people.is_my_user_id(item.user_id), is_current_user: people.is_my_user_id(item.user_id),
show_email: settings_org.show_email(), display_email: people.email_for_user_settings(item),
user: item, user: item,
})); }));
$row.find(".last_active").append(get_rendered_last_activity(item)); $row.find(".last_active").append(get_rendered_last_activity(item));
@ -248,7 +248,7 @@ function populate_users(realm_people_data) {
modifier: function (item) { modifier: function (item) {
return render_admin_user_list({ return render_admin_user_list({
user: item, user: item,
show_email: settings_org.show_email(), display_email: people.email_for_user_settings(item),
can_modify: page_params.is_admin, can_modify: page_params.is_admin,
}); });
}, },

View File

@ -4,13 +4,9 @@
<span class="user_name">{{full_name}}</span> <span class="user_name">{{full_name}}</span>
<i class="fa fa-ban deactivated-user-icon" title="{{t 'User is deactivated' }}" {{#if is_active}}style="display: none;"{{/if}}></i> <i class="fa fa-ban deactivated-user-icon" title="{{t 'User is deactivated' }}" {{#if is_active}}style="display: none;"{{/if}}></i>
</td> </td>
{{#if ../show_email}} {{#if ../display_email}}
<td> <td>
{{#if ../can_modify}} <span class="email">{{../display_email}}</span>
<span class="email">{{delivery_email}}</span>
{{else}}
<span class="email">{{email}}</span>
{{/if}}
</td> </td>
{{/if}} {{/if}}
{{#unless is_bot}} {{#unless is_bot}}