diff --git a/ee/api/test/test_action.py b/ee/api/test/test_action.py index 9bce79647a5a6..a0d6c3a19b28d 100644 --- a/ee/api/test/test_action.py +++ b/ee/api/test/test_action.py @@ -78,8 +78,8 @@ def test_actions_does_not_nplus1(self): action.tagged_items.create(tag=tag) # django_session + user + team + look up if rate limit is enabled (cached after first lookup) - # + organizationmembership + organization + action + taggeditem + plugin_configs - with self.assertNumQueries(10): + # + organizationmembership + organization + action + taggeditem + with self.assertNumQueries(9): response = self.client.get(f"/api/projects/{self.team.id}/actions") self.assertEqual(response.json()["results"][0]["tags"][0], "tag") self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--dark.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--dark.png index 276d107e34099..c17ac6fed82be 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--dark.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--light.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--light.png index fad2fb74402bc..c05b3612b8244 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--light.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration--light.png differ diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--dark.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--dark.png index 717b0a545c5e0..3efdc65f86255 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--dark.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--light.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--light.png index 280efb3c1cbec..ce84d8cbf0015 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--light.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-edit-configuration-stateless-plugin--light.png differ diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--dark.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--dark.png index 4141f3b538ba3..4de622b5503c7 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--dark.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--light.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--light.png index 23eeb49aff7c4..a4802dccf5183 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--light.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-sequence-timer--light.png differ diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 189e60685019c..d5b03a24513bb 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -931,6 +931,17 @@ const api = { async list(params?: string): Promise> { return await new ApiRequest().actions().withQueryString(params).get() }, + async listMatchingPluginConfigs( + actionId: ActionType['id'] + ): Promise> { + return await new ApiRequest() + .actionsDetail(actionId) + .withAction('plugin_configs') + .withQueryString({ + limit: 1000, + }) + .get() + }, determineDeleteEndpoint(): string { return new ApiRequest().actions().assembleEndpointUrl() }, diff --git a/frontend/src/lib/components/ActionSelect.tsx b/frontend/src/lib/components/ActionSelect.tsx deleted file mode 100644 index dced31ff66dfa..0000000000000 --- a/frontend/src/lib/components/ActionSelect.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { LemonButtonProps } from '@posthog/lemon-ui' -import { useValues } from 'kea' -import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' -import { TaxonomicPopover } from 'lib/components/TaxonomicPopover/TaxonomicPopover' -import { actionLogic } from 'scenes/actions/actionLogic' - -interface LemonSelectActionProps { - value?: number | null - onChange?: (value: number | null) => void - disabled?: boolean - placeholder?: string - allowClear?: boolean - size?: LemonButtonProps['size'] - fullWidth?: boolean -} - -export function LemonSelectAction({ - value, - onChange = () => {}, - disabled, - placeholder = 'Select an action', - allowClear, - size, -}: LemonSelectActionProps): JSX.Element { - const { action } = useValues(actionLogic({ id: value ?? undefined })) - - return ( - - v !== null ? ( - <> - {value} - - {action?.name ?? 'Loading...'} - - ) : null - } - allowClear={allowClear} - size={size} - fullWidth - /> - ) -} diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index a16143d42ce01..6739b24a102ed 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -205,7 +205,7 @@ export const FEATURE_FLAGS = { HEATMAPS_UI: 'heatmaps-ui', // owner: @benjackwhite THEME: 'theme', // owner: @aprilfools SESSION_TABLE_PROPERTY_FILTERS: 'session-table-property-filters', // owner: @robbie-c - PLUGINS_ACTION_MATCHING: 'plugins-action-matching', // owner: @benjackwhite + PLUGINS_FILTERING: 'plugins-filtering', // owner: @benjackwhite SESSION_REPLAY_HOG_QL_FILTERING: 'session-replay-hogql-filtering', // owner: #team-replay INSIGHT_LOADING_BAR: 'insight-loading-bar', // owner: @aspicer SESSION_REPLAY_ARTIFICIAL_LAG: 'artificial-lag-query-performance', // owner: #team-replay diff --git a/frontend/src/scenes/actions/Action.stories.tsx b/frontend/src/scenes/actions/Action.stories.tsx index f3586f06350a7..9f1e4d9f2bf26 100644 --- a/frontend/src/scenes/actions/Action.stories.tsx +++ b/frontend/src/scenes/actions/Action.stories.tsx @@ -75,6 +75,7 @@ const meta: Meta = { get: { '/api/projects/:team_id/actions/': toPaginatedResponse([MOCK_ACTION]), '/api/projects/:team_id/actions/1/': MOCK_ACTION, + '/api/projects/:team_id/actions/1/plugin_configs': toPaginatedResponse([]), }, }), ], diff --git a/frontend/src/scenes/actions/ActionEdit.tsx b/frontend/src/scenes/actions/ActionEdit.tsx index 358da0d965562..c21ebcf5dcd64 100644 --- a/frontend/src/scenes/actions/ActionEdit.tsx +++ b/frontend/src/scenes/actions/ActionEdit.tsx @@ -161,7 +161,7 @@ export function ActionEdit({ action: loadedAction, id }: ActionEditLogicProps):

