feat(feedback): ignore feedbacks in issue search #57528

JoshFerge · 2023-10-04T22:57:12Z

We want to display feedbacks within the "Feedbacks" UI section, and have them be hidden in Issues. This PR makes it so issue search does not show them.

ref: https://github.com/getsentry/team-replay/issues/227

Adds a new query param referrer to organization_issues_search endpoint
We pass this referrer through search logic, and if the referrer is not feedback, then hide the feedbacks.

JoshFerge · 2023-10-04T22:58:21Z

src/sentry/api/endpoints/organization_group_index.py

@@ -168,6 +168,8 @@ class OrganizationGroupIndexEndpoint(OrganizationEventsEndpointBase):
        },


need to figure out where to add tests

getsantry · 2023-10-27T07:00:37Z

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

codecov · 2023-10-30T20:07:39Z

Codecov Report

Merging #57528 (3c1c023) into master (dd4fee4) will decrease coverage by 0.28%.
Report is 7 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #57528      +/-   ##
==========================================
- Coverage   81.03%   80.76%   -0.28%     
==========================================
  Files        5492     5164     -328     
  Lines      234889   226125    -8764     
  Branches    38019    38056      +37     
==========================================
- Hits       190348   182626    -7722     
+ Misses      42503    37958    -4545     
- Partials     2038     5541    +3503

Files	Coverage Δ
...c/sentry/api/endpoints/organization_group_index.py	`85.54% <100.00%> (+0.34%)`	⬆️
src/sentry/issues/search.py	`83.51% <100.00%> (+0.37%)`	⬆️
src/sentry/search/snuba/executors.py	`90.96% <100.00%> (-2.33%)`	⬇️

... and 1660 files with indirect coverage changes

ryan953 · 2023-11-01T01:31:45Z

src/sentry/issues/search.py

+        if referrer != "api.feedback_index":
+            category_ids.discard(GroupCategory.FEEDBACK.value)


While we are here can we also apply the inverse:
If the referrer is feedback, replace any categories and ensure GroupCategory.FEEDBACK is present and the only value set.

I can follow up with this -- probably want to slightly reorganize how we're doing this.

bruno-garcia · 2023-11-06T18:46:11Z

src/sentry/api/endpoints/organization_group_index.py

