fix: data migration and cleanup for userId attribute #2400
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Hey @pandeymangg , tested the migration script in different scenarios, works well 🚀✅, other changes looks good too 🙌🏻
Can we just remove truncateMiddle function, since it is not being used any where now !
There was a problem hiding this comment.
@pandeymangg thanks; looks good :-)
Only two smaller things. If they are fixed let's do the data migration on cloud together :-)
| name: "userId", | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
we need to add a timeout, at least:
{
timeout: 60000,
}
maybe even more, else it will fail on large databases with many people (like formbricks cloud).
packages/js-core/src/index.ts
Outdated
| @@ -41,6 +41,11 @@ const setEmail = async (email: string): Promise<void> => { | |||
| }; | |||
|
|
|||
| const setAttribute = async (key: string, value: any): Promise<void> => { | |||
| if (key === "userId") { | |||
There was a problem hiding this comment.
why did we decide to do this?
I think we can allow since we cleaned up everything now :-) It wouldn't have the affect the user might want to have and he will get a new attribute class named userId, but if they want this we maybe shouldn't stop them from it in the future.
What does this PR do?
Removes the old userId attribute code, and has a data migration which transforms the userId attributes to the userId column in the Person table
Fixes # (issue)
How should this be tested?
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated