feat(cdp): mapping #26655
feat(cdp): mapping #26655
Conversation
|
Size Change: +60 B (+0.01%) Total Size: 1.11 MB ℹ️ View Unchanged
|
frontend/src/loadPostHogJS.tsx
Outdated
| @@ -67,6 +67,7 @@ export function loadPostHogJS(): void { | |||
| capture_copied_text: true, | |||
| }, | |||
| person_profiles: 'always', | |||
| __preview_remote_config: true, | |||
There was a problem hiding this comment.
This breaks flags - I'm working on the proper fix atm but just beware of merging
There was a problem hiding this comment.
Removed this line for now... I'm also hoping this is what caused alerts.cy and onboarding.cy to flake as well 🤞 😅
| return ( | ||
| <> | ||
| {mappings.map((mapping, index) => ( | ||
| <div key={index} className="border bg-bg-light rounded p-3 space-y-2"> |
There was a problem hiding this comment.
More of a follow up comment - this new thing breaks the design pattern of having clear panels for different sections. I might fly by and bring some opinionated UI changes to see if we can make it a little cleaner. Definitely a comment on my work, not yours 😅
There was a problem hiding this comment.
Definitely feel free to step all over my "designer with 2 left hands" work (to not throw backend engineers under the bus again)
posthog/api/hog_function.py
Outdated
| attrs["filters"]["transpiled"] = {"lang": "ts", "code": code, "stl": list(compiler.stl_functions)} | ||
| if "bytecode" in attrs["filters"]: | ||
| del attrs["filters"]["bytecode"] | ||
| attrs["mappings"] = attrs.get("mappings") or [] |
There was a problem hiding this comment.
I actually thing this should default to None. Reason being that in the future mappings will essentially override the default so we need a way to indicate that someone has created a destination with no mappings - therefore a noop.
Doesn't really make a difference today but I already know it will be a future migration effort so would be nice to keep it clean now.
posthog/api/hog_function.py
Outdated
|
|
||
| validate_input_and_filters(attrs, attrs["type"]) | ||
|
|
||
| if "mappings" in attrs and attrs["mappings"] is not None: |
There was a problem hiding this comment.
Would be good currently to explicitly raise if mappings are provided for non site-destinations - again thinking about keeping things clean just in case for the future follow up.
| response += "\n})" | ||
| # Wrap in IIFE = Immediately Invoked (invokable) Function Expression = to avoid polluting global scope | ||
| # Add collected STL functions above the generated code | ||
| response = "(function() {\n" + compiler.get_stl_code() + "\n" + response + "\n})" |
|
|
||
| blank_site_destination: HogFunctionTemplate = HogFunctionTemplate( | ||
| status="client-side", | ||
| type="site_destination", | ||
| id="template-blank-site-destination", | ||
| name="New client-side destination", | ||
| description="Run code on your website when an event is sent to PostHog. Works only with posthog-js when opt_in_site_apps is set to true.", | ||
| description="New destination with complex event mapping. Works only with posthog-js when opt_in_site_apps is set to true.", |
There was a problem hiding this comment.
This is fine whilst we are testing internally but i think once we have a "real" destination using mappings we should revert this to be a simple single mapping function
| @@ -255,3 +262,24 @@ def test_get_transpiled_function_with_complex_filters(self): | |||
| assert "const filterMatches = " in result | |||
| assert '__getGlobal("event") == "$pageview"' in result | |||
| assert "https://example.com" in result | |||
|
|
|||
| def test_get_transpiled_function_with_mappings(self): | |||
There was a problem hiding this comment.
Really would love to get proper testing working for these... I have the little node runner which is good for just seeing if there are compilation errors but I think we could likely mock out enough browser-y things to actually do proper tests via node at some point...
There was a problem hiding this comment.
Totally agreed. I'll try to think of something, but in a followup. Even node + jsdom would go a long way.
Problem
Currently you need to set up multiple copies of one destination in order to have it send slightly different payloads on different events.
Changes
Creates a new mapping system, only for site destinations for now:
Features
How did you test this code?