fix(nimbus): fix rollout features form incorrectly submitting

experimenter/experimenter/nimbus_ui/new/forms.py

@@ -278,14 +278,13 @@ def __init__(self, *args, **kwargs):
                     self.instance.feature_config.slug
                 )
 
+        feature_config = (
+            self.instance.feature_config if self.instance.feature_config_id else None
+        )
+
         if (
-            self.instance.id is not None
-            and self.instance.feature_config
-            and (
-                schema := self.instance.feature_config.schemas.filter(
-                    version=None
-                ).first()
-            )
+            feature_config
+            and (schema := feature_config.schemas.filter(version=None).first())
             and schema is not None
             and schema.schema is not None
         ):
@@ -303,10 +302,34 @@ def clean_value(self):
         return value
 
 
+class RolloutBranchFeatureValueForm(NimbusBranchFeatureValueForm):
+    class Meta:
+        model = NimbusBranchFeatureValue
+        fields = ("feature_config", "value")
+        widgets = {"feature_config": forms.HiddenInput()}
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        if (
+            self.is_bound
+            and not self.instance.feature_config_id
+            and (feature_config_id := self.data.get(f"{self.prefix}-feature_config"))
+        ):
+            self.instance.feature_config = NimbusFeatureConfig.objects.filter(
+                id=feature_config_id
+            ).first()
+
+    def clean_feature_config(self):
+        return self.cleaned_data.get("feature_config") or (
+            self.instance.feature_config if self.instance.feature_config_id else None
+        )
+
+
 RolloutBranchFeatureValueFormSet = inlineformset_factory(
     NimbusBranch,
     NimbusBranchFeatureValue,
-    form=NimbusBranchFeatureValueForm,
+    form=RolloutBranchFeatureValueForm,
     extra=0,
 )
 
@@ -693,7 +716,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
         self.branch_feature_values = RolloutBranchFeatureValueFormSet(
-            data=self.data or None,
+            data=self.get_branch_feature_values_data(),
             instance=self.reference_branch,
             prefix="branch-feature-value",
         )
@@ -717,6 +740,47 @@ def __init__(self, *args, **kwargs):
         # will remain unused as rollouts donot have results data
         self.fields["rollout_experience"].initial = self.instance.takeaways_summary
 
+    def get_branch_feature_values_data(self):
+        # Add temporary formset rows so newly selected, unsaved features get JSON
+        # editors during the HTMX preview before Save persists them.
+        if not self.is_bound:
+            return None
+
+        data = self.data.copy()
+        prefix = "branch-feature-value"
+        total_forms_key = "branch-feature-value-TOTAL_FORMS"
+        total_forms = int(data[total_forms_key])
+
+        selected_feature_config_values = (
+            data.getlist("feature_configs")
+            if hasattr(data, "getlist")
+            else data.get("feature_configs", [])
+        )
+        selected_feature_config_ids = [
+            int(feature_config_id)
+            for feature_config_id in selected_feature_config_values
+            if feature_config_id
+        ]
+        submitted_feature_config_ids = {
+            int(feature_config_id)
+            for index in range(total_forms)
+            if (feature_config_id := data.get(f"{prefix}-{index}-feature_config"))
+        }
+
+        new_feature_config_ids = [
+            feature_config_id
+            for feature_config_id in selected_feature_config_ids
+            if feature_config_id not in submitted_feature_config_ids
+        ]
+
+        for feature_config_id in new_feature_config_ids:
+            data[f"{prefix}-{total_forms}-feature_config"] = feature_config_id
+            data[f"{prefix}-{total_forms}-value"] = "{}"
+            total_forms += 1
+
+        data[total_forms_key] = str(total_forms)
+        return data
+
     @property
     def errors(self):
         errors = super().errors

experimenter/experimenter/nimbus_ui/new/views.py

@@ -159,17 +159,16 @@ class NewRolloutFeaturesUpdateView(CardMixin, NewCardUpdateView):
     display_template = "new/rollouts/rollout_features/card.html"
     template_name = "new/rollouts/rollout_features/edit_form.html"
 
