Skip to content

Commit d90ebba

Browse files
moibra05moibra05
committed
fix(nimbus): hide weekly metrics from displaying on results page popout card if not available
1 parent e8be902 commit d90ebba

File tree

3 files changed

+215
-58
lines changed

3 files changed

+215
-58
lines changed

‎experimenter/experimenter/experiments/models.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,11 +1659,13 @@ def get_weekly_metric_data(self, analysis_basis, segment, reference_branch):
16591659
abs_list = branch_data.get("absolute") or []
16601660
rel_list = branch_data.get("relative") or []
16611661

1662-
weekly_data[branch_slug] = list(
1663-
zip_longest(abs_list, rel_list, fillvalue=None)
1664-
)
1662+
if abs_list or rel_list:
1663+
weekly_data[branch_slug] = list(
1664+
zip_longest(abs_list, rel_list, fillvalue=None)
1665+
)
16651666

1666-
weekly_metric_data[metric_metadata["slug"]] = weekly_data
1667+
if weekly_data:
1668+
weekly_metric_data[metric_metadata["slug"]] = weekly_data
16671669

16681670
return weekly_metric_data
16691671

‎experimenter/experimenter/experiments/tests/test_models.py‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,6 +1418,159 @@ def test_targeting_config_returns_None_with_invalid_slug(self):
14181418
experiment = NimbusExperimentFactory.create(targeting_config_slug="invalid slug")
14191419
self.assertIsNone(experiment.targeting_config)
14201420

