settings_org: Use actual value for value attribute of create-stream-policy.

For few settings like `waiting_period_threshold` it makes sense to have the
"value" attribute of option to have a value other than the actual setting
value because multiple settings are depending upon this dropdown, so
handling them in JS code makes more sense. But for many settings (which has
integer values), we have followed a wrong trend over the time of
representing every new dropdown with human-readable values and manually
handling them in JS Code, where it makes more sense to use actual setting
value. The result of which is code has become less concise, sensible and
less likely to be mistaken.
This commit is contained in:
Pragati Agrawal 2020-01-23 11:31:34 +05:30
parent fad5c509ac
commit 1420922705
5 changed files with 170 additions and 33 deletions

View File

@ -115,7 +115,7 @@ casper.then(function () {
casper.test.info("Test setting create streams policy to 'admins only'.");
casper.waitUntilVisible("#id_realm_create_stream_policy", function () {
casper.evaluate(function () {
$("#id_realm_create_stream_policy").val("by_admins_only").change();
$("#id_realm_create_stream_policy").val(settings_org.create_stream_policy_values.by_admins_only.code).change();
});
submit_permissions_change();
});
@ -134,7 +134,7 @@ casper.then(function () {
casper.test.info("Test setting create streams policy to 'members and admins'.");
casper.waitUntilVisible("#id_realm_create_stream_policy", function () {
casper.evaluate(function () {
$("#id_realm_create_stream_policy").val("by_members").change();
$("#id_realm_create_stream_policy").val(settings_org.create_stream_policy_values.by_members.code).change();
});
submit_permissions_change();
});
@ -153,7 +153,7 @@ casper.then(function () {
casper.test.info("Test setting create streams policy to 'waiting period.");
casper.waitUntilVisible("#id_realm_create_stream_policy", function () {
casper.evaluate(function () {
$("#id_realm_create_stream_policy").val("by_full_members").change();
$("#id_realm_create_stream_policy").val(settings_org.create_stream_policy_values.by_full_members.code).change();
});
submit_permissions_change();
});

View File

@ -197,6 +197,7 @@ function test_submit_settings_form(submit_form) {
realm_default_language: '"es"',
realm_default_twenty_four_hour_time: 'false',
realm_invite_to_stream_policy: 2,
realm_create_stream_policy: 1,
});
global.patch_builtin('setTimeout', func => func());
@ -223,10 +224,17 @@ function test_submit_settings_form(submit_form) {
save_button.replace = () => {
return `${subsection}`;
};
$("#id_realm_create_stream_policy").val("by_members");
$("#id_realm_invite_to_stream_policy").val("by_members");
$("#id_realm_waiting_period_threshold").val(10);
const create_stream_policy_elem = $("#id_realm_create_stream_policy");
create_stream_policy_elem.val('2');
create_stream_policy_elem.attr("id", 'id_realm_create_stream_policy');
create_stream_policy_elem.data = () => {
return "integer";
};
const add_emoji_by_admins_only_elem = $("#id_realm_add_emoji_by_admins_only");
add_emoji_by_admins_only_elem.val("by_anyone");
add_emoji_by_admins_only_elem.attr("id", 'id_realm_add_emoji_by_admins_only');
@ -250,6 +258,7 @@ function test_submit_settings_form(submit_form) {
bot_creation_policy_elem,
email_address_visibility_elem,
add_emoji_by_admins_only_elem,
create_stream_policy_elem,
]);
patched = false;
@ -261,7 +270,7 @@ function test_submit_settings_form(submit_form) {
invite_to_stream_policy: 1,
email_address_visibility: '1',
add_emoji_by_admins_only: false,
create_stream_policy: 1,
create_stream_policy: '2',
waiting_period_threshold: 10,
};
assert.deepEqual(data, expected_value);
@ -478,7 +487,8 @@ function test_sync_realm_settings() {
page_params.realm_create_stream_policy = 3;
settings_org.sync_realm_settings('create_stream_policy');
assert.equal($("#id_realm_create_stream_policy").val(), "by_full_members");
assert.equal($("#id_realm_create_stream_policy").val(),
settings_org.create_stream_policy_values.by_full_members.code);
}
{
@ -493,7 +503,8 @@ function test_sync_realm_settings() {
page_params.realm_create_stream_policy = 1;
settings_org.sync_realm_settings('create_stream_policy');
assert.equal($("#id_realm_create_stream_policy").val(), "by_members");
assert.equal($("#id_realm_create_stream_policy").val(),
settings_org.create_stream_policy_values.by_members.code);
}
{
@ -508,7 +519,8 @@ function test_sync_realm_settings() {
page_params.realm_create_stream_policy = 2;
settings_org.sync_realm_settings('create_stream_policy');
assert.equal($("#id_realm_create_stream_policy").val(), "by_admins_only");
assert.equal($("#id_realm_create_stream_policy").val(),
settings_org.create_stream_policy_values.by_admins_only.code);
}
{
@ -810,6 +822,106 @@ run_test('set_up', () => {
settings_org.render_notifications_stream_ui = stub_render_notifications_stream_ui;
});
run_test('test get_organization_settings_options', () => {
const sorted_option_values = settings_org.get_organization_settings_options();
const sorted_create_stream_policy_values = sorted_option_values.create_stream_policy_values;
const expected_create_stream_policy_values = [
{
key: 'by_admins_only',
order: 1,
code: 2,
description: i18n.t("Admins"),
},
{
key: 'by_full_members',
order: 2,
code: 3,
description: i18n.t("Admins and full members"),
},
{
key: 'by_members',
order: 3,
code: 1,
description: i18n.t("Admins and members"),
},
];
assert.deepEqual(sorted_create_stream_policy_values, expected_create_stream_policy_values);
});
run_test('test get_sorted_options_list', () => {
const option_values_1 = {
by_admins_only: {
order: 3,
code: 2,
description: i18n.t("Admins"),
},
by_members: {
order: 2,
code: 1,
description: i18n.t("Admins and members"),
},
by_full_members: {
order: 1,
code: 3,
description: i18n.t("Admins and full members"),
},
};
let expected_option_values = [
{
key: 'by_full_members',
order: 1,
code: 3,
description: i18n.t("Admins and full members"),
},
{
key: 'by_members',
order: 2,
code: 1,
description: i18n.t("Admins and members"),
},
{
key: 'by_admins_only',
order: 3,
code: 2,
description: i18n.t("Admins"),
},
];
assert.deepEqual(settings_org.get_sorted_options_list(option_values_1), expected_option_values);
const option_values_2 = {
by_admins_only: {
code: 1,
description: i18n.t("Admins"),
},
by_members: {
code: 2,
description: i18n.t("Admins and members"),
},
by_full_members: {
code: 3,
description: i18n.t("Admins and full members"),
},
};
expected_option_values = [
{
key: 'by_admins_only',
code: 1,
description: i18n.t("Admins"),
},
{
key: 'by_full_members',
code: 3,
description: i18n.t("Admins and full members"),
},
{
key: 'by_members',
code: 2,
description: i18n.t("Admins and members"),
},
];
assert.deepEqual(settings_org.get_sorted_options_list(option_values_2), expected_option_values);
});
run_test('misc', () => {
page_params.is_admin = false;

View File

@ -83,6 +83,7 @@ exports.build_page = function () {
options.msg_delete_limit_dropdown_values = settings_org.msg_delete_limit_dropdown_values;
options.bot_creation_policy_values = settings_bots.bot_creation_policy_values;
options.email_address_visibility_values = settings_org.email_address_visibility_values;
_.extend(options, settings_org.get_organization_settings_options());
if (options.realm_logo_source !== 'D' && options.realm_night_logo_source === 'D') {
// If no night mode logo is specified but a day mode one is,

View File

@ -52,6 +52,53 @@ exports.email_address_visibility_values = {
},
};
exports.create_stream_policy_values = {
by_admins_only: {
order: 1,
code: 2,
description: i18n.t("Admins"),
},
by_full_members: {
order: 2,
code: 3,
description: i18n.t("Admins and full members"),
},
by_members: {
order: 3,
code: 1,
description: i18n.t("Admins and members"),
},
};
exports.get_sorted_options_list = function (option_values_object) {
const options_list = Object.keys(option_values_object).map((key) => {
return _.extend(option_values_object[key], {key: key});
});
let comparator = (x, y) => x.order - y.order;
if (!options_list[0].order) {
comparator = (x, y) => {
const key_x = x.key.toUpperCase();
const key_y = y.key.toUpperCase();
if (key_x < key_y) {
return -1;
}
if (key_x > key_y) {
return 1;
}
return 0;
};
}
options_list.sort(comparator);
return options_list;
};
exports.get_organization_settings_options = () => {
const options = {};
options.create_stream_policy_values = exports.get_sorted_options_list(
exports.create_stream_policy_values);
return options;
};
exports.show_email = function () {
// TODO: Extend this when we add support for admins_and_members above.
if (page_params.realm_email_address_visibility ===
@ -93,18 +140,6 @@ function get_property_value(property_name) {
return "custom_days";
}
if (property_name === 'realm_create_stream_policy') {
if (page_params.realm_create_stream_policy === 2) {
return "by_admins_only";
}
if (page_params.realm_create_stream_policy === 1) {
return "by_members";
}
if (page_params.realm_create_stream_policy === 3) {
return "by_full_members";
}
}
if (property_name === 'realm_invite_to_stream_policy') {
if (page_params.realm_invite_to_stream_policy === 1) {
return "by_members";
@ -818,7 +853,6 @@ exports.build_page = function () {
JSON.stringify(parseInt(new_message_retention_days, 10)) : null;
} else if (subsection === 'other_permissions') {
const waiting_period_threshold = $("#id_realm_waiting_period_setting").val();
const create_stream_policy = $("#id_realm_create_stream_policy").val();
const invite_to_stream_policy = $("#id_realm_invite_to_stream_policy").val();
const user_group_edit_policy = $("#id_realm_user_group_edit_policy").val();
const private_message_policy = $("#id_realm_private_message_policy").val();
@ -830,14 +864,6 @@ exports.build_page = function () {
data.add_emoji_by_admins_only = false;
}
if (create_stream_policy === "by_admins_only") {
data.create_stream_policy = 2;
} else if (create_stream_policy === "by_members") {
data.create_stream_policy = 1;
} else if (create_stream_policy === "by_full_members") {
data.create_stream_policy = 3;
}
if (invite_to_stream_policy === "by_admins_only") {
data.invite_to_stream_policy = 2;
} else if (invite_to_stream_policy === "by_members") {

View File

@ -89,10 +89,8 @@
<div class="input-group">
<label for="realm_create_stream_policy" class="dropdown-title">{{t "Who can create streams" }}</label>
<select name="realm_create_stream_policy" id="id_realm_create_stream_policy" class="prop-element">
<option value="by_admins_only">{{t "Admins" }}</option>
<option value="by_full_members">{{t "Admins and full members" }}</option>
<option value="by_members">{{t "Admins and members" }}</option>
<select name="realm_create_stream_policy" id="id_realm_create_stream_policy" class="prop-element" data-setting-widget-type="integer">
{{> dropdown_options_widget option_values=create_stream_policy_values}}
</select>
</div>