From be0298314afd35ad83265e0430a802f62a2adf56 Mon Sep 17 00:00:00 2001 From: Shubham Dhama Date: Thu, 15 Mar 2018 04:15:42 +0530 Subject: [PATCH] org settings: Make each subsection independent for saving changes. This makes each subsection(like "Message feed") independent of changes done in any other subsection and the save button of each subsection saves the changes done in that subsection only. --- frontend_tests/casper_tests/10-admin.js | 12 +- .../casper_tests/12-toggle-message-editing.js | 43 +++--- frontend_tests/node_tests/settings_org.js | 43 +++--- static/js/settings_org.js | 138 +++++++----------- static/styles/settings.css | 15 ++ .../organization-settings-admin.handlebars | 47 +++--- 6 files changed, 139 insertions(+), 159 deletions(-) diff --git a/frontend_tests/casper_tests/10-admin.js b/frontend_tests/casper_tests/10-admin.js index e3071a62c5..9fbb854b83 100644 --- a/frontend_tests/casper_tests/10-admin.js +++ b/frontend_tests/casper_tests/10-admin.js @@ -317,9 +317,6 @@ casper.then(function () { }); }); -function submit_org_settings_change() { - casper.click('form.org-settings-form button.button'); -} casper.then(function () { casper.click("li[data-section='organization-settings']"); @@ -328,14 +325,15 @@ casper.then(function () { casper.evaluate(function () { $('#id_realm_default_language').val('de').change(); }); - submit_org_settings_change(); + casper.test.assertSelectorHasText('#org-submit-language-notify', "Save"); + casper.click('#org-submit-language-notify'); }); }); casper.then(function () { - casper.waitUntilVisible('#admin-realm-default-language-status', function () { - casper.test.assertSelectorHasText('#admin-realm-default-language-status', - 'Default language changed!'); + casper.waitUntilVisible('#org-submit-language-notify[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-language-notify', + 'Saved'); }); }); diff --git a/frontend_tests/casper_tests/12-toggle-message-editing.js b/frontend_tests/casper_tests/12-toggle-message-editing.js index f138847312..0ede995514 100644 --- a/frontend_tests/casper_tests/12-toggle-message-editing.js +++ b/frontend_tests/casper_tests/12-toggle-message-editing.js @@ -6,19 +6,10 @@ function heading(heading_str) { }); } -function submit() { - // Casper 1.1.4 has a strange bug related to dispatching functions - // twice. We call save_organization_settings() to try to minimize - // the moving parts involved in troubleshooting. - casper.evaluate(function () { - settings_org.save_organization_settings(); - }); -} - function submit_checked() { casper.then(function () { casper.waitUntilVisible('input:checked[type="checkbox"][id="id_realm_allow_message_editing"] + span', function () { - submit(); + casper.click('#org-submit-msg-editing'); }); }); } @@ -26,7 +17,7 @@ function submit_checked() { function submit_unchecked() { casper.then(function () { casper.waitUntilVisible('input:not(:checked)[type="checkbox"][id="id_realm_allow_message_editing"] + span', function () { - submit(); + casper.click('#org-submit-msg-editing'); }); }); } @@ -114,8 +105,9 @@ common.then_click('input[type="checkbox"][id="id_realm_allow_message_editing"] + submit_unchecked(); casper.then(function () { - casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can no longer edit their past messages!'); + casper.waitUntilVisible('#org-submit-msg-editing[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-msg-editing', + 'Saved'); casper.test.assertEval(function () { return !(document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked); }, 'Allow message editing Setting de-activated'); @@ -167,8 +159,9 @@ common.then_click('input[type="checkbox"][id="id_realm_allow_message_editing"] + submit_checked(); casper.then(function () { - casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit topics for all their messages, and the content of messages which are less than 10 minutes old.'); + casper.waitUntilVisible('#org-submit-msg-editing[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-msg-editing', + 'Saved'); casper.test.assertEval(function () { return document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked; }, 'Allow message editing Setting re-activated'); @@ -204,8 +197,9 @@ common.then_click('input[type="checkbox"][id="id_realm_allow_message_editing"] + submit_unchecked(); casper.then(function () { - casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can no longer edit their past messages!'); + casper.waitUntilVisible('#org-submit-msg-editing[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-msg-editing', + 'Saved'); casper.test.assertEval(function () { return !(document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked); }, 'Allow message editing Setting de-activated'); @@ -222,8 +216,9 @@ common.then_click('input[type="checkbox"][id="id_realm_allow_message_editing"] + submit_checked(); casper.then(function () { - casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit topics for all their messages, and the content of messages which are less than 4 minutes old.'); + casper.waitUntilVisible('#org-submit-msg-editing[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-msg-editing', + 'Saved'); casper.test.assertEval(function () { return document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked; }, 'Allow message editing Setting activated'); @@ -247,8 +242,9 @@ casper.then(function () { submit_checked(); casper.then(function () { - casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit the content and topics of all their past messages!'); + casper.waitUntilVisible('#org-submit-msg-editing[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-msg-editing', + 'Saved'); casper.test.assertEval(function () { return document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked; }, 'Allow message editing Setting still activated'); @@ -273,8 +269,9 @@ common.then_click('input[type="checkbox"][id="id_realm_allow_message_editing"] + submit_unchecked(); casper.then(function () { - casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can no longer edit their past messages!'); + casper.waitUntilVisible('#org-submit-msg-editing[data-status="saved"]', function () { + casper.test.assertSelectorHasText('#org-submit-msg-editing', + 'Saved'); casper.test.assertEval(function () { return !(document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked); }, 'Allow message editing Setting de-activated'); diff --git a/frontend_tests/node_tests/settings_org.js b/frontend_tests/node_tests/settings_org.js index 341b6370b0..095f595fb5 100644 --- a/frontend_tests/node_tests/settings_org.js +++ b/frontend_tests/node_tests/settings_org.js @@ -180,6 +180,7 @@ function test_submit_settings_form(submit_form) { var ev = { preventDefault: noop, stopPropagation: noop, + target: '#org-submit-msg-editing', }; $('#id_realm_default_language').val('fr'); @@ -190,14 +191,21 @@ function test_submit_settings_form(submit_form) { channel.patch = function (req) { patched = true; assert.equal(req.url, '/json/realm'); - - var data = req.data; - - assert.equal(data.default_language, '"fr"'); - success_callback = req.success; }; + var stub_save_button = $('#org-submit-msg-editing'); + stub_save_button.attr = function () { + return 'org-submit-msg-editing'; + }; + var stub_save_button_header = $('.subsection-header'); + stub_save_button.closest = function () { + return stub_save_button_header; + }; + stub_save_button_header.set_find_results( + '.subsection-changes-discard button', $.create('#org-discard-msg-editing') + ); + submit_form(ev); assert(patched); @@ -208,26 +216,19 @@ function test_submit_settings_form(submit_form) { success_callback(response_data); - var editing_status = $('#admin-realm-message-editing-status').val(); - assert(editing_status.indexOf('content of messages which are less than') > 0); + var updated_value_from_response = $('#id_realm_message_content_edit_limit_minutes').val(); + assert(updated_value_from_response, 3); + + $('#id_realm_message_content_edit_limit_minutes').val = function (time_limit) { + updated_value_from_response = time_limit; + }; response_data = { allow_message_editing: true, - message_content_edit_limit_seconds: 0, + message_content_edit_limit_seconds: 10, }; success_callback(response_data); - - assert.equal($('#admin-realm-message-editing-status').val(), - 'translated: Users can now edit the content and topics ' + - 'of all their past messages!'); - - response_data = { - allow_message_editing: false, - }; - success_callback(response_data); - - assert.equal($('#admin-realm-message-editing-status').val(), - 'translated: Users can no longer edit their past messages!'); + assert(updated_value_from_response, 0); } function test_submit_permissions_form(submit_form) { @@ -469,7 +470,7 @@ function test_change_allow_subdomains(change_allow_subdomains) { var submit_permissions_form; var submit_profile_form; $('.organization').on = function (action, selector, f) { - if (selector === 'button.save-message-org-settings') { + if (selector === '.subsection-header .subsection-changes-save button') { assert.equal(action, 'click'); submit_settings_form = f; } diff --git a/static/js/settings_org.js b/static/js/settings_org.js index 311f98a46a..92de545d57 100644 --- a/static/js/settings_org.js +++ b/static/js/settings_org.js @@ -418,104 +418,76 @@ function _set_up() { settings_ui.disable_sub_setting_onchange(this.checked, "id_realm_message_content_edit_limit_minutes", true); }); - exports.save_organization_settings = function () { - _.each(property_types.settings, function (v, k) { - property_type_status_element(k).hide(); - }); - - var message_editing_status = $("#admin-realm-message-editing-status").expectOne(); - // grab the first alert available and use it for the status. - var $alerts = $(".settings-section.show .alert").hide(); - // grab the first alert available and use it for the status. - var status = $("#admin-realm-notifications-stream-status"); - - var compose_textarea_edit_limit_minutes = $("#id_realm_message_content_edit_limit_minutes").val(); - var new_allow_message_editing = $("#id_realm_allow_message_editing").prop("checked"); - - // If allow_message_editing is unchecked, message_content_edit_limit_minutes - // is irrelevant. Hence if allow_message_editing is unchecked, and - // message_content_edit_limit_minutes is poorly formed, we set the latter to - // a default value to prevent the server from returning an error. - if (!new_allow_message_editing) { - if ((parseInt(compose_textarea_edit_limit_minutes, 10).toString() !== - compose_textarea_edit_limit_minutes) || - compose_textarea_edit_limit_minutes < 0) { - // Realm.DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS / 60 - compose_textarea_edit_limit_minutes = 10; - } - } - - var url = "/json/realm"; - var data = {}; - data = populate_data_for_request({ - allow_message_editing: JSON.stringify(new_allow_message_editing), - message_content_edit_limit_seconds: - JSON.stringify(parseInt(compose_textarea_edit_limit_minutes, 10) * 60), - }, property_types.settings); - + exports.save_organization_settings = function (data, save_button, success_continuation) { + var failed_alert_elem = $('#admin-realm-failed-change-status'); + save_button.text(i18n.t("Saving")); + save_button.attr("data-status", "saving"); channel.patch({ - url: url, + url: "/json/realm", data: data, - success: function (response_data) { - $alerts.hide(); - if (response_data.allow_message_editing !== undefined) { - // We expect message_content_edit_limit_seconds was sent in the - // response as well - var data_message_content_edit_limit_minutes = - Math.ceil(response_data.message_content_edit_limit_seconds / 60); - if (response_data.allow_message_editing) { - if (response_data.message_content_edit_limit_seconds > 0) { - ui_report.success( - i18n.t("Users can now edit topics for all their messages, and the content of messages which are less than __num_minutes__ minutes old.", - {num_minutes : data_message_content_edit_limit_minutes}), - message_editing_status); - } else { - ui_report.success(i18n.t("Users can now edit the content and topics of all their past messages!"), message_editing_status); - } - } else { - ui_report.success(i18n.t("Users can no longer edit their past messages!"), message_editing_status); - } - // message_content_edit_limit_seconds could have been changed earlier - // in this function, so update the field just in case - $("#id_realm_message_content_edit_limit_minutes").val(data_message_content_edit_limit_minutes); - } - - process_response_data(response_data, 'settings'); - // Check if no changes made - var no_changes_made = true; - for (var key in response_data) { - if (['msg', 'result'].indexOf(key) < 0) { - no_changes_made = false; - } - } - if (no_changes_made) { - ui_report.success(i18n.t("No changes to save!"), status); + failed_alert_elem.hide(); + save_button.text(i18n.t("Saved")); + save_button.attr("data-status", "saved"); + if (success_continuation !== undefined) { + success_continuation(response_data); } }, error: function (xhr) { - $alerts.hide(); - ui_report.error(i18n.t("Failed"), xhr, status); + save_button.attr("data-status", "failed"); + save_button.text(i18n.t("Save")); + ui_report.error(i18n.t("Failed"), xhr, failed_alert_elem); }, }); }; - // For historical reasons, the save_organization_settings() function handles - // saving data for two different sections of the "Organization settings" panel. - // TODO: Make two separate click handlers, so that we only save changes that - // make sense for the individual buttons. - $(".organization").on("click", "button.save-message-org-settings", function (e) { + $(".organization").on("click", ".subsection-header .subsection-changes-save button", function (e) { e.preventDefault(); e.stopPropagation(); + var save_button = $(e.target); + var subsection_id = save_button.attr('id').replace("org-submit-", ""); + var subsection = subsection_id.split('-').join('_'); - exports.save_organization_settings(e); - }); + var data = {}; + var success_continuation; - $(".organization").on("click", "button.save-language-org-settings", function (e) { - e.preventDefault(); - e.stopPropagation(); + if (subsection === 'msg_editing') { + var compose_textarea_edit_limit_minutes = $("#id_realm_message_content_edit_limit_minutes").val(); + var new_allow_message_editing = $("#id_realm_allow_message_editing").prop("checked"); + // If allow_message_editing is unchecked, message_content_edit_limit_minutes + // is irrelevant. Hence if allow_message_editing is unchecked, and + // message_content_edit_limit_minutes is poorly formed, we set the latter to + // a default value to prevent the server from returning an error. + if (!new_allow_message_editing) { + if ((parseInt(compose_textarea_edit_limit_minutes, 10).toString() !== + compose_textarea_edit_limit_minutes) || + compose_textarea_edit_limit_minutes < 0) { + // Realm.DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS / 60 + compose_textarea_edit_limit_minutes = 10; + } + } - exports.save_organization_settings(e); + data = { + allow_message_editing: JSON.stringify(new_allow_message_editing), + message_content_edit_limit_seconds: + JSON.stringify(parseInt(compose_textarea_edit_limit_minutes, 10) * 60), + }; + + success_continuation = function (response_data) { + if (response_data.allow_message_editing !== undefined) { + // We expect message_content_edit_limit_seconds was sent in the + // response as well + var data_message_content_edit_limit_minutes = + Math.ceil(response_data.message_content_edit_limit_seconds / 60); + // message_content_edit_limit_seconds could have been changed earlier + // in this function, so update the field just in case + $("#id_realm_message_content_edit_limit_minutes").val(data_message_content_edit_limit_minutes); + } + }; + } + + data = populate_data_for_request(data, org_settings[subsection]); + exports.save_organization_settings(data, save_button, success_continuation); }); $("#id_realm_create_stream_permission").change(function () { diff --git a/static/styles/settings.css b/static/styles/settings.css index 0e1966eaaf..2cc1206954 100644 --- a/static/styles/settings.css +++ b/static/styles/settings.css @@ -359,6 +359,21 @@ input[type=checkbox] + .inline-block { margin-top: 0px; } +.org-settings-form .subsection-header h3 { + display: inline; +} + +.org-settings-form .subsection-header div[class*="subsection-changes"] { + display: inline; +} + +.org-settings-form .subsection-header div[class*="subsection-changes"] button { + display: inline-block; + margin-top: -4px; + min-width: 80px; + line-height: 10px; +} + #default_language { margin-left: 20px; } diff --git a/static/templates/settings/organization-settings-admin.handlebars b/static/templates/settings/organization-settings-admin.handlebars index a44d5b87c9..96b108d276 100644 --- a/static/templates/settings/organization-settings-admin.handlebars +++ b/static/templates/settings/organization-settings-admin.handlebars @@ -2,19 +2,18 @@
-
-
- -
-
-
-
-
-
+

{{t "Message editing" }}

+ {{#if is_admin }} +
+ +
+ {{/if}}
@@ -69,6 +68,13 @@

{{t "Message feed" }}

+ {{#if is_admin }} +
+ +
+ {{/if}}
{{#if false}} @@ -123,17 +129,16 @@
- {{#if is_admin }} -
- -
- {{/if}} -

{{t "Language & notifications" }}

+ {{#if is_admin }} +
+ +
+ {{/if}}
@@ -156,14 +161,6 @@
- {{#if is_admin }} -
- -
- {{/if}} -