Skip to content

Commit

Permalink
refactor(insights): remove query-based-insights-saving flag (#24547)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Aug 23, 2024
1 parent 577f7ab commit f68218a
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 171 deletions.
17 changes: 1 addition & 16 deletions frontend/src/layout/navigation-3000/sidebars/insights.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { afterMount, connect, kea, listeners, path, reducers, selectors } from 'kea'
import { subscriptions } from 'kea-subscriptions'
import { FEATURE_FLAGS } from 'lib/constants'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { deleteInsightWithUndo } from 'lib/utils/deleteWithUndo'
import { insightsApi } from 'scenes/insights/utils/api'
import { INSIGHTS_PER_PAGE, savedInsightsLogic } from 'scenes/saved-insights/savedInsightsLogic'
Expand All @@ -28,8 +26,6 @@ export const insightsSidebarLogic = kea<insightsSidebarLogicType>([
['activeScene', 'sceneParams'],
navigation3000Logic,
['searchTerm'],
featureFlagLogic,
['featureFlags'],
],
actions: [savedInsightsLogic, ['loadInsights', 'setSavedInsightsFilters', 'duplicateInsight']],
})),
Expand All @@ -49,10 +45,6 @@ export const insightsSidebarLogic = kea<insightsSidebarLogicType>([
],
})),
selectors(({ actions, values, cache }) => ({
queryBasedInsightSaving: [
(s) => [s.featureFlags],
(featureFlags) => !!featureFlags[FEATURE_FLAGS.QUERY_BASED_INSIGHTS_SAVING],
],
contents: [
(s) => [s.insights, s.infiniteInsights, s.insightsLoading, teamLogic.selectors.currentTeamId],
(insights, infiniteInsights, insightsLoading, currentTeamId) => [
Expand Down Expand Up @@ -102,9 +94,6 @@ export const insightsSidebarLogic = kea<insightsSidebarLogicType>([
object: insight,
endpoint: `projects/${currentTeamId}/insights`,
callback: actions.loadInsights,
options: {
writeAsQuery: values.queryBasedInsightSaving,
},
})
},
status: 'danger',
Expand All @@ -114,11 +103,7 @@ export const insightsSidebarLogic = kea<insightsSidebarLogicType>([
},
],
onRename: async (newName) => {
const updatedItem = await insightsApi.update(
insight.id,
{ name: newName },
{ writeAsQuery: values.queryBasedInsightSaving }
)
const updatedItem = await insightsApi.update(insight.id, { name: newName })
insightsModel.actions.renameInsightSuccess(updatedItem)
},
} as BasicListItem
Expand Down
1 change: 0 additions & 1 deletion frontend/src/lib/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export const FEATURE_FLAGS = {
PRODUCT_SPECIFIC_ONBOARDING: 'product-specific-onboarding', // owner: @raquelmsmith
REDIRECT_SIGNUPS_TO_INSTANCE: 'redirect-signups-to-instance', // owner: @raquelmsmith
APPS_AND_EXPORTS_UI: 'apps-and-exports-ui', // owner: @benjackwhite
QUERY_BASED_INSIGHTS_SAVING: 'query-based-insights-saving', // owner: @thmsobrmlr
HOGQL_DASHBOARD_ASYNC: 'hogql-dashboard-async', // owner: @webjunkie
WEBHOOKS_DENYLIST: 'webhooks-denylist', // owner: #team-pipeline
PIPELINE_UI: 'pipeline-ui', // owner: #team-pipeline
Expand Down
7 changes: 2 additions & 5 deletions frontend/src/lib/utils/deleteWithUndo.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { lemonToast } from '@posthog/lemon-ui'
import api from 'lib/api'
import { getInsightModel, InsightsApiOptions } from 'scenes/insights/utils/api'

import { QueryBasedInsightModel } from '~/types'

Expand Down Expand Up @@ -40,18 +39,16 @@ export async function deleteWithUndo<T extends Record<string, any>>({
* when given a query based insight */
export async function deleteInsightWithUndo({
undo = false,
options,
...props
}: {
undo?: boolean
endpoint: string
object: QueryBasedInsightModel
idField?: keyof QueryBasedInsightModel
callback?: (undo: boolean, object: QueryBasedInsightModel) => void
options: InsightsApiOptions
}): Promise<void> {
await api.update(`api/${props.endpoint}/${props.object[props.idField || 'id']}`, {
...getInsightModel(props.object, options.writeAsQuery),
...props.object,
deleted: !undo,
})
props.callback?.(undo, props.object)
Expand All @@ -66,7 +63,7 @@ export async function deleteInsightWithUndo({
? undefined
: {
label: 'Undo',
action: () => deleteInsightWithUndo({ undo: true, options, ...props }),
action: () => deleteInsightWithUndo({ undo: true, ...props }),
},
}
)
Expand Down
24 changes: 5 additions & 19 deletions frontend/src/models/insightsModel.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { LemonDialog, LemonInput } from '@posthog/lemon-ui'
import { actions, connect, kea, listeners, path, selectors } from 'kea'
import { FEATURE_FLAGS } from 'lib/constants'
import { actions, connect, kea, listeners, path } from 'kea'
import { LemonField } from 'lib/lemon-ui/LemonField'
import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { insightsApi } from 'scenes/insights/utils/api'
import { teamLogic } from 'scenes/teamLogic'

Expand All @@ -13,7 +11,7 @@ import type { insightsModelType } from './insightsModelType'

export const insightsModel = kea<insightsModelType>([
path(['models', 'insightsModel']),
connect({ values: [featureFlagLogic, ['featureFlags']], logic: [teamLogic] }),
connect({ logic: [teamLogic] }),
actions(() => ({
renameInsight: (item: QueryBasedInsightModel) => ({ item }),
renameInsightSuccess: (item: QueryBasedInsightModel) => ({ item }),
Expand All @@ -25,13 +23,7 @@ export const insightsModel = kea<insightsModelType>([
insightIds,
}),
})),
selectors({
queryBasedInsightSaving: [
(s) => [s.featureFlags],
(featureFlags) => !!featureFlags[FEATURE_FLAGS.QUERY_BASED_INSIGHTS_SAVING],
],
}),
listeners(({ actions, values }) => ({
listeners(({ actions }) => ({
renameInsight: async ({ item }) => {
LemonDialog.openForm({
title: 'Rename insight',
Expand All @@ -45,11 +37,7 @@ export const insightsModel = kea<insightsModelType>([
insightName: (name) => (!name ? 'You must enter a name' : undefined),
},
onSubmit: async ({ insightName }) => {
const updatedItem = await insightsApi.update(
item.id,
{ name: insightName },
{ writeAsQuery: values.queryBasedInsightSaving }
)
const updatedItem = await insightsApi.update(item.id, { name: insightName })
lemonToast.success(
<>
Renamed insight from <b>{item.name}</b> to <b>{insightName}</b>
Expand All @@ -60,9 +48,7 @@ export const insightsModel = kea<insightsModelType>([
})
},
duplicateInsight: async ({ item }) => {
const addedItem = await insightsApi.duplicate(item, {
writeAsQuery: values.queryBasedInsightSaving,
})
const addedItem = await insightsApi.duplicate(item)

actions.duplicateInsightSuccess(addedItem)
lemonToast.success('Insight duplicated')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
InsightNodeKind,
InsightQueryNode,
LifecycleFilterLegacy,
Node,
NodeKind,
PathsFilterLegacy,
RetentionFilterLegacy,
Expand All @@ -22,14 +21,13 @@ import {
isDataWarehouseNode,
isEventsNode,
isFunnelsQuery,
isInsightVizNode,
isLifecycleQuery,
isPathsQuery,
isRetentionQuery,
isStickinessQuery,
isTrendsQuery,
} from '~/queries/utils'
import { ActionFilter, EntityTypes, FilterType, InsightType, QueryBasedInsightModel } from '~/types'
import { ActionFilter, EntityTypes, FilterType, InsightType } from '~/types'

type FilterTypeActionsAndEvents = {
events?: ActionFilter[]
Expand Down Expand Up @@ -123,28 +121,6 @@ const nodeKindToFilterKey: Record<InsightNodeKind, string> = {
[NodeKind.LifecycleQuery]: 'lifecycleFilter',
}

/** Returns a `query` or converted `filters` for a query based insight,
* depending on the feature flag. This is necessary as we want to
* transition to query based insights on the frontend, while the backend
* still has filter based insights (and or conversion function is frontend side).
*
* The feature flag can be enabled once we want to persist query based insights
* backend side. Once the flag is rolled out 100% this function becomes obsolete.
*/
export const getInsightFilterOrQueryForPersistance = (
insight: QueryBasedInsightModel,
queryBasedInsightSavingFlag: boolean
): { filters: Partial<FilterType> | undefined; query: Node<Record<string, any>> | null | undefined } => {
let filters
let query
if (!queryBasedInsightSavingFlag && isInsightVizNode(insight.query)) {
filters = queryNodeToFilter(insight.query.source)
} else {
query = insight.query
}
return { filters, query }
}

export const queryNodeToFilter = (query: InsightQueryNode): Partial<FilterType> => {
const filters: Partial<FilterType> = objectClean({
insight: nodeKindToInsightType[query.kind],
Expand Down
15 changes: 3 additions & 12 deletions frontend/src/scenes/insights/InsightPageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,9 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
const { setInsightMode } = useActions(insightSceneLogic)

// insightLogic
const {
insightProps,
canEditInsight,
insight,
queryBasedInsightSaving,
insightChanged,
insightSaving,
hasDashboardItemId,
} = useValues(insightLogic(insightLogicProps))
const { insightProps, canEditInsight, insight, insightChanged, insightSaving, hasDashboardItemId } = useValues(
insightLogic(insightLogicProps)
)
const { setInsightMetadata, saveAs, saveInsight } = useActions(insightLogic(insightLogicProps))

// savedInsightsLogic
Expand Down Expand Up @@ -248,9 +242,6 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
loadInsights()
push(urls.savedInsights())
},
options: {
writeAsQuery: queryBasedInsightSaving,
},
})
}
fullWidth
Expand Down
45 changes: 12 additions & 33 deletions frontend/src/scenes/insights/insightLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ import { LemonDialog, LemonInput } from '@posthog/lemon-ui'
import { actions, connect, events, kea, key, listeners, LogicWrapper, path, props, reducers, selectors } from 'kea'
import { loaders } from 'kea-loaders'
import { router } from 'kea-router'
import { DashboardPrivilegeLevel, FEATURE_FLAGS } from 'lib/constants'
import { DashboardPrivilegeLevel } from 'lib/constants'
import { LemonField } from 'lib/lemon-ui/LemonField'
import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { objectsEqual } from 'lib/utils'
import { eventUsageLogic, InsightEventSource } from 'lib/utils/eventUsageLogic'
import { dashboardLogic } from 'scenes/dashboard/dashboardLogic'
Expand All @@ -29,7 +28,7 @@ import { teamLogic } from '../teamLogic'
import { insightDataLogic } from './insightDataLogic'
import type { insightLogicType } from './insightLogicType'
import { getInsightId } from './utils'
import { insightsApi, InsightsApiOptions } from './utils/api'
import { insightsApi } from './utils/api'

export const UNSAVED_INSIGHT_MIN_REFRESH_INTERVAL_MINUTES = 3

Expand Down Expand Up @@ -59,8 +58,6 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
['mathDefinitions'],
userLogic,
['user'],
featureFlagLogic,
['featureFlags'],
],
actions: [tagsModel, ['loadTags']],
logic: [eventUsageLogic, dashboardsModel],
Expand Down Expand Up @@ -113,9 +110,7 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
return values.insight
}

const response = await insightsApi.update(values.insight.id as number, insightUpdate, {
writeAsQuery: values.queryBasedInsightSaving,
})
const response = await insightsApi.update(values.insight.id as number, insightUpdate)
breakpoint()
const updatedInsight: QueryBasedInsightModel = {
...response,
Expand Down Expand Up @@ -144,9 +139,7 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
beforeUpdates[key] = values.savedInsight[key]
}

const response = await insightsApi.update(values.insight.id as number, metadataUpdate, {
writeAsQuery: values.queryBasedInsightSaving,
})
const response = await insightsApi.update(values.insight.id as number, metadataUpdate)
breakpoint()

savedInsightsLogic.findMounted()?.actions.loadInsights()
Expand All @@ -158,9 +151,7 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
label: 'Undo',
dataAttr: 'edit-insight-undo',
action: async () => {
const response = await insightsApi.update(values.insight.id as number, beforeUpdates, {
writeAsQuery: values.queryBasedInsightSaving,
})
const response = await insightsApi.update(values.insight.id as number, beforeUpdates)
savedInsightsLogic.findMounted()?.actions.loadInsights()
dashboardsModel.actions.updateDashboardInsight(response)
actions.setInsight(response, { overrideQuery: false, fromPersistentApi: true })
Expand Down Expand Up @@ -267,10 +258,6 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
(s) => [(state) => insightDataLogic.findMounted(s.insightProps(state))?.values.query || null],
(node): Node | null => node,
],
queryBasedInsightSaving: [
(s) => [s.featureFlags],
(featureFlags) => !!featureFlags[FEATURE_FLAGS.QUERY_BASED_INSIGHTS_SAVING],
],
insightProps: [() => [(_, props) => props], (props): InsightLogicProps => props],
isInDashboardContext: [() => [(_, props) => props], ({ dashboardId }) => !!dashboardId],
hasDashboardItemId: [
Expand Down Expand Up @@ -333,12 +320,9 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
tags,
}

const options: InsightsApiOptions = {
writeAsQuery: values.queryBasedInsightSaving,
}
savedInsight = insightNumericId
? await insightsApi.update(insightNumericId, insightRequest, options)
: await insightsApi.create(insightRequest, options)
? await insightsApi.update(insightNumericId, insightRequest)
: await insightsApi.create(insightRequest)
savedInsightsLogic.findMounted()?.actions.loadInsights() // Load insights afresh
actions.saveInsightSuccess()
} catch (e) {
Expand Down Expand Up @@ -407,16 +391,11 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
})
},
saveAsConfirmation: async ({ name, redirectToViewMode, persist }) => {
const insight = await insightsApi.create(
{
name,
query: values.query,
saved: true,
},
{
writeAsQuery: values.queryBasedInsightSaving,
}
)
const insight = await insightsApi.create({
name,
query: values.query,
saved: true,
})
lemonToast.info(
`You're now working on a copy of ${values.insight.name || values.insight.derived_name || name}`
)
Expand Down
Loading

0 comments on commit f68218a

Please sign in to comment.