From 1c332f5d6aac5720af274ba67a02c6b4c43c57ca Mon Sep 17 00:00:00 2001 From: Leo Franchi Date: Thu, 6 Feb 2014 15:21:21 -0500 Subject: [PATCH] Standardize on 'subscription' as event type name for subscription changes 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) --- static/js/server_events.js | 2 +- zerver/lib/actions.py | 16 ++++++++-------- zerver/test_events.py | 10 +++++----- zerver/test_subs.py | 10 +++++----- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/static/js/server_events.js b/static/js/server_events.js index a3a957cdc0..b2f594dbab 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -93,7 +93,7 @@ function get_events_success(events) { subs.update_subscription_properties(event.name, event.property, event.value); } break; - case 'subscriptions': + case 'subscription': if (event.op === 'add') { _.each(event.subscriptions, function (sub) { subs.mark_subscribed(sub.name, sub); diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 3e7d6cd1d3..fb58656132 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -906,7 +906,7 @@ def notify_subscriptions_added(user_profile, sub_pairs, stream_emails, no_log=Fa description=stream.description, subscribers=stream_emails(stream)) for (subscription, stream) in sub_pairs] - event = dict(type="subscriptions", op="add", + event = dict(type="subscription", op="add", subscriptions=payload) 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) if other_user_ids: for user_profile in new_users: - event = dict(type="subscriptions", op="peer_add", + event = dict(type="subscription", op="peer_add", subscriptions=[stream.name], user_email=user_profile.email) 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) 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], user_email=user_profile.email) send_event(event, user_ids) @@ -1055,7 +1055,7 @@ def notify_subscriptions_removed(user_profile, streams, no_log=False): 'domain': stream.realm.domain}) payload = [dict(name=stream.name) for stream in streams] - event = dict(type="subscriptions", op="remove", + event = dict(type="subscription", op="remove", subscriptions=payload) send_event(event, [user_profile.id]) @@ -1072,7 +1072,7 @@ def notify_subscriptions_removed(user_profile, streams, no_log=False): continue 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, user_email=user_profile.email) 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, property_name, value) - event = dict(type="subscriptions", + event = dict(type="subscription", op="update", email=user_profile.email, 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, 'used': user_profile.invites_used} - if want('subscriptions'): + if want('subscription'): subscriptions, unsubscribed, email_dict = gather_subscriptions_helper(user_profile) state['subscriptions'] = subscriptions state['unsubscribed'] = unsubscribed @@ -2053,7 +2053,7 @@ def apply_events(state, events, user_profile): elif event['type'] == 'realm': field = 'realm_' + event['property'] state[field] = event['value'] - elif event['type'] == "subscriptions": + elif event['type'] == "subscription": if event['op'] in ["add"]: # Convert the user_profile IDs to emails since that's what register() returns # TODO: Clean up this situation diff --git a/zerver/test_events.py b/zerver/test_events.py index 6d92f42625..92f699da52 100644 --- a/zerver/test_events.py +++ b/zerver/test_events.py @@ -285,12 +285,12 @@ class EventsRegisterTest(AuthedTestCase): ]) ) add_schema_checker = check_dict([ - ('type', equals('subscriptions')), + ('type', equals('subscription')), ('op', equals('add')), ('subscriptions', subscription_schema_checker), ]) remove_schema_checker = check_dict([ - ('type', equals('subscriptions')), + ('type', equals('subscription')), ('op', equals('remove')), ('subscriptions', check_list( check_dict([ @@ -299,13 +299,13 @@ class EventsRegisterTest(AuthedTestCase): )), ]) peer_add_schema_checker = check_dict([ - ('type', equals('subscriptions')), + ('type', equals('subscription')), ('op', equals('peer_add')), ('user_email', check_string), ('subscriptions', check_list(check_string)), ]) peer_remove_schema_checker = check_dict([ - ('type', equals('subscriptions')), + ('type', equals('subscription')), ('op', equals('peer_remove')), ('user_email', 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") - events = self.do_test(action) + events = self.do_test(action, event_types=["subscription", "realm_user"]) error = add_schema_checker('events[0]', events[0]) self.assert_on_error(error) diff --git a/zerver/test_subs.py b/zerver/test_subs.py index 0b57aa58e0..9e77071551 100644 --- a/zerver/test_subs.py +++ b/zerver/test_subs.py @@ -230,7 +230,7 @@ class StreamAdminTest(AuthedTestCase): deletion_event = events[0]['event'] self.assertEqual(deletion_event, dict( op='remove', - type='subscriptions', + type='subscription', subscriptions=[{'name': active_name}] )) else: @@ -935,7 +935,7 @@ class SubscriptionAPITest(AuthedTestCase): self.assert_length(events, 2, True) 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['users'], [get_user_profile_by_email(self.test_email).id]) self.assertEqual( @@ -944,7 +944,7 @@ class SubscriptionAPITest(AuthedTestCase): ) 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']['user_email'], self.test_email) @@ -962,7 +962,7 @@ class SubscriptionAPITest(AuthedTestCase): self.assert_length(events, 2, True) 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['users'], [get_user_profile_by_email(email3).id]) self.assertEqual( @@ -971,7 +971,7 @@ class SubscriptionAPITest(AuthedTestCase): ) 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']['user_email'], email3)