-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(aci): Update API to allow filter monitors by assignee #95501
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.
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
base: master
Are you sure you want to change the base?
Conversation
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #95501 +/- ##
===========================================
+ Coverage 36.17% 76.33% +40.16%
===========================================
Files 9960 10589 +629
Lines 557832 610119 +52287
Branches 23990 23990
===========================================
+ Hits 201783 465733 +263950
+ Misses 355640 142454 -213186
- Partials 409 1932 +1523 |
const query = typeof location.query.query === 'string' ? location.query.query : ''; | ||
|
||
const filterKeys: TagCollection = Object.fromEntries( | ||
Object.keys(DETECTOR_FILTER_KEYS).map(key => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: IMO we should convert the constant DETECTOR_FILTER_KEYS into a hook useDetectorFilterKeys()
so that we can use the useAssignedSearchValues()
hook inside of it. That way all the logic stays together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in #95930
elif actor is None: | ||
assignee_query |= Q(owner_team_id__isnull=True, owner_user_id__isnull=True) | ||
else: | ||
# Unknown actor type, return a query that matches nothing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to raise here or otherwise report an error if there are types coming back from convert_actor_or_none_value
that we don't support.
I think officially the right thing is assert_never
.
status_code=400, | ||
) | ||
assert "owner" in response.data | ||
def setUp(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for us to have a test that actually filters by assignee?
Allows filtering with assignee:
/api/0/organizations/sentry/detectors/?query=assignee%3A{me|my_teams|none|team_slug|user_email}&...