Skip to content

chore(nimbus): always use transaction.atomic for form saves #14189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jaredlockhart merged 1 commit into from
Dec 15, 2025
Merged

chore(nimbus): always use transaction.atomic for form saves #14189

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions experimenter/experimenter/nimbus_ui/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import markus
from django import forms
from django.contrib.auth.models import User
from django.db import transaction
from django.db.models import Case, When
from django.forms import BaseInlineFormSet, BaseModelFormSet, inlineformset_factory
from django.http import HttpRequest
Expand Down Expand Up @@ -48,6 +49,7 @@ def __init__(self, *args, request: HttpRequest = None, **kwargs):
def get_changelog_message(self) -> str:
raise NotImplementedError

@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)

Expand Down Expand Up @@ -137,6 +139,7 @@ def clean(self):
cleaned_data["slug"] = slugify(cleaned_data["name"])
return cleaned_data

@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)

Expand Down Expand Up @@ -186,6 +189,7 @@ def clean(self):
def get_changelog_message(self):
return f"{self.request.user} cloned this experiment from {self.instance.name}"

@transaction.atomic
def save(self):
return self.instance.clone(self.cleaned_data["name"], self.cleaned_data["owner"])

Expand Down Expand Up @@ -213,6 +217,7 @@ class Meta:
model = NimbusExperiment
fields = ["owner", "name", "slug"]

@transaction.atomic
def save(self):
return self.instance.clone(
self.cleaned_data["name"],
Expand All @@ -226,6 +231,7 @@ class Meta:
model = NimbusExperiment
fields = []

@transaction.atomic
def save(self):
self.instance.is_archived = not self.instance.is_archived
super().save()
Expand All @@ -249,6 +255,7 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._initial_qa_status = self.instance.qa_status if self.instance.pk else None

@transaction.atomic
def save(self, *args, **kwargs):
new_qa_status = self.cleaned_data.get("qa_status")
if self._initial_qa_status != new_qa_status:
Expand Down Expand Up @@ -418,6 +425,7 @@ def __init__(self, *args, **kwargs):
def is_valid(self):
return super().is_valid() and self.documentation_links.is_valid()

@transaction.atomic
def save(self):
experiment = super().save()
self.documentation_links.save()
Expand All @@ -428,6 +436,7 @@ def get_changelog_message(self):


class DocumentationLinkCreateForm(OverviewForm):
@transaction.atomic
def save(self, *args, **kwargs):
super().save(*args, **kwargs)
self.instance.documentation_links.create()
Expand All @@ -444,6 +453,7 @@ class Meta:
model = NimbusExperiment
fields = [*OverviewForm.Meta.fields, "link_id"]

@transaction.atomic
def save(self, *args, **kwargs):
super().save(*args, **kwargs)
documentation_link = self.cleaned_data["link_id"]
Expand Down Expand Up @@ -616,6 +626,7 @@ def clean(self):
cleaned_data["slug"] = slugify(cleaned_data["name"])
return cleaned_data

@transaction.atomic
def save(self, *args, **kwargs):
branch = super().save(*args, **kwargs)
self.branch_feature_values.save()
Expand Down Expand Up @@ -803,6 +814,7 @@ def clean(self):

return cleaned_data

@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)
self.branches.save()
Expand Down Expand Up @@ -847,6 +859,7 @@ def get_changelog_message(self):


class NimbusBranchCreateForm(NimbusBranchesForm):
@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)
if not experiment.reference_branch:
Expand Down Expand Up @@ -889,6 +902,7 @@ def clean_branch_id(self):
raise forms.ValidationError("You cannot delete the reference branch.")
return branch

@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)
branch = self.cleaned_data["branch_id"]
Expand All @@ -909,6 +923,7 @@ class Meta:
*NimbusBranchesForm.Meta.fields,
]

@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)
branch = self.cleaned_data["branch_id"]
Expand All @@ -929,6 +944,7 @@ class Meta:
*NimbusBranchesForm.Meta.fields,
]

