Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(insights): query based duplicateInsight #23435

Merged
merged 15 commits into from
Jul 12, 2024
Merged
117 changes: 61 additions & 56 deletions frontend/src/layout/navigation-3000/sidebars/insights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { urls } from 'scenes/urls'

import { navigation3000Logic } from '~/layout/navigation-3000/navigationLogic'
import { insightsModel } from '~/models/insightsModel'
import { getQueryBasedInsightModel } from '~/queries/nodes/InsightViz/utils'
import { InsightModel } from '~/types'

import { BasicListItem, SidebarCategory } from '../types'
Expand Down Expand Up @@ -52,65 +53,69 @@ export const insightsSidebarLogic = kea<insightsSidebarLogicType>([
key: 'insights',
noun: 'insight',
onAdd: urls.insightNew(),
items: infiniteInsights.map(
(insight) =>
insight &&
({
key: insight.short_id,
name: insight.name || insight.derived_name || 'Untitled',
isNamePlaceholder: !insight.name,
url: urls.insightView(insight.short_id),
searchMatch: findSearchTermInItemName(
insight.name || insight.derived_name || '',
values.searchTerm
),
menuItems: (initiateRename) => [
{
items: [
{
to: urls.insightEdit(insight.short_id),
label: 'Edit',
},
{
onClick: () => {
actions.duplicateInsight(insight)
},
label: 'Duplicate',
},
],
},
{
items: [
{
onClick: initiateRename,
label: 'Rename',
keyboardShortcut: ['enter'],
},
{
onClick: () => {
void deleteWithUndo({
object: insight,
endpoint: `projects/${currentTeamId}/insights`,
callback: actions.loadInsights,
})
},
status: 'danger',
label: 'Delete insight',
items: infiniteInsights.map((legacyInsight) => {
if (!legacyInsight) {
return undefined
}

const insight = getQueryBasedInsightModel(legacyInsight)

return {
key: insight.short_id,
name: insight.name || insight.derived_name || 'Untitled',
isNamePlaceholder: !insight.name,
url: urls.insightView(insight.short_id),
searchMatch: findSearchTermInItemName(
insight.name || insight.derived_name || '',
values.searchTerm
),
menuItems: (initiateRename) => [
{
items: [
{
to: urls.insightEdit(insight.short_id),
label: 'Edit',
},
{
onClick: () => {
actions.duplicateInsight(insight)
},
],
},
],
onRename: async (newName) => {
const updatedItem = await api.update(
`api/projects/${teamLogic.values.currentTeamId}/insights/${insight.id}`,
label: 'Duplicate',
},
],
},
{
items: [
{
name: newName,
}
)
insightsModel.actions.renameInsightSuccess(updatedItem)
onClick: initiateRename,
label: 'Rename',
keyboardShortcut: ['enter'],
},
{
onClick: () => {
void deleteWithUndo({
object: insight,
endpoint: `projects/${currentTeamId}/insights`,
callback: actions.loadInsights,
})
},
status: 'danger',
label: 'Delete insight',
},
],
},
} as BasicListItem)
),
],
onRename: async (newName) => {
const updatedItem = await api.update(
`api/projects/${teamLogic.values.currentTeamId}/insights/${insight.id}`,
{
name: newName,
}
)
insightsModel.actions.renameInsightSuccess(updatedItem)
},
} as BasicListItem
}),
loading: insightsLoading,
remote: {
isItemLoaded: (index) => !!(cache.requestedInsights[index] || infiniteInsights[index]),
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,9 @@ const api = {
)
.get()
},
async create(data: any): Promise<InsightModel> {
return await new ApiRequest().insights().create({ data })
},
},

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

type FilterTypeActionsAndEvents = {
events?: ActionFilter[]
Expand Down Expand Up @@ -121,6 +123,28 @@ const filterMap: 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: insightMap[query.kind],
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/scenes/insights/InsightPageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { urls } from 'scenes/urls'

import { tagsModel } from '~/models/tagsModel'
import { DataTableNode, NodeKind } from '~/queries/schema'
import { ExporterFormat, InsightLogicProps, InsightModel, InsightShortId, ItemMode, NotebookNodeType } from '~/types'
import { ExporterFormat, InsightLogicProps, ItemMode, NotebookNodeType } from '~/types'

export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: InsightLogicProps }): JSX.Element {
// insightSceneLogic
Expand Down Expand Up @@ -69,14 +69,14 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
<>
<SubscriptionsModal
isOpen={insightMode === ItemMode.Subscriptions}
closeModal={() => push(urls.insightView(insight.short_id as InsightShortId))}
closeModal={() => push(urls.insightView(insight.short_id))}
insightShortId={insight.short_id}
subscriptionId={subscriptionId}
/>
<SharingModal
title="Insight sharing"
isOpen={insightMode === ItemMode.Sharing}
closeModal={() => push(urls.insightView(insight.short_id as InsightShortId))}
closeModal={() => push(urls.insightView(insight.short_id))}
insightShortId={insight.short_id}
insight={insight}
previewIframe
Expand All @@ -89,8 +89,8 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
/>
<AlertsModal
isOpen={insightMode === ItemMode.Alerts}
closeModal={() => push(urls.insightView(insight.short_id as InsightShortId))}
insightShortId={insight.short_id as InsightShortId}
closeModal={() => push(urls.insightView(insight.short_id))}
insightShortId={insight.short_id}
alertId={subscriptionId}
/>
<NewDashboardModal />
Expand All @@ -105,7 +105,7 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
{hasDashboardItemId && (
<>
<LemonButton
onClick={() => duplicateInsight(legacyInsight as InsightModel, true)}
onClick={() => duplicateInsight(insight, true)}
fullWidth
data-attr="duplicate-insight-from-insight-view"
>
Expand Down
42 changes: 22 additions & 20 deletions frontend/src/scenes/insights/insightLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,18 @@ import { dashboardsModel } from '~/models/dashboardsModel'
import { groupsModel } from '~/models/groupsModel'
import { insightsModel } from '~/models/insightsModel'
import { tagsModel } from '~/models/tagsModel'
import { queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter'
import { getInsightFilterOrQueryForPersistance } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter'
import { getQueryBasedInsightModel } from '~/queries/nodes/InsightViz/utils'
import { InsightVizNode } from '~/queries/schema'
import { isInsightVizNode } from '~/queries/utils'
import { FilterType, InsightLogicProps, InsightModel, InsightShortId, ItemMode, SetInsightOptions } from '~/types'
import {
FilterType,
InsightLogicProps,
InsightModel,
InsightShortId,
ItemMode,
QueryBasedInsightModel,
SetInsightOptions,
} from '~/types'

import { teamLogic } from '../teamLogic'
import type { insightLogicType } from './insightLogicType'
Expand Down Expand Up @@ -314,7 +321,10 @@ export const insightLogic = kea<insightLogicType>([
(s) => [s.featureFlags],
(featureFlags) => !!featureFlags[FEATURE_FLAGS.QUERY_BASED_INSIGHTS_SAVING],
],
queryBasedInsight: [(s) => [s.legacyInsight], (legacyInsight) => getQueryBasedInsightModel(legacyInsight)],
queryBasedInsight: [
(s) => [s.legacyInsight],
(legacyInsight) => getQueryBasedInsightModel(legacyInsight) as QueryBasedInsightModel,
],
insightProps: [() => [(_, props) => props], (props): InsightLogicProps => props],
isInDashboardContext: [() => [(_, props) => props], ({ dashboardId }) => !!dashboardId],
hasDashboardItemId: [
Expand Down Expand Up @@ -367,14 +377,10 @@ export const insightLogic = kea<insightLogicType>([
const { name, description, favorited, deleted, dashboards, tags } = values.legacyInsight

let savedInsight: InsightModel
let filters
let query

if (!values.queryBasedInsightSaving && isInsightVizNode(values.queryBasedInsight.query)) {
filters = queryNodeToFilter(values.queryBasedInsight.query.source)
} else {
query = values.queryBasedInsight.query
}
const { filters, query } = getInsightFilterOrQueryForPersistance(
values.queryBasedInsight,
values.queryBasedInsightSaving
)

try {
// We don't want to send ALL the insight properties back to the API, so only grabbing fields that might have changed
Expand Down Expand Up @@ -435,14 +441,10 @@ export const insightLogic = kea<insightLogicType>([
}
},
saveAsNamingSuccess: async ({ name }) => {
let filters
let query
if (!values.queryBasedInsightSaving && isInsightVizNode(values.queryBasedInsight.query)) {
filters = queryNodeToFilter(values.queryBasedInsight.query.source)
} else {
query = values.queryBasedInsight.query
}

const { filters, query } = getInsightFilterOrQueryForPersistance(
values.queryBasedInsight,
values.queryBasedInsightSaving
)
const insight: InsightModel = await api.create(`api/projects/${teamLogic.values.currentTeamId}/insights/`, {
name,
filters,
Expand Down
26 changes: 26 additions & 0 deletions frontend/src/scenes/insights/utils/api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import api from 'lib/api'

import { getInsightFilterOrQueryForPersistance } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter'
import { getQueryBasedInsightModel } from '~/queries/nodes/InsightViz/utils'
import { InsightModel, QueryBasedInsightModel } from '~/types'

type ReturnedInsightModelByFlag<Flag extends boolean> = Flag extends true ? QueryBasedInsightModel : InsightModel

export const insightsApi = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially did the handling in lib/api.ts, but that doesn't work as (1) it's included in the toolbar and importing anything there explodes size (2) doing so breaks kea, likely due to some logics needing to be mounted in the right order. Therefore this wrapper.

async create<Flag extends boolean>(
insight: QueryBasedInsightModel,
options: {
writeAsQuery: boolean
readAsQuery: Flag
}
): Promise<ReturnedInsightModelByFlag<Flag>> {
const data = {
...insight,
...getInsightFilterOrQueryForPersistance(insight, options.writeAsQuery),
}
const legacyInsight: InsightModel = await api.insights.create(data)
return (
options.readAsQuery ? getQueryBasedInsightModel(legacyInsight) : legacyInsight
) as ReturnedInsightModelByFlag<Flag>
},
}
Loading
Loading