refactor(taxonomic-filter): extract errorTrackingTaxonomicGroupsLogic#59827
refactor(taxonomic-filter): extract errorTrackingTaxonomicGroupsLogic#59827pauldambra wants to merge 2 commits into
Conversation
|
🎭 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. |
|
Size Change: 0 B Total Size: 80.6 MB ℹ️ View Unchanged
|
|
👋 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. |
cb8629f to
9526b64
Compare
4c536fc to
4abe0d5
Compare
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/errorTrackingTaxonomicGroupsLogic.ts:51
The `valuesEndpoint` lambda mixes a template literal for the static portion with string concatenation for `key`. Using a single template literal is more consistent with modern JS/TS style and avoids any accidental encoding differences if `key` were ever changed to an object.
```suggestion
valuesEndpoint: (key) => `api/environments/${projectId}/error_tracking/issues/values?key=${key}`,
```
Reviews (1): Last reviewed commit: "refactor(taxonomic-filter): extract erro..." | Re-trigger Greptile |
75063b7 to
ee00776
Compare
b7225b4 to
e0e374e
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Clean structural refactor moving pre-existing error tracking group definitions verbatim into a dedicated kea logic, matching the established pattern of sibling files. Behavior is unchanged and the one resolved inline comment was a minor style suggestion on code moved verbatim that the author correctly deferred.
ee00776 to
822c6f4
Compare
e0e374e to
367a0e7
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Pure structural refactor moving pre-existing error tracking group definitions verbatim into a dedicated kea logic file, matching the established pattern of sibling files in the same directory. Behavior is unchanged and no showstoppers are present.
fdcf048 to
c70b12f
Compare
367a0e7 to
389d6d7
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
389d6d7 to
4e00b66
Compare
75b6b7e to
60b55c4
Compare
4e00b66 to
92475c2
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Clean structural refactor moving pre-existing error tracking group definitions verbatim into a dedicated kea logic file, matching the established pattern of sibling files in the same directory. Behavior is unchanged and the one resolved inline comment was a minor style suggestion on code moved verbatim.
92475c2 to
642f26a
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
Twelfth slice. `Issues` and `Exception properties` are both error tracking concerns and read from the parent's `excludedProperties`, `currentTeam`, `currentProjectId`. Also: this slice would have pushed the parent's `taxonomicGroups` input tuple past kea's 16-input cap (we already had 16 inputs after adding `posthogResourcesTaxonomicGroups`). So this PR also introduces an **`extractedTaxonomicGroups` bundle selector** in the parent that collapses all 10 per-data-type child group arrays into a single object-shaped input. The parent's `taxonomicGroups` selector now has room to keep growing as further data types are pulled out. This bundle selector is a transitional shape — the end state (per the user's stack strategy: "one data type at a time with a parent logic gathering the different types") is to lift `taxonomicGroups` itself into a composition logic that consumes per-data-type child outputs. The bundle is the seam where that composition will eventually live. New `errorTrackingTaxonomicGroupsLogic.ts`: - Connects to `teamLogic`, `projectLogic`. - Owns its own `excludedProperties` selector (small duplication — the parent still has the same selector for tabs that haven't moved yet). - Owns `errorTrackingTaxonomicGroups` returning Issues + Exception properties tabs. Parent `taxonomicFilterLogic`: - New `extractedTaxonomicGroups` bundle selector wrapping all ten extracted per-data-type group arrays (cohort, hogQLExpression, eventMetadata, maxAIContext, suggestedFilters, apm, recentPinned, replay, posthogResources, errorTracking). - `taxonomicGroups` now consumes the bundle as one selector input (replacing what would have been 10 individual inputs), then destructures back to the individual arrays inside the body. - Removes the two inline error tracking tabs, replaces with spread. 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
…rent Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
642f26a to
c3496d3
Compare
|
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. |

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