1421+
def test_get_weekly_metric_data(self):
1422+
experiment = NimbusExperimentFactory.create()
1423+
branch_a = NimbusBranchFactory.create(
1424+
experiment=experiment, name="Branch A", slug="branch-a"
1425+
)
1426+
branch_b = NimbusBranchFactory.create(
1427+
experiment=experiment, name="Branch B", slug="branch-b"
1428+
)
1429+
1430+
experiment.results_data = {
1431+
"v3": {
1432+
"weekly": {
1433+
"enrollments": {
1434+
"all": {
1435+
branch_a.slug: {
1436+
"branch_data": {
1437+
"other_metrics": {
1438+
"retained": {
1439+
"absolute": {
1440+
"all": [
1441+
{
1442+
"lower": 140,
1443+
"upper": 160,
1444+
"point": 150,
1445+
},
1446+
{
1447+
"lower": 130,
1448+
"upper": 150,
1449+
"point": 140,
1450+
},
1451+
{
1452+
"lower": 120,
1453+
"upper": 140,
1454+
"point": 130,
1455+
},
1456+
]
1457+
},
1458+
"relative_uplift": {
1459+
branch_a.slug: {"all": []},
1460+
},
1461+
}
1462+
}
1463+
},
1464+
},
1465+
branch_b.slug: {
1466+
"branch_data": {
1467+
"other_metrics": {
1468+
"retained": {
1469+
"absolute": {
1470+
"all": [
1471+
{
1472+
"lower": 140,
1473+
"upper": 160,
1474+
"point": 150,
1475+
},
1476+
{
1477+
"lower": 130,
1478+
"upper": 150,
1479+
"point": 140,
1480+
},
1481+
{
1482+
"lower": 120,
1483+
"upper": 140,
1484+
"point": 130,
1485+
},
1486+
]
1487+
},
1488+
"relative_uplift": {
1489+
branch_a.slug: {
1490+
"all": [
1491+
{
1492+
"lower": 10,
1493+
"upper": 20,
1494+
"point": 15,
1495+
},
1496+
{
1497+
"lower": 5,
1498+
"upper": 15,
1499+
"point": 10,
1500+
},
1501+
{
1502+
"lower": 0,
1503+
"upper": 10,
1504+
"point": 5,
1505+
},
1506+
]
1507+
},
1508+
branch_b.slug: {"all": []},
1509+
},
1510+
}
1511+
}
1512+
}
1513+
},
1514+
}
1515+
}
1516+
}
1517+
}
1518+
}
1519+
experiment.save()
1520+
1521+
expected_weekly_data = {
1522+
"retained": {
1523+
branch_a.slug: [
1524+
(
1525+
{"lower": 140, "upper": 160, "significance": "neutral"},
1526+
None,
1527+
),
1528+
(
1529+
{"lower": 130, "upper": 150, "significance": "neutral"},
1530+
None,
1531+
),
1532+
(
1533+
{"lower": 120, "upper": 140, "significance": "neutral"},
1534+
None,
1535+
),
1536+
],
1537+
branch_b.slug: [
1538+
(
1539+
{"lower": 140, "upper": 160, "significance": "neutral"},
1540+
{
1541+
"avg_rel_change": 15,
1542+
"lower": 10,
1543+
"upper": 20,
1544+
"significance": "neutral",
1545+
},
1546+
),
1547+
(
1548+
{"lower": 130, "upper": 150, "significance": "neutral"},
1549+
{
1550+
"avg_rel_change": 10,
1551+
"lower": 5,
1552+
"upper": 15,
1553+
"significance": "neutral",
1554+
},
1555+
),
1556+
(
1557+
{"lower": 120, "upper": 140, "significance": "neutral"},
1558+
{
1559+
"avg_rel_change": 5,
1560+
"lower": 0,
1561+
"upper": 10,
1562+
"significance": "neutral",
1563+
},
1564+
),
1565+
],
1566+
}
1567+
}
1568+
1569+
self.assertEqual(
1570+
experiment.get_weekly_metric_data("enrollments", "all", branch_a.slug),
1571+
expected_weekly_data,
1572+
)
1573+
14211574
@parameterized.expand(
14221575
[
14231576
[

‎experimenter/experimenter/nimbus_ui/templates/common/metric_popout.html‎

Lines changed: 56 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -78,66 +78,68 @@ <h2 class="accordion-header">
7878
</div>
7979
</div>
8080
</div>
81-
<div class="accordion-item p-3 px-4 rounded-4 border border-1">
82-
<h2 class="accordion-header">
83-
<button class="accordion-button fw-bold shadow-none bg-transparent text-body"
84-
type="button"
85-
data-bs-toggle="collapse"
86-
data-bs-target="#{{ experiment_slug }}-{{ metric_info.slug }}-weekly-collapse"
87-
aria-expanded="true"
88-
aria-controls="{{ experiment_slug }}-{{ metric_info.slug }}-weekly-collapse">
89-
Weekly breakdown
90-
</button>
91-
</h2>
92-
{% with weekly_metric_data=all_weekly_metric_data|dict_get:metric_info.slug %}
93-
<div class="accordion-collapse collapse show"
94-
id="{{ experiment_slug }}-{{ metric_info.slug }}-weekly-collapse">
95-
<div class="accordion-body d-flex">
96-
<div class="col-2">
97-
<p class="text-muted mb-0 invisible" aria-hidden="true">Metrics</p>
98-
<p class="fs-5 mb-3 invisible" aria-hidden="true">Baseline</p>
99-
{% with reference_branch_weekly=weekly_metric_data|dict_get:reference_branch %}
100-
{% for week in experiment.get_weekly_dates %}
101-
<div class="{% if forloop.last %}mb-4{% endif %} mb-3 d-flex text-center rounded-4 w-100 text-start position-relative d-flex flex-column align-items-center justify-content-center"
102-
style="height: 100px">
103-
<small class="text-muted">Week {{ forloop.counter }}</small>
104-
<p class="mb-0">{{ week.0|date:"M j" }} - {{ week.1|date:"M j" }}</p>
105-
</div>
106-
{% endfor %}
107-
{% endwith %}
108-
</div>
109-
<div class="col position-relative w-25">
110-
<div class="row flex-row flex-nowrap overflow-auto mx-2 pb-3 {% if branch_data|length > 3 %}mx-4{% endif %}">
111-
{% if branch_data|length > 3 %}
112-
<div class="branch-fade branch-fade-left"></div>
113-
<div class="branch-fade branch-fade-right"></div>
114-
{% endif %}
115-
{% for branch in branch_data %}
116-
<div class="{% if branch.slug == selected_reference_branch %}col-2{% elif branch_data|length > 3 %}col-4{% else %}col{% endif %} d-flex flex-column align-items-center">
117-
<p class="text-muted mb-0 text-truncate">{{ branch.name }}</p>
118-
{% if branch.slug == selected_reference_branch %}
119-
<p class="fs-5 mb-3">Baseline</p>
120-
{% else %}
121-
<p class="fs-5">{{ branch.name }}</p>
122-
{% endif %}
123-
<div class="d-flex flex-column gap-3 w-100">
124-
{% for curr_branch_slug, branch_weekly_metric_data in weekly_metric_data.items %}
125-
{% if curr_branch_slug == branch.slug %}
126-
{% for weekly_data_point in branch_weekly_metric_data %}
127-
{% include "common/metric_card.html" with slug=branch.slug reference_branch=selected_reference_branch absolute_lower=weekly_data_point.0.lower absolute_upper=weekly_data_point.0.upper significance=weekly_data_point.1.significance percentage=weekly_data_point.1.avg_rel_change %}
81+
{% with weekly_metric_data=all_weekly_metric_data|dict_get:metric_info.slug %}
82+
{% if weekly_metric_data %}
83+
<div class="accordion-item p-3 px-4 rounded-4 border border-1">
84+
<h2 class="accordion-header">
85+
<button class="accordion-button fw-bold shadow-none bg-transparent text-body"
86+
type="button"
87+
data-bs-toggle="collapse"
88+
data-bs-target="#{{ experiment_slug }}-{{ metric_info.slug }}-weekly-collapse"
89+
aria-expanded="true"
90+
aria-controls="{{ experiment_slug }}-{{ metric_info.slug }}-weekly-collapse">
91+
Weekly breakdown
92+
</button>
93+
</h2>
94+
<div class="accordion-collapse collapse show"
95+
id="{{ experiment_slug }}-{{ metric_info.slug }}-weekly-collapse">
96+
<div class="accordion-body d-flex">
97+
<div class="col-2">
98+
<p class="text-muted mb-0 invisible" aria-hidden="true">Metrics</p>
99+
<p class="fs-5 mb-3 invisible" aria-hidden="true">Baseline</p>
100+
{% with reference_branch_weekly=weekly_metric_data|dict_get:reference_branch %}
101+
{% for week in experiment.get_weekly_dates %}
102+
<div class="{% if forloop.last %}mb-4{% endif %} mb-3 d-flex text-center rounded-4 w-100 text-start position-relative d-flex flex-column align-items-center justify-content-center"
103+
style="height: 100px">
104+
<small class="text-muted">Week {{ forloop.counter }}</small>
105+
<p class="mb-0">{{ week.0|date:"M j" }} - {{ week.1|date:"M j" }}</p>
106+
</div>
107+
{% endfor %}
108+
{% endwith %}
109+
</div>
110+
<div class="col position-relative w-25">
111+
<div class="row flex-row flex-nowrap overflow-auto mx-2 pb-3 {% if branch_data|length > 3 %}mx-4{% endif %}">
112+
{% if branch_data|length > 3 %}
113+
<div class="branch-fade branch-fade-left"></div>
114+
<div class="branch-fade branch-fade-right"></div>
115+
{% endif %}
116+
{% for branch in branch_data %}
117+
<div class="{% if branch.slug == selected_reference_branch %}col-2{% elif branch_data|length > 3 %}col-4{% else %}col{% endif %} d-flex flex-column align-items-center">
118+
<p class="text-muted mb-0 text-truncate">{{ branch.name }}</p>
119+
{% if branch.slug == selected_reference_branch %}
120+
<p class="fs-5 mb-3">Baseline</p>
121+
{% else %}
122+
<p class="fs-5">{{ branch.name }}</p>
123+
{% endif %}
124+
<div class="d-flex flex-column gap-3 w-100">
125+
{% for curr_branch_slug, branch_weekly_metric_data in weekly_metric_data.items %}
126+
{% if curr_branch_slug == branch.slug %}
127+
{% for weekly_data_point in branch_weekly_metric_data %}
128+
{% include "common/metric_card.html" with slug=branch.slug reference_branch=selected_reference_branch absolute_lower=weekly_data_point.0.lower absolute_upper=weekly_data_point.0.upper significance=weekly_data_point.1.significance percentage=weekly_data_point.1.avg_rel_change %}
128129

129-
{% endfor %}
130-
{% endif %}
131-
{% endfor %}
130+
{% endfor %}
131+
{% endif %}
132+
{% endfor %}
133+
</div>
132134
</div>
133-
</div>
134-
{% endfor %}
135+
{% endfor %}
136+
</div>
135137
</div>
136138
</div>
137139
</div>
138140
</div>
139-
{% endwith %}
140-
</div>
141+
{% endif %}
142+
{% endwith %}
141143
</div>
142144
</div>
143145
</div>

0 commit comments

Comments
 (0)