-
Notifications
You must be signed in to change notification settings - Fork 941
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
chore: handle people and attributes separately to improve sync performance #2476
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pandeymangg Looks great! :-)
A few suggestion I found while going through the code. Since we are now moving towards Formbricks 2.0 I think it's good if we clean up some things if we notice them :-)
.../app/(app)/environments/[environmentId]/(peopleAndSegments)/people/components/PersonCard.tsx
Outdated
Show resolved
Hide resolved
.../app/(app)/environments/[environmentId]/(peopleAndSegments)/people/components/PersonCard.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/api/v1/client/[environmentId]/app/sync/[userId]/route.ts
Outdated
Show resolved
Hide resolved
…rmbricks into feature/split-person-model
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.
LGTM!! 🚀
Draft: not fully implemented, @pandeymangg will take over
What does this PR do?
handle people and attributes separately to improve sync performance
Since we are not always needing the whole person model with all attributes (e.g. for checks if a person exists) it doesn't make sense to query the whole model where we also might not have a cache hit if the attributes were updated.