-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(aci): UI to filter monitors by assignee #95930
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
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.
Bug: Detector Filter Key Definition Regression
The fieldDefinitionGetter
in DetectorSearch
was incorrectly changed from the custom getDetectorFilterKeyDefinition
to the generic getFieldDefinition
. The original function was specifically designed to create FieldDefinition
objects for DETECTOR_FILTER_KEYS
(e.g., 'name', 'type', 'assignee') by extracting their specific properties. The generic getFieldDefinition
lacks knowledge of these detector-specific keys, preventing the SearchQueryBuilder
from properly recognizing and handling them, thus breaking search functionality for detector-specific filters.
static/app/views/detectors/components/detectorSearch.tsx#L21-L22
sentry/static/app/views/detectors/components/detectorSearch.tsx
Lines 21 to 22 in 7894e87
searchSource="detectors-list" | |
fieldDefinitionGetter={getFieldDefinition} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
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.
nice! mind adding a few tests as well?
export function useDetectorFilterKeys(): TagCollection { | ||
const assignedValues = useAssignedSearchValues(); | ||
|
||
return useMemo(() => { | ||
return Object.fromEntries( | ||
Object.keys(DETECTOR_FILTER_KEYS).map(key => { | ||
const {values} = DETECTOR_FILTER_KEYS[key] ?? {}; | ||
|
||
// Special handling for assignee field to provide user/team values | ||
if (key === 'assignee') { | ||
return [ | ||
key, | ||
{ | ||
key, | ||
name: key, | ||
predefined: true, | ||
values: assignedValues, | ||
}, | ||
]; | ||
} | ||
|
||
return [ | ||
key, | ||
{ | ||
key, | ||
name: key, | ||
predefined: values !== undefined, | ||
values, | ||
}, | ||
]; | ||
}) | ||
); | ||
}, [assignedValues]); | ||
} |
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: felt like it was a little more concise using entries
rather than keys and then inlining the checks for isAssignee
.
export function useDetectorFilterKeys(): TagCollection {
const assignedValues = useAssignedSearchValues();
return useMemo(() => {
return Object.fromEntries(
Object.entries(DETECTOR_FILTER_KEYS).map(([key, config]) => {
const { values } = config ?? {};
const isAssignee = key === 'assignee';
return [key, {
key,
name: key,
predefined: isAssignee || values !== undefined,
values: isAssignee ? assignedValues : values,
}];
})
);
}, [assignedValues]);
}
Step 1
Step 2
Step 3