paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-10 17:07:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

bulk_add_subscriptions: Fix confusing access to user_profile.realm.

Browse Source 
Previously, we frequently accessed user_profile.realm from outside the
loops that interact with UserProfile objects.  This variable reuse
outside the loop could be confusing and should be a style/lint
violation.

While in this case, the behavior was correct (in that all users in the
loops were within the same realm), extracting a separate `realm`
variable significantly clarifies what's going on here.
This commit is contained in:
Tim Abbott
2018-12-03 10:35:32 -08:00
parent 779ed37cfa
commit d96624490e
1 changed files with 9 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
zerver/lib/actions.py
View File

@@ -2551,6 +2551,8 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
for sub in all_subs_query: for sub in all_subs_query:
subs_by_user[sub.user_profile_id].append(sub) subs_by_user[sub.user_profile_id].append(sub)
realm = users[0].realm
already_subscribed = [] # type: List[Tuple[UserProfile, Stream]] already_subscribed = [] # type: List[Tuple[UserProfile, Stream]]
subs_to_activate = [] # type: List[Tuple[Subscription, Stream]] subs_to_activate = [] # type: List[Tuple[Subscription, Stream]]
new_subs = [] # type: List[Tuple[UserProfile, int, Stream]] new_subs = [] # type: List[Tuple[UserProfile, int, Stream]]
@@ -2586,11 +2588,11 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
# TODO: XXX: This transaction really needs to be done at the serializeable # TODO: XXX: This transaction really needs to be done at the serializeable
# transaction isolation level. # transaction isolation level.
with transaction.atomic(): with transaction.atomic():
occupied_streams_before = list(get_occupied_streams(user_profile.realm)) occupied_streams_before = list(get_occupied_streams(realm))
Subscription.objects.bulk_create([sub for (sub, stream) in subs_to_add]) Subscription.objects.bulk_create([sub for (sub, stream) in subs_to_add])
sub_ids = [sub.id for (sub, stream) in subs_to_activate] sub_ids = [sub.id for (sub, stream) in subs_to_activate]
Subscription.objects.filter(id__in=sub_ids).update(active=True) Subscription.objects.filter(id__in=sub_ids).update(active=True)
occupied_streams_after = list(get_occupied_streams(user_profile.realm)) occupied_streams_after = list(get_occupied_streams(realm))
# Log Subscription Activities in RealmAuditLog # Log Subscription Activities in RealmAuditLog
event_time = timezone_now() event_time = timezone_now()
@@ -2598,7 +2600,7 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
all_subscription_logs = [] # type: (List[RealmAuditLog]) all_subscription_logs = [] # type: (List[RealmAuditLog])
for (sub, stream) in subs_to_add: for (sub, stream) in subs_to_add:
all_subscription_logs.append(RealmAuditLog(realm=sub.user_profile.realm, all_subscription_logs.append(RealmAuditLog(realm=realm,
acting_user=acting_user, acting_user=acting_user,
modified_user=sub.user_profile, modified_user=sub.user_profile,
modified_stream=stream, modified_stream=stream,
@@ -2606,7 +2608,7 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
event_type=RealmAuditLog.SUBSCRIPTION_CREATED, event_type=RealmAuditLog.SUBSCRIPTION_CREATED,
event_time=event_time)) event_time=event_time))
for (sub, stream) in subs_to_activate: for (sub, stream) in subs_to_activate:
all_subscription_logs.append(RealmAuditLog(realm=sub.user_profile.realm, all_subscription_logs.append(RealmAuditLog(realm=realm,
acting_user=acting_user, acting_user=acting_user,
modified_user=sub.user_profile, modified_user=sub.user_profile,
modified_stream=stream, modified_stream=stream,
@@ -2623,7 +2625,7 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
event = dict(type="stream", op="occupy", event = dict(type="stream", op="occupy",
streams=[stream.to_dict() streams=[stream.to_dict()
for stream in new_occupied_streams]) for stream in new_occupied_streams])
send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id)) send_event(realm, event, active_user_ids(realm.id))
# Notify all existing users on streams that users have joined # Notify all existing users on streams that users have joined
@@ -2655,7 +2657,7 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
# they get the "subscribe" notification, and the latter so # they get the "subscribe" notification, and the latter so
# they can manage the new stream. # they can manage the new stream.
# Realm admins already have all created private streams. # Realm admins already have all created private streams.
realm_admin_ids = [user.id for user in user_profile.realm.get_admin_users()] realm_admin_ids = [user.id for user in realm.get_admin_users()]
new_users_ids = [user.id for user in users if (user.id, stream.id) in new_streams and new_users_ids = [user.id for user in users if (user.id, stream.id) in new_streams and
user.id not in realm_admin_ids] user.id not in realm_admin_ids]
send_stream_creation_event(stream, new_users_ids) send_stream_creation_event(stream, new_users_ids)
@@ -2692,7 +2694,7 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
event = dict(type="subscription", op="peer_add", event = dict(type="subscription", op="peer_add",
subscriptions=[stream.name], subscriptions=[stream.name],
user_id=new_user_id) user_id=new_user_id)
send_event(stream.realm, event, peer_user_ids) send_event(realm, event, peer_user_ids)
return ([(user_profile, stream) for (user_profile, recipient_id, stream) in new_subs] + return ([(user_profile, stream) for (user_profile, recipient_id, stream) in new_subs] +
[(sub.user_profile, stream) for (sub, stream) in subs_to_activate], [(sub.user_profile, stream) for (sub, stream) in subs_to_activate],