Permalink
Browse files

Fix type mismatches in streams.py.

  • Loading branch information...
timabbott committed Jan 26, 2016
1 parent e6e2584 commit 620411c0ea757c9f3f0ae08597e77de84f20ee97
Showing with 2 additions and 2 deletions.
  1. +2 −2 zerver/views/streams.py
View
@@ -199,7 +199,7 @@ def remove_subscriptions_backend(request, user_profile,
people_to_unsub = set(principal_to_user_profile(
user_profile, principal) for principal in principals)
else:
- people_to_unsub = [user_profile]
+ people_to_unsub = set([user_profile])
result = dict(removed=[], not_subscribed=[])
(removed, not_subscribed) = bulk_remove_subscriptions(people_to_unsub, streams)
@@ -279,7 +279,7 @@ def add_subscriptions_backend(request, user_profile,
return json_error("You can only invite other mit.edu users to invite-only streams.")
subscribers = set(principal_to_user_profile(user_profile, principal) for principal in principals)
else:
- subscribers = [user_profile]
+ subscribers = set([user_profile])
(subscribed, already_subscribed) = bulk_add_subscriptions(streams, subscribers)

2 comments on commit 620411c

@dimaqq

This comment has been minimized.

Show comment
Hide comment
@dimaqq

dimaqq Oct 14, 2016

I feel this change is terrible, it hides bad design.

IMO buld_add/remove_subs should accept an iterable.

OT, it's weird that order of arguments for add is opposite of that for remove.

I feel this change is terrible, it hides bad design.

IMO buld_add/remove_subs should accept an iterable.

OT, it's weird that order of arguments for add is opposite of that for remove.

@timabbott

This comment has been minimized.

Show comment
Hide comment
@timabbott

timabbott Oct 14, 2016

Member

Actually, bulk_add_subscriptions and bulk_remove_subscriptions do take Iterables; the problem here isn't that.

The issue mypy flagged here is that the subscribers variable on line 284 is a different type depending on which code path has been followed, which is confusing and creates risk of future bugs.

Member

timabbott replied Oct 14, 2016

Actually, bulk_add_subscriptions and bulk_remove_subscriptions do take Iterables; the problem here isn't that.

The issue mypy flagged here is that the subscribers variable on line 284 is a different type depending on which code path has been followed, which is confusing and creates risk of future bugs.

Please sign in to comment.