feat: define new insight urls in one place #14778
Conversation
| @@ -617,3 +620,14 @@ export function sortDates(dates: Array<string | null>): Array<string | null> { | |||
| export function getResponseBytes(apiResponse: Response): number { | |||
| return parseInt(apiResponse.headers.get('Content-Length') ?? '0') | |||
| } | |||
|
|
|||
| export const insightTypeURL: Record<InsightType, string> = { | |||
There was a problem hiding this comment.
So urls.insightNew({ insight: insightType }) shouldn't be used directly? 🧐 That seems like a redundant level of complexity. Shouldn't we do this branching inside insightNew so that there's only one way of doing this?
There was a problem hiding this comment.
There are 48 calls to insightNew either because they're passing a specific Filter or are passing nothing happy to get the default.
This was only to pull into one place "please can I have a new insight of a specific type with defaults" as opposed to "can I have the default new insight" or "can I have this specific new insight"
Many of the calls will change as we swap from Filter and Query
There was a problem hiding this comment.
Hm, yeah, it's gonna get fun with Querys 😅
* master: (101 commits) feat: feedback tab - improve the UI and switch to HogQL (#14777) feat(experiments): cache experiment results (#14742) fix(empty-state): properly fix query timeout state (#14793) chore(feature-flag): release json payload flag (#14775) feat(hogql): make sure joins work with properties (#14790) test(frontend): Tune visual regression test failure threshold (#14766) feat(hogql): count distinct and count star (#14786) feat(perf): only load experiment results once (#14772) feat: allow new insight type choice from sidebar (#14759) fix(persons): hide persons in the future (#14308) feat: allow query cards in saved insights grid (#14784) fix(lemon-ui): Align padding for small/large buttons with icons (#14750) chore(deps): Upgrade TypeScript from 4.8 to 4.9 in the frontend (#14609) feat: rename insight query tab to json tab (#14781) feat: use same top heading for insight card and exported insight (#14780) feat: define new insight urls in one place (#14778) fix(hogql): person properties in a funnel breakdown (#14765) fix: hide query based insights in recently viewed list (#14774) feat: improve query summaries (#14768) feat(hogql): add sampling support (#14733) ...
Problem
new insight urls are built in more than one place, so they'd got out of sync
Changes
defines them in one place
How did you test this code?
👀 locally