Skip to content

Commit

Permalink
feat(cdp): cleanup action webhook stuff (#27759)
Browse files Browse the repository at this point in the history
  • Loading branch information
@meikelmosby
meikelmosby authored Jan 22, 2025
1 parent c78ede2 commit e6dfdd8
Show file tree
Hide file tree
Showing 7 changed files with 5 additions and 411 deletions.
1 change: 0 additions & 1 deletion frontend/src/lib/constants.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ export const FEATURE_FLAGS = {
BIGQUERY_DWH: 'bigquery-dwh', // owner: @Gilbert09 #team-data-warehouse
ENVIRONMENTS: 'environments', // owner: @Twixes #team-product-analytics
BILLING_PAYMENT_ENTRY_IN_APP: 'billing-payment-entry-in-app', // owner: @zach
LEGACY_ACTION_WEBHOOKS: 'legacy-action-webhooks', // owner: @mariusandra #team-cdp
REPLAY_TEMPLATES: 'replay-templates', // owner: @raquelmsmith #team-replay
EXPERIMENTS_HOGQL: 'experiments-hogql', // owner: @jurajmajerik #team-experiments
ROLE_BASED_ACCESS_CONTROL: 'role-based-access-control', // owner: @zach
Expand Down
108 changes: 2 additions & 106 deletions frontend/src/scenes/actions/ActionEdit.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
import { IconInfo, IconPlus } from '@posthog/icons'
import { LemonBanner, LemonCheckbox, LemonDialog, LemonTextArea } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { Form } from 'kea-forms'
import { router } from 'kea-router'
import { EditableField } from 'lib/components/EditableField/EditableField'
import { ObjectTags } from 'lib/components/ObjectTags/ObjectTags'
import { PageHeader } from 'lib/components/PageHeader'
import { FEATURE_FLAGS } from 'lib/constants'
import { IconPlayCircle } from 'lib/lemon-ui/icons'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { LemonField } from 'lib/lemon-ui/LemonField'
import { LemonLabel } from 'lib/lemon-ui/LemonLabel/LemonLabel'
import { Link } from 'lib/lemon-ui/Link'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { ActionHogFunctions } from 'scenes/actions/ActionHogFunctions'
import { pipelineAccessLogic } from 'scenes/pipeline/pipelineAccessLogic'
import { teamLogic } from 'scenes/teamLogic'
import { urls } from 'scenes/urls'

import { tagsModel } from '~/models/tagsModel'
Expand All @@ -30,19 +24,9 @@ export function ActionEdit({ action: loadedAction, id }: ActionEditLogicProps):
action: loadedAction,
}
const logic = actionEditLogic(logicProps)
const { action, actionLoading, actionChanged, migrationLoading, hasCohortFilters } = useValues(logic)
const { submitAction, deleteAction, migrateToHogFunction } = useActions(logic)
const { currentTeam } = useValues(teamLogic)
const { action, actionLoading, actionChanged } = useValues(logic)
const { submitAction, deleteAction } = useActions(logic)
const { tags } = useValues(tagsModel)
const { canEnableNewDestinations: hasDataPipelinesAddon } = useValues(pipelineAccessLogic)
const { featureFlags } = useValues(featureFlagLogic)

const slackEnabled = currentTeam?.slack_incoming_webhook
const showWebhookDelivery =
(!hasDataPipelinesAddon && slackEnabled) ||
action.post_to_slack ||
featureFlags[FEATURE_FLAGS.LEGACY_ACTION_WEBHOOKS]
const showActionMigrationBanner = hasDataPipelinesAddon && action.post_to_slack

const deleteButton = (): JSX.Element => (
<LemonButton
Expand Down Expand Up @@ -217,94 +201,6 @@ export function ActionEdit({ action: loadedAction, id }: ActionEditLogicProps):
)}
</LemonField>
</div>