{({ value: stepsValue, onChange }) => ( -
+
{stepsValue.map((step: ActionStepType, index: number) => { const identifier = String(JSON.stringify(step)) return ( diff --git a/frontend/src/scenes/actions/ActionPlugins.tsx b/frontend/src/scenes/actions/ActionPlugins.tsx index cd4484446c240..56d6807cf35a8 100644 --- a/frontend/src/scenes/actions/ActionPlugins.tsx +++ b/frontend/src/scenes/actions/ActionPlugins.tsx @@ -1,6 +1,7 @@ -import { LemonButton } from '@posthog/lemon-ui' -import { useValues } from 'kea' +import { LemonButton, LemonTable } from '@posthog/lemon-ui' +import { useActions, useValues } from 'kea' import { LemonTableLink } from 'lib/lemon-ui/LemonTable/LemonTableLink' +import { useEffect } from 'react' import { actionLogic } from 'scenes/actions/actionLogic' import { PluginImage } from 'scenes/plugins/plugin/PluginImage' import { urls } from 'scenes/urls' @@ -8,42 +9,59 @@ import { urls } from 'scenes/urls' import { PipelineNodeTab, PipelineStage } from '~/types' export function ActionPlugins(): JSX.Element | null { - const { action } = useValues(actionLogic) + const { matchingPluginConfigs } = useValues(actionLogic) + const { loadMatchingPluginConfigs } = useActions(actionLogic) - if (!action?.plugin_configs?.length) { + useEffect(() => { + loadMatchingPluginConfigs() + }, []) + + if (!matchingPluginConfigs?.length) { return null } return ( <> -

Connected apps

- - {action.plugin_configs.map((pluginConfig) => ( -
- - - +

Connected data pipelines

- - Configure - -
- ))} + ( +
+ + +
+ ), + }, + { + title: '', + width: 0, + render: (_, config) => ( + + Configure + + ), + }, + ]} + /> ) } diff --git a/frontend/src/scenes/actions/actionLogic.ts b/frontend/src/scenes/actions/actionLogic.ts index b2b547dcce9d9..c640029bca9c5 100644 --- a/frontend/src/scenes/actions/actionLogic.ts +++ b/frontend/src/scenes/actions/actionLogic.ts @@ -5,7 +5,7 @@ import { DataManagementTab } from 'scenes/data-management/DataManagementScene' import { Scene } from 'scenes/sceneTypes' import { urls } from 'scenes/urls' -import { ActionType, Breadcrumb } from '~/types' +import { ActionType, Breadcrumb, PluginConfigWithPluginInfoNew } from '~/types' import { actionEditLogic } from './actionEditLogic' import type { actionLogicType } from './actionLogicType' @@ -35,6 +35,16 @@ export const actionLogic = kea([ }, }, ], + matchingPluginConfigs: [ + null as PluginConfigWithPluginInfoNew[] | null, + { + loadMatchingPluginConfigs: async () => { + const res = await api.actions.listMatchingPluginConfigs(props.id!) + + return res.results + }, + }, + ], })), reducers(() => ({ pollTimeout: [ diff --git a/frontend/src/scenes/pipeline/PipelinePluginConfiguration.tsx b/frontend/src/scenes/pipeline/PipelinePluginConfiguration.tsx index 481b6e085a31a..0cdb81f7400c1 100644 --- a/frontend/src/scenes/pipeline/PipelinePluginConfiguration.tsx +++ b/frontend/src/scenes/pipeline/PipelinePluginConfiguration.tsx @@ -2,16 +2,19 @@ import { IconLock } from '@posthog/icons' import { LemonButton, LemonInput, LemonSwitch, LemonTextArea, SpinnerOverlay, Tooltip } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { Form } from 'kea-forms' -import { LemonSelectAction } from 'lib/components/ActionSelect' import { NotFound } from 'lib/components/NotFound' import { PageHeader } from 'lib/components/PageHeader' +import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' +import { TestAccountFilterSwitch } from 'lib/components/TestAccountFiltersSwitch' import { LemonField } from 'lib/lemon-ui/LemonField' import { LemonMarkdown } from 'lib/lemon-ui/LemonMarkdown' import React from 'react' +import { ActionFilter } from 'scenes/insights/filters/ActionFilter/ActionFilter' +import { MathAvailability } from 'scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow' import { getConfigSchemaArray, isValidField } from 'scenes/pipeline/configUtils' import { PluginField } from 'scenes/plugins/edit/PluginField' -import { PipelineStage } from '~/types' +import { EntityTypes, PipelineStage } from '~/types' import { pipelinePluginConfigurationLogic } from './pipelinePluginConfigurationLogic' import { RenderApp } from './utils' @@ -37,7 +40,7 @@ export function PipelinePluginConfiguration({ requiredFields, loading, configurationChanged, - actionMatchingEnabled, + pluginFilteringEnabled, } = useValues(logic) const { submitConfiguration, resetConfiguration } = useActions(logic) @@ -173,33 +176,74 @@ export function PipelinePluginConfiguration({
- {actionMatchingEnabled ? ( -
- - + {pluginFilteringEnabled ? ( +
+ + {({ value, onChange }) => ( + <> + onChange({ ...value, filter_test_accounts: val })} + fullWidth + /> + { + onChange({ + ...payload, + filter_test_accounts: value?.filter_test_accounts, + }) + }} + typeKey="plugin-filters" + mathAvailability={MathAvailability.None} + hideRename + hideDuplicate + showNestedArrow={false} + actionsTaxonomicGroupTypes={[ + TaxonomicFilterGroupType.Events, + TaxonomicFilterGroupType.Actions, + ]} + propertiesTaxonomicGroupTypes={[ + TaxonomicFilterGroupType.EventProperties, + TaxonomicFilterGroupType.EventFeatureFlags, + TaxonomicFilterGroupType.Elements, + TaxonomicFilterGroupType.PersonProperties, + ]} + propertyFiltersPopover + addFilterDefaultOptions={{ + id: '$pageview', + name: '$pageview', + type: EntityTypes.EVENTS, + }} + buttonCopy="Add event filter" + /> + + )} + +

+ This destination will be triggered if any of the above filters match. +

) : null}
-
- <> - {fields.length ? ( - fields - ) : ( - - This app does not have specific configuration options - - )} - +
+
+ <> + {fields.length ? ( + fields + ) : ( + + This app does not have specific configuration options + + )} + +
+
{buttons}
- -
{buttons}
) diff --git a/frontend/src/scenes/pipeline/pipelinePluginConfigurationLogic.tsx b/frontend/src/scenes/pipeline/pipelinePluginConfigurationLogic.tsx index cd3b8eb72885a..a1c97b022b670 100644 --- a/frontend/src/scenes/pipeline/pipelinePluginConfigurationLogic.tsx +++ b/frontend/src/scenes/pipeline/pipelinePluginConfigurationLogic.tsx @@ -9,7 +9,15 @@ import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { teamLogic } from 'scenes/teamLogic' import { urls } from 'scenes/urls' -import { PipelineNodeTab, PipelineStage, PluginConfigWithPluginInfoNew, PluginType } from '~/types' +import { + FilterType, + PipelineNodeTab, + PipelineStage, + PluginConfigFilters, + PluginConfigTypeNew, + PluginConfigWithPluginInfoNew, + PluginType, +} from '~/types' import { defaultConfigForPlugin, @@ -35,7 +43,7 @@ const PLUGIN_URL_LEGACY_ACTION_WEBHOOK = 'https://github.com/PostHog/legacy-acti function getConfigurationFromPluginConfig(pluginConfig: PluginConfigWithPluginInfoNew): Record { return { ...pluginConfig.config, - match_action: pluginConfig.match_action, + filters: pluginConfig.filters, enabled: pluginConfig.enabled, order: pluginConfig.order, name: pluginConfig.name ? pluginConfig.name : pluginConfig.plugin_info.name, @@ -52,6 +60,39 @@ function getDefaultConfiguration(plugin: PluginType): Record { } } +function sanitizeFilters(filters?: FilterType): PluginConfigTypeNew['filters'] { + if (!filters) { + return null + } + const sanitized: PluginConfigFilters = {} + + if (filters.events) { + sanitized.events = filters.events.map((f) => ({ + id: f.id, + type: 'events', + name: f.name, + order: f.order, + properties: f.properties, + })) + } + + if (filters.actions) { + sanitized.actions = filters.actions.map((f) => ({ + id: f.id, + type: 'actions', + name: f.name, + order: f.order, + properties: f.properties, + })) + } + + if (filters.filter_test_accounts) { + sanitized.filter_test_accounts = filters.filter_test_accounts + } + + return Object.keys(sanitized).length > 0 ? sanitized : undefined +} + // Should likely be somewhat similar to pipelineBatchExportConfigurationLogic export const pipelinePluginConfigurationLogic = kea([ props({} as PipelinePluginConfigurationLogicProps), @@ -107,7 +148,7 @@ export const pipelinePluginConfigurationLogic = kea ({ + listeners(({ props, actions, values }) => ({ updatePluginConfigSuccess: ({ pluginConfig }) => { if (!pluginConfig) { return } + + // Reset the form so that it doesn't think there are unsaved changes + actions.resetConfiguration(values.configuration) + // Navigating back to the list views gets the updated plugin info without refreshing if (props.stage === PipelineStage.Transformation) { pipelineTransformationsLogic.findMounted()?.actions.updatePluginConfig(pluginConfig) @@ -227,15 +275,14 @@ export const pipelinePluginConfigurationLogic = kea [p.pluginConfigId], (pluginConfigId): boolean => !pluginConfigId], stage: [(_, p) => [p.stage], (stage) => stage], - actionMatchingEnabled: [ + pluginFilteringEnabled: [ (s) => [s.featureFlags, s.pluginConfig, s.plugin], (featureFlags, pluginConfig, plugin) => { - const actionMatchingFlag = featureFlags[FEATURE_FLAGS.PLUGINS_ACTION_MATCHING] - const actionMatchingEnabled = - (actionMatchingFlag || pluginConfig?.match_action) && + const pluginFilteringEnabled = featureFlags[FEATURE_FLAGS.PLUGINS_FILTERING] + return ( + (pluginFilteringEnabled || pluginConfig?.filters) && plugin?.url === PLUGIN_URL_LEGACY_ACTION_WEBHOOK - - return actionMatchingEnabled + ) }, ], })), diff --git a/frontend/src/scenes/plugins/edit/PluginDrawer.tsx b/frontend/src/scenes/plugins/edit/PluginDrawer.tsx index fc783dc9ff688..6313663808f93 100644 --- a/frontend/src/scenes/plugins/edit/PluginDrawer.tsx +++ b/frontend/src/scenes/plugins/edit/PluginDrawer.tsx @@ -82,7 +82,7 @@ export function PluginDrawer(): JSX.Element { form.resetFields() } updateInvisibleAndRequiredFields() - }, [editingPlugin?.id, editingPlugin?.config_schema, editingPlugin?.pluginConfig?.match_action]) + }, [editingPlugin?.id, editingPlugin?.config_schema]) return ( <> diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 46810402396e6..b8e653738eaa9 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -532,7 +532,6 @@ export interface ActionType { action_id?: number // alias of id to make it compatible with event definitions uuid bytecode?: any[] bytecode_error?: string - plugin_configs?: PluginConfigWithPluginInfoNew[] } /** Sync with plugin-server/src/types.ts */ @@ -1838,7 +1837,6 @@ export interface PluginConfigType { error?: PluginErrorType delivery_rate_24h?: number | null created_at?: string - match_action?: ActionType['id'] } /** @deprecated in favor of PluginConfigWithPluginInfoNew */ @@ -1859,7 +1857,29 @@ export interface PluginConfigTypeNew { updated_at: string delivery_rate_24h?: number | null config: Record - match_action?: ActionType['id'] + filters?: PluginConfigFilters | null +} + +// subset of EntityFilter +export interface PluginConfigFilterBase { + id: string + name: string | null + order: number + properties: (EventPropertyFilter | PersonPropertyFilter | ElementPropertyFilter)[] +} + +export interface PluginConfigFilterEvents extends PluginConfigFilterBase { + type: 'events' +} + +export interface PluginConfigFilterActions extends PluginConfigFilterBase { + type: 'actions' +} + +export interface PluginConfigFilters { + events?: PluginConfigFilterEvents[] + actions?: PluginConfigFilterActions[] + filter_test_accounts?: boolean } // TODO: Rename to PluginConfigWithPluginInfo once the are removed from the frontend diff --git a/latest_migrations.manifest b/latest_migrations.manifest index 6192a4dd5953b..6b35c7e36bbec 100644 --- a/latest_migrations.manifest +++ b/latest_migrations.manifest @@ -5,7 +5,7 @@ contenttypes: 0002_remove_content_type_name ee: 0016_rolemembership_organization_member otp_static: 0002_throttling otp_totp: 0002_auto_20190420_0723 -posthog: 0417_remove_organizationmembership_only_one_owner_per_organization_and_more +posthog: 0418_pluginconfig_filters sessions: 0001_initial social_django: 0010_uid_db_index two_factor: 0007_auto_20201201_1019 diff --git a/plugin-server/src/main/pluginsServer.ts b/plugin-server/src/main/pluginsServer.ts index f6cbbd0f20dfc..3a1ba51f87992 100644 --- a/plugin-server/src/main/pluginsServer.ts +++ b/plugin-server/src/main/pluginsServer.ts @@ -387,7 +387,7 @@ export async function startPluginsServer( ) const actionManager = hub?.actionManager ?? new ActionManager(postgres, serverConfig) - const actionMatcher = hub?.actionMatcher ?? new ActionMatcher(postgres, actionManager) + const actionMatcher = hub?.actionMatcher ?? new ActionMatcher(postgres, actionManager, teamManager) const groupTypeManager = new GroupTypeManager(postgres, teamManager, serverConfig.SITE_URL) const { stop: webhooksStopConsumer, isHealthy: isWebhooksIngestionHealthy } = diff --git a/plugin-server/src/types.ts b/plugin-server/src/types.ts index 93ff65e015b28..7fffd6930bac2 100644 --- a/plugin-server/src/types.ts +++ b/plugin-server/src/types.ts @@ -426,7 +426,7 @@ export interface PluginConfig { // we'll need to know which method this plugin is using to call it the right way // undefined for old plugins with multiple or deprecated methods method?: PluginMethod - match_action_id?: number + filters?: PluginConfigFilters } export interface PluginJsonConfig { @@ -585,6 +585,9 @@ export interface Team { session_recording_opt_in: boolean ingested_event: boolean person_display_name_properties: string[] | null + test_account_filters: + | (EventPropertyFilter | PersonPropertyFilter | ElementPropertyFilter | CohortPropertyFilter)[] + | null } /** Properties shared by RawEventMessage and EventMessage. */ @@ -961,6 +964,30 @@ export interface ActionStep { properties: PropertyFilter[] | null } +// subset of EntityFilter +export interface PluginConfigFilterBase { + id: string + name: string | null + order: number + properties: (EventPropertyFilter | PersonPropertyFilter | ElementPropertyFilter)[] +} + +export interface PluginConfigFilterEvents extends PluginConfigFilterBase { + type: 'events' +} + +export interface PluginConfigFilterActions extends PluginConfigFilterBase { + type: 'actions' +} + +export type PluginConfigFilter = PluginConfigFilterEvents | PluginConfigFilterActions + +export interface PluginConfigFilters { + events?: PluginConfigFilterEvents[] + actions?: PluginConfigFilterActions[] + filter_test_accounts?: boolean +} + /** Raw Action row from database. */ export interface RawAction { id: number diff --git a/plugin-server/src/utils/db/hub.ts b/plugin-server/src/utils/db/hub.ts index ae629f066444a..331f95c95d6bb 100644 --- a/plugin-server/src/utils/db/hub.ts +++ b/plugin-server/src/utils/db/hub.ts @@ -154,7 +154,7 @@ export async function createHub( ) const actionManager = new ActionManager(postgres, serverConfig) - const actionMatcher = new ActionMatcher(postgres, actionManager) + const actionMatcher = new ActionMatcher(postgres, actionManager, teamManager) const enqueuePluginJob = async (job: EnqueuedPluginJob) => { // NOTE: we use the producer directly here rather than using the wrapper diff --git a/plugin-server/src/utils/db/sql.ts b/plugin-server/src/utils/db/sql.ts index c0977b3b15b84..6aab87a5f9ceb 100644 --- a/plugin-server/src/utils/db/sql.ts +++ b/plugin-server/src/utils/db/sql.ts @@ -11,7 +11,7 @@ function pluginConfigsInForceQuery(specificField?: keyof PluginConfig): string { posthog_pluginconfig.enabled, posthog_pluginconfig.order, posthog_pluginconfig.config, - posthog_pluginconfig.match_action_id, + posthog_pluginconfig.filters, posthog_pluginconfig.updated_at, posthog_pluginconfig.created_at ` diff --git a/plugin-server/src/worker/ingestion/action-manager.ts b/plugin-server/src/worker/ingestion/action-manager.ts index a002f0c7f9935..09ac1865f319b 100644 --- a/plugin-server/src/worker/ingestion/action-manager.ts +++ b/plugin-server/src/worker/ingestion/action-manager.ts @@ -1,3 +1,4 @@ +import { captureException } from '@sentry/node' import * as schedule from 'node-schedule' import { Action, Hook, PluginConfig, PluginsServerConfig, RawAction, Team } from '../../types' @@ -115,10 +116,8 @@ export async function fetchAllActionsGroupedByTeam( const restHooks = await fetchActionRestHooks(client) const restHookActionIds = restHooks.map(({ resource_id }) => resource_id) - const rawPluginsWithActionMatching = await fetchPluginConfigsWithMatchActions(client) - const pluginConfigActionMatchIds = rawPluginsWithActionMatching.map(({ match_action_id }) => match_action_id) - - const additionalActionIds = [...restHookActionIds, ...pluginConfigActionMatchIds] + const pluginConfigFiltersActionIds = await fetchPluginConfigsRelatedActionIds(client) + const additionalActionIds = [...restHookActionIds, ...pluginConfigFiltersActionIds] const rawActions = ( await client.query( @@ -211,17 +210,30 @@ export async function fetchAction(client: PostgresRouter, id: Action['id']): Pro return action.post_to_slack || action.hooks.length > 0 ? action : null } -export async function fetchPluginConfigsWithMatchActions( - postgres: PostgresRouter -): Promise[]> { - const { rows }: { rows: Pick[] } = await postgres.query( +async function fetchPluginConfigsRelatedActionIds(postgres: PostgresRouter): Promise { + // TODO: Do we need to trigger a reload when the plugin config changes? + const { rows }: { rows: Pick[] } = await postgres.query( PostgresUse.COMMON_READ, - `SELECT id, team_id, match_action_id + `SELECT id, team_id, filters FROM posthog_pluginconfig - WHERE match_action_id IS NOT NULL + WHERE filters IS NOT NULL AND enabled`, undefined, - 'fetchPluginConfigsWithMatchActions' + 'fetchPluginConfigsRelatedActionIds' ) - return rows + + const actionIds: number[] = [] + + for (const row of rows) { + try { + row.filters?.actions?.forEach((filter) => { + actionIds.push(parseInt(filter.id)) + }) + } catch (e) { + // Badly formatted actions in the list + captureException(e) + } + } + + return actionIds } diff --git a/plugin-server/src/worker/ingestion/action-matcher.ts b/plugin-server/src/worker/ingestion/action-matcher.ts index 0b9869c68ba0f..b3fd48cadefa5 100644 --- a/plugin-server/src/worker/ingestion/action-matcher.ts +++ b/plugin-server/src/worker/ingestion/action-matcher.ts @@ -13,6 +13,7 @@ import { ElementPropertyFilter, EventPropertyFilter, PersonPropertyFilter, + PluginConfigFilters, PostIngestionEvent, PropertyFilter, PropertyFilterWithOperator, @@ -24,6 +25,7 @@ import { stringToBoolean } from '../../utils/env-utils' import { mutatePostIngestionEventWithElementsList } from '../../utils/event' import { stringify } from '../../utils/utils' import { ActionManager } from './action-manager' +import { TeamManager } from './team-manager' /** These operators can only be matched if the provided filter's value has the right type. */ const propertyOperatorToRequiredValueType: Partial> = { @@ -132,13 +134,11 @@ export function matchString(actual: string, expected: string, matching: StringMa } export class ActionMatcher { - private postgres: PostgresRouter - private actionManager: ActionManager - - constructor(postgres: PostgresRouter, actionManager: ActionManager) { - this.postgres = postgres - this.actionManager = actionManager - } + constructor( + private postgres: PostgresRouter, + private actionManager: ActionManager, + private teamManager: TeamManager + ) {} public hasWebhooks(teamId: number): boolean { return Object.keys(this.actionManager.getTeamActions(teamId)).length > 0 @@ -161,6 +161,47 @@ export class ActionMatcher { return matches } + public async checkFilters(event: PostIngestionEvent, filters: PluginConfigFilters): Promise { + const allFilters = [...(filters.events || []), ...(filters.actions || [])] + + if (allFilters.length) { + for (const filter of allFilters) { + switch (filter.type) { + case 'events': + if (filter.name && filter.name !== event.event) { + continue + } + break + case 'actions': + const action = this.actionManager.getTeamActions(event.teamId)[parseInt(filter.id)] + + if (!(await this.checkAction(event, action))) { + continue + } + break + default: + return false + } + + if (!filter.properties.length) { + return true + } + + return filter.properties.every((x) => this.checkEventAgainstFilterSync(event, x)) + } + return false + } + + if (filters.filter_test_accounts) { + const internalFilters = (await this.teamManager.fetchTeam(event.teamId))?.test_account_filters + if (internalFilters?.length) { + return internalFilters.every((x) => this.checkEventAgainstFilterSync(event, x)) + } + } + + return true + } + public getActionById(teamId: number, actionId: number): Action | undefined { return this.actionManager.getTeamActions(teamId)[actionId] } @@ -298,7 +339,7 @@ export class ActionMatcher { if (step.properties && step.properties.length) { // EVERY FILTER MUST BE A MATCH for (const filter of step.properties) { - if (!(await this.checkEventAgainstFilter(event, filter))) { + if (!(await this.checkEventAgainstFilterAsync(event, filter))) { return false } } @@ -309,7 +350,7 @@ export class ActionMatcher { /** * Sublevel 3 of action matching. */ - private async checkEventAgainstFilter(event: PostIngestionEvent, filter: PropertyFilter): Promise { + private checkEventAgainstFilterSync(event: PostIngestionEvent, filter: PropertyFilter): boolean { switch (filter.type) { case 'event': return this.checkEventAgainstEventFilter(event, filter) @@ -317,6 +358,22 @@ export class ActionMatcher { return this.checkEventAgainstPersonFilter(event, filter) case 'element': return this.checkEventAgainstElementFilter(event, filter) + default: + return false + } + } + + /** + * Sublevel 3 of action matching. + */ + private async checkEventAgainstFilterAsync(event: PostIngestionEvent, filter: PropertyFilter): Promise { + const match = this.checkEventAgainstFilterSync(event, filter) + + if (match) { + return match + } + + switch (filter.type) { case 'cohort': return await this.checkEventAgainstCohortFilter(event, filter) default: diff --git a/plugin-server/src/worker/ingestion/hooks.ts b/plugin-server/src/worker/ingestion/hooks.ts index 65106ef338525..9d87b9d2e02f9 100644 --- a/plugin-server/src/worker/ingestion/hooks.ts +++ b/plugin-server/src/worker/ingestion/hooks.ts @@ -7,10 +7,10 @@ import { PostgresRouter, PostgresUse } from '../../utils/db/postgres' import { convertToHookPayload } from '../../utils/event' import { trackedFetch } from '../../utils/fetch' import { status } from '../../utils/status' -import { ActionWebhookFormatter } from './action-webhook-formatter' import { AppMetric, AppMetrics } from './app-metrics' import { OrganizationManager } from './organization-manager' import { TeamManager } from './team-manager' +import { WebhookFormatter } from './webhook-formatter' export const webhookProcessStepDuration = new Histogram({ name: 'webhook_process_event_duration', @@ -113,15 +113,16 @@ export class HookCommander { ): Record { const endTimer = webhookProcessStepDuration.labels('messageFormatting').startTimer() try { - const actionWebhookFormatter = new ActionWebhookFormatter( + const webhookFormatter = new WebhookFormatter({ webhookUrl, messageFormat, - action, event, team, - this.siteUrl - ) - return actionWebhookFormatter.generateWebhookPayload() + siteUrl: this.siteUrl, + sourceName: action.name ?? 'Unamed action', + sourcePath: `/action/${action.id}`, + }) + return webhookFormatter.generateWebhookPayload() } finally { endTimer() } diff --git a/plugin-server/src/worker/ingestion/team-manager.ts b/plugin-server/src/worker/ingestion/team-manager.ts index 862c482c00cbe..72258e72d2a50 100644 --- a/plugin-server/src/worker/ingestion/team-manager.ts +++ b/plugin-server/src/worker/ingestion/team-manager.ts @@ -156,7 +156,8 @@ export async function fetchTeam(client: PostgresRouter, teamId: Team['id']): Pro slack_incoming_webhook, session_recording_opt_in, ingested_event, - person_display_name_properties + person_display_name_properties, + test_account_filters FROM posthog_team WHERE id = $1 `, @@ -179,7 +180,8 @@ export async function fetchTeamByToken(client: PostgresRouter, token: string): P api_token, slack_incoming_webhook, session_recording_opt_in, - ingested_event + ingested_event, + test_account_filters FROM posthog_team WHERE api_token = $1 LIMIT 1 diff --git a/plugin-server/src/worker/ingestion/action-webhook-formatter.ts b/plugin-server/src/worker/ingestion/webhook-formatter.ts similarity index 76% rename from plugin-server/src/worker/ingestion/action-webhook-formatter.ts rename to plugin-server/src/worker/ingestion/webhook-formatter.ts index 4d297b376924a..a9c0f6d207995 100644 --- a/plugin-server/src/worker/ingestion/action-webhook-formatter.ts +++ b/plugin-server/src/worker/ingestion/webhook-formatter.ts @@ -1,7 +1,7 @@ import { Webhook } from '@posthog/plugin-scaffold' import { format } from 'util' -import { Action, PostIngestionEvent, Team } from '../../types' +import { PostIngestionEvent, Team } from '../../types' import { getPropertyValueByPath, stringify } from '../../utils/utils' enum WebhookType { @@ -23,34 +23,38 @@ const determineWebhookType = (url: string): WebhookType => { return WebhookType.Other } -export class ActionWebhookFormatter { +export type WebhookFormatterOptions = { + webhookUrl: string + messageFormat: string + event: PostIngestionEvent + team: Team + siteUrl: string + sourcePath: string + sourceName: string +} + +export class WebhookFormatter { private webhookType: WebhookType private projectUrl: string private personLink: string - private actionLink: string private eventLink: string + private sourceLink: string - constructor( - private webhookUrl: string, - private messageFormat: string, - private action: Action, - private event: PostIngestionEvent, - private team: Team, - private siteUrl: string - ) { - this.webhookType = determineWebhookType(webhookUrl) - this.projectUrl = `${siteUrl}/project/${team.id}` - - this.personLink = `${this.projectUrl}/person/${encodeURIComponent(event.distinctId)}` - this.actionLink = `${this.projectUrl}/action/${action.id}` - this.eventLink = `${this.projectUrl}/events/${encodeURIComponent(event.eventUuid)}/${encodeURIComponent( - event.timestamp + constructor(private options: WebhookFormatterOptions) { + this.webhookType = determineWebhookType(options.webhookUrl) + this.projectUrl = `${options.siteUrl}/project/${options.team.id}` + + this.personLink = `${this.projectUrl}/person/${encodeURIComponent(options.event.distinctId)}` + this.eventLink = `${this.projectUrl}/events/${encodeURIComponent(options.event.eventUuid)}/${encodeURIComponent( + options.event.timestamp )}` + + this.sourceLink = `${this.projectUrl}${options.sourcePath}` } composeWebhook(): Webhook { return { - url: this.webhookUrl, + url: this.options.webhookUrl, body: JSON.stringify(this.generateWebhookPayload()), headers: { 'Content-Type': 'application/json', @@ -91,9 +95,9 @@ export class ActionWebhookFormatter { messageText = format(tokenizedMessage, ...values) messageMarkdown = format(tokenizedMessage, ...markdownValues) } catch (error) { - const [actionName, actionMarkdown] = this.getActionDetails() - messageText = `⚠ Error: There are one or more formatting errors in the message template for action "${actionName}".` - messageMarkdown = `*⚠ Error: There are one or more formatting errors in the message template for action "${actionMarkdown}".*` + const [sourceName, sourceMarkdown] = this.getSourceDetails() + messageText = `⚠ Error: There are one or more formatting errors in the message template for source "${sourceName}".` + messageMarkdown = `*⚠ Error: There are one or more formatting errors in the message template for source "${sourceMarkdown}".*` } return [messageText, messageMarkdown] @@ -145,24 +149,26 @@ export class ActionWebhookFormatter { private getPersonDetails(): [string, string] { // Sync the logic below with the frontend `asDisplay` const personDisplayNameProperties = - this.team.person_display_name_properties ?? PERSON_DEFAULT_DISPLAY_NAME_PROPERTIES - const customPropertyKey = personDisplayNameProperties.find((x) => this.event.person_properties?.[x]) - const propertyIdentifier = customPropertyKey ? this.event.person_properties[customPropertyKey] : undefined + this.options.team.person_display_name_properties ?? PERSON_DEFAULT_DISPLAY_NAME_PROPERTIES + const customPropertyKey = personDisplayNameProperties.find((x) => this.options.event.person_properties?.[x]) + const propertyIdentifier = customPropertyKey + ? this.options.event.person_properties[customPropertyKey] + : undefined const customIdentifier: string = typeof propertyIdentifier !== 'string' ? JSON.stringify(propertyIdentifier) : propertyIdentifier - const display: string | undefined = (customIdentifier || this.event.distinctId)?.trim() + const display: string | undefined = (customIdentifier || this.options.event.distinctId)?.trim() return this.toWebhookLink(display, this.personLink) } - private getActionDetails(): [string, string] { - return this.toWebhookLink(this.action.name, this.actionLink) + private getSourceDetails(): [string, string] { + return this.toWebhookLink(this.options.sourceName, this.sourceLink) } private getEventDetails(): [string, string] { - return this.toWebhookLink(this.event.event, this.eventLink) + return this.toWebhookLink(this.options.event.event, this.eventLink) } private getGroupLink(groupIndex: number, groupKey: string): string { @@ -173,9 +179,9 @@ export class ActionWebhookFormatter { // This finds property value tokens, basically any string contained in square brackets // Examples: "[foo]" is matched in "bar [foo]", "[action.name]" is matched in "action [action.name]" // The backslash is used as an escape character - "\[foo\]" is not matched, allowing square brackets in messages - const matchedTokens = this.messageFormat.match(TOKENS_REGEX_BRACKETS_EXCLUDED) || [] + const matchedTokens = this.options.messageFormat.match(TOKENS_REGEX_BRACKETS_EXCLUDED) || [] // Replace the tokens with placeholders, and unescape leftover brackets - const tokenizedMessage = this.messageFormat + const tokenizedMessage = this.options.messageFormat .replace(TOKENS_REGEX_BRACKETS_INCLUDED, '%s') .replace(/\\(\[|\])/g, '$1') return [matchedTokens, tokenizedMessage] @@ -192,7 +198,7 @@ export class ActionWebhookFormatter { ;[text, markdown] = this.getPersonDetails() } else { const propertyName = `$${tokenParts[1]}` - const property = this.event.properties?.[propertyName] + const property = this.options.event.properties?.[propertyName] markdown = text = this.webhookEscape(property) } } else if (tokenParts[0] === 'person') { @@ -201,16 +207,16 @@ export class ActionWebhookFormatter { } else if (tokenParts[1] === 'link') { markdown = text = this.webhookEscape(this.personLink) } else if (tokenParts[1] === 'properties' && tokenParts.length > 2) { - const property = this.event.person_properties - ? getPropertyValueByPath(this.event.person_properties, tokenParts.slice(2)) + const property = this.options.event.person_properties + ? getPropertyValueByPath(this.options.event.person_properties, tokenParts.slice(2)) : undefined markdown = text = this.webhookEscape(property) } - } else if (tokenParts[0] === 'action') { + } else if (tokenParts[0] === 'action' || tokenParts[0] === 'source') { if (tokenParts[1] === 'name') { - ;[text, markdown] = this.getActionDetails() + ;[text, markdown] = this.getSourceDetails() } else if (tokenParts[1] === 'link') { - markdown = text = this.webhookEscape(this.actionLink) + markdown = text = this.webhookEscape(this.sourceLink) } } else if (tokenParts[0] === 'event') { if (tokenParts.length === 1) { @@ -218,19 +224,19 @@ export class ActionWebhookFormatter { } else if (tokenParts[1] === 'link') { markdown = text = this.webhookEscape(this.eventLink) } else if (tokenParts[1] === 'uuid') { - markdown = text = this.webhookEscape(this.event.eventUuid) + markdown = text = this.webhookEscape(this.options.event.eventUuid) } else if (tokenParts[1] === 'name') { // deprecated - markdown = text = this.webhookEscape(this.event.event) + markdown = text = this.webhookEscape(this.options.event.event) } else if (tokenParts[1] === 'event') { - markdown = text = this.webhookEscape(this.event.event) + markdown = text = this.webhookEscape(this.options.event.event) } else if (tokenParts[1] === 'timestamp') { - markdown = text = this.webhookEscape(this.event.timestamp) + markdown = text = this.webhookEscape(this.options.event.timestamp) } else if (tokenParts[1] === 'distinct_id') { - markdown = text = this.webhookEscape(this.event.distinctId) + markdown = text = this.webhookEscape(this.options.event.distinctId) } else if (tokenParts[1] === 'properties' && tokenParts.length > 2) { - const property = this.event.properties - ? getPropertyValueByPath(this.event.properties, tokenParts.slice(2)) + const property = this.options.event.properties + ? getPropertyValueByPath(this.options.event.properties, tokenParts.slice(2)) : undefined markdown = text = this.webhookEscape(property) } @@ -239,7 +245,7 @@ export class ActionWebhookFormatter { // Summarise groups as a list markdown = text = 'NOT SUPPORTED' } else if (tokenParts.length > 1) { - const relatedGroup = this.event.groups?.[tokenParts[1]] + const relatedGroup = this.options.event.groups?.[tokenParts[1]] if (!relatedGroup) { // What to return if no matching group? diff --git a/plugin-server/src/worker/plugins/run.ts b/plugin-server/src/worker/plugins/run.ts index 39345d5fb8ee2..7b24bc10a4a0e 100644 --- a/plugin-server/src/worker/plugins/run.ts +++ b/plugin-server/src/worker/plugins/run.ts @@ -1,7 +1,6 @@ import { PluginEvent, Webhook } from '@posthog/plugin-scaffold' -import { captureException } from '@sentry/node' -import { Action, Hub, PluginConfig, PluginTaskType, PostIngestionEvent, VMMethodsConcrete } from '../../types' +import { Hub, PluginConfig, PluginTaskType, PostIngestionEvent, VMMethodsConcrete } from '../../types' import { processError } from '../../utils/db/error' import { convertToOnEventPayload, @@ -11,7 +10,7 @@ import { import { trackedFetch } from '../../utils/fetch' import { status } from '../../utils/status' import { IllegalOperationError } from '../../utils/utils' -import { ActionWebhookFormatter } from '../ingestion/action-webhook-formatter' +import { WebhookFormatter } from '../ingestion/webhook-formatter' import { pluginActionMsSummary } from '../metrics' const PLUGIN_URL_LEGACY_ACTION_WEBHOOK = 'https://github.com/PostHog/legacy-action-webhook' @@ -95,7 +94,26 @@ async function runSingleTeamPluginComposeWebhook( const event = convertToPostHogEvent(postIngestionEvent) let maybeWebhook: Webhook | null = null try { - maybeWebhook = composeWebhook(event) + if (pluginConfig.plugin?.url === PLUGIN_URL_LEGACY_ACTION_WEBHOOK) { + const team = await hub.teamManager.fetchTeam(event.team_id) + + if (team) { + const webhookFormatter = new WebhookFormatter({ + webhookUrl: pluginConfig.config.webhook_url as string, + messageFormat: pluginConfig.config.message_format as string, + event: postIngestionEvent, + team, + siteUrl: hub.SITE_URL || '', + // TODO: What about pluginConfig.name ? + sourceName: pluginConfig.plugin.name || 'Unnamed plugin', + sourcePath: `/pipeline/destinations/${pluginConfig.id}`, + }) + maybeWebhook = webhookFormatter.composeWebhook() + } + } else { + maybeWebhook = composeWebhook(event) + } + if (!maybeWebhook) { // TODO: ideally we'd queryMetric it as skipped, but that's not an option atm status.debug('Skipping composeWebhook returned null', { @@ -217,10 +235,7 @@ async function runSingleTeamPluginComposeWebhook( export async function runComposeWebhook(hub: Hub, event: PostIngestionEvent): Promise { // Runs composeWebhook for all plugins for this team in parallel let pluginMethodsToRun = await getPluginMethodsForTeam(hub, event.teamId, 'composeWebhook') - pluginMethodsToRun = await filterPluginMethodsForActionMatches(hub, event, pluginMethodsToRun) - - // Here we load the special "LegacyActionWebhook" plugin which is actually run in process - pluginMethodsToRun = pluginMethodsToRun.concat(await getLegacyActionWebhookPluginsForTeam(hub, event)) + pluginMethodsToRun = await filterPluginMethods(hub, event, pluginMethodsToRun) await Promise.all( pluginMethodsToRun.map(([pluginConfig, composeWebhook]) => @@ -388,54 +403,7 @@ async function getPluginMethodsForTeam( return methodsObtainedFiltered } -async function getLegacyActionWebhookPluginsForTeam( - hub: Hub, - event: PostIngestionEvent -): Promise<[PluginConfig, () => Webhook | null][]> { - const pluginConfigs = hub.pluginConfigsPerTeam.get(event.teamId) || [] - if (pluginConfigs.length === 0) { - return [] - } - - const filteredList: [PluginConfig, () => Webhook | null][] = [] - - await Promise.all( - pluginConfigs - // TODO: We probably want a stronger check than just the plugin name... - .map(async (pluginConfig) => { - if (pluginConfig.plugin?.url !== PLUGIN_URL_LEGACY_ACTION_WEBHOOK || !pluginConfig.match_action_id) { - return - } - - const matchedAction = await getActionMatchingPluginConfigs(hub, pluginConfig, event) - const team = await hub.teamManager.fetchTeam(event.teamId) - - if (!matchedAction || !team) { - return - } - - filteredList.push([ - pluginConfig, - () => { - // TODO: Fix the forced conversion here - const actionWebhookFormatter = new ActionWebhookFormatter( - pluginConfig.config.webhook_url as string, - pluginConfig.config.message_format as string, - matchedAction, - event, - team, - hub.SITE_URL || '' - ) - return actionWebhookFormatter.composeWebhook() - }, - ]) - }) - ) - - return filteredList -} - -async function filterPluginMethodsForActionMatches( +async function filterPluginMethods( hub: Hub, event: PostIngestionEvent, pluginMethods: [PluginConfig, T][] @@ -444,45 +412,34 @@ async function filterPluginMethodsForActionMatches( await Promise.all( pluginMethods.map(async ([pluginConfig, method]) => { - if (pluginConfig.match_action_id) { - const matchedAction = await getActionMatchingPluginConfigs(hub, pluginConfig, event) - if (!matchedAction) { + if (pluginConfig.filters) { + try { + const matchedFilters = await hub.actionMatcher.checkFilters(event, pluginConfig.filters) + if (!matchedFilters) { + return + } + } catch (error) { + // We consider an exception to be bad enough to drop the event (we should also log it as a processing error) + + await hub.appMetrics.queueError( + { + teamId: event.teamId, + pluginConfigId: pluginConfig.id, + category: 'composeWebhook', + failures: 1, + }, + { + error: `Error occurred when processing filters: ${error}`, + event, + } + ) return } } + filteredList.push([pluginConfig, method]) }) ) return filteredList } - -async function getActionMatchingPluginConfigs( - hub: Hub, - pluginConfig: PluginConfig, - event: PostIngestionEvent -): Promise { - if (!pluginConfig.match_action_id) { - return null - } - const relatedAction = hub.actionMatcher.getActionById(event.teamId, pluginConfig.match_action_id) - - if (!relatedAction) { - captureException(new Error('Could not find action for PluginConfig!'), { - extra: { - pluginConfigId: pluginConfig.id, - teamId: event.teamId, - }, - }) - status.error('🔴', 'Could not find action for PluginConfig!', { - pluginConfigId: pluginConfig.id, - teamId: event.teamId, - }) - - return null - } - - const matched = await hub.actionMatcher.checkAction(event, relatedAction) - - return matched ? relatedAction : null -} diff --git a/plugin-server/tests/main/db.test.ts b/plugin-server/tests/main/db.test.ts index 249c547942439..e5c08a092099b 100644 --- a/plugin-server/tests/main/db.test.ts +++ b/plugin-server/tests/main/db.test.ts @@ -992,6 +992,7 @@ describe('DB', () => { slack_incoming_webhook: null, uuid: expect.any(String), person_display_name_properties: [], + test_account_filters: {} as any, // NOTE: Test insertion data gets set as an object weirdly } as Team) }) @@ -1017,6 +1018,7 @@ describe('DB', () => { session_recording_opt_in: true, slack_incoming_webhook: null, uuid: expect.any(String), + test_account_filters: {} as any, // NOTE: Test insertion data gets set as an object weirdly }) }) diff --git a/plugin-server/tests/main/ingestion-queues/each-batch-webhooks.test.ts b/plugin-server/tests/main/ingestion-queues/each-batch-webhooks.test.ts index feca2c7d31225..344bf200f766a 100644 --- a/plugin-server/tests/main/ingestion-queues/each-batch-webhooks.test.ts +++ b/plugin-server/tests/main/ingestion-queues/each-batch-webhooks.test.ts @@ -54,8 +54,8 @@ describe('eachMessageWebhooksHandlers', () => { }) it('calls runWebhooksHandlersEventPipeline', async () => { - const actionManager = new ActionManager(hub.postgres) - const actionMatcher = new ActionMatcher(hub.postgres, actionManager) + const actionManager = new ActionManager(hub.postgres, hub) + const actionMatcher = new ActionMatcher(hub.postgres, actionManager, hub.teamManager) const hookCannon = new HookCommander( hub.postgres, hub.teamManager, diff --git a/plugin-server/tests/main/ingestion-queues/each-batch.test.ts b/plugin-server/tests/main/ingestion-queues/each-batch.test.ts index 4f75159ed335e..98f5f5d965a36 100644 --- a/plugin-server/tests/main/ingestion-queues/each-batch.test.ts +++ b/plugin-server/tests/main/ingestion-queues/each-batch.test.ts @@ -168,7 +168,11 @@ describe('eachBatchX', () => { describe('eachBatchWebhooksHandlers', () => { it('calls runWebhooksHandlersEventPipeline', async () => { const actionManager = new ActionManager(queue.pluginsServer.postgres, queue.pluginsServer) - const actionMatcher = new ActionMatcher(queue.pluginsServer.postgres, actionManager) + const actionMatcher = new ActionMatcher( + queue.pluginsServer.postgres, + actionManager, + queue.pluginsServer.teamManager + ) const hookCannon = new HookCommander( queue.pluginsServer.postgres, queue.pluginsServer.teamManager, diff --git a/plugin-server/tests/sql.test.ts b/plugin-server/tests/sql.test.ts index 95b31f0943fe9..24c294a6a97c2 100644 --- a/plugin-server/tests/sql.test.ts +++ b/plugin-server/tests/sql.test.ts @@ -59,7 +59,7 @@ describe('sql', () => { team_id: 2, created_at: expect.anything(), updated_at: expect.anything(), - match_action_id: null, + filters: null, } const rows1 = await getPluginConfigRows(hub) diff --git a/plugin-server/tests/worker/ingestion/__snapshots__/action-webhook-formatter.test.ts.snap b/plugin-server/tests/worker/ingestion/__snapshots__/action-webhook-formatter.test.ts.snap deleted file mode 100644 index 4b9a2536d1eea..0000000000000 --- a/plugin-server/tests/worker/ingestion/__snapshots__/action-webhook-formatter.test.ts.snap +++ /dev/null @@ -1,72 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ActionWebhookFormatter slack webhook formatting options { messageFormat: '[action.name]' } 1`] = `"action1"`; - -exports[`ActionWebhookFormatter slack webhook formatting options { messageFormat: '[action.name]' } 2`] = `""`; - -exports[`ActionWebhookFormatter slack webhook formatting options { messageFormat: '[event]', event: [Object] } 1`] = `"text><new link"`; - -exports[`ActionWebhookFormatter slack webhook formatting options { messageFormat: '[event]', event: [Object] } 2`] = `""`; - -exports[`ActionWebhookFormatter slack webhook formatting options { messageFormat: '[person]' } 1`] = `"test@posthog.com"`; - -exports[`ActionWebhookFormatter slack webhook formatting options { messageFormat: '[person]' } 2`] = `""`; - -exports[`ActionWebhookFormatter webhook formatting options { - messageFormat: '[person.properties.enjoys_broccoli_on_pizza]', - personProperties: [Object] -} 1`] = `"false"`; - -exports[`ActionWebhookFormatter webhook formatting options { - messageFormat: '[person.properties.pizza_ingredient_ranking]', - personProperties: [Object] -} 1`] = `"[\\"pineapple\\",\\"broccoli\\",\\"aubergine\\"\\\\]"`; - -exports[`ActionWebhookFormatter webhook formatting options { - messageFormat: '[person]', - personProperties: [Object], - team: [Object] -} 1`] = `"[Brzęczyszczykiewicz](http://localhost:8000/project/123/person/WALL-E)"`; - -exports[`ActionWebhookFormatter webhook formatting options { - messageFormat: '[user.name] from [user.browser] on [event.properties.page_title] page with [event.properties.fruit], [event.properties.with space]', - event: [Object] -} 1`] = `"[test@posthog.com](http://localhost:8000/project/123/person/2) from Chrome on Pricing page with undefined, yes sir"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[action.name\\\\] got done by \\\\[user.name\\\\]' } 1`] = `"[action.name] got done by [user.name]"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[action.name] was done by [user.name]' } 1`] = `"[action1](http://localhost:8000/project/123/action/1) was done by [test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[action.name]' } 1`] = `"[action1](http://localhost:8000/project/123/action/1)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[event.distinct_id]' } 1`] = `"WALL-E"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[event.event]' } 1`] = `"$pageview"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[event.name]' } 1`] = `"$pageview"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[event.uuid]' } 1`] = `"123"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[event]' } 1`] = `"[$pageview](http://localhost:8000/project/123/events/123/2021-10-31T00%253A44%253A00.000Z)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[event]', event: [Object] } 1`] = `"[text\\\\]\\\\(yes\\\\!\\\\), \\\\[new link](http://localhost:8000/project/123/events/\\\\*\\\\*\\\\)/2021-10-31T00%253A44%253A00.000Z)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[groups.missing]' } 1`] = `"(event without 'missing')"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[groups.organization.properties.plan]' } 1`] = `"paid"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[groups.organization]' } 1`] = `"[PostHog](http://localhost:8000/project/123/groups/1/123)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[groups.project]' } 1`] = `"[234](http://localhost:8000/project/123/groups/2/234)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[groups]' } 1`] = `"NOT SUPPORTED"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[person.link]' } 1`] = `"http://localhost:8000/project/123/person/WALL-E"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[person]' } 1`] = `"[test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[user.browser]' } 1`] = `"Chrome"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[user.missing_property]' } 1`] = `"undefined"`; - -exports[`ActionWebhookFormatter webhook formatting options { messageFormat: '[user.name]' } 1`] = `"[test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; diff --git a/plugin-server/tests/worker/ingestion/__snapshots__/webhook-formatter.test.ts.snap b/plugin-server/tests/worker/ingestion/__snapshots__/webhook-formatter.test.ts.snap new file mode 100644 index 0000000000000..c64b13e0431ce --- /dev/null +++ b/plugin-server/tests/worker/ingestion/__snapshots__/webhook-formatter.test.ts.snap @@ -0,0 +1,76 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`WebhookFormatter slack webhook formatting options { messageFormat: '[action.name]' } 1`] = `"action1"`; + +exports[`WebhookFormatter slack webhook formatting options { messageFormat: '[action.name]' } 2`] = `""`; + +exports[`WebhookFormatter slack webhook formatting options { messageFormat: '[event]', event: [Object] } 1`] = `"text><new link"`; + +exports[`WebhookFormatter slack webhook formatting options { messageFormat: '[event]', event: [Object] } 2`] = `""`; + +exports[`WebhookFormatter slack webhook formatting options { messageFormat: '[person]' } 1`] = `"test@posthog.com"`; + +exports[`WebhookFormatter slack webhook formatting options { messageFormat: '[person]' } 2`] = `""`; + +exports[`WebhookFormatter webhook formatting options { + messageFormat: '[person.properties.enjoys_broccoli_on_pizza]', + personProperties: [Object] +} 1`] = `"false"`; + +exports[`WebhookFormatter webhook formatting options { + messageFormat: '[person.properties.pizza_ingredient_ranking]', + personProperties: [Object] +} 1`] = `"[\\"pineapple\\",\\"broccoli\\",\\"aubergine\\"\\\\]"`; + +exports[`WebhookFormatter webhook formatting options { + messageFormat: '[person]', + personProperties: [Object], + team: [Object] +} 1`] = `"[Brzęczyszczykiewicz](http://localhost:8000/project/123/person/WALL-E)"`; + +exports[`WebhookFormatter webhook formatting options { + messageFormat: '[user.name] from [user.browser] on [event.properties.page_title] page with [event.properties.fruit], [event.properties.with space]', + event: [Object] +} 1`] = `"[test@posthog.com](http://localhost:8000/project/123/person/2) from Chrome on Pricing page with undefined, yes sir"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[action.name\\\\] got done by \\\\[user.name\\\\]' } 1`] = `"[action.name] got done by [user.name]"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[action.name] was done by [user.name]' } 1`] = `"[action1](http://localhost:8000/project/123/action/1) was done by [test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[action.name]' } 1`] = `"[action1](http://localhost:8000/project/123/action/1)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[event.distinct_id]' } 1`] = `"WALL-E"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[event.event]' } 1`] = `"$pageview"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[event.name]' } 1`] = `"$pageview"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[event.uuid]' } 1`] = `"123"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[event]' } 1`] = `"[$pageview](http://localhost:8000/project/123/events/123/2021-10-31T00%253A44%253A00.000Z)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[event]', event: [Object] } 1`] = `"[text\\\\]\\\\(yes\\\\!\\\\), \\\\[new link](http://localhost:8000/project/123/events/\\\\*\\\\*\\\\)/2021-10-31T00%253A44%253A00.000Z)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[groups.missing]' } 1`] = `"(event without 'missing')"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[groups.organization.properties.plan]' } 1`] = `"paid"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[groups.organization]' } 1`] = `"[PostHog](http://localhost:8000/project/123/groups/1/123)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[groups.project]' } 1`] = `"[234](http://localhost:8000/project/123/groups/2/234)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[groups]' } 1`] = `"NOT SUPPORTED"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[person.link]' } 1`] = `"http://localhost:8000/project/123/person/WALL-E"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[person]' } 1`] = `"[test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[source.name] was done by [user.name]' } 1`] = `"[action1](http://localhost:8000/project/123/action/1) was done by [test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[source.name]' } 1`] = `"[action1](http://localhost:8000/project/123/action/1)"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[user.browser]' } 1`] = `"Chrome"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[user.missing_property]' } 1`] = `"undefined"`; + +exports[`WebhookFormatter webhook formatting options { messageFormat: '[user.name]' } 1`] = `"[test@posthog.com](http://localhost:8000/project/123/person/WALL-E)"`; diff --git a/plugin-server/tests/worker/ingestion/action-matcher.test.ts b/plugin-server/tests/worker/ingestion/action-matcher.test.ts index 140140d6f7519..a66a8d03bb2be 100644 --- a/plugin-server/tests/worker/ingestion/action-matcher.test.ts +++ b/plugin-server/tests/worker/ingestion/action-matcher.test.ts @@ -8,6 +8,9 @@ import { Hub, ISOTimestamp, Person, + PluginConfigFilterActions, + PluginConfigFilterEvents, + PluginConfigFilters, PostIngestionEvent, PropertyOperator, RawAction, @@ -23,6 +26,25 @@ import { getFirstTeam, insertRow, resetTestDatabase } from '../../helpers/sql' jest.mock('../../../src/utils/status') +/** Return a test event created on a common base using provided property overrides. */ +function createTestEvent(overrides: Partial = {}): PostIngestionEvent { + const url: string = overrides.properties?.$current_url ?? 'http://example.com/foo/' + return { + eventUuid: 'uuid1', + distinctId: 'my_id', + ip: '127.0.0.1', + teamId: 2, + timestamp: new Date().toISOString() as ISOTimestamp, + event: '$pageview', + properties: { $current_url: url }, + elementsList: [], + person_id: 'F99FA0A1-E0C2-4CFE-A09A-4C3C4327A4C8', + person_created_at: DateTime.fromSeconds(18000000).toISO() as ISOTimestamp, + person_properties: {}, + ...overrides, + } +} + describe('ActionMatcher', () => { let hub: Hub let closeServer: () => Promise @@ -35,7 +57,7 @@ describe('ActionMatcher', () => { ;[hub, closeServer] = await createHub() actionManager = new ActionManager(hub.db.postgres, hub) await actionManager.start() - actionMatcher = new ActionMatcher(hub.db.postgres, actionManager) + actionMatcher = new ActionMatcher(hub.db.postgres, actionManager, hub.teamManager) actionCounter = 0 }) @@ -88,25 +110,6 @@ describe('ActionMatcher', () => { } } - /** Return a test event created on a common base using provided property overrides. */ - function createTestEvent(overrides: Partial = {}): PostIngestionEvent { - const url: string = overrides.properties?.$current_url ?? 'http://example.com/foo/' - return { - eventUuid: 'uuid1', - distinctId: 'my_id', - ip: '127.0.0.1', - teamId: 2, - timestamp: new Date().toISOString() as ISOTimestamp, - event: '$pageview', - properties: { $current_url: url }, - elementsList: [], - person_id: 'F99FA0A1-E0C2-4CFE-A09A-4C3C4327A4C8', - person_created_at: DateTime.fromSeconds(18000000).toISO() as ISOTimestamp, - person_properties: {}, - ...overrides, - } - } - describe('#match()', () => { it('returns no match if action has no steps', async () => { await createTestAction(null) @@ -1333,6 +1336,186 @@ describe('ActionMatcher', () => { }) }) +describe('ActionMatcher.checkFilters', () => { + const mockTeamManager = { + fetchTeam: jest.fn(() => + Promise.resolve({ + test_account_filters: [ + { + key: '$host', + type: 'event', + value: '^(localhost|127\\.0\\.0\\.1)($|:)', + operator: 'not_regex', + }, + ], + }) + ), + } + const mockActionManager = { + getTeamActions: jest.fn( + (): Record> => ({ + 1: { + id: 1, + name: 'my-action', + deleted: false, + steps: [ + { + event: '$pageview', + tag_name: null, + text: null, + text_matching: null, + href: null, + href_matching: null, + selector: null, + url: null, + url_matching: null, + properties: null, + }, + ], + }, + }) + ), + } + const actionMatcher = new ActionMatcher({} as any, mockActionManager as any, mockTeamManager as any) + + const f = ( + partials: Partial[], + filterTestAccounts = false + ): PluginConfigFilters => { + const events = partials + .filter((x) => !x.type || x.type === 'events') + .map((x) => x as Partial) + const actions = partials.filter((x) => x.type === 'actions').map((x) => x as Partial) + return { + events: events.map((x) => ({ + id: '0', + type: 'events', + name: '$pageview', + order: 0, + properties: [], + ...x, + })), + actions: actions.map((x) => ({ + id: '0', + type: 'actions', + name: 'my-action', + order: 0, + properties: [], + ...x, + })), + filter_test_accounts: filterTestAccounts, + } + } + + const testCases: [Partial, PluginConfigFilters, boolean][] = [ + // No filters should be true + [{ event: '$pageview' }, f([]), true], + [{ event: '$pageview' }, f([{ name: null }]), true], + [{ event: '$pageview' }, f([{ name: '$pageview' }]), true], + [{ event: '$not-pageview' }, f([{ name: '$pageview' }]), false], + [{ event: '$pageview', properties: { $current_url: 'https://posthog.com' } }, f([{ name: '$pageview' }]), true], + [ + { event: '$pageview', properties: { $current_url: 'https://posthog.com' } }, + f([ + { + name: '$pageview', + properties: [ + { + key: '$current_url', + type: 'event', + value: ['https://posthog.com'], + operator: PropertyOperator.Exact, + }, + ], + }, + ]), + true, + ], + [ + { event: '$pageview', properties: { $current_url: 'https://posthog.com' } }, + f([ + { + name: '$pageview', + properties: [ + { + key: '$current_url', + type: 'event', + value: ['posthog.com'], + operator: PropertyOperator.IContains, + }, + ], + }, + ]), + true, + ], + [ + { event: '$pageview', properties: { $current_url: 'https://posthog.com' } }, + f([ + { + name: '$pageview', + properties: [ + { + key: '$current_url', + type: 'event', + value: ['not-posthog.com'], + operator: PropertyOperator.IContains, + }, + ], + }, + ]), + false, + ], + [ + { event: '$pageview', properties: { $current_url: 'https://posthog.com' } }, + f([ + { + name: '$pageview', + properties: [ + { + key: '$current_url', + type: 'event', + value: ['not-posthog.com', 'posthog.com'], + operator: PropertyOperator.IContains, + }, + ], + }, + ]), + true, + ], + + [{ event: '$pageview' }, f([{ type: 'actions', id: '1' }]), true], + [{ event: '$not-pageview' }, f([{ type: 'actions', id: '1' }]), false], + [ + { event: '$pageview' }, + f([ + { + type: 'actions', + id: '1', + properties: [ + { + key: '$current_url', + type: 'event', + value: ['not-correct'], + operator: PropertyOperator.Exact, + }, + ], + }, + ]), + false, + ], + [{ event: '$not-pageview' }, f([{ type: 'actions', id: '1' }, { name: '$not-pageview' }]), true], + // test internal filters + [{ event: '$pageview', properties: { $host: 'localhost:8000' } }, f([], true), false], + [{ event: '$pageview', properties: { $host: 'posthog.com' } }, f([], true), true], + ] + + it.each(testCases)('should correctly match filters %o %o', async (partialEvent, filters, expectation) => { + const event = createTestEvent(partialEvent) + + expect(await actionMatcher.checkFilters(event, filters)).toEqual(expectation) + }) +}) + describe('castingCompare', () => { it('compares exact', () => { expect(castingCompare(2, '2', PropertyOperator.Exact)).toBeTruthy() diff --git a/plugin-server/tests/worker/ingestion/event-pipeline/event-pipeline-integration.test.ts b/plugin-server/tests/worker/ingestion/event-pipeline/event-pipeline-integration.test.ts index 819d2046f5820..bc4bf1254d2e0 100644 --- a/plugin-server/tests/worker/ingestion/event-pipeline/event-pipeline-integration.test.ts +++ b/plugin-server/tests/worker/ingestion/event-pipeline/event-pipeline-integration.test.ts @@ -47,7 +47,7 @@ describe('Event Pipeline integration test', () => { actionManager = new ActionManager(hub.db.postgres, hub) await actionManager.start() - actionMatcher = new ActionMatcher(hub.db.postgres, actionManager) + actionMatcher = new ActionMatcher(hub.db.postgres, actionManager, hub.teamManager) hookCannon = new HookCommander( hub.db.postgres, hub.teamManager, diff --git a/plugin-server/tests/worker/ingestion/action-webhook-formatter.test.ts b/plugin-server/tests/worker/ingestion/webhook-formatter.test.ts similarity index 82% rename from plugin-server/tests/worker/ingestion/action-webhook-formatter.test.ts rename to plugin-server/tests/worker/ingestion/webhook-formatter.test.ts index f3be84edce81b..5e26261087ba6 100644 --- a/plugin-server/tests/worker/ingestion/action-webhook-formatter.test.ts +++ b/plugin-server/tests/worker/ingestion/webhook-formatter.test.ts @@ -1,22 +1,12 @@ -import { Action, ISOTimestamp, PostIngestionEvent, Team } from '../../../src/types' -import { ActionWebhookFormatter } from '../../../src/worker/ingestion/action-webhook-formatter' +import { ISOTimestamp, PostIngestionEvent, Team } from '../../../src/types' +import { WebhookFormatter, WebhookFormatterOptions } from '../../../src/worker/ingestion/webhook-formatter' -type ActionWebhookFormatterOptions = { - webhookUrl?: string - messageFormat?: string - action?: Action - event?: PostIngestionEvent - team?: Team - siteUrl?: string +type TestWebhookFormatterOptions = Partial & { personProperties?: PostIngestionEvent['person_properties'] } -describe('ActionWebhookFormatter', () => { +describe('WebhookFormatter', () => { const team = { id: 123, person_display_name_properties: null } as Team - const action = { - id: 1, - name: 'action1', - } as Action const event: PostIngestionEvent = { event: '$pageview', eventUuid: '123', @@ -45,18 +35,19 @@ describe('ActionWebhookFormatter', () => { }, } - const createFormatter = (options?: ActionWebhookFormatterOptions) => { - return new ActionWebhookFormatter( - options?.webhookUrl ?? 'https://example.com/', - options?.messageFormat ?? 'User [person] did [action.name]', - options?.action ?? action, - { + const createFormatter = (options?: TestWebhookFormatterOptions) => { + return new WebhookFormatter({ + webhookUrl: options?.webhookUrl ?? 'https://example.com/', + messageFormat: options?.messageFormat ?? 'User [person] did [action.name]', + sourceName: options?.sourceName ?? 'action1', + sourcePath: options?.sourcePath ?? '/action/1', + event: { ...(options?.event ?? event), person_properties: options?.personProperties ?? event.person_properties, }, - options?.team ?? team, - options?.siteUrl ?? 'http://localhost:8000' - ) + team: options?.team ?? team, + siteUrl: options?.siteUrl ?? 'http://localhost:8000', + }) } beforeEach(() => { @@ -64,13 +55,15 @@ describe('ActionWebhookFormatter', () => { }) describe('webhook formatting options', () => { - const cases: [ActionWebhookFormatterOptions][] = [ + const cases: [TestWebhookFormatterOptions][] = [ [{ messageFormat: '[person]' }], [{ messageFormat: '[person.link]' }], [{ messageFormat: '[user.name]' }], // Alias for person name [{ messageFormat: '[user.browser]' }], // Otherwise just alias to event properties [{ messageFormat: '[action.name]' }], [{ messageFormat: '[action.name] was done by [user.name]' }], + [{ messageFormat: '[source.name]' }], + [{ messageFormat: '[source.name] was done by [user.name]' }], // Handle escaping brackets [{ messageFormat: '[action.name\\] got done by \\[user.name\\]' }], [{ messageFormat: '[event]' }], @@ -134,7 +127,7 @@ describe('ActionWebhookFormatter', () => { describe('slack webhook formatting options', () => { // Additional checks for the standard slack webhook formats - const cases: [ActionWebhookFormatterOptions][] = [ + const cases: [TestWebhookFormatterOptions][] = [ [{ messageFormat: '[person]' }], [{ messageFormat: '[action.name]' }], [{ messageFormat: '[event]', event: { ...event, eventUuid: '**>)', event: 'text> { beforeEach(() => { composeWebhook = jest.fn() mockPluginConfig = { + id: 123, plugin_id: 100, team_id: 2, enabled: false, @@ -276,7 +277,7 @@ describe('runComposeWebhook', () => { queueMetric: jest.fn(), queueError: jest.fn(), } as any, - actionMatcher: new ActionMatcher(mockPostgres, mockActionManager), + actionMatcher: new ActionMatcher(mockPostgres, mockActionManager, {} as any), } }) @@ -296,15 +297,72 @@ describe('runComposeWebhook', () => { `) }) - it('filters out if has matching action that cannot be found', async () => { - mockPluginConfig.match_action_id = 1 + it('filters in if has matching action', async () => { + mockPluginConfig.filters = { + actions: [ + { + id: '1', + type: 'actions', + properties: [], + name: '', + order: 0, + }, + ], + } + + mockActionManager.getTeamActions.mockImplementation(() => ({ + 1: { + steps: [ + { + event: '$autocapture', + }, + ], + }, + })) await runComposeWebhook(mockHub as Hub, createEvent()) - expect(composeWebhook).toHaveBeenCalledTimes(0) + expect(composeWebhook).toHaveBeenCalledTimes(1) + }) + + it('filters in if has matching event filter', async () => { + mockPluginConfig.filters = { + events: [ + { + id: '0', + type: 'events', + name: '$autocapture', + order: 0, + properties: [], + }, + ], + } + + await runComposeWebhook(mockHub as Hub, createEvent()) + + expect(composeWebhook).toHaveBeenCalledTimes(1) }) - it('filters out if has matching action that does not match event', async () => { - mockPluginConfig.match_action_id = 1 + it('filters in if has matching match action _or_ event filter', async () => { + mockPluginConfig.filters = { + events: [ + { + type: 'events', + name: '$not-autcapture', + order: 0, + properties: [], + id: '0', + }, + ], + actions: [ + { + id: '1', + type: 'actions', + properties: [], + name: '', + order: 0, + }, + ], + } mockActionManager.getTeamActions.mockImplementation(() => ({ 1: { @@ -315,8 +373,88 @@ describe('runComposeWebhook', () => { ], }, })) + await runComposeWebhook(mockHub as Hub, createEvent()) expect(composeWebhook).toHaveBeenCalledTimes(1) }) + + it('filters out if has matching action that cannot be found', async () => { + mockPluginConfig.filters = { + actions: [ + { + id: '1', + type: 'actions', + properties: [], + name: '', + order: 0, + }, + ], + } + await runComposeWebhook(mockHub as Hub, createEvent()) + + expect(composeWebhook).toHaveBeenCalledTimes(0) + }) + + it('filters out if has non-matching action and event filter', async () => { + mockPluginConfig.filters = { + events: [ + { + type: 'events', + name: '$not-autcapture', + order: 0, + properties: [], + id: '0', + }, + ], + actions: [ + { + id: '1', + type: 'actions', + properties: [], + name: '', + order: 0, + }, + ], + } + + mockActionManager.getTeamActions.mockImplementation(() => ({ + 1: { + steps: [ + { + event: '$also-not-autocapture', + }, + ], + }, + })) + + await runComposeWebhook(mockHub as Hub, createEvent()) + + expect(composeWebhook).toHaveBeenCalledTimes(0) + }) + + it('handles malformed filters and does logs an error', async () => { + mockPluginConfig.filters = { + events: {}, + } as any + + await runComposeWebhook(mockHub as Hub, createEvent()) + + expect(composeWebhook).toHaveBeenCalledTimes(0) + expect(mockHub.appMetrics?.queueError).toHaveBeenCalledTimes(1) + expect(mockHub.appMetrics?.queueError).toHaveBeenLastCalledWith( + { + category: 'composeWebhook', + failures: 1, + pluginConfigId: 123, + teamId: 2, + }, + { + error: 'Error occurred when processing filters: TypeError: (filters.events || []) is not iterable', + event: expect.objectContaining({ + event: '$autocapture', + }), + } + ) + }) }) diff --git a/posthog/api/action.py b/posthog/api/action.py index 3659c2f8505ac..8b2ac8b2291f0 100644 --- a/posthog/api/action.py +++ b/posthog/api/action.py @@ -2,6 +2,9 @@ from rest_framework import serializers, viewsets from django.db.models import Count +from rest_framework.decorators import action +from rest_framework.request import Request +from rest_framework.response import Response from rest_framework.settings import api_settings from rest_framework_csv import renderers as csvrenderers @@ -15,6 +18,7 @@ from posthog.event_usage import report_user_action from posthog.models import Action from posthog.models.action.action import ACTION_STEP_MATCHING_OPTIONS +from posthog.models.plugin import PluginConfig from .forbid_destroy_model import ForbidDestroyModel from .tagged_item import TaggedItemSerializerMixin, TaggedItemViewSetMixin @@ -38,7 +42,6 @@ class ActionSerializer(TaggedItemSerializerMixin, serializers.HyperlinkedModelSe created_by = UserBasicSerializer(read_only=True) is_calculating = serializers.SerializerMethodField() is_action = serializers.BooleanField(read_only=True, default=True) - plugin_configs = PluginConfigSerializer(many=True, read_only=True) class Meta: model = Action @@ -58,7 +61,6 @@ class Meta: "team_id", "is_action", "bytecode_error", - "plugin_configs", ] read_only_fields = [ "team_id", @@ -105,10 +107,6 @@ def create(self, validated_data: Any) -> Any: return instance def update(self, instance: Any, validated_data: dict[str, Any]) -> Any: - if validated_data.get("deleted"): - if instance.plugin_configs.count(): - raise serializers.ValidationError("Actions with plugins cannot be deleted. Remove the plugin first.") - instance = super().update(instance, validated_data) report_user_action( @@ -140,5 +138,16 @@ def safely_get_queryset(self, queryset): queryset = queryset.filter(deleted=False) queryset = queryset.annotate(count=Count(TREND_FILTER_TYPE_EVENTS)) - queryset = queryset.prefetch_related("plugin_configs") return queryset.filter(team_id=self.team_id).order_by(*self.ordering) + + @action(methods=["GET"], detail=True) + def plugin_configs(self, request: Request, *args: Any, **kwargs: Any) -> Response: + queryset = ( + PluginConfig.objects.all() + .filter(team=self.team_id, enabled=True) + .filter(filters__contains={"actions": [{"id": str(self.get_object().id)}]}) + ) + + page = self.paginate_queryset(queryset) + serializer = PluginConfigSerializer(page, many=True, context=self.get_serializer_context()) + return self.get_paginated_response(serializer.data) diff --git a/posthog/api/plugin.py b/posthog/api/plugin.py index b4d8631d2699d..6f9d36a6bd090 100644 --- a/posthog/api/plugin.py +++ b/posthog/api/plugin.py @@ -21,8 +21,8 @@ from rest_framework.response import Response from posthog.api.routing import TeamAndOrgViewSetMixin +from posthog.api.shared import FiltersSerializer from posthog.models import Plugin, PluginAttachment, PluginConfig, User -from posthog.models.action.action import Action from posthog.models.activity_logging.activity_log import ( ActivityPage, Change, @@ -563,11 +563,6 @@ class PluginConfigSerializer(serializers.ModelSerializer): plugin_info = serializers.SerializerMethodField() delivery_rate_24h = serializers.SerializerMethodField() error = serializers.SerializerMethodField() - match_action = serializers.PrimaryKeyRelatedField( - queryset=Action.objects.all(), - required=False, - allow_null=True, - ) class Meta: model = PluginConfig @@ -586,7 +581,7 @@ class Meta: "name", "description", "deleted", - "match_action", + "filters", ] read_only_fields = [ "id", @@ -655,12 +650,10 @@ def get_error(self, plugin_config: PluginConfig) -> None: # error details instead. return None - def validate_match_action(self, value: Action): - if value: - if value.team_id != self.context["team_id"]: - raise ValidationError("Action must belong to the same project as the plugin config.") - - return value + def validate_filters(self, value: dict) -> dict: + serializer = FiltersSerializer(data=value) + serializer.is_valid(raise_exception=True) + return serializer.validated_data def create(self, validated_data: dict, *args: Any, **kwargs: Any) -> PluginConfig: if not can_configure_plugins(self.context["get_organization"]()): diff --git a/posthog/api/shared.py b/posthog/api/shared.py index d8c15f11459f3..97fb8a5cd288e 100644 --- a/posthog/api/shared.py +++ b/posthog/api/shared.py @@ -93,3 +93,17 @@ def get_membership_level(self, organization: Organization) -> Optional[Organizat organization=organization, user=self.context["request"].user ).first() return membership.level if membership is not None else None + + +class FilterBaseSerializer(serializers.Serializer): + type = serializers.ChoiceField(choices=["events", "actions"]) + id = serializers.CharField(required=False) + name = serializers.CharField(required=False, allow_null=True) + order = serializers.IntegerField(required=False) + properties = serializers.ListField(child=serializers.DictField(), default=[]) + + +class FiltersSerializer(serializers.Serializer): + events = FilterBaseSerializer(many=True, required=False) + actions = FilterBaseSerializer(many=True, required=False) + filter_test_accounts = serializers.BooleanField(required=False) diff --git a/posthog/api/test/__snapshots__/test_action.ambr b/posthog/api/test/__snapshots__/test_action.ambr index 11d845eafe5be..6015a9dea55bf 100644 --- a/posthog/api/test/__snapshots__/test_action.ambr +++ b/posthog/api/test/__snapshots__/test_action.ambr @@ -144,30 +144,6 @@ ''' # --- # name: TestActionApi.test_listing_actions_is_not_nplus1.11 - ''' - SELECT "posthog_pluginconfig"."id", - "posthog_pluginconfig"."team_id", - "posthog_pluginconfig"."plugin_id", - "posthog_pluginconfig"."enabled", - "posthog_pluginconfig"."order", - "posthog_pluginconfig"."config", - "posthog_pluginconfig"."error", - "posthog_pluginconfig"."web_token", - "posthog_pluginconfig"."created_at", - "posthog_pluginconfig"."updated_at", - "posthog_pluginconfig"."name", - "posthog_pluginconfig"."description", - "posthog_pluginconfig"."deleted", - "posthog_pluginconfig"."match_action_id" - FROM "posthog_pluginconfig" - WHERE "posthog_pluginconfig"."match_action_id" IN (1, - 2, - 3, - 4, - 5 /* ... */) - ''' -# --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.12 ''' SELECT "posthog_user"."id", "posthog_user"."password", @@ -199,7 +175,7 @@ LIMIT 21 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.13 +# name: TestActionApi.test_listing_actions_is_not_nplus1.12 ''' SELECT "posthog_team"."id", "posthog_team"."uuid", @@ -254,7 +230,7 @@ LIMIT 21 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.14 +# name: TestActionApi.test_listing_actions_is_not_nplus1.13 ''' SELECT "posthog_organizationmembership"."id", "posthog_organizationmembership"."organization_id", @@ -286,7 +262,7 @@ WHERE "posthog_organizationmembership"."user_id" = 2 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.15 +# name: TestActionApi.test_listing_actions_is_not_nplus1.14 ''' SELECT "posthog_organization"."id", "posthog_organization"."name", @@ -312,7 +288,7 @@ LIMIT 21 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.16 +# name: TestActionApi.test_listing_actions_is_not_nplus1.15 ''' SELECT COUNT(*) FROM @@ -325,7 +301,7 @@ GROUP BY 1) subquery ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.17 +# name: TestActionApi.test_listing_actions_is_not_nplus1.16 ''' SELECT "posthog_action"."id", "posthog_action"."name", @@ -382,54 +358,6 @@ LIMIT 100 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.18 - ''' - SELECT "posthog_pluginconfig"."id", - "posthog_pluginconfig"."team_id", - "posthog_pluginconfig"."plugin_id", - "posthog_pluginconfig"."enabled", - "posthog_pluginconfig"."order", - "posthog_pluginconfig"."config", - "posthog_pluginconfig"."error", - "posthog_pluginconfig"."web_token", - "posthog_pluginconfig"."created_at", - "posthog_pluginconfig"."updated_at", - "posthog_pluginconfig"."name", - "posthog_pluginconfig"."description", - "posthog_pluginconfig"."deleted", - "posthog_pluginconfig"."match_action_id" - FROM "posthog_pluginconfig" - WHERE "posthog_pluginconfig"."match_action_id" IN (1, - 2, - 3, - 4, - 5 /* ... */) - ''' -# --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.19 - ''' - SELECT "posthog_pluginconfig"."id", - "posthog_pluginconfig"."team_id", - "posthog_pluginconfig"."plugin_id", - "posthog_pluginconfig"."enabled", - "posthog_pluginconfig"."order", - "posthog_pluginconfig"."config", - "posthog_pluginconfig"."error", - "posthog_pluginconfig"."web_token", - "posthog_pluginconfig"."created_at", - "posthog_pluginconfig"."updated_at", - "posthog_pluginconfig"."name", - "posthog_pluginconfig"."description", - "posthog_pluginconfig"."deleted", - "posthog_pluginconfig"."match_action_id" - FROM "posthog_pluginconfig" - WHERE "posthog_pluginconfig"."match_action_id" IN (1, - 2, - 3, - 4, - 5 /* ... */) - ''' -# --- # name: TestActionApi.test_listing_actions_is_not_nplus1.2 ''' SELECT "posthog_organizationmembership"."id", @@ -462,30 +390,6 @@ WHERE "posthog_organizationmembership"."user_id" = 2 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.20 - ''' - SELECT "posthog_actionstep"."id", - "posthog_actionstep"."action_id", - "posthog_actionstep"."tag_name", - "posthog_actionstep"."text", - "posthog_actionstep"."text_matching", - "posthog_actionstep"."href", - "posthog_actionstep"."href_matching", - "posthog_actionstep"."selector", - "posthog_actionstep"."url", - "posthog_actionstep"."url_matching", - "posthog_actionstep"."event", - "posthog_actionstep"."properties", - "posthog_actionstep"."name" - FROM "posthog_actionstep" - WHERE "posthog_actionstep"."action_id" IN (1, - 2, - 3, - 4, - 5 /* ... */) - ORDER BY "posthog_actionstep"."id" ASC - ''' -# --- # name: TestActionApi.test_listing_actions_is_not_nplus1.3 ''' SELECT "posthog_organization"."id", diff --git a/posthog/api/test/__snapshots__/test_api_docs.ambr b/posthog/api/test/__snapshots__/test_api_docs.ambr index 9b3f0d468a7b4..b4cd8f94d8631 100644 --- a/posthog/api/test/__snapshots__/test_api_docs.ambr +++ b/posthog/api/test/__snapshots__/test_api_docs.ambr @@ -11,10 +11,6 @@ '/home/runner/work/posthog/posthog/ee/api/role.py: Warning [RoleViewSet > RoleSerializer]: unable to resolve type hint for function "get_associated_flags". Consider using a type hint or @extend_schema_field. Defaulting to string.', '/home/runner/work/posthog/posthog/ee/api/role.py: Warning [RoleMembershipViewSet]: could not derive type of path parameter "organization_id" because model "ee.models.role.RoleMembership" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/posthog/api/action.py: Warning [ActionViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.action.action.Action" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', - '/home/runner/work/posthog/posthog/posthog/api/plugin.py: Warning [ActionViewSet > ActionSerializer > PluginConfigSerializer]: unable to resolve type hint for function "get_config". Consider using a type hint or @extend_schema_field. Defaulting to string.', - '/home/runner/work/posthog/posthog/posthog/api/plugin.py: Warning [ActionViewSet > ActionSerializer > PluginConfigSerializer]: unable to resolve type hint for function "get_error". Consider using a type hint or @extend_schema_field. Defaulting to string.', - '/home/runner/work/posthog/posthog/posthog/api/plugin.py: Warning [ActionViewSet > ActionSerializer > PluginConfigSerializer]: unable to resolve type hint for function "get_plugin_info". Consider using a type hint or @extend_schema_field. Defaulting to string.', - '/home/runner/work/posthog/posthog/posthog/api/plugin.py: Warning [ActionViewSet > ActionSerializer > PluginConfigSerializer]: unable to resolve type hint for function "get_delivery_rate_24h". Consider using a type hint or @extend_schema_field. Defaulting to string.', '/home/runner/work/posthog/posthog/posthog/api/activity_log.py: Warning [ActivityLogViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.activity_logging.activity_log.ActivityLog" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/posthog/api/annotation.py: Warning [AnnotationsViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.annotation.Annotation" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', "/home/runner/work/posthog/posthog/posthog/api/app_metrics.py: Error [AppMetricsViewSet]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'AppMetricsViewSet' should either include a `serializer_class` attribute, or override the `get_serializer_class()` method.)", diff --git a/posthog/api/test/test_action.py b/posthog/api/test/test_action.py index 198f79cc12ff5..a1323bd868a1d 100644 --- a/posthog/api/test/test_action.py +++ b/posthog/api/test/test_action.py @@ -61,7 +61,6 @@ def test_create_action(self, patch_capture, *args): "is_action": True, "bytecode_error": None, "tags": [], - "plugin_configs": [], } # Assert analytics are sent @@ -223,8 +222,8 @@ def test_update_action(self, patch_capture, *args): # test queries with self.assertNumQueries(FuzzyInt(7, 8)): - # Django session, PostHog user, PostHog team, PostHog org membership, PostHog org - # PostHog action, PostHog action step, plugin_configs + # Django session, user, team, org membership, instance setting, org, + # count, action self.client.get(f"/api/projects/{self.team.id}/actions/") def test_update_action_remove_all_steps(self, *args): @@ -331,7 +330,7 @@ def test_listing_actions_is_not_nplus1(self) -> None: created_by=User.objects.create_and_join(self.organization, "a", ""), ) - with self.assertNumQueries(8), snapshot_postgres_queries_context(self): + with self.assertNumQueries(7), snapshot_postgres_queries_context(self): self.client.get(f"/api/projects/{self.team.id}/actions/") Action.objects.create( @@ -340,7 +339,7 @@ def test_listing_actions_is_not_nplus1(self) -> None: created_by=User.objects.create_and_join(self.organization, "b", ""), ) - with self.assertNumQueries(8), snapshot_postgres_queries_context(self): + with self.assertNumQueries(7), snapshot_postgres_queries_context(self): self.client.get(f"/api/projects/{self.team.id}/actions/") def test_get_tags_on_non_ee_returns_empty_list(self): diff --git a/posthog/api/test/test_plugin.py b/posthog/api/test/test_plugin.py index 2336cdd50752b..c201f091d8c39 100644 --- a/posthog/api/test/test_plugin.py +++ b/posthog/api/test/test_plugin.py @@ -1013,7 +1013,7 @@ def test_create_plugin_config(self, mock_get, mock_reload): "name": "name in ui", "description": "description in ui", "deleted": False, - "match_action": None, + "filters": None, }, ) plugin_config = PluginConfig.objects.first() @@ -1043,7 +1043,7 @@ def test_create_plugin_config(self, mock_get, mock_reload): "name": "name in ui", "description": "description in ui", "deleted": False, - "match_action": None, + "filters": None, }, ) @@ -1082,7 +1082,7 @@ def test_create_plugin_config(self, mock_get, mock_reload): "name": "name in ui", "description": "description in ui", "deleted": False, - "match_action": None, + "filters": None, }, ) @@ -1400,7 +1400,7 @@ def test_create_plugin_config_with_secrets(self, mock_get, mock_reload): "name": "Hello World", "description": "Greet the World and Foo a Bar, JS edition!", "deleted": False, - "match_action": None, + "filters": None, }, ) @@ -1430,7 +1430,7 @@ def test_create_plugin_config_with_secrets(self, mock_get, mock_reload): "name": "Hello World", "description": "Greet the World and Foo a Bar, JS edition!", "deleted": False, - "match_action": None, + "filters": None, }, ) @@ -1462,7 +1462,7 @@ def test_create_plugin_config_with_secrets(self, mock_get, mock_reload): "name": "Hello World", "description": "Greet the World and Foo a Bar, JS edition!", "deleted": False, - "match_action": None, + "filters": None, }, ) plugin_config = PluginConfig.objects.get(plugin=plugin_id) @@ -1511,7 +1511,7 @@ def test_plugin_config_list(self, mock_get, mock_reload): "name": None, "description": None, "deleted": False, - "match_action": None, + "filters": None, }, { "id": plugin_config2.pk, @@ -1528,7 +1528,7 @@ def test_plugin_config_list(self, mock_get, mock_reload): "name": "ui name", "description": "ui description", "deleted": False, - "match_action": None, + "filters": None, }, ], ) @@ -1653,45 +1653,48 @@ def test_get_all_activity(self, _, mock_reload): ] ) - def test_update_plugin_match_action(self, mock_get, mock_reload): - action_res = self.client.post(f"/api/projects/{self.team.id}/actions/", {"name": "test action"}) - assert action_res.status_code == 201, action_res.json() - action = action_res.json() - + def test_update_plugin_filters(self, mock_get, mock_reload): plugin = self._create_plugin() response = self.client.post( "/api/plugin_config/", - {"plugin": plugin["id"], "enabled": True, "order": 0, "match_action": action["id"]}, + { + "plugin": plugin["id"], + "enabled": True, + "order": 0, + "filters": json.dumps( + { + "events": [ + { + "name": "$pageview", + "properties": [], + "type": "events", + } + ] + } + ), + }, format="multipart", ) assert response.status_code == 201, response.json() - assert response.json()["match_action"] == action["id"] - - def test_update_plugin_match_action_fails_for_missing_action(self, mock_get, mock_reload): - plugin = self._create_plugin() - response = self.client.post( - "/api/plugin_config/", - {"plugin": plugin["id"], "enabled": True, "order": 0, "match_action": 99999}, - format="multipart", - ) - - assert response.status_code == 400, response.json() - assert response.json()["code"] == "does_not_exist" - def test_update_plugin_match_action_fails_for_wrong_team(self, mock_get, mock_reload): - other_team = Team.objects.create(organization=self.organization, name="Another Team") - action_res = self.client.post(f"/api/projects/{other_team.id}/actions/", {"name": "test action"}) + assert response.json()["filters"] == {"events": [{"name": "$pageview", "properties": [], "type": "events"}]} + def test_update_plugin_filters_fails_for_bad_formatting(self, mock_get, mock_reload): plugin = self._create_plugin() response = self.client.post( "/api/plugin_config/", - {"plugin": plugin["id"], "enabled": True, "order": 0, "match_action": action_res.json()["id"]}, + { + "plugin": plugin["id"], + "enabled": True, + "order": 0, + "filters": json.dumps({"events": "wrong"}), + }, format="multipart", ) assert response.status_code == 400, response.json() - assert response.json()["detail"] == "Action must belong to the same project as the plugin config." + assert response.json()["code"] == "not_a_list" class TestPluginsAccessLevelAPI(APIBaseTest): diff --git a/posthog/migrations/0418_pluginconfig_filters.py b/posthog/migrations/0418_pluginconfig_filters.py new file mode 100644 index 0000000000000..4d0bf579fe2aa --- /dev/null +++ b/posthog/migrations/0418_pluginconfig_filters.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.11 on 2024-05-23 11:54 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("posthog", "0417_remove_organizationmembership_only_one_owner_per_organization_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="pluginconfig", + name="filters", + field=models.JSONField(blank=True, null=True), + ), + ] diff --git a/posthog/models/plugin.py b/posthog/models/plugin.py index 7c4c8cb122dae..b2776a04f6cfd 100644 --- a/posthog/models/plugin.py +++ b/posthog/models/plugin.py @@ -254,7 +254,11 @@ class Meta: description: models.CharField = models.CharField(max_length=1000, null=True, blank=True) # Used in the frontend to hide pluginConfigs that user deleted deleted: models.BooleanField = models.BooleanField(default=False, null=True) - # If set, the plugin-server will only trigger this plugin for events that match this action + + # If set we will filter the plugin triggers for this event + filters: models.JSONField = models.JSONField(null=True, blank=True) + + # DEPRECATED - this never actually got used - filters is the way to go match_action = models.ForeignKey( "posthog.Action", on_delete=models.SET_NULL,