@transaction.atomic
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)
screenshot = self.cleaned_data["screenshot_id"]
Expand Down Expand Up @@ -1243,6 +1259,7 @@ def check_rollout_dirty(self, instance):
):
instance.is_rollout_dirty = True

@transaction.atomic
def save(self, *args, **kwargs):
instance = super().save(*args, **kwargs)
self.check_rollout_dirty(instance)
Expand All @@ -1265,6 +1282,7 @@ class Meta:
model = NimbusExperiment
fields = []

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
experiment.subscribers.add(self.request.user)
Expand All @@ -1279,6 +1297,7 @@ class Meta:
model = NimbusExperiment
fields = []

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
experiment.subscribers.remove(self.request.user)
Expand All @@ -1289,12 +1308,14 @@ def get_changelog_message(self):


class FeatureSubscribeForm(FeatureSubscriberFormMixin):
@transaction.atomic
def save(self, commit=True):
self.instance.subscribers.add(self.request.user)
return self.instance


class FeatureUnsubscribeForm(FeatureSubscriberFormMixin):
@transaction.atomic
def save(self, commit=True):
self.instance.subscribers.remove(self.request.user)
return self.instance
Expand All @@ -1303,6 +1324,7 @@ def save(self, commit=True):
class SlackNotificationMixin:
slack_action = None

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
if self.slack_action:
Expand All @@ -1325,6 +1347,7 @@ class Meta:
model = NimbusExperiment
fields = []

@transaction.atomic
def save(self, commit=True):
self.instance.status = self.status
self.instance.status_next = self.status_next
Expand Down Expand Up @@ -1362,6 +1385,7 @@ class DraftToPreviewForm(UpdateStatusForm):
def get_changelog_message(self):
return f"{self.request.user} launched experiment to Preview"

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
experiment.allocate_bucket_range()
Expand Down Expand Up @@ -1398,6 +1422,7 @@ class PreviewToDraftForm(UpdateStatusForm):
def get_changelog_message(self):
return f"{self.request.user} moved the experiment back to Draft"

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
nimbus_synchronize_preview_experiments_in_kinto.apply_async(countdown=5)
Expand Down Expand Up @@ -1433,6 +1458,7 @@ class ReviewToApproveForm(UpdateStatusForm):
def get_changelog_message(self):
return f"{self.request.user} approved the review."

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
experiment.allocate_bucket_range()
Expand Down Expand Up @@ -1488,6 +1514,7 @@ class ApproveEndEnrollmentForm(UpdateStatusForm):
def get_changelog_message(self):
return f"{self.request.user} approved the end enrollment request"

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
nimbus_check_kinto_push_queue_by_collection.apply_async(
Expand All @@ -1514,6 +1541,7 @@ class ApproveEndExperimentForm(UpdateStatusForm):
def get_changelog_message(self):
return f"{self.request.user} approved the end experiment request"

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
nimbus_check_kinto_push_queue_by_collection.apply_async(
Expand Down Expand Up @@ -1608,6 +1636,7 @@ class ApproveUpdateRolloutForm(UpdateStatusForm):
def get_changelog_message(self):
return f"{self.request.user} approved the update review request"

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
experiment.allocate_bucket_range()
Expand Down Expand Up @@ -1712,6 +1741,7 @@ def clean(self):
if len(names) != len(set(names)):
raise forms.ValidationError(NimbusUIConstants.ERROR_TAG_DUPLICATE_NAME)

@transaction.atomic
def create_tag(self):
# Create a new tag with a unique name and random color
base_name = "Tag"
Expand Down Expand Up @@ -1766,6 +1796,7 @@ def __init__(self, *args, **kwargs):
if self.instance and self.instance.pk:
self.fields["collaborators"].initial = self.instance.subscribers.all()

@transaction.atomic
def save(self, commit=True):
experiment = super().save(commit=commit)
if commit:
Expand Down