@@ -182,7 +189,7 @@ def _search(
                query_kwargs.pop("sort_by")
                result = inbox_search(**query_kwargs)
            else:
-                query_kwargs["referrer"] = "search.group_index"
+                query_kwargs["referrer"] = referrer


Should we default to DEFAULT_REFERRER here too?

referrer will always be passed in and defined here -- the default is set in the callers

malwilley

Overall looks good, left one comment/question about the referrer approach

malwilley · 2023-11-06T19:30:50Z

src/sentry/issues/search.py

        category_ids = {gc.value for gc in categories}
+        if referrer != "api.feedback_index":


Are there other places in the codebase where we change behavior based on the referrer? To me it's a bit unexpected and I think I'd prefer a more explicit way of telling this code path you either do or don't want feedbacks. But if this is something we do elsewhere then this is fine.

I think ideally we'd have a separate endpoint that only queried feedbacks, and feedbacks by default are hidden in this endpoint.

This would allow us to remove all of the referrer based behavior here. I'm not sure if we modify behavior by referrer elsewhere.

I agree that it is a bit weird -- I can attempt some refactors and making a separate endpoint as a follow up and then remove the referrer logic here -- what do you think?

The followup with separate endpoints sounds like a great solution to me!

okay sounds good!

+1 on this feeling awkward, I'd rather see that as a specific param or different endpoint path.

vartec · 2023-11-06T20:21:22Z

src/sentry/issues/search.py

@@ -211,6 +211,7 @@ def _query_params_for_generic(
    conditions: Sequence[Any],
    actor: Optional[Any] = None,
    categories: Optional[Sequence[GroupCategory]] = None,
+    referrer: str = None,


nit: Optional if it's to accept None as a default value

vartec · 2023-11-06T20:24:32Z

src/sentry/issues/search.py

        category_ids = {gc.value for gc in categories}
+        if referrer != "api.feedback_index":


+1 on this feeling awkward, I'd rather see that as a specific param or different endpoint path.

JoshFerge · 2023-11-06T20:26:19Z

I'll go ahead and close this, not happy w/ how hacky it feels. thanks for reviews folks. will follow up with something better.

…59505) Previously: #57528 Instead of trying to control the display of feedbacks, lets just hide them by default unless they are filtered on. Aspirationally, we'd like to eventually have our own feedbacks endpoint that only displays feedbacks, and have feedbacks hidden from issues search entirely, but I do not see a good way of doing this without it being either extremely hacky, or a lot of work.

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 4, 2023

JoshFerge commented Oct 4, 2023

View reviewed changes

vercel bot deployed to Preview October 4, 2023 22:59 View deployment

Base automatically changed from jferg/feedback-issue-type to master October 5, 2023 17:40

getsantry bot added the Stale label Oct 27, 2023

feat(feedback): ignore feedbacks in issue search

dfd94d7

JoshFerge force-pushed the jferg/ignore-feedbacks-in-search branch from 2494d96 to dfd94d7 Compare October 30, 2023 19:17

vercel bot deployed to Preview October 30, 2023 19:21 View deployment

use query parameter, add test

da29ba1

vercel bot deployed to Preview October 31, 2023 21:31 View deployment

JoshFerge added 2 commits October 31, 2023 17:42

add test

32ef7fd

cleanup

4f42cca

vercel bot deployed to Preview November 1, 2023 00:47 View deployment

fix tests

42e519c

JoshFerge marked this pull request as ready for review November 1, 2023 01:17

JoshFerge requested a review from a team as a code owner November 1, 2023 01:17

JoshFerge requested a review from a team November 1, 2023 01:17

vercel bot deployed to Preview November 1, 2023 01:17 View deployment

ryan953 reviewed Nov 1, 2023

View reviewed changes

add start and end to search

79ceb6f

vercel bot deployed to Preview November 1, 2023 18:23 View deployment

add referrer to delete and put

42ea00f

vercel bot deployed to Preview November 1, 2023 19:45 View deployment

fix type

9e1e886

vercel bot deployed to Preview November 1, 2023 20:34 View deployment

JoshFerge removed the Stale label Nov 6, 2023

Merge branch 'master' into jferg/ignore-feedbacks-in-search

3c1c023

vercel bot deployed to Preview November 6, 2023 18:43 View deployment

bruno-garcia reviewed Nov 6, 2023

View reviewed changes

malwilley approved these changes Nov 6, 2023

View reviewed changes

vartec reviewed Nov 6, 2023

View reviewed changes

JoshFerge closed this Nov 6, 2023

JoshFerge mentioned this pull request Nov 7, 2023

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

Merged

github-actions bot locked and limited conversation to collaborators Nov 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(feedback): ignore feedbacks in issue search #57528

feat(feedback): ignore feedbacks in issue search #57528

JoshFerge commented Oct 4, 2023 •

edited

Loading

JoshFerge Oct 4, 2023

getsantry bot commented Oct 27, 2023

codecov bot commented Oct 30, 2023 •

edited

Loading

ryan953 Nov 1, 2023

JoshFerge Nov 1, 2023

bruno-garcia Nov 6, 2023

JoshFerge Nov 6, 2023

malwilley left a comment

malwilley Nov 6, 2023

JoshFerge Nov 6, 2023

malwilley Nov 6, 2023

JoshFerge Nov 6, 2023

vartec Nov 6, 2023

vartec Nov 6, 2023

vartec Nov 6, 2023

JoshFerge commented Nov 6, 2023

		@@ -168,6 +168,8 @@ class OrganizationGroupIndexEndpoint(OrganizationEventsEndpointBase):
		},

		if referrer != "api.feedback_index":
		category_ids.discard(GroupCategory.FEEDBACK.value)

		category_ids = {gc.value for gc in categories}
		if referrer != "api.feedback_index":

feat(feedback): ignore feedbacks in issue search #57528

feat(feedback): ignore feedbacks in issue search #57528

Conversation

JoshFerge commented Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

getsantry bot commented Oct 27, 2023

codecov bot commented Oct 30, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malwilley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshFerge commented Nov 6, 2023

JoshFerge commented Oct 4, 2023 •

edited

Loading

codecov bot commented Oct 30, 2023 •

edited

Loading