posthog icon indicating copy to clipboard operation
posthog copied to clipboard

feat: Action steps refactor

Open benjackwhite opened this issue 1 year ago â€ĸ 6 comments

Problem

We have ActionSteps as a separate model that is never used in any clever way.

It would be much simpler to have it all stored in the action itself as a json map. This would make it easier to load and reason on

Changes

  • [x] Adds a new steps_json to the model
  • [x] Maps steps to create a new class from the related fields or the stored json
  • [x] Moves all code over to use this new class
  • [x] Adds migration code
  • [x] Writes to both places, reading from the steps_json if it exists
  • [x] Removes properties that weren't referenced anymore such as name

Split up PRs

  • Part 1 - API - https://github.com/PostHog/posthog/pull/22091
  • Part 2 - Plugin server - https://github.com/PostHog/posthog/pull/22092
  • Part 3 - Deprecate old model https://github.com/PostHog/posthog/pull/22093

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

benjackwhite avatar May 02 '24 10:05 benjackwhite

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: posthog/hogql/property.py

Function Unhandled Issue
property_to_expr TypeError: Property.init() missing 1 required positional argument: 'key' posthog.ta...
Event Count: 90
property_to_expr Cohort.DoesNotExist: Cohort matching query does not exist. posthog.tasks.tasks.process_...
Event Count: 4
property_to_expr NotImplementedError: property_to_expr for element selector only supports exact and is_not operators, not regex ...
Event Count: 4
property_to_expr NotImplementedError: property_to_expr for element selector only supports exact and is_not operators, not icontains ...
Event Count: 2
property_to_expr TypeError: Property.init() missing 1 required positional argument: 'key' posthog.ta...
Event Count: 1

Did you find this useful? React with a 👍 or 👎

sentry-io[bot] avatar May 02 '24 10:05 sentry-io[bot]

Schema purity 📉 Practicality 📈 ✅

Twixes avatar May 02 '24 15:05 Twixes

Size Change: -32 B (0%)

Total Size: 1.05 MB

â„šī¸ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.05 MB -32 B (0%)

compressed-size-action

github-actions[bot] avatar May 03 '24 08:05 github-actions[bot]

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot avatar May 03 '24 08:05 posthog-bot

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot avatar May 03 '24 08:05 posthog-bot

Screenshot 2024-05-06 at 16 39 02

I love how many commits come after this one đŸ¤Ŗ

pauldambra avatar May 06 '24 15:05 pauldambra