Skip to content

Commit 7b4f672

Browse files
authored
fix(spans-migration): changed reason format (#101466)
To display more cohesive warning messaging for dropped fields in the frontend we're changing the way dropped fields are being passed back for orderbys. We are changing it to mimic the columns and equations fields where the reason can be a list of fields that were dropped as part of the orderby.
1 parent d053d29 commit 7b4f672

File tree

3 files changed

+13
-15
lines changed

3 files changed

+13
-15
lines changed

src/sentry/discover/translation/mep_to_eap.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class QueryParts(TypedDict):
2525
class DroppedFields(TypedDict):
2626
selected_columns: list[str]
2727
equations: list[dict[str, list[str]]]
28-
orderby: list[dict[str, str]]
28+
orderby: list[dict[str, str | list[str]]]
2929

3030

3131
COLUMNS_TO_DROP = (
@@ -357,14 +357,14 @@ def translate_orderbys(orderbys, equations, dropped_equations, new_equations):
357357

358358
# checks if equation index is out of bounds
359359
if len(equations) < equation_index + 1:
360-
dropped_orderby_reason = "equation at this index doesn't exist"
360+
dropped_orderby_reason = "equation issue"
361361

362362
# if there are equations
363363
elif len(equations) > 0:
364364
selected_equation = equations[equation_index]
365365
# if equation was dropped, drop the orderby too
366366
if selected_equation in dropped_equations:
367-
dropped_orderby_reason = "equation was dropped"
367+
dropped_orderby_reason = "dropped"
368368
decoded_orderby = (
369369
selected_equation if not is_negated else f"-{selected_equation}"
370370
)
@@ -376,12 +376,12 @@ def translate_orderbys(orderbys, equations, dropped_equations, new_equations):
376376
new_equation_index = new_equations.index(translated_equation)
377377
translated_orderby = [f"equation[{new_equation_index}]"]
378378
except (IndexError, ValueError):
379-
dropped_orderby_reason = "equation was dropped"
379+
dropped_orderby_reason = "dropped"
380380
decoded_orderby = (
381381
selected_equation if not is_negated else f"-{selected_equation}"
382382
)
383383
else:
384-
dropped_orderby_reason = "no equations in this query"
384+
dropped_orderby_reason = "no equations"
385385
decoded_orderby = orderby
386386

387387
# if orderby is an equation
@@ -390,17 +390,15 @@ def translate_orderbys(orderbys, equations, dropped_equations, new_equations):
390390
[orderby_without_neg]
391391
)
392392
if len(dropped_orderby_equation) > 0:
393-
dropped_orderby_reason = "fields were dropped: " + ", ".join(
394-
dropped_orderby_equation[0]["reason"]
395-
)
393+
dropped_orderby_reason = dropped_orderby_equation[0]["reason"]
396394

397395
# if orderby is a field/function
398396
else:
399397
translated_orderby, dropped_orderby = translate_columns(
400398
[orderby_without_neg], need_equation=True
401399
)
402400
if len(dropped_orderby) > 0:
403-
dropped_orderby_reason = "fields were dropped: " + ", ".join(dropped_orderby)
401+
dropped_orderby_reason = dropped_orderby
404402

405403
# add translated orderby to the list and record dropped orderbys
406404
if dropped_orderby_reason is None:

tests/sentry/discover/translation/test_mep_to_eap.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -338,15 +338,15 @@ def test_mep_to_eap_simple_equations(
338338
[
339339
{
340340
"orderby": "-count_miserable(user,300)",
341-
"reason": "fields were dropped: count_miserable(user,300)",
341+
"reason": ["count_miserable(user,300)"],
342342
},
343343
{
344344
"orderby": "count_web_vitals(user,300)",
345-
"reason": "fields were dropped: count_web_vitals(user,300)",
345+
"reason": ["count_web_vitals(user,300)"],
346346
},
347347
{
348348
"orderby": "any(transaction.duration)",
349-
"reason": "fields were dropped: any(span.duration)",
349+
"reason": ["any(span.duration)"],
350350
},
351351
],
352352
),
@@ -366,14 +366,14 @@ def test_mep_to_eap_simple_equations(
366366
[
367367
{
368368
"orderby": "equation|count_miserable(user,300) + 3",
369-
"reason": "equation was dropped",
369+
"reason": "dropped",
370370
}
371371
],
372372
),
373373
pytest.param(
374374
["equation[3453]"],
375375
[],
376-
[{"orderby": "equation[3453]", "reason": "equation at this index doesn't exist"}],
376+
[{"orderby": "equation[3453]", "reason": "equation issue"}],
377377
),
378378
],
379379
)

tests/sentry/explore/translation/test_discover_translation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ def test_translate_drop_swap_function_field_orderby_filter_discover_to_explore_q
219219
assert new_explore_query.changed_reason["orderby"] == [
220220
{
221221
"orderby": "-count_miserable(users)",
222-
"reason": "fields were dropped: count_miserable(users)",
222+
"reason": ["count_miserable(users)"],
223223
}
224224
]
225225

0 commit comments

Comments
 (0)