-    def render_valid_response(self):
-        self.object.refresh_from_db()
-
+    def form_valid(self, form):
         # If the request came from the explicit Save button, return the read-only card
         # view so the UI swaps back to the card. When the form is posted for intermediate
         # updates (e.g. feature_configs changed via hx-post on change), return the
         # editable form so the user can continue editing.
+
         if "save" in self.request.POST:
-            return super().render_valid_response()
+            return super().form_valid(form)
 
-        return self.render_to_response(self.get_context_data())
+        return self.render_to_response(self.get_context_data(form=form))
 
 
 class NewQAUpdateView(CardMixin, NewCardUpdateView):

experimenter/experimenter/nimbus_ui/templates/new/rollouts/rollout_features/edit_form.html

@@ -35,6 +35,7 @@
         {{ branch_feature_values.management_form }}
         {% for branch_feature_values_form in branch_feature_values %}
           {{ branch_feature_values_form.id }}
+          {{ branch_feature_values_form.feature_config }}
           <div class="row mt-2">
             <div class="col">
               <label class="small mb-2 text-secondary">
@@ -85,5 +86,5 @@
 </div>
 {% block extrascripts %}
   {% if branch_form_data %}{{ branch_form_data|json_script:"branch-form-data" }}{% endif %}
-  <script src="{% static 'nimbus_ui/edit_branches.bundle.js' %}"></script>
+  <script src="{% static 'nimbus_ui/new_edit_branches.bundle.js' %}"></script>
 {% endblock extrascripts %}

experimenter/experimenter/nimbus_ui/tests/test_new_forms.py

@@ -678,6 +678,105 @@ def test_form_sets_initial_rollout_experience_and_filters_feature_configs(self):
             )
         )
 
+    def test_selected_feature_without_value_gets_temporary_form(self):
+        feature_config = NimbusFeatureConfigFactory.create(
+            application=NimbusExperiment.Application.DESKTOP,
+            slug="rollout-feature-preview",
+        )
+        experiment = NimbusExperimentFactory.create_with_lifecycle(
+            NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
+            feature_configs=[],
+        )
+
+        form = RolloutFeaturesForm(
+            instance=experiment,
+            data={
+                "rollout_experience": "Updated rollout experience",
+                "feature_configs": [feature_config.id],
+                "branch-feature-value-TOTAL_FORMS": "0",
+                "branch-feature-value-INITIAL_FORMS": "0",
+                "branch-feature-value-MIN_NUM_FORMS": "0",
+                "branch-feature-value-MAX_NUM_FORMS": "1000",
+            },
+            request=self.request,
+        )
+
+        self.assertEqual(len(form.branch_feature_values.forms), 1)
+        feature_value_form = form.branch_feature_values.forms[0]
+        self.assertEqual(feature_value_form.instance.feature_config, feature_config)
+        self.assertEqual(feature_value_form["value"].value(), "{}")
+        self.assertEqual(experiment.feature_configs.count(), 0)
+
+    def test_temporary_feature_value_form_saves_typed_value(self):
+        feature_config = NimbusFeatureConfigFactory.create(
+            application=NimbusExperiment.Application.DESKTOP,
+            slug="rollout-feature-preview-save",
+        )
+        experiment = NimbusExperimentFactory.create_with_lifecycle(
+            NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
+            feature_configs=[],
+        )
+
+        form = RolloutFeaturesForm(
+            instance=experiment,
+            data={
+                "rollout_experience": "Updated rollout experience",
+                "feature_configs": [feature_config.id],
+                "branch-feature-value-TOTAL_FORMS": "1",
+                "branch-feature-value-INITIAL_FORMS": "0",
+                "branch-feature-value-MIN_NUM_FORMS": "0",
+                "branch-feature-value-MAX_NUM_FORMS": "1000",
+                "branch-feature-value-0-feature_config": feature_config.id,
+                "branch-feature-value-0-value": '{"enabled": true}',
+            },
+            request=self.request,
+        )
+
+        self.assertTrue(form.is_valid(), form.errors)
+        experiment = form.save()
+        experiment.refresh_from_db()
+
+        feature_value = experiment.reference_branch.feature_values.get(
+            feature_config=feature_config
+        )
+        self.assertEqual(feature_value.value, '{"enabled": true}')
+
+    def test_selected_feature_without_submitted_value_saves_default_value(self):
+        feature_config = NimbusFeatureConfigFactory.create(
+            application=NimbusExperiment.Application.DESKTOP,
+            slug="rollout-feature-default-value",
+        )
+        experiment = NimbusExperimentFactory.create_with_lifecycle(
+            NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
+            feature_configs=[],
+        )
+
+        form = RolloutFeaturesForm(
+            instance=experiment,
+            data={
+                "rollout_experience": "Updated rollout experience",
+                "feature_configs": [feature_config.id],
+                "branch-feature-value-TOTAL_FORMS": "0",
+                "branch-feature-value-INITIAL_FORMS": "0",
+                "branch-feature-value-MIN_NUM_FORMS": "0",
+                "branch-feature-value-MAX_NUM_FORMS": "1000",
+            },
+            request=self.request,
+        )
+
+        self.assertTrue(form.is_valid(), form.errors)
+        form.branch_feature_values.save = lambda: None
+        experiment = form.save()
+        experiment.refresh_from_db()
+
+        feature_value = experiment.reference_branch.feature_values.get(
+            feature_config=feature_config
+        )
+        self.assertEqual(feature_value.value, "{}")
+
     def test_form_reports_branch_feature_value_errors(self):
         feature_config = NimbusFeatureConfigFactory.create(
             application=NimbusExperiment.Application.DESKTOP,
@@ -699,6 +798,7 @@ def test_form_reports_branch_feature_value_errors(self):
                 "branch-feature-value-INITIAL_FORMS": "1",
                 "branch-feature-value-MIN_NUM_FORMS": "0",
                 "branch-feature-value-MAX_NUM_FORMS": "1000",
+                "branch-feature-value-0-feature_config": feature_config.id,
                 "branch-feature-value-0-value": "{}",
             },
             request=self.request,
