Skip to content

feat(nimbus): add rollout setup progress and issues modal#16239

Open
RJAK11 wants to merge 3 commits into
mainfrom
15927
Open

feat(nimbus): add rollout setup progress and issues modal#16239
RJAK11 wants to merge 3 commits into
mainfrom
15927

Conversation

@RJAK11

@RJAK11 RJAK11 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Because

  • users need to know their rollout setup progress

This commit

  • adds a setup completion progress bar to the rollout sidebar
  • adds an issues modal that lists invalid fields with links to the relevant card

Fixes #15927

Screen.Recording.2026-07-08.at.3.01.38.PM.mov

@RJAK11

RJAK11 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

When testing locally with the "this is a rollout" checkbox checked, you'll hit 3 additional warnings not in the video for the population percent, total enrolled clients, and reference branch fields.

A few thoughts on handling these:

  • Total enrolled clients doesn't apply to the new UI, and I was told we can just default it to 1 when we add the "create new rollout" flow, until we create a dedicated rollout serializer. I think the same idea works for Reference branch.
  • For Population percent, the current serializer doesn't support the new rollout phases model, so I was thinking maybe we could also hardcode a value here too since it'll get overwritten anyway once the user requests to start a phase.

Separately, we might need to bring over the AI risk field from the old UI into the new Risks card, and Public description into the Overview card or we can handle them the same way since we don’t currently have those fields. Let me know what you think!

@RJAK11
RJAK11 marked this pull request as draft July 13, 2026 19:10
@RJAK11
RJAK11 force-pushed the 15927 branch 3 times, most recently from 254cbe9 to 26798a0 Compare July 15, 2026 14:52
* adds a setup completion progress bar to the rollout sidebar
* adds an issues modal that lists invalid fields with links to the relevant card
* allows rollouts to set 0% population, 0 enrolled clients, and no reference branch description
* validates the rollout schedule (at least one phase, nonzero first phase) via NimbusRolloutReviewSerializer

Fixes #15927
@RJAK11

RJAK11 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Update: instead of hard-coding some values to prevent the warnings mentioned previously, I added a dedicated NimbusRolloutReviewSerializer that the new UI uses (the old UI keeps the base serializer, so this is all scoped to the new UI). That clears the 3 warnings:

  • Total enrolled clients and Reference branch: changed to allow empty values for rollouts (since neither seems to apply to the new UI).
  • Population percent: allows 0% since population now comes from the rollout phases model. To keep that meaningful, I added schedule validation (>= 1 phase, first phase non-zero) that shows up as a setup issue on the Rollout schedule card.

However, we may still want to add AI risk in the Risks card and Public description in the Overview card.

@RJAK11
RJAK11 marked this pull request as ready for review July 15, 2026 16:52
Comment thread experimenter/experimenter/nimbus_ui/templates/new/rollouts/schedule/card.html Outdated
RJAK11 added 2 commits July 17, 2026 15:25
# Conflicts:
#	experimenter/experimenter/nimbus_ui/templates/new/rollouts/audience/card.html
#	experimenter/experimenter/nimbus_ui/templates/new/rollouts/rollout_features/card.html
#	experimenter/experimenter/nimbus_ui/tests/test_new_views.py
Because

* rollout authors need to know how much and what is left before a rollout can be launched

This commit

* adds a setup completion progress bar to the rollout sidebar
* adds an issues modal that lists invalid fields with links to the relevant rollout card

@yashikakhurana yashikakhurana left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @RJAK11 this is really great, nice approach to use separate serializer, and I have tested it locally, majorly works fine, some improvements to make, otherwise very good work

Comment on lines +135 to +145
tracked_fields = set()
for fields in self.SETUP_SECTIONS.values():
tracked_fields.update(fields)
tracked_fields.update(self.NON_FIELD_ISSUE_CARDS)
tracked_fields.update(error_keys)

invalid_tracked = error_keys & tracked_fields
context["setup_completion_percent"] = round(
100 * (len(tracked_fields) - len(invalid_tracked)) / len(tracked_fields)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tracked_fields.update(error_keys) adds every erroring field into the denominator, which causes two issues:

error_keys & tracked_fields then always equals error_keys, so the & never filters anything (could just be len(error_keys)).
The total keeps changing — an unexpected error grows the denominator, so "100%" isn't a stable baseline.
Can we keep the denominator fixed to the known fields and only count broken ones?

tracked_fields = set()
for fields in self.SETUP_SECTIONS.values():
tracked_fields.update(fields)
tracked_fields.update(self.NON_FIELD_ISSUE_CARDS)
# don't add error_keys — keep total fixed

broken = error_keys & tracked_fields
percent = round(100 * (len(tracked_fields) - len(broken)) / len(tracked_fields))

One decision for us to make first: should an unexpected error drag the progress bar down, or just show up in the issues list without affecting the percent? With the fixed denominator it does the latter. Right now test_setup_progress_untracked_issue_lowers_completion expects the former — so let's agree on the intended behavior and make the code + test match. My vote is the bar tracks only known setup fields, and unknown errors just appear as issues.

max_value=100.0,
required=True,
error_messages={"min_value": NimbusConstants.ERROR_POPULATION_PERCENT_MIN},
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

min_value=0.0 is what we want for rollouts, but you're still passing the old "must be >= 0.0001" message — it can never fire

queryset=NimbusRolloutPhase.objects.all(),
)

def validate_reference_branch(self, value):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a comment here why we are simply returning the value for the future purpose

Comment on lines +99 to +119
SETUP_SECTIONS = {
"Overview": {*RolloutOverviewForm.Meta.fields},
"Risks": {*RolloutRisksForm.Meta.fields},
"Features": {*RolloutFeaturesForm.Meta.fields},
"Audience": {*RolloutAudienceForm.Meta.fields},
}

SECTION_CARD_IDS = {
"Overview": "overview",
"Risks": "risks",
"Features": "rollout-features",
"Audience": "audience",
}

NON_FIELD_ISSUE_CARDS = {
"rollout_phases": {
"section": "Rollout schedule",
"card_id": "schedule",
"label": "Rollout Schedule",
},
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SETUP_SECTIONS, SECTION_CARD_IDS, NON_FIELD_ISSUE_CARDS
All keyed by section, so easy to fall out of sync. Could we merge into one structure so a new section is added in one place?

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.

Add Setup Completion Progress Bar

2 participants