Skip to content

Conversation

@ameliahsu
Copy link
Member

rather than adding a temporary type field on condition subfilters, this change adds logic to check for either an attribute or key field on the subfilter to determine if it's an attribute or tag filter

using the type to determine the filter (attribute/tag) would not work when loading an existing alert into the builder when editing an alert, since it was just a temporary field being added in the ui during the alert creation flow

@ameliahsu ameliahsu requested a review from a team as a code owner October 22, 2025 22:10
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 22, 2025
const {subfilter, subfilter_id, onUpdate} = useSubfilterContext();

if (!subfilter.type) {
if ('attribute' in subfilter || 'key' in subfilter) {
Copy link
Member

Choose a reason for hiding this comment

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

It might be helpful to define the types as type Subfilter = AttributeSubfilter | KeySubfilter

And then you can use a user defined type predicate to narrow the type:

function isAttributeSubfilter(subfilter: Subfilter): subfilter is AttributeSubfilter {
  return ''attribute' in subfilter
}

This improves the readability and gives better type safety.

I'm noticing that subfilter is typed as Record<string, any> which we can definitely go back and improve at some point (doesn't have to be in this PR)

cursor[bot]

This comment was marked as outdated.

@ameliahsu ameliahsu merged commit b00a977 into master Oct 23, 2025
47 checks passed
@ameliahsu ameliahsu deleted the mia/aci/remove-subfilter-type branch October 23, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants