Skip to content

fix(nimbus): fix rollout features form incorrectly submitting#16138

Open
moibra05 wants to merge 1 commit into
mainfrom
16119
Open

fix(nimbus): fix rollout features form incorrectly submitting#16138
moibra05 wants to merge 1 commit into
mainfrom
16119

Conversation

@moibra05

Copy link
Copy Markdown
Contributor

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

RJAK11 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rollout features form incorrect behaviour

2 participants