Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identity schema with FTUE use case selection #15

Merged
merged 3 commits into from
Jan 19, 2022

Conversation

ouchadam
Copy link
Contributor

@ouchadam ouchadam commented Jan 13, 2022

Draft of a Identity schema for typing the properties used by Posthog.Identify

  • Uses the existing event schema to create a pseudo Identity event
  • User properties are defined as individual properties on the parent event
  • Adds the ftueUseCaseSelection to the identity properties
    • Clients will locally store the FTUE onboarding messaging use case selection and will apply as a user property when initiating and identifying a consented analytics session
  • Also adds examples of applying descriptions to the readme
// example usage
posthog?.identify(
  id, 
  Identity(ftueUseCaseSelection = fetchStoredSelection()).getProperties()
)

@ouchadam ouchadam force-pushed the adm/ftue-use-case-via-identity branch from 758223a to bc12dd0 Compare January 13, 2022 15:04
]
}
},
"required": [],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the properties be enforced? Clients opting in vs opting out

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say it's fine to leave ftueUseCaseSelection as optional. Potentially for this one event, we might want to leave all the properties as optional so we could use $set from elsewhere in the app on a property by property basis. WDYT @novocaine?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense with the caveat that clients will need to have a $set wrapper that still does the validation

Copy link
Member

@pixlwave pixlwave Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds sensible to me. If I've interpreted the docs correctly, I think the right option here would be to have a setUserProperties(from identity: IdentityAnalyticsEvent) method that caches the Identity event and adds the additional $set property to the next event we send.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this a bit more, caching the identity event would also mean serialising the event to prevent data loss, which I'd prefer to avoid if possible!

from the docs I couldn't find any mention of avoiding calling identify directly, we could have a mix of

fun updateUserProperties(identity: IdentityEvent) {
  // calls identify with the currently known analytics id and identity payload
}

fun capture(
  event: Event, 
  setIdentity: IdentityEvent? = null, 
  setOnceIdentity: IdentityEvent? = null
) {
  // applies optional $set and $set_once properties if present to the event payload
}

what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

caching the identity event would also mean serialising the event to prevent data loss, which I'd prefer to avoid if possible!

Should we have a way to serialise the identity anyway? For example - I sign up and choose not to opt in when prompted for analytics. Later on I find the toggle in settings and decide to opt in. It would be a shame to lose any potential user properties in this instance (although this is probably more of an edge case in reality).

Maybe we should discuss. Although ultimately it's no more than an implementation detail specific to the client. So long as we're sticking to the schema, it probably doesn't matter that much.

@ouchadam ouchadam marked this pull request as ready for review January 17, 2022 09:29
"description": "The selected messaging use case during the onboarding flow.",
"type": "string",
"enum": [
"FriendsAndFamily",
Copy link
Contributor Author

@ouchadam ouchadam Jan 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniellekirkwood are there any preferred tracking values for the use case options?

I've used the UI as a reference for the values

property - ftueUseCaseSelection
options =

FriendsAndFamily
Teams,
Communities,
Skipped"

@ouchadam ouchadam force-pushed the adm/ftue-use-case-via-identity branch from bc12dd0 to 46afd36 Compare January 18, 2022 16:11
"description": "The selected messaging use case during the onboarding flow.",
"type": "string",
"oneOf": [
{"const": "PersonalMessaging", "description": "The first option, Friends and family."},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @daniellekirkwood here's the updated options (with descriptions)

Copy link

@daniellekirkwood daniellekirkwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

"description": "The user properties to apply when identifying",
"properties": {
"eventName": {
"enum": ["Identity"]
Copy link
Contributor

@novocaine novocaine Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably kind of unnecessary .. or should be $identify? I guess that'll mess heartily with the type gen

Copy link
Contributor Author

@ouchadam ouchadam Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's only added because eventName is mandatory for the type gen, is completely unused by the clients, what's $identify?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah nm, I was thinking posthog's identify call lets you set user properties as in mixpanel, but its entirely separate

@ouchadam ouchadam merged commit b8138cc into main Jan 19, 2022
@ouchadam ouchadam deleted the adm/ftue-use-case-via-identity branch January 19, 2022 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants