actions: Use transaction.atomic properly when removing subscriptions.

Previously, the transaction.atomic() was not properly scoped to ensure
that RealmAuditLog entries were created in the same transaction,
making it possible for state changes to not be properly recorded in
RealmAuditLog.
This commit is contained in:
Tim Abbott 2020-10-15 15:10:33 -07:00
parent 0b91526f28
commit caa939d2d5
1 changed files with 18 additions and 16 deletions

View File

@ -3064,8 +3064,8 @@ def bulk_remove_subscriptions(users: Iterable[UserProfile],
our_realm = users[0].realm
# TODO: XXX: This transaction really needs to be done at the serializeable
# transaction isolation level.
# We do all the database changes in a transaction to ensure
# RealmAuditLog entries are atomically created when making changes.
with transaction.atomic():
occupied_streams_before = list(get_occupied_streams(our_realm))
Subscription.objects.filter(
@ -3078,13 +3078,15 @@ def bulk_remove_subscriptions(users: Iterable[UserProfile],
event_last_message_id = get_last_message_id()
all_subscription_logs: (List[RealmAuditLog]) = []
for (sub, stream) in subs_to_deactivate:
all_subscription_logs.append(RealmAuditLog(realm=sub.user_profile.realm,
audit_log_entry = RealmAuditLog(
realm=sub.user_profile.realm,
acting_user=acting_user,
modified_user=sub.user_profile,
modified_stream=stream,
event_last_message_id=event_last_message_id,
event_type=RealmAuditLog.SUBSCRIPTION_DEACTIVATED,
event_time=event_time))
event_time=event_time)
all_subscription_logs.append(audit_log_entry)
# Now since we have all log objects generated we can do a bulk insert
RealmAuditLog.objects.bulk_create(all_subscription_logs)