Properly split up "Your Account" and "Notifications".

The "Your Account" and "Notifications" boxes on the Settings
page each had their own border and their own "Save changes"
button, but they were within the same form and sending to the
same back end point.

This commit creates a separate form and endpoint for each
of the two boxes.

(imported from commit 04d4d16938f20749a18d2c6887da3ed3cf21ef74)
This commit is contained in:
Steve Howell 2013-10-25 16:41:35 -04:00
parent 3494ce6ebc
commit 68942a8f3a
5 changed files with 68 additions and 28 deletions

View File

@ -984,7 +984,7 @@ $(function () {
.text(message).stop(true).fadeTo(0,1);
}
$("#settings-change-box form").ajaxForm({
$("form.your-account-settings").expectOne().ajaxForm({
dataType: 'json', // This seems to be ignored. We still get back an xhr.
beforeSubmit: function (arr, form, options) {
// FIXME: Check that the two password fields match
@ -1014,6 +1014,36 @@ $(function () {
}
update_gravatars();
settings_status.removeClass(status_classes)
.addClass('alert-success')
.text(message).stop(true).fadeTo(0,1);
// TODO: In theory we should auto-reload or something if
// you changed the email address or other fields that show
// up on all screens
},
error: function (xhr, error_type, xhn) {
var response = "Error changing settings";
if (xhr.status.toString().charAt(0) === "4") {
// Only display the error response for 4XX, where we've crafted
// a nice response.
response += ": " + $.parseJSON(xhr.responseText).msg;
}
settings_change_error(response);
},
complete: function (xhr, statusText) {
// Whether successful or not, clear the password boxes.
// TODO: Clear these earlier, while the request is still pending.
clear_password_change();
}
});
$("form.notify-settings").expectOne().ajaxForm({
dataType: 'json', // This seems to be ignored. We still get back an xhr.
success: function (resp, statusText, xhr, form) {
var message = "Updated notification settings!";
var result = $.parseJSON(xhr.responseText);
if (result.enable_desktop_notifications !== undefined) {
page_params.desktop_notifications_enabled = result.enable_desktop_notifications;
}
@ -1044,11 +1074,6 @@ $(function () {
response += ": " + $.parseJSON(xhr.responseText).msg;
}
settings_change_error(response);
},
complete: function (xhr, statusText) {
// Whether successful or not, clear the password boxes.
// TODO: Clear these earlier, while the request is still pending.
clear_password_change();
}
});

View File

@ -6,7 +6,7 @@
<div id="settings-change-box">
<form action="/json/settings/change" method="post"
class="form-horizontal">{% csrf_token %}
class="form-horizontal your-account-settings">{% csrf_token %}
<div id="account-settings" class="settings-section">
<div class="settings-section-title"><i class="icon-vector-user settings-section-icon"></i>Your Account</div>
@ -85,7 +85,10 @@
</div>
</div>
</form>
<form action="/json/notify_settings/change" method="post"
class="form-horizontal notify-settings">{% csrf_token %}
<div id="notification-settings" class="settings-section">
<div class="settings-section-title"><i class="icon-vector-warning-sign settings-section-icon"></i>Notifications <i class="tiny icon-vector-question-sign" id="notification-docs"></i></div>

View File

@ -2173,17 +2173,12 @@ class ChangeSettingsTest(AuthedTestCase):
post_params = {"full_name": "Foo Bar",
"old_password": initial_password("hamlet@zulip.com"),
"new_password": "foobar1", "confirm_password": "foobar1",
"enable_desktop_notifications": "",
"enable_offline_email_notifications": "",
"enable_sounds": ""}
}
post_params.update(modified_params)
return self.client.post("/json/settings/change", dict(post_params))
def check_well_formed_change_settings_response(self, result):
self.assertIn("full_name", result)
self.assertIn("enable_desktop_notifications", result)
self.assertIn("enable_sounds", result)
self.assertIn("enable_offline_email_notifications", result)
def test_successful_change_settings(self):
"""
@ -2197,13 +2192,19 @@ class ChangeSettingsTest(AuthedTestCase):
self.check_well_formed_change_settings_response(result)
self.assertEqual(get_user_profile_by_email("hamlet@zulip.com").
full_name, "Foo Bar")
self.assertEqual(get_user_profile_by_email("hamlet@zulip.com").
enable_desktop_notifications, False)
self.client.post('/accounts/logout/')
self.login("hamlet@zulip.com", "foobar1")
user_profile = get_user_profile_by_email('hamlet@zulip.com')
self.assertEqual(self.client.session['_auth_user_id'], user_profile.id)
def test_notify_settings(self):
# This is basically a don't-explode test.
self.login("hamlet@zulip.com")
json_result = self.client.post("/json/notify_settings/change", {})
self.assert_json_success(json_result)
self.assertEqual(get_user_profile_by_email("hamlet@zulip.com").
enable_desktop_notifications, False)
def test_missing_params(self):
"""
full_name, old_password, and new_password are all required POST

View File

@ -1669,19 +1669,11 @@ def get_subscribers_backend(request, user_profile, stream_name=REQ('stream')):
@authenticated_json_post_view
@has_request_variables
def json_change_settings(request, user_profile, full_name=REQ,
old_password=REQ, new_password=REQ,
confirm_password=REQ,
# enable_desktop_notification needs to default to False
# because browsers POST nothing for an unchecked checkbox
enable_desktop_notifications=REQ(converter=lambda x: x == "on",
default=False),
enable_sounds=REQ(converter=lambda x: x == "on",
default=False),
enable_offline_email_notifications=REQ(converter=lambda x: x == "on",
default=False),
enable_offline_push_notifications=REQ(converter=lambda x: x == "on",
default=False)):
def json_change_settings(request, user_profile,
full_name=REQ,
old_password=REQ,
new_password=REQ,
confirm_password=REQ):
if new_password != "" or confirm_password != "":
if new_password != confirm_password:
return json_error("New password must match confirmation password!")
@ -1704,6 +1696,24 @@ def json_change_settings(request, user_profile, full_name=REQ,
do_change_full_name(user_profile, new_full_name)
result['full_name'] = new_full_name
return json_success(result)
@authenticated_json_post_view
@has_request_variables
def json_change_notify_settings(request, user_profile,
# enable_desktop_notification needs to default to False
# because browsers POST nothing for an unchecked checkbox
enable_desktop_notifications=REQ(converter=lambda x: x == "on",
default=False),
enable_sounds=REQ(converter=lambda x: x == "on",
default=False),
enable_offline_email_notifications=REQ(converter=lambda x: x == "on",
default=False),
enable_offline_push_notifications=REQ(converter=lambda x: x == "on",
default=False)):
result = {}
if user_profile.enable_desktop_notifications != enable_desktop_notifications:
do_change_enable_desktop_notifications(user_profile, enable_desktop_notifications)
result['enable_desktop_notifications'] = enable_desktop_notifications

View File

@ -100,6 +100,7 @@ urlpatterns += patterns('zerver.views',
url(r'^json/invite_users$', 'json_invite_users'),
url(r'^json/bulk_invite_users$', 'json_bulk_invite_users'),
url(r'^json/settings/change$', 'json_change_settings'),
url(r'^json/notify_settings/change$', 'json_change_notify_settings'),
url(r'^json/subscriptions/remove$', 'json_remove_subscriptions'),
url(r'^json/subscriptions/add$', 'json_add_subscriptions'),
url(r'^json/subscriptions/exists$', 'json_stream_exists'),