@@ -787,6 +887,7 @@ def test_form_deletes_removed_reference_branch_feature_values(self):
                 "branch-feature-value-MIN_NUM_FORMS": "0",
                 "branch-feature-value-MAX_NUM_FORMS": "1000",
                 "branch-feature-value-0-id": reference_feature_value.id,
+                "branch-feature-value-0-feature_config": feature_config.id,
                 "branch-feature-value-0-value": reference_feature_value.value,
             },
             request=self.request,

experimenter/experimenter/nimbus_ui/tests/test_new_views.py

@@ -12,6 +12,7 @@
 from experimenter.experiments.tests.factories import (
     NimbusDocumentationLinkFactory,
     NimbusExperimentFactory,
+    NimbusFeatureConfigFactory,
     TagFactory,
 )
 from experimenter.nimbus_ui.new.forms import (
@@ -356,25 +357,34 @@ def test_post_valid_saves_and_returns_display_card(self):
         self.assertTrue(response.context["hx_swap_oob"])
 
     def test_post_change_returns_edit_form(self):
+        feature_config = NimbusFeatureConfigFactory.create(
+            application=NimbusExperiment.Application.DESKTOP,
+            slug="rollout-feature",
+        )
         experiment = NimbusExperimentFactory.create_with_lifecycle(
             NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
             feature_configs=[],
+            takeaways_summary="Original rollout experience",
         )
 
         response = self.client.post(
             reverse(self.url_name, kwargs={"slug": experiment.slug}),
             {
                 "rollout_experience": "Updated rollout experience",
-                "feature_configs": [],
+                "feature_configs": [feature_config.id],
                 "branch-feature-value-TOTAL_FORMS": "0",
                 "branch-feature-value-INITIAL_FORMS": "0",
             },
         )
 
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, "new/rollouts/rollout_features/edit_form.html")
+        self.assertContains(response, "rollout-feature")
+        self.assertContains(response, "value-editor")
         experiment.refresh_from_db()
-        self.assertEqual(experiment.takeaways_summary, "Updated rollout experience")
+        self.assertEqual(experiment.takeaways_summary, "Original rollout experience")
+        self.assertEqual(experiment.feature_configs.count(), 0)
 
 
 class TestNewQAUpdateView(NewViewTestMixin, AuthTestCase):