-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add a new crypto session state event #103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few remarks, but nothing blocking.
@@ -59,6 +59,21 @@ | |||
{"const": "Group", "description": "Modern layout."}, | |||
{"const": "Compact", "description": "Modern layout with compact option enabled."} | |||
] | |||
}, | |||
"verificationState": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a description please? Can be copy paste of what's in CryptoSessionState.json.
] | ||
}, | ||
"recoveryState": { | ||
"type": "string", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a description please? Can be copy paste of what's in CryptoSessionState.json.
schemas/CryptoSessionState.json
Outdated
}, | ||
"required": ["eventName","verificationState", "recoveryState"], | ||
"additionalProperties": false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So IIUC, this Event will be used for state change, so maybe rename to something like CryptoSessionStateChange
(not sure what's best)?
Add a new CryptoSessionState to get information on Verification And recovery state.
The same propeties are also added as UserProperties.
Adding as UserProperties will allow to have the current state of a user, as the new values will overwrite the previous one (posthog
$set
-will replace any value that may have been set on a person for a specific property-)We also keep an event
CryptoSessionState
that records the historical values (could allow to know the average time to get from unverified to verified, or to check when the state is reverted).For the UserProperty to work as expected, different devices should not report to the same posthog
User
. So it depends how platforms are identifyign users