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

feat: pre-fill existing tags for insights, actions, event definitions, and property definitions #12544

Merged
merged 12 commits into from
Nov 24, 2022
14 changes: 14 additions & 0 deletions frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,10 @@ class ApiRequest {
return this.projectsDetail(teamId).addPathComponent('event_definitions')
}

public eventDefinitionsTags(teamId?: TeamType['id']): ApiRequest {
return this.projectsDetail(teamId).addPathComponent('event_definitions').addPathComponent('tags')
}

public eventDefinitionDetail(eventDefinitionId: EventDefinition['id'], teamId?: TeamType['id']): ApiRequest {
return this.projectsDetail(teamId).addPathComponent('event_definitions').addPathComponent(eventDefinitionId)
}
Expand All @@ -213,6 +217,10 @@ class ApiRequest {
return this.projectsDetail(teamId).addPathComponent('property_definitions')
}

public propertyDefinitionsTags(teamId?: TeamType['id']): ApiRequest {
return this.projectsDetail(teamId).addPathComponent('property_definitions').addPathComponent('tags')
}

public propertyDefinitionDetail(
propertyDefinitionId: PropertyDefinition['id'],
teamId?: TeamType['id']
Expand Down Expand Up @@ -529,6 +537,9 @@ const api = {
.withQueryString(toParams({ limit, ...params }))
.get()
},
async list_tags(teamId: TeamType['id'] = getCurrentTeamId()): Promise<string[]> {
return new ApiRequest().eventDefinitionsTags(teamId).get()
},
determineListEndpoint({
limit = EVENT_DEFINITIONS_PER_PAGE,
teamId = getCurrentTeamId(),
Expand Down Expand Up @@ -589,6 +600,9 @@ const api = {
)
.get()
},
async list_tags(teamId: TeamType['id'] = getCurrentTeamId()): Promise<string[]> {
return new ApiRequest().propertyDefinitionsTags(teamId).get()
},
determineListEndpoint({
limit = EVENT_PROPERTY_DEFINITIONS_PER_PAGE,
teamId = getCurrentTeamId(),
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/models/actionsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { kea } from 'kea'
import api from 'lib/api'
import { ActionType } from '~/types'
import type { actionsModelType } from './actionsModelType'
import { uniqueBy } from 'lib/utils'

export interface ActionsModelProps {
params?: string
Expand Down Expand Up @@ -43,6 +44,15 @@ export const actionsModel = kea<actionsModelType>({
(actions): Partial<Record<string | number, ActionType>> =>
Object.fromEntries(actions.map((action) => [action.id, action])),
],
actionsTags: [
(s) => [s.actions],
(actions): string[] => {
return uniqueBy(
actions.flatMap(({ tags }) => tags || ''),
(item) => item
).sort()
},
],
}),

events: ({ actions }) => ({
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/scenes/actions/ActionEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { LemonInput } from 'lib/components/LemonInput/LemonInput'
import { Form } from 'kea-forms'
import { LemonLabel } from 'lib/components/LemonLabel/LemonLabel'
import { IconPlayCircle } from 'lib/components/icons'
import { actionsModel } from '~/models/actionsModel'

export function ActionEdit({ action: loadedAction, id, onSave, temporaryToken }: ActionEditLogicProps): JSX.Element {
const logicProps: ActionEditLogicProps = {
Expand All @@ -34,6 +35,7 @@ export function ActionEdit({ action: loadedAction, id, onSave, temporaryToken }:
const { submitAction, deleteAction } = useActions(logic)
const { currentTeam } = useValues(teamLogic)
const { hasAvailableFeature } = useValues(userLogic)
const { actionsTags } = useValues(actionsModel)

const slackEnabled = currentTeam?.slack_incoming_webhook

Expand Down Expand Up @@ -132,6 +134,7 @@ export function ActionEdit({ action: loadedAction, id, onSave, temporaryToken }:
onChange={(_, newTags) => onChange(newTags)}
className="action-tags"
saving={actionLoading}
tagsAvailable={actionsTags.filter((tag) => !action.tags?.includes(tag))}
/>
)}
</Field>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Form } from 'kea-forms'

export function DefinitionEdit(props: DefinitionEditLogicProps): JSX.Element {
const logic = definitionEditLogic(props)
const { definitionLoading, definition, hasTaxonomyFeatures, isEvent } = useValues(logic)
const { definitionLoading, definition, hasTaxonomyFeatures, isEvent, tags, tagsLoading } = useValues(logic)
const { setPageMode, saveDefinition } = useActions(logic)

return (
Expand Down Expand Up @@ -90,10 +90,11 @@ export function DefinitionEdit(props: DefinitionEditLogicProps): JSX.Element {
{({ value, onChange }) => (
<ObjectTags
className="definition-tags"
saving={definitionLoading}
saving={definitionLoading || tagsLoading}
tags={value || []}
onChange={(_, tags) => onChange(tags)}
style={{ marginBottom: 4 }}
tagsAvailable={tags}
/>
)}
</Field>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,18 @@ describe('definitionEditLogic', () => {
beforeEach(async () => {
useMocks({
get: {
'/api/projects/:team/event_definitions/:id': mockEventDefinitions[0],
'/api/projects/:team/property_definitions/:id': mockEventPropertyDefinition,
'/api/projects/:team/event_definitions/:id': (req) => {
if (req.params['id'] === 'tags') {
return [200, ['the', 'event', 'tags', 'array']]
}
return [200, mockEventDefinitions[0]]
},
'/api/projects/:team/property_definitions/:id': (req) => {
if (req.params['id'] === 'tags') {
return [200, ['the', 'property', 'tags', 'array']]
}
return [200, mockEventPropertyDefinition]
},
'/api/projects/@current/event_definitions/': {
results: mockEventDefinitions,
count: mockEventDefinitions.length,
Expand All @@ -32,22 +42,71 @@ describe('definitionEditLogic', () => {
},
})
initKeaTests()
await expectLogic(definitionLogic({ id: '1' })).toFinishAllListeners()
eventDefinitionsTableLogic.mount()
eventPropertyDefinitionsTableLogic.mount()
logic = definitionEditLogic({ id: '1', definition: mockEventDefinitions[0] })
logic.mount()
})

it('save definition', async () => {
router.actions.push(urls.eventDefinition('1'))
await expectLogic(logic, () => {
logic.actions.saveDefinition(mockEventDefinitions[0])
}).toDispatchActionsInAnyOrder([
'saveDefinition',
'setPageMode',
'setDefinition',
eventDefinitionsTableLogic.actionCreators.setLocalEventDefinition(mockEventDefinitions[0]),
])
describe('for event definitions', () => {
beforeEach(async () => {
router.actions.push(urls.eventDefinition('1'))
await expectLogic(definitionLogic({ id: '1' })).toFinishAllListeners()
eventDefinitionsTableLogic.mount()
eventPropertyDefinitionsTableLogic.mount()
logic = definitionEditLogic({ id: '1', definition: mockEventDefinitions[0] })
logic.mount()
})

it('save definition', async () => {
await expectLogic(logic, () => {
logic.actions.saveDefinition(mockEventDefinitions[0])
}).toDispatchActionsInAnyOrder([
'saveDefinition',
'setPageMode',
'setDefinition',
eventDefinitionsTableLogic.actionCreators.setLocalEventDefinition(mockEventDefinitions[0]),
])
})

it('can load tags', async () => {
await expectLogic(logic, () => {
logic.actions.loadTags()
})
.toFinishAllListeners()
.toMatchValues({
tags: ['the', 'event', 'tags', 'array'],
})
})
})

describe('for property definitions', () => {
beforeEach(async () => {
router.actions.push(urls.eventPropertyDefinition('1'))
await expectLogic(definitionLogic({ id: '1' })).toFinishAllListeners()
eventDefinitionsTableLogic.mount()
eventPropertyDefinitionsTableLogic.mount()
logic = definitionEditLogic({ id: '1', definition: mockEventDefinitions[0] })
logic.mount()
})

it('save definition', async () => {
await expectLogic(logic, () => {
logic.actions.saveDefinition(mockEventPropertyDefinition)
}).toDispatchActionsInAnyOrder([
'saveDefinition',
'setPageMode',
'setDefinition',
eventPropertyDefinitionsTableLogic.actionCreators.setLocalEventPropertyDefinition(
mockEventPropertyDefinition
),
])
})

it('can load tags', async () => {
await expectLogic(logic, () => {
logic.actions.loadTags()
})
.toFinishAllListeners()
.toMatchValues({
tags: ['the', 'property', 'tags', 'array'],
})
})
})
})
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { beforeUnmount, connect, kea, key, path, props } from 'kea'
import { afterMount, beforeUnmount, connect, kea, key, path, props } from 'kea'
import { Definition, EventDefinition, PropertyDefinition } from '~/types'
import { forms } from 'kea-forms'
import { loaders } from 'kea-loaders'
Expand Down Expand Up @@ -46,6 +46,15 @@ export const definitionEditLogic = kea<definitionEditLogicType>([
},
})),
loaders(({ values, props, actions }) => ({
tags: {
loadTags: async () => {
if (values.isEvent) {
return await api.eventDefinitions.list_tags()
} else {
return await api.propertyDefinitions.list_tags()
}
},
},
definition: [
{ ...props.definition } as Definition,
{
Expand Down Expand Up @@ -95,4 +104,7 @@ export const definitionEditLogic = kea<definitionEditLogicType>([
beforeUnmount(({ actions }) => {
actions.setPageMode(DefinitionPageMode.View)
}),
afterMount(({ actions }) => {
actions.loadTags()
}),
])
4 changes: 3 additions & 1 deletion frontend/src/scenes/insights/Insight.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export function Insight({ insightId }: { insightId: InsightShortId | 'new' }): J
const { cohortsById } = useValues(cohortsModel)
const { mathDefinitions } = useValues(mathsLogic)

const { insightsTags } = useValues(savedInsightsLogic)

useEffect(() => {
reportInsightViewedForRecentInsights()
}, [insightId])
Expand Down Expand Up @@ -255,7 +257,7 @@ export function Insight({ insightId }: { insightId: InsightShortId | 'new' }): J
tags={insight.tags ?? []}
onChange={(_, tags) => setInsightMetadata({ tags: tags ?? [] })}
saving={tagLoading}
tagsAvailable={[]}
tagsAvailable={insightsTags}
className="insight-metadata-tags"
data-attr="insight-tags"
/>
Expand Down
15 changes: 12 additions & 3 deletions frontend/src/scenes/saved-insights/savedInsightsLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { dashboardsModel } from '~/models/dashboardsModel'

jest.spyOn(api, 'create')

const createInsight = (id: number, string = 'hi'): InsightModel =>
const createInsight = (id: number, string = 'hi', tags: string[] = []): InsightModel =>
({
id: id || 1,
name: `${string} ${id || 1}`,
Expand All @@ -25,7 +25,7 @@ const createInsight = (id: number, string = 'hi'): InsightModel =>
is_sample: false,
updated_at: 'now',
result: {},
tags: [],
tags: tags,
color: null,
created_at: 'now',
dashboard: null,
Expand All @@ -36,7 +36,11 @@ const createInsight = (id: number, string = 'hi'): InsightModel =>
} as any as InsightModel)
const createSavedInsights = (string = 'hello'): InsightsResult => ({
count: 3,
results: [createInsight(1, string), createInsight(2, string), createInsight(3, string)],
results: [
createInsight(1, string, ['marketing', 'vip']),
createInsight(2, string, ['seo']),
createInsight(3, string),
],
})

describe('savedInsightsLogic', () => {
Expand Down Expand Up @@ -182,6 +186,7 @@ describe('savedInsightsLogic', () => {
expect.objectContaining({ name: 'should be copied (copy)' })
)
})

it('can duplicate using name', async () => {
const sourceInsight = createInsight(123, 'hello')
sourceInsight.name = 'should be copied'
Expand All @@ -198,4 +203,8 @@ describe('savedInsightsLogic', () => {
dashboardsModel.actions.duplicateDashboardSuccess({} as DashboardType, {} as any)
}).toDispatchActions(['loadInsights'])
})

it('gathers tags from loaded insights', async () => {
await expectLogic(logic).toMatchValues({ insightsTags: ['marketing', 'seo', 'vip'] })
})
})
10 changes: 9 additions & 1 deletion frontend/src/scenes/saved-insights/savedInsightsLogic.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { kea } from 'kea'
import { router } from 'kea-router'
import api from 'lib/api'
import { objectDiffShallow, objectsEqual, toParams } from 'lib/utils'
import { objectDiffShallow, objectsEqual, toParams, uniqueBy } from 'lib/utils'
import { InsightModel, LayoutView, SavedInsightsTabs } from '~/types'
import type { savedInsightsLogicType } from './savedInsightsLogicType'
import { dayjs } from 'lib/dayjs'
Expand Down Expand Up @@ -140,6 +140,14 @@ export const savedInsightsLogic = kea<savedInsightsLogicType>({
],
},
selectors: ({ actions }) => ({
insightsTags: [
(s) => [s.insights],
(insights) =>
uniqueBy(
insights.results.flatMap(({ tags }) => tags || ''),
(item) => item
).sort(),
],
filters: [(s) => [s.rawFilters], (rawFilters): SavedInsightFilters => cleanFilters(rawFilters || {})],
count: [(s) => [s.insights], (insights) => insights.count],
usingFilters: [
Expand Down
14 changes: 13 additions & 1 deletion posthog/api/event_definition.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from typing import Type

from django.db.models import Prefetch
from rest_framework import mixins, permissions, serializers, viewsets
from rest_framework import mixins, permissions, request, response, serializers, status, viewsets
from rest_framework.decorators import action

from posthog.api.routing import StructuredViewSetMixin
from posthog.api.shared import UserBasicSerializer
Expand Down Expand Up @@ -121,3 +122,14 @@ def get_serializer_class(self) -> Type[serializers.ModelSerializer]:

serializer_class = EnterpriseEventDefinitionSerializer # type: ignore
return serializer_class

@action(methods=["GET"], detail=False)
def tags(self, request: request.Request, **kwargs) -> response.Response:
if not self.is_licensed():
return response.Response([], status=status.HTTP_402_PAYMENT_REQUIRED)

return response.Response(
TaggedItem.objects.filter(tag__team=self.team, event_definition__isnull=False)
.values_list("tag__name", flat=True)
.distinct()
)
14 changes: 13 additions & 1 deletion posthog/api/property_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

from django.db import connection
from django.db.models import Prefetch
from rest_framework import mixins, permissions, serializers, viewsets
from rest_framework import mixins, permissions, request, response, serializers, status, viewsets
from rest_framework.decorators import action
from rest_framework.pagination import LimitOffsetPagination

from posthog.api.routing import StructuredViewSetMixin
Expand Down Expand Up @@ -325,3 +326,14 @@ def get_object(self):
new_enterprise_property.save()
return new_enterprise_property
return PropertyDefinition.objects.get(id=id)

@action(methods=["GET"], detail=False)
def tags(self, request: request.Request, **kwargs) -> response.Response:
if not self.is_licensed():
return response.Response([], status=status.HTTP_402_PAYMENT_REQUIRED)

return response.Response(
TaggedItem.objects.filter(tag__team=self.team, property_definition__isnull=False)
.values_list("tag__name", flat=True)
.distinct()
)
Loading