{showWebhookDelivery ? (
<div className="my-4 space-y-2">
<h2 className="subtitle">Webhook delivery</h2>

{showActionMigrationBanner ? (
<LemonBanner
type="error"
action={{
children: 'Upgrade to new version',
onClick: () =>
LemonDialog.open({
title: 'Upgrade webhook',
width: '30rem',
description:
'This will create a new Destination in the upgraded system. The action will have its webhook disabled. There will be slight difference in the placeholder tags, so double check that everything works as expected.',
secondaryButton: {
type: 'secondary',
children: 'Cancel',
},
primaryButton: {
type: 'primary',
onClick: () => migrateToHogFunction(),
children: 'Upgrade',
},
}),
disabledReason: hasCohortFilters
? 'Can not upgrade because action has a cohort filter.'
: migrationLoading
? 'Loading...'
: actionChanged
? 'Please save the action first'
: undefined,
}}
>
Action Webhooks have been replaced by the new and improved <b>Pipeline Destinations</b>.{' '}
{!hasCohortFilters && !actionChanged ? 'Click to upgrade.' : ''}
</LemonBanner>
) : null}

<LemonField name="post_to_slack">
{({ value, onChange }) => (
<div className="flex items-center gap-2 flex-wrap">
<LemonCheckbox
id="webhook-checkbox"
bordered
checked={!!value}
onChange={onChange}
disabledReason={!slackEnabled ? 'Configure webhooks in project settings' : null}
label={
<>
<span>Post to webhook when this action is triggered.</span>
</>
}
/>
<Link to={urls.settings('project-integrations', 'integration-webhooks')}>
{slackEnabled ? 'Configure' : 'Enable'} webhooks in project settings.
</Link>
</div>
)}
</LemonField>
{action.post_to_slack && (
<LemonField name="slack_message_format">
{({ value, onChange }) => (
<>
<LemonLabel showOptional>Slack message format</LemonLabel>
<LemonTextArea
placeholder="Default: [action.name] triggered by [person]"
value={value}
onChange={onChange}
disabled={!slackEnabled || !action.post_to_slack}
data-attr="edit-slack-message-format"
maxLength={1200 /** Must be same as in posthog/models/action/action.py */}
/>
<small>
<Link
to="https://posthog.com/docs/webhooks#message-formatting"
target="_blank"
>
See documentation on how to format webhook messages.
</Link>
</small>
</>
)}
</LemonField>
)}
</div>
) : null}
</Form>
</div>
)
Expand Down
23 changes: 1 addition & 22 deletions frontend/src/scenes/actions/actionEditLogic.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
import { Link } from 'lib/lemon-ui/Link'
import { deleteWithUndo } from 'lib/utils/deleteWithUndo'
import { eventDefinitionsTableLogic } from 'scenes/data-management/events/eventDefinitionsTableLogic'
import { hogFunctionListLogic } from 'scenes/pipeline/hogfunctions/list/hogFunctionListLogic'
import { sceneLogic } from 'scenes/sceneLogic'
import { urls } from 'scenes/urls'

Expand Down Expand Up @@ -137,34 +136,14 @@ export const actionEditLogic = kea<actionEditLogicType>([
],
}),

