Support for posthog super properties #2579
Conversation
|
Generated by 🚫 Danger Swift against 24c858e |
| /// Super Properties are properties associated with events that are set once and then sent with every capture call, be it a $screen, an autocaptured button click, or anything else. | ||
| /// It is different from user properties that will be attached to the user and not events. | ||
| /// Not persisted for now, should be set on start. | ||
| private var superProperties: [String: Any] = [:] |
There was a problem hiding this comment.
Why don't we define and type these similar to AnalyticsEvent.UserProperties?
There was a problem hiding this comment.
We need this property in Screen events to have common filters for each platforms. Screen events are built in, and not defined in the analytics repo
There was a problem hiding this comment.
Oh I see, just define a blank object in the repo
|
|
||
| // Add super property cryptoSDK to the captured events, to allow easy | ||
| // filtering of events across different client by using same filter. | ||
| superProperties["cryptoSDK"] = "Rust" |
There was a problem hiding this comment.
.. would make this less.. stringy
There was a problem hiding this comment.
Agreed, we shouldn't be sending anything to PostHog that isn't coming from the schema.
There was a problem hiding this comment.
We can't define that in the schema (properties added to build in events)
There was a problem hiding this comment.
It should be possible to make a SuperProperties event and attach it's .properties to the event's that we're sending instead of an untyped dictionary though?
There was a problem hiding this comment.
| } | ||
|
|
||
| func screen(_ event: AnalyticsScreenProtocol) { | ||
| guard isRunning else { return } | ||
| postHog?.screen(event.screenName.rawValue, properties: attachUserProperties(to: event.properties)) | ||
| postHog?.screen(event.screenName.rawValue, properties: attachUserProperties(to: attachSuperProperties(to: event.properties))) |
There was a problem hiding this comment.
This is not the most straightforward thing, some tests are in order. Please see UnitTests/AnalyticsTests, especiall the *UserProperties part
|
@BillCarsonFr planning on coming back to this PR at some point or can we close it? |
|
Closing in favour of #2774 |
Pull Request Checklist
A quick PR to add super properties to captured event. Super properties are added to every captured event.
In this case I added on super property for
cryptoSDK, we used this property as a filter on$screenevents (as well as E2E errors). It allows to build a ratio of errors for all different clients just using this filter.Let me know if ok
UI changes have been tested with: