Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/sentry/explore/endpoints/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from rest_framework.serializers import ListField

from sentry.constants import ALL_ACCESS_PROJECTS
from sentry.discover.arithmetic import is_equation
from sentry.explore.models import ExploreSavedQueryDataset
from sentry.utils.dates import parse_stats_period, validate_interval

Expand Down Expand Up @@ -222,9 +223,16 @@
raise serializers.ValidationError(
"Metric field is only allowed for metrics dataset"
)
if data["dataset"] == "metrics" and "metric" not in q:

# the metrics field is only required for non-equation queries
has_equations = all(
is_equation(y_axis)
for aggregate_field in q.get("aggregateField") or []
for y_axis in aggregate_field.get("yAxes") or []
)

Check warning on line 232 in src/sentry/explore/endpoints/serializers.py

View check run for this annotation

@sentry/warden / warden: sentry-backend-bugs

all() on empty sequence bypasses metric field validation

Python's `all()` returns `True` for empty iterables. When `aggregateField` is missing, empty, or contains items with empty/missing `yAxes`, the generator produces zero elements and `has_equations` evaluates to `True`. This allows queries without equations (and without aggregate fields) to bypass the metric field validation on the metrics dataset, contrary to the intended behavior of only bypassing validation when all y-axes are equations.
if data["dataset"] == "metrics" and not has_equations and "metric" not in q:
raise serializers.ValidationError(
"Metric field is required for metrics dataset"
"Metric field is required for non-equation queries on the metrics dataset"
)
inner_query = {}
for key in inner_query_keys:
Expand Down
26 changes: 25 additions & 1 deletion tests/sentry/explore/endpoints/test_explore_saved_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,9 @@ def test_post_metrics_dataset_requires_metric_field(self) -> None:
},
)
assert response.status_code == 400, response.content
assert "Metric field is required for metrics dataset" in str(response.data)
assert "Metric field is required for non-equation queries on the metrics dataset" in str(
response.data
)

def test_save_with_start_and_end_time(self) -> None:
with self.feature(self.features):
Expand Down Expand Up @@ -1391,3 +1393,25 @@ def test_post_with_empty_query_is_rejected(self) -> None:
},
)
assert response.status_code == 400

def test_post_with_equation_is_accepted(self) -> None:
with self.feature(self.features):
response = self.client.post(
self.url,
{
"name": "Equation query",
"projects": self.project_ids,
"dataset": "metrics",
"query": [
{
"aggregateField": [{"yAxes": ["equation|A + B"], "chartType": 1}],
"mode": "samples",
"fields": ["A", "B"],
"orderby": "-timestamp",
},
],
},
)
assert response.status_code == 201, response.content
data = response.data
assert data["query"][0].get("metric") is None
Loading