feat: detect and upgrade users with missing metadata #18
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Playwright e2e testsTo view traces locally, unzip the report and run: npx playwright show-report ~/Downloads/playwright-report |
| const setDemoStatus = trpc.auth.setDemoStatus.useMutation(); | ||
|
|
||
| const userHasAlreadyAcceptedTerms = | ||
| user?.publicMetadata?.["labs"]?.["isOnboarded"]; |
There was a problem hiding this comment.
is this a TS complaint that the keys might not exist?
There was a problem hiding this comment.
Yeah. It's a slightly hokey way to wait for user to have loaded and then dig into those fields if they don't exist. publicMetadata is unknown here, so we have to access them as fields rather than properties
| const userHasAlreadyAcceptedTerms = | ||
| user?.publicMetadata?.["labs"]?.["isOnboarded"]; | ||
|
|
||
| // Edge case: Legacy users have already accepted terms but don't have a demo status |
There was a problem hiding this comment.
Sorry being a bit slow here -- where's the condition that they don't have a demo status?
There was a problem hiding this comment.
Ah just seen -- user won't land on this page otherwise
There was a problem hiding this comment.
The case is any user who signed up before started capturing the demo status. Unless we can find a sensible representation of their region (maybe posthog?), our only option is to wait until they come back online to set it
We're soon going to have users landing on labs from OWA who don't have all of our metadata, so this concept of upgrading metadata will likely come in useful anyway
| if (!demoUsers.isDemoStatusSet(user)) { | ||
| return false; | ||
| } | ||
| const isDemoUser = Boolean(user.publicMetadata.labs.isDemoUser ?? "true"); |
There was a problem hiding this comment.
does this need to be publicMetadata.labs?.isDemoUser or is this safe at this point?
There was a problem hiding this comment.
The type of isDemoStatusSet is
isDemoStatusSet(user: LabsUser): user is UserWithDemoStatusSo that's already asserted here!
There was a problem hiding this comment.
I guess my concern is that we might be trusting what is coming through here, and it might be correct in the types, but I'm not sure we validate the actual data from Clerk?
There was a problem hiding this comment.
Do you mean that isDemoStatusSet could be accidentally sanitising incorrect data with a correct type?
Here's the definition:
isDemoStatusSet(user: LabsUser): user is UserWithDemoStatus {
const labsMetadata = user.publicMetadata.labs || {};
return "isDemoUser" in labsMetadata && labsMetadata.isDemoUser !== null;
}| <OakSpan> | ||
| Keep me updated with latest Oak AI experiments, resources and | ||
| other helpful content by email. You can unsubscribe at any time. | ||
| See our{" "} |
There was a problem hiding this comment.
The language here shifts from "me" - the user to "our" - Oak. Should this be "the"?
There was a problem hiding this comment.
This isn't new copy, but you are right that the grammar isn't quite right
The previous sentence has already switched to "You", so I don't think it's as bad as it looks at first glance
That said, "Keep me updated with latest Oak AI experiments" seems to be missing a "the"?
|
|
🎉 This PR is included in version 1.3.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
Existing users were onboarded without us recording their region, which we need to determine their demo status. This PR takes these legacy users through a quick onboarding just to record the region and demo status
It's also a good opportunity to namespace the public metadata to the
labsprefix. Existing users will already be migrated to this structure with a scriptpublicMetadata.labsisOnboardedto public metadataisOnboardedandisDemoUserto Clerk session claims to avoid server round tripisOnboardedsession claim and redirect to /onboardingx-dev-preloadeven if they're protectedisOnboardedHow to test
{ "labs": { "isOnboarded": true } }Screenshots
Current onboarding form. This shouls stay the same for new users
For legacy users without demo status
Checklist