loaders(({ actions, props, values }) => ({
loaders(({ props, values }) => ({
action: [
{ ...props.action } as ActionType,
{
setAction: ({ action, options: { merge } }) =>
(merge ? { ...values.action, ...action } : action) as ActionType,
},
],
migration: [
true,
{
migrateToHogFunction: async () => {
if (props.id) {
const hogFunction = await api.actions.migrate(props.id)
actions.setActionValues({ post_to_slack: false })
actions.loadActions()
if (hogFunctionListLogic.isMounted()) {
hogFunctionListLogic.actions.addHogFunction(hogFunction)
}
if (actionLogic({ id: props.id }).isMounted()) {
actionLogic({ id: props.id }).actions.updateAction({ post_to_slack: false })
}
lemonToast.success('Action migrated to a destination!')
}
return true
},
},
],
})),

listeners(({ values, actions }) => ({
Expand Down
19 changes: 0 additions & 19 deletions posthog/api/action.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from django.db.models import Count
from rest_framework import request, serializers, viewsets
from rest_framework.decorators import action
from rest_framework.response import Response
from rest_framework.settings import api_settings
from rest_framework_csv import renderers as csvrenderers
Expand All @@ -17,7 +16,6 @@
from posthog.models.action.action import ACTION_STEP_MATCHING_OPTIONS

from .forbid_destroy_model import ForbidDestroyModel
from .hog_function import HogFunctionSerializer
from .tagged_item import TaggedItemSerializerMixin, TaggedItemViewSetMixin


Expand Down Expand Up @@ -162,20 +160,3 @@ def list(self, request: request.Request, *args: Any, **kwargs: Any) -> Response:
actions, many=True, context={"request": request}
).data # type: ignore
return Response({"results": actions_list})

@action(methods=["POST"], url_path="migrate", detail=True)
def migrate(self, request: request.Request, **kwargs):
obj = self.get_object()

from posthog.management.commands.migrate_action_webhooks import convert_to_hog_function

hog_function = convert_to_hog_function(obj, inert=False)
if not hog_function:
return Response({"detail": "Failed to convert action to function"}, status=400)
hog_function.save()
hog_function_serializer = HogFunctionSerializer(hog_function, context=self.get_serializer_context())
if obj.post_to_slack:
obj.post_to_slack = False
obj.save()

return Response(hog_function_serializer.data)
37 changes: 1 addition & 36 deletions posthog/api/test/test_action.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from freezegun import freeze_time
from rest_framework import status

from posthog.models import Action, Tag, User, HogFunction
from posthog.models import Action, Tag, User
from posthog.test.base import (
APIBaseTest,
ClickhouseTestMixin,
Expand Down Expand Up @@ -107,41 +107,6 @@ def test_create_action_generates_bytecode(self):
action = Action.objects.get(pk=response.json()["id"])
assert action.bytecode == ["_H", 1, 32, "%/signup%", 32, "$current_url", 32, "properties", 1, 2, 17]

def test_action_migration(self):
self.team.slack_incoming_webhook = "https://slack.com"
self.team.save()
response = self.client.post(
f"/api/projects/{self.team.id}/actions/",
data={
"name": "user signed up",
"steps": [
{
"text": "sign up",
"selector": "div > button",
"url": "/signup",
}
],
"description": "Test description",
"post_to_slack": True,
},
HTTP_ORIGIN="http://testserver",
)
action = Action.objects.get(pk=response.json()["id"])
assert action.post_to_slack is True

migrate_response = self.client.post(f"/api/projects/{self.team.id}/actions/{action.id}/migrate/")
self.assertEqual(migrate_response.status_code, status.HTTP_200_OK)

action.refresh_from_db()
assert action.post_to_slack is False

function: HogFunction | None = None # type: ignore
for hog_function in HogFunction.objects.all():
if action.id in hog_function.filter_action_ids:
function = hog_function
assert function is not None
assert function.enabled

def test_cant_create_action_with_the_same_name(self, *args):
original_action = Action.objects.create(name="user signed up", team=self.team)
user2 = self._create_user("tim2")
Expand Down
90 changes: 0 additions & 90 deletions posthog/management/commands/migrate_action_webhooks.py
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import re
from typing import Optional
from django.core.management.base import BaseCommand
from django.core.paginator import Paginator
from django.db.models import QuerySet

from posthog.cdp.filters import compile_filters_bytecode
from posthog.cdp.validation import compile_hog, validate_inputs
from posthog.models.action.action import Action
Expand Down Expand Up @@ -150,52 +146,6 @@ def convert_to_hog_function(action: Action, inert=False) -> Optional[HogFunction
return hog_function


def migrate_action_webhooks(action_ids: list[int], team_ids: list[int], dry_run=False, inert=False):
if action_ids and team_ids:
print("Please provide either action_ids or team_ids, not both") # noqa: T201
return
query = Action.objects.select_related("team").filter(post_to_slack=True, deleted=False).order_by("id")
if team_ids:
print("Migrating all actions for teams:", team_ids) # noqa: T201
query = query.filter(team_id__in=team_ids)
elif action_ids:
print("Migrating actions:", action_ids) # noqa: T201
query = query.filter(id__in=action_ids)
else:
print(f"Migrating all actions") # noqa T201
paginator = Paginator(query.all(), 100)
for page_number in paginator.page_range:
page = paginator.page(page_number)
hog_functions: list[HogFunction] = []
actions_to_update: list[Action] = []
for action in page.object_list:
if len(action.steps) == 0:
continue
try:
hog_function = convert_to_hog_function(action, inert)
if hog_function:
hog_functions.append(hog_function)
if not inert:
action.post_to_slack = False
actions_to_update.append(action)
except Exception as e:
print(f"Failed to migrate action {action.id}: {e}") # noqa: T201
if not dry_run:
HogFunction.objects.bulk_create(hog_functions)
if actions_to_update:
Action.objects.bulk_update(actions_to_update, ["post_to_slack"])
else:
print("Would have created the following HogFunctions:") # noqa: T201
for hog_function in hog_functions:
print(hog_function, hog_function.inputs, hog_function.filters) # noqa: T201
if not dry_run:
reload_all_hog_functions_on_workers()


def get_all_inert_hogfunctions() -> QuerySet[HogFunction, HogFunction]:
return HogFunction.objects.filter(name__startswith="[CDP-TEST-HIDDEN]")


def migrate_all_teams_action_webhooks(dry_run=False, inert=False):
"""Migrate actions for all teams in the system."""
print("Starting migration of actions for all teams") # noqa: T201
Expand Down Expand Up @@ -250,43 +200,3 @@ def migrate_all_teams_action_webhooks(dry_run=False, inert=False):

if not dry_run:
reload_all_hog_functions_on_workers()


class Command(BaseCommand):
help = "Migrate action webhooks to HogFunctions"

def add_arguments(self, parser):
parser.add_argument(
"--dry-run",
action="store_true",
help="If set, will not actually perform the migration, but will print out what would have been done",
)
parser.add_argument(
"--inert", action="store_true", help="Create inert HogFunctions that will not fetch but just print"
)
parser.add_argument("--migrate-all-teams-actions", action="store_true", help="Migrate actions for all teams")
parser.add_argument("--action-ids", type=str, help="Comma separated list of action ids to sync")
parser.add_argument("--team-ids", type=str, help="Comma separated list of team ids to sync")

def handle(self, *args, **options):
dry_run = options["dry_run"]
action_ids = options["action_ids"]
team_ids = options["team_ids"]
inert = options["inert"]
migrate_all_teams = options["migrate_all_teams_actions"]

print(f"Migrating action webhooks with options: {options}") # noqa: T201

if sum(bool(x) for x in [action_ids, team_ids, migrate_all_teams]) > 1:
print("Please provide only one of: action_ids, team_ids, or migrate-all-teams-actions") # noqa: T201
return

if migrate_all_teams:
migrate_all_teams_action_webhooks(dry_run=dry_run, inert=inert)
else:
migrate_action_webhooks(
action_ids=[int(x) for x in action_ids.split(",")] if action_ids else [],
team_ids=[int(x) for x in team_ids.split(",")] if team_ids else [],
dry_run=dry_run,
inert=inert,
)
Loading

0 comments on commit e6dfdd8

Please sign in to comment.