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

peer_remove: Send user_id, not email, for unsubscribe events.

This commit is contained in:
Steve Howell 2016-11-08 06:04:18 -08:00 committed by Tim Abbott
parent 61625ce51e
commit a03a7d4c95
5 changed files with 29 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
frontend_tests/node_tests/dispatch.js
View File

@ -309,7 +309,7 @@ var event_fixtures = {
subscription__peer_remove: {
type: 'subscription',
op: 'peer_remove',
user_email: 'bob@example.com',
user_id: 555,
subscriptions: [
{
stream_id: 42
@ -658,7 +658,7 @@ run(function (override, capture, args) {
override('stream_data', 'remove_subscriber', capture(['sub', 'email']));
dispatch(event);
assert_same(args.sub, event.subscriptions[0]);
assert_same(args.email, event.user_email);
assert_same(args.email, 'bob@example.com');
event = event_fixtures.subscription__remove;
var stream_id_looked_up;

12
static/js/server_events.js
View File

@ -120,6 +120,9 @@ function dispatch_normal_event(event) {
break;
case 'subscription':
var person;
var email;
if (event.op === 'add') {
_.each(event.subscriptions, function (sub) {
subs.mark_subscribed(sub.name, sub);
@ -127,16 +130,19 @@ function dispatch_normal_event(event) {
} else if (event.op === 'peer_add') {
// TODO: remove email shim here and fix called functions
// to use user_ids
var person = people.get_person_from_user_id(event.user_id);
var email = person.email;
person = people.get_person_from_user_id(event.user_id);
email = person.email;
_.each(event.subscriptions, function (sub) {
stream_data.add_subscriber(sub, email);
$(document).trigger('peer_subscribe.zulip',
{stream_name: sub, user_email: email});
});
} else if (event.op === 'peer_remove') {
// TODO: remove email shim here and fix called functions
// to use user_ids
person = people.get_person_from_user_id(event.user_id);
email = person.email;
_.each(event.subscriptions, function (sub) {
var email = event.user_email;
stream_data.remove_subscriber(sub, email);
$(document).trigger('peer_unsubscribe.zulip',
{stream_name: sub, user_email: email});

4
zerver/lib/actions.py
View File

@ -1614,7 +1614,7 @@ def bulk_remove_subscriptions(users, streams):
event = dict(type="subscription",
op="peer_remove",
subscriptions=[stream.name],
user_email=removed_user.email)
user_id=removed_user.id)
send_event(event, peer_user_ids)
return ([(sub.user_profile, stream) for (sub, stream) in subs_to_deactivate],
@ -3059,7 +3059,7 @@ def apply_events(state, events, user_profile):
user_id not in sub['subscribers']):
sub['subscribers'].append(user_id)
elif event['op'] == 'peer_remove':
user_id = get_user_profile_by_email(event['user_email']).id
user_id = event['user_id']
for sub in state['subscriptions']:
if (sub['name'] in event['subscriptions'] and
user_id in sub['subscribers']):

2
zerver/tests/test_events.py
View File

@ -820,7 +820,7 @@ class EventsRegisterTest(ZulipTestCase):
peer_remove_schema_checker = check_dict([
('type', equals('subscription')),
('op', equals('peer_remove')),
('user_email', check_string),
('user_id', check_int),
('subscriptions', check_list(check_string)),
])
stream_update_schema_checker = check_dict([

30
zerver/tests/test_subs.py
View File

@ -1513,37 +1513,37 @@ class SubscriptionAPITest(ZulipTestCase):
for event in peer_events:
for user_id in event['users']:
for stream_name in event['event']['subscriptions']:
email = event['event']['user_email']
notifications.add((user_id, email, stream_name))
removed_user_id = event['event']['user_id']
notifications.add((user_id, removed_user_id, stream_name))
# POSITIVE CASES FIRST
self.assertIn((user3.id, email1, 'stream1'), notifications)
self.assertIn((user4.id, email1, 'stream1'), notifications)
self.assertIn((user3.id, user1.id, 'stream1'), notifications)
self.assertIn((user4.id, user1.id, 'stream1'), notifications)
self.assertIn((user3.id, email2, 'stream1'), notifications)
self.assertIn((user4.id, email2, 'stream1'), notifications)
self.assertIn((user3.id, user2.id, 'stream1'), notifications)
self.assertIn((user4.id, user2.id, 'stream1'), notifications)
self.assertIn((user1.id, email2, 'stream2'), notifications)
self.assertIn((user3.id, email2, 'stream2'), notifications)
self.assertIn((user4.id, email2, 'stream2'), notifications)
self.assertIn((user1.id, user2.id, 'stream2'), notifications)
self.assertIn((user3.id, user2.id, 'stream2'), notifications)
self.assertIn((user4.id, user2.id, 'stream2'), notifications)
self.assertIn((user3.id, email1, 'private_stream'), notifications)
self.assertIn((user3.id, email2, 'private_stream'), notifications)
self.assertIn((user3.id, user1.id, 'private_stream'), notifications)
self.assertIn((user3.id, user2.id, 'private_stream'), notifications)
# NEGATIVE
# don't be notified if you are being removed yourself
self.assertNotIn((user1.id, email1, 'stream1'), notifications)
self.assertNotIn((user1.id, user1.id, 'stream1'), notifications)
# don't send false notifications for folks that weren't actually
# subscribed int he first place
self.assertNotIn((user3.id, email1, 'stream2'), notifications)
self.assertNotIn((user3.id, user1.id, 'stream2'), notifications)
# don't send notifications for random people
self.assertNotIn((user3.id, email4, 'stream2'), notifications)
self.assertNotIn((user3.id, user4.id, 'stream2'), notifications)
# don't send notifications to unsubscribed people for private streams
self.assertNotIn((user4.id, email1, 'private_stream'), notifications)
self.assertNotIn((user4.id, user1.id, 'private_stream'), notifications)
def test_bulk_subscribe_MIT(self):
# type: () -> None