Skip to content

Commit a88b836

Browse files
committed
feat(nimbus): add rollout setup progress and issues modal
* 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
1 parent 369f248 commit a88b836

16 files changed

Lines changed: 451 additions & 10 deletions

File tree

experimenter/experimenter/experiments/api/v5/serializers.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
NimbusExperiment,
3636
NimbusFeatureConfig,
3737
NimbusFeatureVersion,
38+
NimbusRolloutPhase,
3839
NimbusVersionedSchema,
3940
)
4041
from experimenter.features.manifests.nimbus_fml_loader import NimbusFmlLoader
@@ -1994,6 +1995,39 @@ def validate(self, data):
19941995
return data
19951996

19961997

1998+
class NimbusRolloutReviewSerializer(NimbusReviewSerializer):
1999+
population_percent = serializers.DecimalField(
2000+
7,
2001+
4,
2002+
min_value=0.0,
2003+
max_value=100.0,
2004+
required=True,
2005+
error_messages={"min_value": NimbusConstants.ERROR_POPULATION_PERCENT_MIN},
2006+
)
2007+
total_enrolled_clients = serializers.IntegerField(required=False, min_value=0)
2008+
rollout_phases = serializers.PrimaryKeyRelatedField(
2009+
many=True,
2010+
required=False,
2011+
queryset=NimbusRolloutPhase.objects.all(),
2012+
)
2013+
2014+
def validate_reference_branch(self, value):
2015+
return value
2016+
2017+
def validate_rollout_phases(self, value):
2018+
if self.instance and not self.instance.is_rollout:
2019+
return value
2020+
2021+
phases = list(value)
2022+
if not phases:
2023+
raise serializers.ValidationError(NimbusConstants.ERROR_ROLLOUT_NO_PHASES)
2024+
if not phases[0].population_percent:
2025+
raise serializers.ValidationError(
2026+
NimbusConstants.ERROR_ROLLOUT_FIRST_PHASE_ZERO
2027+
)
2028+
return value
2029+
2030+
19972031
class LocalizationError(Exception):
19982032
"""An error that occurs during localization substitution."""
19992033

experimenter/experimenter/experiments/constants.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,10 @@ class AnalysisWindow(models.TextChoices):
721721
"Launching experiments has been temporarily disabled by the site administrators."
722722
)
723723
ERROR_POPULATION_PERCENT_MIN = "Ensure this value is greater than or equal to 0.0001."
724+
ERROR_ROLLOUT_NO_PHASES = "Add at least one rollout phase to the schedule."
725+
ERROR_ROLLOUT_FIRST_PHASE_ZERO = (
726+
"The first rollout phase must have a population percent greater than 0."
727+
)
724728
ERROR_FIREFOX_VERSION_MIN = (
725729
"Ensure this value is less than or equal to the maximum version"
726730
)

experimenter/experimenter/experiments/models.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,11 +2422,14 @@ def has_results_errors(self):
24222422
return True
24232423
return False
24242424

2425-
def get_invalid_fields_errors(self):
2425+
def get_invalid_fields_errors(self, serializer_class=None):
24262426
from experimenter.experiments.api.v5.serializers import NimbusReviewSerializer
24272427

2428-
serializer_data = NimbusReviewSerializer(self).data
2429-
serializer = NimbusReviewSerializer(self, data=serializer_data)
2428+
if serializer_class is None:
2429+
serializer_class = NimbusReviewSerializer
2430+
2431+
serializer_data = serializer_class(self).data
2432+
serializer = serializer_class(self, data=serializer_data)
24302433

24312434
errors = {}
24322435
if not serializer.is_valid():

experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212
from experimenter.base.tests.factories import (
1313
LocaleFactory,
1414
)
15-
from experimenter.experiments.api.v5.serializers import NimbusReviewSerializer
15+
from experimenter.experiments.api.v5.serializers import (
16+
NimbusReviewSerializer,
17+
NimbusRolloutReviewSerializer,
18+
)
1619
from experimenter.experiments.constants import (
1720
ApplicationConfig,
1821
FirefoxLabs,
@@ -31,6 +34,7 @@
3134
NimbusExperimentFactory,
3235
NimbusFeatureConfigFactory,
3336
NimbusFmlErrorDataClass,
37+
NimbusRolloutPhaseFactory,
3438
NimbusVersionedSchemaFactory,
3539
)
3640
from experimenter.openidc.tests.factories import UserFactory
@@ -92,6 +96,15 @@ def get_review_serializer(self, experiment):
9296
context={"user": self.user},
9397
)
9498

