diff --git a/static/js/subs.js b/static/js/subs.js index 165db5c341..11b80a3ec8 100644 --- a/static/js/subs.js +++ b/static/js/subs.js @@ -47,13 +47,10 @@ exports.stream_id = function (stream_name) { }; function set_stream_property(stream_name, property, value) { + var sub_data = {stream: stream_name, property: property, value: value}; return channel.post({ url: '/json/subscriptions/property', - data: { - "property": property, - "stream_name": stream_name, - "value": value - }, + data: {"subscription_data": JSON.stringify([sub_data])}, timeout: 10*1000 }); } diff --git a/zerver/test_subs.py b/zerver/test_subs.py index 601c542204..4aa1fe51b0 100644 --- a/zerver/test_subs.py +++ b/zerver/test_subs.py @@ -457,30 +457,6 @@ class DefaultStreamTest(AuthedTestCase): self.assertFalse(stream_name in self.get_default_stream_names(user_profile.realm)) class SubscriptionPropertiesTest(AuthedTestCase): - - def test_get_stream_color(self): - """ - A GET request to - /json/subscriptions/property?property=color+stream_name=foo returns - the color for stream foo. - """ - test_email = "hamlet@zulip.com" - self.login(test_email) - subs = gather_subscriptions(get_user_profile_by_email(test_email))[0] - result = self.client.get("/json/subscriptions/property", - {"property": "color", - "stream_name": subs[0]['name']}) - - self.assert_json_success(result) - json = ujson.loads(result.content) - - self.assertIn("stream_name", json) - self.assertIn("value", json) - self.assertIsInstance(json["stream_name"], basestring) - self.assertIsInstance(json["value"], basestring) - self.assertEqual(json["stream_name"], subs[0]["name"]) - self.assertEqual(json["value"], subs[0]["color"]) - def test_set_stream_color(self): """ A POST request to /json/subscriptions/property with stream_name and @@ -493,10 +469,11 @@ class SubscriptionPropertiesTest(AuthedTestCase): sub = old_subs[0] stream_name = sub['name'] new_color = "#ffffff" # TODO: ensure that this is different from old_color - result = self.client.post("/json/subscriptions/property", - {"property": "color", - "stream_name": stream_name, - "value": "#ffffff"}) + result = self.client.post( + "/json/subscriptions/property", + {"subscription_data": ujson.dumps([{"property": "color", + "stream": stream_name, + "value": "#ffffff"}])}) self.assert_json_success(result) @@ -520,15 +497,17 @@ class SubscriptionPropertiesTest(AuthedTestCase): def test_set_color_missing_stream_name(self): """ - Updating the color property requires a stream_name. + Updating the color property requires a `stream` key. """ test_email = "hamlet@zulip.com" self.login(test_email) - result = self.client.post("/json/subscriptions/property", - {"property": "color", - "value": "#ffffff"}) + result = self.client.post( + "/json/subscriptions/property", + {"subscription_data": ujson.dumps([{"property": "color", + "value": "#ffffff"}])}) - self.assert_json_error(result, "Missing 'stream_name' argument") + self.assert_json_error( + result, "stream key is missing from subscription_data[0]") def test_set_color_missing_color(self): """ @@ -537,11 +516,13 @@ class SubscriptionPropertiesTest(AuthedTestCase): test_email = "hamlet@zulip.com" self.login(test_email) subs = gather_subscriptions(get_user_profile_by_email(test_email))[0] - result = self.client.post("/json/subscriptions/property", - {"property": "color", - "stream_name": subs[0]["name"]}) + result = self.client.post( + "/json/subscriptions/property", + {"subscription_data": ujson.dumps([{"property": "color", + "stream": subs[0]["name"]}])}) - self.assert_json_error(result, "Missing 'value' argument") + self.assert_json_error( + result, "value key is missing from subscription_data[0]") def test_set_invalid_property(self): """ @@ -550,9 +531,11 @@ class SubscriptionPropertiesTest(AuthedTestCase): test_email = "hamlet@zulip.com" self.login(test_email) subs = gather_subscriptions(get_user_profile_by_email(test_email))[0] - result = self.client.post("/json/subscriptions/property", - {"property": "bad", - "stream_name": subs[0]["name"]}) + result = self.client.post( + "/json/subscriptions/property", + {"subscription_data": ujson.dumps([{"property": "bad", + "value": "bad", + "stream": subs[0]["name"]}])}) self.assert_json_error(result, "Unknown subscription property: bad") diff --git a/zerver/views/__init__.py b/zerver/views/__init__.py index 0a16cf7d94..c51187b1dc 100644 --- a/zerver/views/__init__.py +++ b/zerver/views/__init__.py @@ -51,8 +51,8 @@ from openid.consumer.consumer import SUCCESS as openid_SUCCESS from openid.extensions import ax from zerver.lib import bugdown from zerver.lib.alert_words import user_alert_words -from zerver.lib.validator import check_string, check_list, check_dict, check_int, check_bool - +from zerver.lib.validator import check_string, check_list, check_dict, \ + check_int, check_bool, check_variable_type from zerver.decorator import require_post, \ authenticated_api_view, authenticated_json_post_view, \ has_request_variables, authenticated_json_view, to_non_negative_int, \ @@ -1615,34 +1615,53 @@ def get_subscription_or_die(stream_name, user_profile): @authenticated_json_view @has_request_variables -def json_subscription_property(request, user_profile, stream_name=REQ, - property=REQ): +def json_subscription_property(request, user_profile, subscription_data=REQ( + validator=check_list( + check_dict([["stream", check_string], + ["property", check_string], + ["value", check_variable_type( + [check_string, check_bool])]])))): """ - This is the entry point to accessing or changing subscription - properties. - """ - property_converters = dict(color=lambda x: x, - in_home_view=json_to_bool, - desktop_notifications=json_to_bool, - audible_notifications=json_to_bool) - if property not in property_converters: - return json_error("Unknown subscription property: %s" % (property,)) + This is the entry point to changing subscription properties. This + is a bulk endpoint: requestors always provide a subscription_data + list containing dictionaries for each stream of interest. - sub = get_subscription_or_die(stream_name, user_profile)[0] - if request.method == "GET": - return json_success({'stream_name': stream_name, - 'value': getattr(sub, property)}) - elif request.method == "POST": - @has_request_variables - def do_set_property(request, - value=REQ(converter=property_converters[property])): - do_change_subscription_property(user_profile, sub, stream_name, - property, value) - do_set_property(request) - return json_success() - else: + Requests are of the form: + + [{"stream": "devel", "property": "in_home_view", "value": False}, + {"stream": "devel", "property": "color", "value": "#c2c2c2"}] + """ + if request.method != "POST": return json_error("Invalid verb") + property_converters = {"color": check_string, "in_home_view": check_bool, + "desktop_notifications": check_bool, + "audible_notifications": check_bool} + response_data = [] + + for change in subscription_data: + stream_name = change["stream"] + property = change["property"] + value = change["value"] + + if property not in property_converters: + return json_error("Unknown subscription property: %s" % (property,)) + + sub = get_subscription_or_die(stream_name, user_profile)[0] + + property_conversion = property_converters[property](property, value) + if property_conversion: + return json_error(property_conversion) + + do_change_subscription_property(user_profile, sub, stream_name, + property, value) + + response_data.append({'stream': stream_name, + 'property': property, + 'value': value}) + + return json_success({"subscription_data": response_data}) + @csrf_exempt @require_post @has_request_variables