refactor(taxonomic-filter): extract eventPropertiesTaxonomicGroupsLogic#59834
Closed
pauldambra wants to merge 3 commits into
Closed
Conversation
This was referenced May 24, 2026
Member
Author
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
|
Size Change: 0 B Total Size: 80.6 MB ℹ️ View Unchanged
|
This was referenced May 24, 2026
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
acb4bba to
7652b3f
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
dd77d58 to
42f5b66
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
frontend/src/lib/components/TaxonomicFilter/eventPropertiesTaxonomicGroupsLogic.ts:27-116
**Extracted but not integrated — logic now lives in two places**
The identical `Event properties` and `Internal event properties` group definitions (and all their supporting selectors) still exist verbatim in `taxonomicFilterLogic.tsx` (lines 750–814) and no caller references `eventPropertiesTaxonomicGroupsLogic` yet. The refactoring is incomplete: extracting to a new file without removing or wiring in the original means the same logic is expressed twice, which breaks the OnceAndOnlyOnce principle. The integration step — connecting this logic via `connect()` in `taxonomicFilterLogic.tsx` and removing the duplicate block — appears to be missing from this PR.
Reviews (1): Last reviewed commit: "refactor(taxonomic-filter): extract even..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
42f5b66 to
e254dbf
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
0805859 to
b44a7b3
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
278e472 to
394021f
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
b44a7b3 to
ca62644
Compare
Contributor
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
394021f to
ddd0817
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
ca62644 to
d7ac0a5
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
ddd0817 to
2d7e104
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
d7ac0a5 to
2aa1968
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
2d7e104 to
fefd2b6
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
2aa1968 to
1a02aee
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
fefd2b6 to
32e3529
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
1a02aee to
2241ef3
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
32e3529 to
90b795b
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
2241ef3 to
5e695d6
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
90b795b to
c804380
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
5e695d6 to
b225155
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
c804380 to
0c80e0c
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
b225155 to
8a0106d
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
0c80e0c to
731f858
Compare
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
8a0106d to
a859ef4
Compare
pauldambra
force-pushed
the
posthog-code/extract-custom-events-taxonomic-logic
branch
from
731f858 to
3d10889
Compare
Eighteenth slice. The `Event properties` tab is the most complex tab definition in the parent file (scoped vs unscoped endpoints, `expandLabel` for scoped/unscoped counts, virtual-property exclusion gated on a feature flag, autocapture event-type keyword shortcuts). It sits next to `Internal event properties` (a tiny related tab), so the two move together as the event-property data type. This extraction also retires `featureFlags` from the parent's connect block and `taxonomicGroups` selector inputs — the gate on `FEATURE_FLAGS.TRAFFIC_TYPE_VIRTUAL_PROPERTIES` was the only remaining parent consumer of feature flags. Parent `taxonomicFilterLogic` exports `TRAFFIC_TYPE_VIRTUAL_PROPERTIES` (previously file-local) so the child can import it. New `eventPropertiesTaxonomicGroupsLogic.ts`: - Connects to `projectLogic` and `featureFlagLogic`. - Owns `eventNames`, `excludedProperties`, `propertyAllowList` prop-passthrough selectors. - Owns `eventPropertiesTaxonomicGroups` returning the two-element array (Event properties + Internal event properties). Parent `taxonomicFilterLogic`: - Connects to the new logic, added to `extractedTaxonomicGroups` bundle. - Removes the two inline tabs, replaces with the spread. - Drops `featureFlagLogic` connect and `featureFlags` selector input/ signature param. - Drops now-unused `buildEventTypeFilterShortcuts`, `withKeywordShortcuts`, `FEATURE_FLAGS` imports. No behaviour change. Agent-written. Tests: - `hogli test frontend/src/lib/components/TaxonomicFilter/` — 362/362 pass. - TS check on touched files — clean. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
…parent Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
pauldambra
force-pushed
the
posthog-code/extract-event-properties-taxonomic-logic
branch
from
a859ef4 to
e001c8a
Compare
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
Member
Author
|
Closing — the extraction approach hit kea's 16-dep SelectorTuple limit repeatedly and needs a different architectural strategy. The stack is being abandoned and master is being reverted back to the state right after #59816. See the upcoming revert PR for details. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Changes
How did you test this code?
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Publish to changelog?
Docs update
🤖 Agent context