Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

streams: Migrate stream property changes to new REST endpoint. 

This is one of the last major endpoints that were still done in the
pre-REST style.

While we're at it, we change the endpoint to expect a stream ID, not a
stream name.
This commit is contained in:
kunall17 2017-05-09 10:31:42 +05:30 committed by Tim Abbott
parent a75af36039
commit e087bc24f8
6 changed files with 72 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
static/js/stream_edit.js
View File

@ -245,10 +245,9 @@ function stream_home_view_clicked(e) {
exports.set_stream_property = function (sub, property, value) {
// TODO: Fix backend so it takes a stream id.
var stream_name = sub.name;
var sub_data = {stream: stream_name, property: property, value: value};
var sub_data = {stream_id: sub.stream_id, property: property, value: value};
return channel.post({
url: '/json/subscriptions/property',
url: '/json/users/me/subscriptions/properties',
data: {subscription_data: JSON.stringify([sub_data])},
timeout: 10*1000,
});

117
zerver/tests/test_subs.py
View File

@ -797,7 +797,7 @@ class SubscriptionPropertiesTest(ZulipTestCase):
def test_set_stream_color(self):
# type: () -> None
"""
A POST request to /json/subscriptions/property with stream_name and
A POST request to /api/v1/users/me/subscriptions/properties with stream_id and
color data sets the stream color, and for that stream only.
"""
test_user = self.example_user('hamlet')
@ -806,20 +806,21 @@ class SubscriptionPropertiesTest(ZulipTestCase):
old_subs, _ = gather_subscriptions(test_user)
sub = old_subs[0]
stream_name = sub['name']
stream_id = sub['stream_id']
new_color = "#ffffff" # TODO: ensure that this is different from old_color
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "color",
"stream": stream_name,
"value": "#ffffff"}])})
"stream_id": stream_id,
"value": "#ffffff"}])},
**self.api_auth(test_email))
self.assert_json_success(result)
new_subs = gather_subscriptions(get_user_profile_by_email(test_email))[0]
found_sub = None
for sub in new_subs:
if sub['name'] == stream_name:
if sub['stream_id'] == stream_id:
found_sub = sub
break
@ -828,28 +829,30 @@ class SubscriptionPropertiesTest(ZulipTestCase):
new_subs.remove(found_sub)
for sub in old_subs:
if sub['name'] == stream_name:
if sub['stream_id'] == stream_id:
found_sub = sub
break
old_subs.remove(found_sub)
self.assertEqual(old_subs, new_subs)
def test_set_color_missing_stream_name(self):
def test_set_color_missing_stream_id(self):
# type: () -> None
"""
Updating the color property requires a `stream` key.
Updating the color property requires a `stream_id` key.
"""
test_email = "hamlet@zulip.com"
test_user = self.example_user('hamlet')
test_email = test_user.email
self.login(test_email)
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "color",
"value": "#ffffff"}])})
"value": "#ffffff"}])},
**self.api_auth(test_email))
self.assert_json_error(
result, "stream key is missing from subscription_data[0]")
result, "stream_id key is missing from subscription_data[0]")
def test_set_color_unsubscribed_stream_name(self):
def test_set_color_unsubscribed_stream_id(self):
# type: () -> None
"""
Updating the color property requires a subscribed stream.
@ -857,32 +860,17 @@ class SubscriptionPropertiesTest(ZulipTestCase):
test_email = "hamlet@zulip.com"
self.login(test_email)
unsubs_stream = 'Rome'
subscribed, unsubscribed, never_subscribed = gather_subscriptions_helper(
get_user_profile_by_email(test_email))
not_subbed = unsubscribed + never_subscribed
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "color",
"stream": unsubs_stream,
"value": "#ffffff"}])})
"stream_id": not_subbed[0]["stream_id"],
"value": "#ffffff"}])},
**self.api_auth(test_email))
self.assert_json_error(
result, "Not subscribed to stream %s" % (unsubs_stream,))
def test_json_subscription_property_invalid_verb(self):
# type: () -> None
"""
Called by invalid request method. No other request method other than
'post' is allowed in this case.
"""
test_user = self.example_user('hamlet')
test_email = test_user.email
self.login(test_email)
subs = gather_subscriptions(test_user)[0]
result = self.client_get(
"/json/subscriptions/property",
{"subscription_data": ujson.dumps([{"property": "in_home_view",
"stream": subs[0]["name"],
"value": False}])})
self.assert_json_error(result, "Invalid verb")
result, "Not subscribed to stream id %d" % (not_subbed[0]["stream_id"],))
def test_set_color_missing_color(self):
# type: () -> None
@ -894,9 +882,10 @@ class SubscriptionPropertiesTest(ZulipTestCase):
self.login(test_email)
subs = gather_subscriptions(test_user)[0]
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "color",
"stream": subs[0]["name"]}])})
"stream_id": subs[0]["stream_id"]}])},
**self.api_auth(test_email))
self.assert_json_error(
result, "value key is missing from subscription_data[0]")
@ -904,7 +893,7 @@ class SubscriptionPropertiesTest(ZulipTestCase):
def test_set_pin_to_top(self):
# type: () -> None
"""
A POST request to /json/subscriptions/property with stream_name and
A POST request to /api/v1/users/me/subscriptions/properties with stream_id and
pin_to_top data pins the stream.
"""
user_profile = self.example_user('hamlet')
@ -913,17 +902,18 @@ class SubscriptionPropertiesTest(ZulipTestCase):
old_subs, _ = gather_subscriptions(user_profile)
sub = old_subs[0]
stream_name = sub['name']
stream_id = sub['stream_id']
new_pin_to_top = not sub['pin_to_top']
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "pin_to_top",
"stream": stream_name,
"value": new_pin_to_top}])})
"stream_id": stream_id,
"value": new_pin_to_top}])},
**self.api_auth(test_email))
self.assert_json_success(result)
updated_sub = get_subscription(stream_name, user_profile)
updated_sub = get_subscription(sub['name'], user_profile)
self.assertIsNotNone(updated_sub)
self.assertEqual(updated_sub.pin_to_top, new_pin_to_top)
@ -940,40 +930,44 @@ class SubscriptionPropertiesTest(ZulipTestCase):
property_name = "in_home_view"
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": property_name,
"value": "bad",
"stream": subs[0]["name"]}])})
"stream_id": subs[0]["stream_id"]}])},
**self.api_auth(test_email))
self.assert_json_error(result,
'%s is not a boolean' % (property_name,))
property_name = "desktop_notifications"
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": property_name,
"value": "bad",
"stream": subs[0]["name"]}])})
"stream_id": subs[0]["stream_id"]}])},
**self.api_auth(test_email))
self.assert_json_error(result,
'%s is not a boolean' % (property_name,))
property_name = "audible_notifications"
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": property_name,
"value": "bad",
"stream": subs[0]["name"]}])})
"stream_id": subs[0]["stream_id"]}])},
**self.api_auth(test_email))
self.assert_json_error(result,
'%s is not a boolean' % (property_name,))
property_name = "color"
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": property_name,
"value": False,
"stream": subs[0]["name"]}])})
"stream_id": subs[0]["stream_id"]}])},
**self.api_auth(test_email))
self.assert_json_error(result,
'%s is not a string' % (property_name,))
@ -983,14 +977,14 @@ class SubscriptionPropertiesTest(ZulipTestCase):
test_email = "hamlet@zulip.com"
self.login(test_email)
stream_name = "invalid_stream"
stream_id = 1000
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "in_home_view",
"stream": stream_name,
"value": False}])})
self.assert_json_error(result, "Invalid stream name '%s'" % (stream_name,))
"stream_id": stream_id,
"value": False}])},
**self.api_auth(test_email))
self.assert_json_error(result, "Invalid stream id")
def test_set_invalid_property(self):
# type: () -> None
@ -1002,10 +996,11 @@ class SubscriptionPropertiesTest(ZulipTestCase):
self.login(test_email)
subs = gather_subscriptions(test_user)[0]
result = self.client_post(
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "bad",
"value": "bad",
"stream": subs[0]["name"]}])})
"stream_id": subs[0]["stream_id"]}])},
**self.api_auth(test_email))
self.assert_json_error(result,
"Unknown subscription property: bad")

2
zerver/tests/test_urls.py
View File

@ -67,7 +67,7 @@ class PublicURLTest(ZulipTestCase):
"/json/invite_users",
"/json/settings/change",
"/json/subscriptions/exists",
"/json/subscriptions/property",
"/api/v1/users/me/subscriptions/properties",
"/json/fetch_api_key",
"/json/users/me/pointer",
"/json/users/me/subscriptions",

22
zerver/views/streams.py
View File

@ -23,7 +23,7 @@ from zerver.lib.response import json_success, json_error, json_response
from zerver.lib.streams import access_stream_by_id, access_stream_by_name, \
check_stream_name, check_stream_name_available, filter_stream_authorization, \
list_to_streams
from zerver.lib.validator import check_string, check_list, check_dict, \
from zerver.lib.validator import check_string, check_int, check_list, check_dict, \
check_bool, check_variable_type
from zerver.models import UserProfile, Stream, Realm, Subscription, \
Recipient, get_recipient, get_stream, get_active_user_dicts_in_realm
@ -410,11 +410,10 @@ def json_get_stream_id(request, user_profile, stream_name=REQ('stream')):
(stream, recipient, sub) = access_stream_by_name(user_profile, stream_name)
return json_success({'stream_id': stream.id})
@authenticated_json_view
@has_request_variables
def json_subscription_property(request, user_profile, subscription_data=REQ(
def update_subscription_properties_backend(request, user_profile, subscription_data=REQ(
validator=check_list(
check_dict([("stream", check_string),
check_dict([("stream_id", check_int),
("property", check_string),
("value", check_variable_type(
[check_string, check_bool]))])))):
@ -426,12 +425,9 @@ def json_subscription_property(request, user_profile, subscription_data=REQ(
Requests are of the form:
[{"stream": "devel", "property": "in_home_view", "value": False},
{"stream": "devel", "property": "color", "value": "#c2c2c2"}]
[{"stream_id": "1", "property": "in_home_view", "value": False},
{"stream_id": "1", "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,
@ -439,16 +435,16 @@ def json_subscription_property(request, user_profile, subscription_data=REQ(
response_data = []
for change in subscription_data:
stream_name = change["stream"]
stream_id = change["stream_id"]
property = change["property"]
value = change["value"]
if property not in property_converters:
return json_error(_("Unknown subscription property: %s") % (property,))
(stream, recipient, sub) = access_stream_by_name(user_profile, stream_name)
(stream, recipient, sub) = access_stream_by_id(user_profile, stream_id)
if sub is None:
return json_error(_("Not subscribed to stream %s") % (stream_name,))
return json_error(_("Not subscribed to stream id %d") % (stream_id,))
property_conversion = property_converters[property](property, value)
if property_conversion:
@ -457,7 +453,7 @@ def json_subscription_property(request, user_profile, subscription_data=REQ(
do_change_subscription_property(user_profile, sub, stream,
property, value)
response_data.append({'stream': stream_name,
response_data.append({'stream_id': stream_id,
'property': property,
'value': value})

1
zproject/legacy_urls.py
View File

@ -25,7 +25,6 @@ legacy_urls = [
# pushed to us via the event system.
url(r'^json/subscriptions/exists$', zerver.views.streams.json_stream_exists),
url(r'^json/subscriptions/property$', zerver.views.streams.json_subscription_property),
url(r'^json/fetch_api_key$', zerver.views.auth.json_fetch_api_key),
url(r'^json/tutorial_send_message$', zerver.views.tutorial.json_tutorial_send_message),
url(r'^json/tutorial_status$', zerver.views.tutorial.json_tutorial_status),

4
zproject/urls.py
View File

@ -28,6 +28,7 @@ import zerver.views.unsubscribe
import zerver.views.integrations
import zerver.views.user_settings
import zerver.views.muting
import zerver.views.streams
import confirmation.views
from zerver.lib.rest import rest_dispatch
@ -251,6 +252,9 @@ v1_api_and_json_patterns = [
url(r'^messages/(?P<message_id>\d+)/history$', rest_dispatch,
{'GET': 'zerver.views.messages.get_message_edit_history'}),
url(r'^users/me/subscriptions/properties$', rest_dispatch,
{'POST': 'zerver.views.streams.update_subscription_properties_backend'}),
# reactions -> zerver.view.reactions
# PUT adds a reaction to a message
# DELETE removes a reaction from a message