fix(nimbus): fix rollout features form incorrectly submitting

[moibra05]

Because

• Rollout feature changes should be editable before Save without being persisted by intermediate HTMX updates

This commit

• Prevents intermediate rollout feature form updates from saving
• Renders temporary feature value editors for newly selected rollout features

Fixes #16119

[RJAK11]

I tested the changes locally and it looks great to me!

One small thing: When I fill out the rollout experience description and then go back to the old UI's branch page, the description field there is left empty, which triggers a validation error. It looks like the new rollout experience description is stored separately from the field the old UI uses. Just wanted to confirm that separation was intentional since we might have to make additional changes later on to avoid that validation error.

[moibra05]

I tested the changes locally and it looks great to me!

One small thing: When I fill out the rollout experience description and then go back to the old UI's branch page, the description field there is left empty, which triggers a validation error. It looks like the new rollout experience description is stored separately from the field the old UI uses. Just wanted to confirm that separation was intentional since we might have to make additional changes later on to avoid that validation error.

if im not mistaken the rollout experience field is a new field entirely and i wasnt sure whether the better approach was to introduce it as a new field with a migration, or to reuse a similarly shaped existing field like takeaways_summary.

the main issue i saw with introducing a new field is that all non-rollout experiments would leave it blank since it doesnt apply to them. that said, i can see that the current approach may make it unclear where the rollout experience description is actually stored, especially when switching between the new and old UIs

[RJAK11]

Ah that makes sense! I agree with your approach of adding a new field then since it is different.

One other thing: for preventing intermediate updates from saving I did something similar for the rollout phases, but @yashikakhurana suggested following the pattern from Documentation Links in the old UI, where changes persist right away for simplicity so maybe that might apply here as well but I'm not too sure.

[yashikakhurana]

is this how we are doing with the current forms?

[yashikakhurana]

you have 2 options to do that- 1- add temporary forms after formset init instead of patching POST data

Instead of manipulating raw POST data before passing to the formset, build extra forms directly after:

self.branch_feature_values = RolloutBranchFeatureValueFormSet(
data=self.data or None,
instance=self.reference_branch,
prefix="branch-feature-value",
)

Add temporary forms for newly selected, unsaved features

if self.is_bound:
saved_ids = {
f.instance.feature_config_id
for f in self.branch_feature_values.forms
if f.instance.feature_config_id
}
for feature_config_id in selected_feature_config_ids:
if feature_config_id not in saved_ids:
temp_instance = NimbusBranchFeatureValue(
branch=self.reference_branch,
feature_config_id=feature_config_id,
value="{}",
)
self.branch_feature_values.forms.append(
RolloutBranchFeatureValueForm(instance=temp_instance)
)

2nd- keep current approach but simplify)

Keep get_branch_feature_values_data() but extract the "find new feature IDs" logic:

def _get_new_feature_config_ids(self, data, prefix, total_forms):
selected = [int(id) for id in data.getlist("feature_configs") if id]
submitted = {
int(data[f"{prefix}-{i}-feature_config"])
for i in range(total_forms)
if data.get(f"{prefix}-{i}-feature_config")
}
return [id for id in selected if id not in submitted]