99+
def get_rollout_review_serializer(self, experiment):
100+
return NimbusRolloutReviewSerializer(
101+
experiment,
102+
data=NimbusRolloutReviewSerializer(
103+
experiment, context={"user": self.user}
104+
).data,
105+
context={"user": self.user},
106+
)
107+
95108

96109
class TestNimbusReviewSerializerSingleFeature(
97110
GetReviewSerializerMixin, MockFmlErrorMixin, TestCase
@@ -104,6 +117,26 @@ def setUp(self):
104117
self.user = UserFactory()
105118
self.setup_fml_no_errors()
106119

120+
def create_rollout(self, **overrides):
121+
experiment = NimbusExperimentFactory.create_with_lifecycle(
122+
NimbusExperimentFactory.Lifecycles.CREATED,
123+
population_percent=0.0,
124+
application=NimbusExperiment.Application.DESKTOP,
125+
feature_configs=[
126+
NimbusFeatureConfigFactory(
127+
application=NimbusExperiment.Application.DESKTOP
128+
)
129+
],
130+
is_rollout=True,
131+
is_sticky=True,
132+
firefox_min_version=NimbusExperiment.MIN_REQUIRED_VERSION,
133+
**overrides,
134+
)
135+
for branch in experiment.treatment_branches:
136+
branch.delete()
137+
experiment.save()
138+
return experiment
139+
107140
def test_valid_experiment_with_single_feature(self):
108141
experiment = NimbusExperimentFactory.create_with_lifecycle(
109142
NimbusExperimentFactory.Lifecycles.CREATED,
@@ -503,6 +536,49 @@ def test_valid_experiment_minimum_population_percent(self):
503536
)
504537
self.assertTrue(serializer.is_valid(), serializer.errors)
505538

539+
def test_rollout_serializer_allows_relaxed_fields(self):
540+
experiment = self.create_rollout(total_enrolled_clients=0)
541+
experiment.reference_branch.description = ""
542+
experiment.reference_branch.save()
543+
NimbusRolloutPhaseFactory.create(experiment=experiment, population_percent=10)
544+
serializer = self.get_rollout_review_serializer(experiment)
545+
self.assertTrue(serializer.is_valid(), serializer.errors)
546+
547+
def test_base_serializer_rejects_relaxed_rollout_fields(self):
548+
experiment = self.create_rollout(total_enrolled_clients=0)
549+
experiment.reference_branch.description = ""
550+
experiment.reference_branch.save()
551+
serializer = self.get_review_serializer(experiment)
552+
self.assertFalse(serializer.is_valid())
553+
self.assertIn("population_percent", serializer.errors)
554+
self.assertIn("total_enrolled_clients", serializer.errors)
555+
self.assertIn("reference_branch", serializer.errors)
556+
557+
def test_rollout_serializer_requires_phases(self):
558+
experiment = NimbusExperimentFactory.create_with_lifecycle(
559+
NimbusExperimentFactory.Lifecycles.CREATED,
560+
is_rollout=True,
561+
public_description="",
562+
)
563+
serializer = self.get_rollout_review_serializer(experiment)
564+
self.assertFalse(serializer.is_valid())
565+
self.assertGreater(len(serializer.errors), 1)
566+
self.assertEqual(
567+
serializer.errors["rollout_phases"],
568+
[NimbusConstants.ERROR_ROLLOUT_NO_PHASES],
569+
)
570+
571+
def test_rollout_serializer_requires_nonzero_first_phase(self):
572+
experiment = self.create_rollout()
573+
NimbusRolloutPhaseFactory.create(experiment=experiment, population_percent=0)
574+
NimbusRolloutPhaseFactory.create(experiment=experiment, population_percent=50)
575+
serializer = self.get_rollout_review_serializer(experiment)
576+
self.assertFalse(serializer.is_valid())
577+
self.assertEqual(
578+
serializer.errors["rollout_phases"],
579+
[NimbusConstants.ERROR_ROLLOUT_FIRST_PHASE_ZERO],
580+
)
581+
506582
def test_invalid_experiment_treatment_branch_requires_description(self):
507583
experiment = NimbusExperimentFactory.create_with_lifecycle(
508584
NimbusExperimentFactory.Lifecycles.CREATED,

experimenter/experimenter/nimbus_ui/new/views.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from django.views.generic import DetailView
55
from django.views.generic.edit import UpdateView
66

7+
from experimenter.experiments.api.v5.serializers import NimbusRolloutReviewSerializer
78
from experimenter.experiments.models import NimbusExperiment, Tag
89
from experimenter.nimbus_ui.filtersets import (
910
TagSearchFilterSet,
@@ -93,7 +94,86 @@ def post(self, request, *args, **kwargs):
9394
return super().post(request, *args, **kwargs)
9495

9596

97+
class RolloutSetupProgressMixin:
98+
SETUP_SECTIONS = {
99+
"Overview": {*RolloutOverviewForm.Meta.fields},
100+
"Risks": {*RolloutRisksForm.Meta.fields},
101+
"Features": {*RolloutFeaturesForm.Meta.fields},
102+
"Audience": {*RolloutAudienceForm.Meta.fields},
103+
}
104+
105+
SECTION_CARD_IDS = {
106+
"Overview": "overview",
107+
"Risks": "risks",
108+
"Features": "rollout-features",
109+
"Audience": "audience",
110+
}
111+
112+
NON_FIELD_ISSUE_CARDS = {
113+
"rollout_phases": {
114+
"section": "Rollout schedule",
115+
"card_id": "schedule",
116+
"label": "Rollout Schedule",
117+
},
118+
}
119+
120+
def flatten_errors(self, messages):
121+
if isinstance(messages, dict):
122+
return [m for value in messages.values() for m in self.flatten_errors(value)]
123+
if isinstance(messages, (list, tuple)):
124+
return [m for value in messages for m in self.flatten_errors(value)]
125+
return [str(messages)]
126+
127+
def get_context_data(self, **kwargs):
128+
context = super().get_context_data(**kwargs)
129+
field_errors = self.object.get_invalid_fields_errors(
130+
serializer_class=NimbusRolloutReviewSerializer
131+
)
132+
error_keys = set(field_errors)
133+
134+
tracked_fields = set()
135+
for fields in self.SETUP_SECTIONS.values():
136+
tracked_fields.update(fields)
137+
tracked_fields.update(self.NON_FIELD_ISSUE_CARDS)
138+
tracked_fields.update(error_keys)
139+
140+
invalid_tracked = error_keys & tracked_fields
141+
context["setup_completion_percent"] = round(
142+
100 * (len(tracked_fields) - len(invalid_tracked)) / len(tracked_fields)
143+
)
144+
145+
field_to_section = {
146+
field: section
147+
for section, fields in self.SETUP_SECTIONS.items()
148+
for field in fields
149+
}
150+
issues = []
151+
for field, messages in field_errors.items():
152+
if field in self.NON_FIELD_ISSUE_CARDS:
153+
meta = self.NON_FIELD_ISSUE_CARDS[field]
154+
section = meta["section"]
155+
card_id = meta["card_id"]
156+
label = meta["label"]
157+
else:
158+
section = field_to_section.get(field, "Other")
159+
card_id = self.SECTION_CARD_IDS.get(section)
160+
label = field.replace("_", " ").title()
161+
issues.append(
162+
{
163+
"section": section,
164+
"card_id": card_id,
165+
"label": label,
166+
"messages": self.flatten_errors(messages),
167+
}
168+
)
169+
170+
context["setup_issues"] = issues
171+
context["setup_issues_count"] = len(issues)
172+
return context
173+
174+
96175
class NimbusRolloutDetailView(
176+
RolloutSetupProgressMixin,
97177
NimbusExperimentViewMixin,
98178
CloneExperimentFormMixin,
99179
DetailView,
@@ -117,6 +197,7 @@ def get_context_data(self, **kwargs):
117197

118198

119199
class NewCardUpdateView(
200+
RolloutSetupProgressMixin,
120201
NimbusExperimentViewMixin,
121202
RequestFormMixin,
122203
UpdateRedirectViewMixin,

experimenter/experimenter/nimbus_ui/static/js/index.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,30 @@ const cleanupOrphanedModalBackdrops = () => {
8484
}
8585
};
8686

87+
const setupSetupIssueNav = () => {
88+
document.addEventListener("click", function (event) {
89+
const trigger = event.target.closest("[data-setup-issue-target]");
90+
if (!trigger) {
91+
return;
92+
}
93+
94+
const cardId = trigger.getAttribute("data-setup-issue-target");
95+
const card = document.getElementById(`rollout-card-${cardId}`);
96+
if (!card) {
97+
return;
98+
}
99+
const scrollToCard = () =>
100+
card.scrollIntoView({ behavior: "smooth", block: "start" });
101+
102+
if (trigger.closest(".modal")) {
103+
setTimeout(scrollToCard, 300);
104+
} else {
105+
scrollToCard();
106+
}
107+
});
108+
};
109+
setupSetupIssueNav();
110+
87111
const setupHTMXLoadingOverlay = () => {
88112
document.addEventListener("htmx:beforeRequest", function () {
89113
const loadingOverlay = document.querySelector("#htmx-loading-overlay");
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<div id="sidebar-setup-issues-modal"
2+
{% if oob %}hx-swap-oob="true"{% endif %}>
3+
{% if setup_issues_count %}
4+
<div class="modal fade" id="setupIssuesModal" tabindex="-1">
5+
<div class="modal-dialog modal-dialog-scrollable">
6+
<div class="modal-content">
7+
<div class="modal-header">
8+
<h1 class="modal-title fs-5 d-flex align-items-center gap-2">Rollout Issues Detected</h1>
9+
<button type="button" class="btn-close" data-bs-dismiss="modal"></button>
10+
</div>
11+
<div class="modal-body bg-body-tertiary d-flex flex-column gap-2">
12+
{% for issue in setup_issues %}
13+
<div class="rounded-2 p-3 small"
14+
style="background-color: var(--bs-warning-bg-subtle);
15+
border: 1px solid var(--bs-warning-border-subtle)">
16+
<button type="button"
17+
class="rollout-card-toggle btn btn-link small p-0 w-100 d-flex align-items-center gap-2 text-black text-decoration-none collapsed"
18+
data-bs-toggle="collapse"
19+
data-bs-target="#setupIssue{{ forloop.counter }}">
20+
<i class="fa-solid fa-triangle-exclamation"
21+
style="color: var(--bs-orange)"></i>
22+
<span class="flex-grow-1 small text-start fw-semibold">{{ issue.label }}</span>
23+
<i class="fa-solid fa-chevron-right chevron-icon"
24+
style="font-size: 11px"></i>
25+
</button>
26+
<div class="collapse ms-4" id="setupIssue{{ forloop.counter }}">
27+
{% for message in issue.messages %}<div>{{ message }}</div>{% endfor %}
28+
{% if issue.card_id %}
29+
<button type="button"
30+
class="btn btn-sm mt-2"
31+
style="background-color: var(--bs-warning-border-subtle);
32+
border: 1px solid var(--bs-warning-border-subtle)"
33+
data-bs-dismiss="modal"
34+
data-setup-issue-target="{{ issue.card_id }}">
35+
Go to {{ issue.section }}
36+
<i class="fa-solid fa-arrow-right ms-1" style="font-size: 10px;"></i>
37+
</button>
38+
{% endif %}
39+
</div>
40+
</div>
41+
{% endfor %}
42+
</div>
43+
</div>
44+
</div>
45+
</div>
46+
{% endif %}
47+
</div>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<div id="sidebar-setup-progress" {% if oob %}hx-swap-oob="true"{% endif %}>
2+
<div class="mb-2">
3+
<div class="small mb-1 text-secondary">{{ setup_completion_percent|default:0 }}% complete</div>
4+
<div class="rounded-pill bg-secondary-subtle" style="height: 3px;">
5+
<div class="rounded-pill bg-primary"
6+
style="height: 3px;
7+
width: {{ setup_completion_percent|default:0 }}%"></div>
8+
</div>
9+
</div>
10+
{% if setup_issues_count %}
11+
<div class="d-flex align-items-center justify-content-between gap-2 rounded-2 mb-2 p-2 small"
12+
style="background-color: var(--bs-warning-bg-subtle);
13+
border: 1px solid var(--bs-warning-border-subtle)">
14+
<span class="d-flex align-items-center gap-2">
15+
<i class="fa-solid fa-triangle-exclamation"
16+
style="color: var(--bs-orange)"></i>
17+
{{ setup_issues_count }} issue{{ setup_issues_count|pluralize }} detected
18+
</span>
19+
<button type="button"
20+
class="btn btn-sm py-0 px-2 small"
21+
style="background-color: var(--bs-warning-border-subtle);
22+
border: 1px solid var(--bs-warning-border-subtle)"
23+
data-bs-toggle="modal"
24+
data-bs-target="#setupIssuesModal">View</button>
25+
</div>
26+
{% endif %}
27+
</div>

0 commit comments

Comments
 (0)