Skip to content

Commit 45bc149

Browse files
authored
Don't use or when handling tables that have fields with many-to-* associations. (#8859)
This patch resolves an issue that causes duplicate alert summaries to be returned from the API because of the OR filter on a query that contains many-to-many/many-to-one fields. Specifically, for each alert that an alert summary had, a duplicate alert summary would be created.
1 parent c0cc102 commit 45bc149

File tree

3 files changed

+79
-11
lines changed

3 files changed

+79
-11
lines changed

tests/conftest.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,40 @@ def test_perf_signature_2(test_perf_signature):
779779
)
780780

781781

782+
@pytest.fixture
783+
def test_perf_signature_3(test_perf_signature):
784+
return perf_models.PerformanceSignature.objects.create(
785+
repository=test_perf_signature.repository,
786+
signature_hash=(20 * "t3"),
787+
framework=test_perf_signature.framework,
788+
platform=test_perf_signature.platform,
789+
option_collection=test_perf_signature.option_collection,
790+
suite="mysuite2",
791+
test="mytest3",
792+
has_subtests=test_perf_signature.has_subtests,
793+
extra_options=test_perf_signature.extra_options,
794+
last_updated=datetime.datetime.now(),
795+
tags="cold pageload",
796+
)
797+
798+
799+
@pytest.fixture
800+
def test_perf_signature_4(test_perf_signature):
801+
return perf_models.PerformanceSignature.objects.create(
802+
repository=test_perf_signature.repository,
803+
signature_hash=(20 * "t4"),
804+
framework=test_perf_signature.framework,
805+
platform=test_perf_signature.platform,
806+
option_collection=test_perf_signature.option_collection,
807+
suite="mysuite2",
808+
test="mytest4",
809+
has_subtests=test_perf_signature.has_subtests,
810+
extra_options=test_perf_signature.extra_options,
811+
last_updated=datetime.datetime.now(),
812+
tags="cold pageload",
813+
)
814+
815+
782816
@pytest.fixture
783817
def test_stalled_data_signature(test_perf_signature):
784818
stalled_data_timestamp = datetime.datetime.now() - datetime.timedelta(days=120)
@@ -1209,6 +1243,24 @@ def test_perf_alert_2(
12091243
)
12101244

12111245

1246+
@pytest.fixture
1247+
def test_perf_alert_3(
1248+
test_perf_alert, test_perf_signature_3, test_perf_alert_summary_2
1249+
) -> perf_models.PerformanceAlert:
1250+
return create_perf_alert(
1251+
summary=test_perf_alert_summary_2, series_signature=test_perf_signature_3
1252+
)
1253+
1254+
1255+
@pytest.fixture
1256+
def test_perf_alert_4(
1257+
test_perf_alert, test_perf_signature_4, test_perf_alert_summary_2
1258+
) -> perf_models.PerformanceAlert:
1259+
return create_perf_alert(
1260+
summary=test_perf_alert_summary_2, series_signature=test_perf_signature_4
1261+
)
1262+
1263+
12121264
@pytest.fixture
12131265
def generic_reference_data(test_repository):
12141266
"""

tests/webapp/api/test_performance_alertsummary_api.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,27 @@ def test_alert_summaries_get(
130130
}
131131

132132

133+
def test_alert_summaries_get_multiple_alerts(
134+
client,
135+
test_perf_alert_summary,
136+
test_perf_alert_summary_2,
137+
test_perf_alert_2,
138+
test_perf_alert_3,
139+
test_perf_alert_4,
140+
):
141+
# verify that we get the performance summary + alert on GET
142+
resp = client.get(reverse("performance-alert-summaries-list"))
143+
assert resp.status_code == 200
144+
145+
# make sure only 2 alert summaries are returned when one summary
146+
# has multiple alerts associated with it
147+
data = resp.json()
148+
assert len(data["results"]) == 2
149+
150+
# make sure the 2 alert summaries are unique
151+
assert len(set([result["id"] for result in data["results"]])) == 2
152+
153+
133154
def test_alert_summaries_get_onhold(
134155
client,
135156
test_perf_alert_summary,

treeherder/webapp/api/performance_data.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -474,18 +474,13 @@ class PerformanceAlertSummaryViewSet(viewsets.ModelViewSet):
474474

475475
def list(self, request, *args, **kwargs):
476476
queryset = self.filter_queryset(self.queryset)
477-
if request.query_params.get("monitored_alerts"):
478-
queryset = queryset.filter(
479-
alerts__series_signature__monitor=True,
480-
)
481-
else:
482-
queryset = queryset.filter(
483-
alerts__series_signature__monitor=None,
484-
) | queryset.filter(
485-
alerts__series_signature__monitor=False,
486-
)
487-
488477
pk = request.query_params.get("id")
478+
if not pk:
479+
if request.query_params.get("monitored_alerts"):
480+
queryset = queryset.filter(sheriffed=False)
481+
else:
482+
queryset = queryset.filter(sheriffed=True)
483+
489484
page = self.paginate_queryset(queryset)
490485
if page is not None:
491486
serializer = self.get_serializer(page, many=True)

0 commit comments

Comments
 (0)