Fix onboarding status refacto #6533
Conversation
martmull
force-pushed
the
fix-onboardingStatus-refacto
branch
from
c907134
to
f61f9dc
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The pull request focuses on refining the onboarding process by renaming methods and adding conditional checks to ensure consistency and clarity.
- Renamed Method: Updated method name from
setOnboardingCreateProfileCompletiontosetOnboardingCreateProfilePendingacross multiple files for consistency. - Conditional Check: Added a check in
packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.tsto set the 'create profile pending' variable only if bothfirstNameandlastNameare empty. - Onboarding Initialization: Modified
packages/twenty-server/src/database/commands/upgrade-version/0-23/0-23-backfill-new-onboarding-user-vars.tsto set onboarding variables for users in workspaces with aPENDING_CREATIONstatus. - Listener Update: Adjusted
packages/twenty-server/src/engine/core-modules/workspace/workspace-workspace-member.listener.tsto reflect the renamed method, ensuring consistency in event handling.
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
| workspaceId: workspace.id, | ||
| value: true, | ||
| }); | ||
| if (firstName === '' && lastName === '') { |
There was a problem hiding this comment.
I don't think this is actually right anymore, we should always show the create profile popin, it is prefill with currentInformation
There was a problem hiding this comment.
This is how onboarding worked before the optimisation. When login with google sso, first name and last name are already filled at user creation, so there is no particular reason to display the create profile modale in that case
...-server/src/database/commands/upgrade-version/0-23/0-23-backfill-new-onboarding-user-vars.ts
Show resolved
Hide resolved
| workspaceId: workspace.id, | ||
| value: true, | ||
| }); | ||
| if (firstName === '' && lastName === '') { |
|
@martmull @charlesBochet Merging to avoid increasing the stock of PR while many are away. I agree "firstname === ''" doesn't seem very clean but it makes sense to skip profile creation when going through Google. OK for me to merge like as I don't have something better to offer |
see comments in #6531 (review)