refactor: Extract settings_data.py.

This extracts a new module with three
functions, which we will test with 100%
line coverage:

    - show_email
    - email_for_user_settings
    - get_time_preferences

The first two break several dependencies
in the codebase on `settings_org.js`.  The
`get_time_preferences` breaks an annoying
dependency on `page_params` within people.

The module is pretty cohesive, in terms that
all three functions are just light wrappers
around `page_params` and/or `settings_config`.

Now all the modules that want to call show_email()
only have to require `settings_data`, instead of
having a dependency on the much heavier
`settings_org.js` module.

I also make some of the unit tests here be more
full-stack, where instead of stubbing show_email,
I basically just toggle `page_params.is_admin`.
This commit is contained in:
Steve Howell
2020-02-25 11:46:14 +00:00
committed by Tim Abbott
parent b994889315
commit 979dcfe85b
14 changed files with 140 additions and 90 deletions

View File

@@ -1,13 +1,23 @@
zrequire('people');
set_global('blueslip', global.make_zblueslip());
set_global('message_store', {});
set_global('page_params', {});
set_global('settings_org', {});
set_global('settings_data', {});
set_global('md5', function (s) {
return 'md5-' + s;
});
const settings_config = zrequire('settings_config');
const visibility = settings_config.email_address_visibility_values;
const admins_only = visibility.admins_only.code;
const everyone = visibility.everyone.code;
function set_email_visibility(code) {
page_params.realm_email_address_visibility = code;
}
set_email_visibility(admins_only);
const me = {
email: 'me@example.com',
user_id: 30,
@@ -929,9 +939,7 @@ run_test('filter_for_user_settings_search', () => {
so that is where we do more thorough testing.
This test is just a sanity check for now.
*/
settings_org.show_email = () => {
return false;
};
page_params.is_admin = false;
const fred_smith = {full_name: 'Fred Smith'};
const alice_lee = {full_name: 'Alice Lee'};
@@ -951,23 +959,19 @@ run_test('filter_for_user_settings_search', () => {
run_test('matches_user_settings_search', () => {
const match = people.matches_user_settings_search;
settings_org.show_email = () => {
return false;
};
page_params.is_admin = false;
assert.equal(match({email: 'fred@example.com'}, 'fred'), false);
assert.equal(match({full_name: 'Fred Smith'}, 'fr'), true);
settings_org.show_email = () => {
return true;
};
page_params.is_admin = true;
assert.equal(match({delivery_email: 'fred@example.com'}, 'fr'), true);
assert.equal(
match({delivery_email: 'bogus', email: 'fred@example.com'}, 'fr'),
false);
set_email_visibility(everyone);
page_params.is_admin = false;
assert.equal(match({delivery_email: 'fred@example.com'}, 'fr'), false);
assert.equal(match({email: 'fred@example.com'}, 'fr'), true);
@@ -989,31 +993,6 @@ run_test('matches_user_settings_search', () => {
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);
});
initialize();
run_test('is_valid_full_name_and_user_id', () => {

View File

@@ -7,7 +7,6 @@ zrequire('people');
zrequire('presence');
zrequire('buddy_data');
zrequire('user_status');
zrequire('settings_org');
zrequire('feature_flags');
zrequire('message_edit');

View File

@@ -7,6 +7,10 @@ set_global('message_store', {
});
const settings_config = zrequire('settings_config');
page_params.realm_email_address_visibility =
settings_config.email_address_visibility_values.admins_only.code;
zrequire('typeahead_helper');
set_global('Handlebars', global.make_handlebars());
zrequire('Filter', 'js/filter');
@@ -35,10 +39,8 @@ function init() {
}
init();
page_params.is_admin = true;
set_global('narrow', {});
set_global('settings_org', {
show_email: () => true,
});
topic_data.reset();
@@ -1071,7 +1073,7 @@ run_test('people_suggestion (Admin only email visibility)', () => {
only */
people_suggestion_setup();
const query = 'te';
settings_org.show_email = () => false;
page_params.is_admin = false;
const suggestions = get_suggestions('', query);
const expected = [
"te",

View File

@@ -6,6 +6,10 @@ set_global('message_store', {
});
const settings_config = zrequire('settings_config');
page_params.realm_email_address_visibility =
settings_config.email_address_visibility_values.admins_only.code;
zrequire('typeahead_helper');
set_global('Handlebars', global.make_handlebars());
zrequire('Filter', 'js/filter');
@@ -34,9 +38,8 @@ function init() {
init();
set_global('narrow', {});
set_global('settings_org', {
show_email: () => true,
});
page_params.is_admin = true;
topic_data.reset();

View File

@@ -0,0 +1,42 @@
const settings_data = zrequire('settings_data');
set_global('page_params', {});
/*
Some methods in settings_data are fairly
trivial, so the meaningful tests happen
at the higher layers, such as when we
test people.js.
*/
const isaac = {
email: 'isaac@example.com',
delivery_email: 'isaac-delivery@example.com',
};
run_test('email_for_user_settings', () => {
const email = settings_data.email_for_user_settings;
settings_data.show_email = () => {
return false;
};
assert.equal(email(isaac), undefined);
settings_data.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);
});

View File

@@ -3,6 +3,11 @@ set_global('md5', function (s) {
return 'md5-' + s;
});
const settings_config = zrequire('settings_config');
page_params.realm_email_address_visibility =
settings_config.email_address_visibility_values.admins_only.code;
set_global('Handlebars', global.make_handlebars());
zrequire('recent_senders');
zrequire('pm_conversations');
@@ -14,7 +19,6 @@ zrequire('hash_util');
zrequire('marked', 'third/marked/lib/marked');
const pygments_data = zrequire('pygments_data', 'generated/pygments_data.json');
const actual_pygments_data = Object.assign({}, pygments_data);
zrequire('settings_org');
const ct = zrequire('composebox_typeahead');
const th = zrequire('typeahead_helper');
const LazySet = zrequire('lazy_set.js').LazySet;
@@ -499,7 +503,7 @@ run_test('highlight_with_escaping', () => {
run_test('render_person when emails hidden', () => {
// Test render_person with regular person, under hidden email visiblity case
settings_org.show_email = () => false;
page_params.is_admin = false;
let rendered = false;
global.stub_templates(function (template_name, args) {
assert.equal(template_name, 'typeahead_list_item');
@@ -513,7 +517,7 @@ run_test('render_person when emails hidden', () => {
});
run_test('render_person', () => {
settings_org.show_email = () => true;
page_params.is_admin = true;
// Test render_person with regular person
let rendered = false;
global.stub_templates(function (template_name, args) {