paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-02 21:13:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Standardize on 'subscription' as event type name for subscription changes

Browse Source 
Our overall guideline is the type names for events are singular, and the list of
events of that type are plural. 'subscriptions' was not following this guideline
and (potentially as a result) had a bug where it was impossible for clients to explicitly
subscribe to subscription change events properly.

(imported from commit 7b3162141fd673746e0489199966c29ea32ee876)
This commit is contained in:
Leo Franchi
2014-02-06 15:21:21 -05:00
parent f1e110c177
commit 1c332f5d6a
4 changed files with 19 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
static/js/server_events.js
View File

@@ -93,7 +93,7 @@ function get_events_success(events) {
subs.update_subscription_properties(event.name, event.property, event.value); subs.update_subscription_properties(event.name, event.property, event.value);
} }
break; break;
case 'subscriptions': case 'subscription':
if (event.op === 'add') { if (event.op === 'add') {
_.each(event.subscriptions, function (sub) { _.each(event.subscriptions, function (sub) {
subs.mark_subscribed(sub.name, sub); subs.mark_subscribed(sub.name, sub);

16
zerver/lib/actions.py
View File

@@ -906,7 +906,7 @@ def notify_subscriptions_added(user_profile, sub_pairs, stream_emails, no_log=Fa
description=stream.description, description=stream.description,
subscribers=stream_emails(stream)) subscribers=stream_emails(stream))
for (subscription, stream) in sub_pairs] for (subscription, stream) in sub_pairs]
event = dict(type="subscriptions", op="add", event = dict(type="subscription", op="add",
subscriptions=payload) subscriptions=payload)
send_event(event, [user_profile.id]) send_event(event, [user_profile.id])
@@ -1011,7 +1011,7 @@ def bulk_add_subscriptions(streams, users):
other_user_ids = set(all_subscribed_ids) - set(new_user_ids) other_user_ids = set(all_subscribed_ids) - set(new_user_ids)
if other_user_ids: if other_user_ids:
for user_profile in new_users: for user_profile in new_users:
event = dict(type="subscriptions", op="peer_add", event = dict(type="subscription", op="peer_add",
subscriptions=[stream.name], subscriptions=[stream.name],
user_email=user_profile.email) user_email=user_profile.email)
send_event(event, other_user_ids) send_event(event, other_user_ids)
@@ -1040,7 +1040,7 @@ def do_add_subscription(user_profile, stream, no_log=False):
notify_subscriptions_added(user_profile, [(subscription, stream)], lambda stream: emails_by_stream[stream.id], no_log) notify_subscriptions_added(user_profile, [(subscription, stream)], lambda stream: emails_by_stream[stream.id], no_log)
user_ids = get_other_subscriber_ids(stream, user_profile.id) user_ids = get_other_subscriber_ids(stream, user_profile.id)
event = dict(type="subscriptions", op="peer_add", event = dict(type="subscription", op="peer_add",
subscriptions=[stream.name], subscriptions=[stream.name],
user_email=user_profile.email) user_email=user_profile.email)
send_event(event, user_ids) send_event(event, user_ids)
@@ -1055,7 +1055,7 @@ def notify_subscriptions_removed(user_profile, streams, no_log=False):
'domain': stream.realm.domain}) 'domain': stream.realm.domain})
payload = [dict(name=stream.name) for stream in streams] payload = [dict(name=stream.name) for stream in streams]
event = dict(type="subscriptions", op="remove", event = dict(type="subscription", op="remove",
subscriptions=payload) subscriptions=payload)
send_event(event, [user_profile.id]) send_event(event, [user_profile.id])
@@ -1072,7 +1072,7 @@ def notify_subscriptions_removed(user_profile, streams, no_log=False):
continue continue
stream_names = [stream.name for stream in notifications] stream_names = [stream.name for stream in notifications]
event = dict(type="subscriptions", op="peer_remove", event = dict(type="subscription", op="peer_remove",
subscriptions=stream_names, subscriptions=stream_names,
user_email=user_profile.email) user_email=user_profile.email)
send_event(event, [event_recipient.id]) send_event(event, [event_recipient.id])
@@ -1145,7 +1145,7 @@ def do_change_subscription_property(user_profile, sub, stream_name,
log_subscription_property_change(user_profile.email, stream_name, log_subscription_property_change(user_profile.email, stream_name,
property_name, value) property_name, value)
event = dict(type="subscriptions", event = dict(type="subscription",
op="update", op="update",
email=user_profile.email, email=user_profile.email,
property=property_name, property=property_name,
@@ -2010,7 +2010,7 @@ def fetch_initial_state_data(user_profile, event_types, queue_id):
state['referrals'] = {'granted': user_profile.invites_granted, state['referrals'] = {'granted': user_profile.invites_granted,
'used': user_profile.invites_used} 'used': user_profile.invites_used}
if want('subscriptions'): if want('subscription'):
subscriptions, unsubscribed, email_dict = gather_subscriptions_helper(user_profile) subscriptions, unsubscribed, email_dict = gather_subscriptions_helper(user_profile)
state['subscriptions'] = subscriptions state['subscriptions'] = subscriptions
state['unsubscribed'] = unsubscribed state['unsubscribed'] = unsubscribed
@@ -2053,7 +2053,7 @@ def apply_events(state, events, user_profile):
elif event['type'] == 'realm': elif event['type'] == 'realm':
field = 'realm_' + event['property'] field = 'realm_' + event['property']
state[field] = event['value'] state[field] = event['value']
elif event['type'] == "subscriptions": elif event['type'] == "subscription":
if event['op'] in ["add"]: if event['op'] in ["add"]:
# Convert the user_profile IDs to emails since that's what register() returns # Convert the user_profile IDs to emails since that's what register() returns
# TODO: Clean up this situation # TODO: Clean up this situation

10
zerver/test_events.py
View File

@@ -285,12 +285,12 @@ class EventsRegisterTest(AuthedTestCase):
]) ])
) )
add_schema_checker = check_dict([ add_schema_checker = check_dict([
('type', equals('subscriptions')), ('type', equals('subscription')),
('op', equals('add')), ('op', equals('add')),
('subscriptions', subscription_schema_checker), ('subscriptions', subscription_schema_checker),
]) ])
remove_schema_checker = check_dict([ remove_schema_checker = check_dict([
('type', equals('subscriptions')), ('type', equals('subscription')),
('op', equals('remove')), ('op', equals('remove')),
('subscriptions', check_list( ('subscriptions', check_list(
check_dict([ check_dict([
@@ -299,13 +299,13 @@ class EventsRegisterTest(AuthedTestCase):
)), )),
]) ])
peer_add_schema_checker = check_dict([ peer_add_schema_checker = check_dict([
('type', equals('subscriptions')), ('type', equals('subscription')),
('op', equals('peer_add')), ('op', equals('peer_add')),
('user_email', check_string), ('user_email', check_string),
('subscriptions', check_list(check_string)), ('subscriptions', check_list(check_string)),
]) ])
peer_remove_schema_checker = check_dict([ peer_remove_schema_checker = check_dict([
('type', equals('subscriptions')), ('type', equals('subscription')),
('op', equals('peer_remove')), ('op', equals('peer_remove')),
('user_email', check_string), ('user_email', check_string),
('subscriptions', check_list(check_string)), ('subscriptions', check_list(check_string)),
@@ -319,7 +319,7 @@ class EventsRegisterTest(AuthedTestCase):
]) ])
action = lambda: self.subscribe_to_stream("hamlet@zulip.com", "test_stream") action = lambda: self.subscribe_to_stream("hamlet@zulip.com", "test_stream")
events = self.do_test(action) events = self.do_test(action, event_types=["subscription", "realm_user"])
error = add_schema_checker('events[0]', events[0]) error = add_schema_checker('events[0]', events[0])
self.assert_on_error(error) self.assert_on_error(error)

10
zerver/test_subs.py
View File

@@ -230,7 +230,7 @@ class StreamAdminTest(AuthedTestCase):
deletion_event = events[0]['event'] deletion_event = events[0]['event']
self.assertEqual(deletion_event, dict( self.assertEqual(deletion_event, dict(
op='remove', op='remove',
type='subscriptions', type='subscription',
subscriptions=[{'name': active_name}] subscriptions=[{'name': active_name}]
)) ))
else: else:
@@ -935,7 +935,7 @@ class SubscriptionAPITest(AuthedTestCase):
self.assert_length(events, 2, True) self.assert_length(events, 2, True)
add_event, add_peer_event = events add_event, add_peer_event = events
self.assertEqual(add_event['event']['type'], 'subscriptions') self.assertEqual(add_event['event']['type'], 'subscription')
self.assertEqual(add_event['event']['op'], 'add') self.assertEqual(add_event['event']['op'], 'add')
self.assertEqual(add_event['users'], [get_user_profile_by_email(self.test_email).id]) self.assertEqual(add_event['users'], [get_user_profile_by_email(self.test_email).id])
self.assertEqual( self.assertEqual(
@@ -944,7 +944,7 @@ class SubscriptionAPITest(AuthedTestCase):
) )
self.assertEqual(len(add_peer_event['users']), 2) self.assertEqual(len(add_peer_event['users']), 2)
self.assertEqual(add_peer_event['event']['type'], 'subscriptions') self.assertEqual(add_peer_event['event']['type'], 'subscription')
self.assertEqual(add_peer_event['event']['op'], 'peer_add') self.assertEqual(add_peer_event['event']['op'], 'peer_add')
self.assertEqual(add_peer_event['event']['user_email'], self.test_email) self.assertEqual(add_peer_event['event']['user_email'], self.test_email)
@@ -962,7 +962,7 @@ class SubscriptionAPITest(AuthedTestCase):
self.assert_length(events, 2, True) self.assert_length(events, 2, True)
add_event, add_peer_event = events add_event, add_peer_event = events
self.assertEqual(add_event['event']['type'], 'subscriptions') self.assertEqual(add_event['event']['type'], 'subscription')
self.assertEqual(add_event['event']['op'], 'add') self.assertEqual(add_event['event']['op'], 'add')
self.assertEqual(add_event['users'], [get_user_profile_by_email(email3).id]) self.assertEqual(add_event['users'], [get_user_profile_by_email(email3).id])
self.assertEqual( self.assertEqual(
@@ -971,7 +971,7 @@ class SubscriptionAPITest(AuthedTestCase):
) )
self.assertEqual(len(add_peer_event['users']), 3) self.assertEqual(len(add_peer_event['users']), 3)
self.assertEqual(add_peer_event['event']['type'], 'subscriptions') self.assertEqual(add_peer_event['event']['type'], 'subscription')
self.assertEqual(add_peer_event['event']['op'], 'peer_add') self.assertEqual(add_peer_event['event']['op'], 'peer_add')
self.assertEqual(add_peer_event['event']['user_email'], email3) self.assertEqual(add_peer_event['event']['user_email'], email3)