Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(feedback): hide feedback issues by default if not filtered on #59505

Merged
merged 1 commit into from
Nov 7, 2023
Merged

feat(feedback): hide feedback issues by default if not filtered on #59505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/sentry/search/snuba/executors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,8 @@ def snuba_search(
if gc != GroupCategory.PROFILE.value
or features.has("organizations:issue-platform", organization, actor=actor)
}
# if we're not searching for feedbacks, then hide them by default
group_categories.discard(GroupCategory.FEEDBACK.value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One edge case I can see here is that this would show feedbacks if the user searched using negation, like !issue.category:performance. Is that something we care to prevent?

Copy link
Member Author

@JoshFerge JoshFerge Nov 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay for now. I can follow up on that -- this should cover the majority of usage and that type of query isn't super common

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Everything else looks good 👍


if not features.has("organizations:performance-issues-search", organization):
group_categories.discard(GroupCategory.PERFORMANCE.value)
Expand Down Expand Up @@ -1228,7 +1230,6 @@ def query(
actor: Optional[Any] = None,
aggregate_kwargs: Optional[PrioritySortWeights] = None,
) -> CursorResult[Group]:

if not validate_cdc_search_filters(search_filters):
raise InvalidQueryForExecutor("Search filters invalid for this query executor")

Expand Down
92 changes: 91 additions & 1 deletion tests/snuba/search/test_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from sentry.exceptions import InvalidSearchQuery
from sentry.issues.grouptype import (
ErrorGroupType,
FeedbackGroup,
NoiseConfig,
PerformanceNPlusOneGroupType,
PerformanceRenderBlockingAssetSpanGroupType,
Expand Down Expand Up @@ -1965,7 +1966,6 @@ def test_regressed_in_release(self):
assert set(results) == {group_2}

def test_first_release(self):

# expect no groups within the results since there are no releases

results = self.make_query(search_filter_query="first_release:%s" % "fake")
Expand Down Expand Up @@ -3684,6 +3684,96 @@ def test_rejected_filters(self):
== []
)

def test_feedback_category_hidden_default(self):
with self.feature(
["organizations:issue-platform", FeedbackGroup.build_visible_feature_name()]
):
event_id_1 = uuid.uuid4().hex
_, group_info = process_event_and_issue_occurrence(
self.build_occurrence_data(
**{
"id": uuid.uuid4().hex,
"project_id": 1,
"event_id": event_id_1,
"fingerprint": ["c" * 32],
"issue_title": "User Feedback",
"subtitle": "it was bad",
"culprit": "api/123",
"resource_id": "1234",
"evidence_data": {"Test": 123},
"evidence_display": [
{"name": "hi", "value": "bye", "important": True},
{"name": "what", "value": "where", "important": False},
],
"type": FeedbackGroup.type_id,
"detection_time": datetime.now().timestamp(),
"level": "info",
}
),
{
"event_id": event_id_1,
"project_id": self.project.id,
"title": "some problem",
"platform": "python",
"tags": {"my_tag": "1"},
"timestamp": before_now(minutes=1).isoformat(),
"received": before_now(minutes=1).isoformat(),
},
)
results = self.make_query(
date_from=self.base_datetime,
date_to=self.base_datetime + timedelta(days=10),
)
assert set(results) == set()

def test_feedback_category_show_when_filtered_on(self):
with self.feature(
[
"organizations:issue-platform",
FeedbackGroup.build_visible_feature_name(),
FeedbackGroup.build_ingest_feature_name(),
]
):
event_id_1 = uuid.uuid4().hex
_, group_info = process_event_and_issue_occurrence(
self.build_occurrence_data(
**{
"id": uuid.uuid4().hex,
"project_id": 1,
"event_id": event_id_1,
"fingerprint": ["c" * 32],
"issue_title": "User Feedback",
"subtitle": "it was bad",
"culprit": "api/123",
"resource_id": "1234",
"evidence_data": {"Test": 123},
"evidence_display": [
{"name": "hi", "value": "bye", "important": True},
{"name": "what", "value": "where", "important": False},
],
"type": FeedbackGroup.type_id,
"detection_time": datetime.now().timestamp(),
"level": "info",
}
),
{
"event_id": event_id_1,
"project_id": self.project.id,
"title": "some problem",
"platform": "python",
"tags": {"my_tag": "1"},
"timestamp": before_now(minutes=1).isoformat(),
"received": before_now(minutes=1).isoformat(),
},
)
results = self.make_query(
search_filter_query="issue.category:feedback",
date_from=self.base_datetime,
date_to=self.base_datetime + timedelta(days=10),
)
assert group_info is not None
assert list(results) == [group_info.group]


class CdcEventsSnubaSearchTest(TestCase, SharedSnubaMixin):
@property
Expand Down
Loading