Skip to content

Filter builder refactor#1828

Open
AaronPlave wants to merge 2 commits intodevelopfrom
refactor/filter-builder
Open

Filter builder refactor#1828
AaronPlave wants to merge 2 commits intodevelopfrom
refactor/filter-builder

Conversation

@AaronPlave
Copy link
Copy Markdown
Contributor

@AaronPlave AaronPlave commented Dec 10, 2025

Refactors the activity and external event filter builders to share common code. Closes #1803

@AaronPlave AaronPlave marked this pull request as ready for review January 6, 2026 15:18
@AaronPlave AaronPlave requested a review from a team as a code owner January 6, 2026 15:18
@AaronPlave AaronPlave force-pushed the refactor/filter-builder branch from 160c073 to e128eb9 Compare January 6, 2026 15:19
@AaronPlave AaronPlave requested review from JosephVolosin and duranb and removed request for dandelany, joswig and mattdailis January 6, 2026 15:19
@AaronPlave AaronPlave self-assigned this Jan 6, 2026
@AaronPlave AaronPlave changed the title Filter builder refactor WIP Filter builder refactor Jan 8, 2026
@AaronPlave AaronPlave force-pushed the refactor/filter-builder branch from e128eb9 to 7c3fddf Compare January 13, 2026 21:49
@AaronPlave AaronPlave force-pushed the refactor/filter-builder branch from 7c3fddf to 025d690 Compare January 20, 2026 15:46
@AaronPlave AaronPlave force-pushed the refactor/filter-builder branch from d9ea755 to fa9bcc1 Compare January 22, 2026 19:08
const matchingName = !!acc[parameterName];
const matchingEntry = matchingName && acc[parameterName].type === 'variant';
if (matchingEntry) {
// handle enums too?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this now handle enums with this refactor? I'm a little confused as to what the comment means since it's in a function called handleEnumAttribute

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe it already handled enums in the line below so the comment should be outdated? @JosephVolosin might be able to confirm?

const matchingName = !!acc[key];
const matchingEntry = matchingName && acc[key].type === type;
if (matchingEntry) {
// handle enums too?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similar question here as my above question in regards to the comment about enums. Is this just cleanup of a comment that was long addressed before? I'm seeing a reference to "enum" in the logic directly above, so I'm not sure what the previous state was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor ActivityFilterBuilder & ExternalEventFilterBuilder

2 participants