fix: hubspot signup form issue when user is invited #5317
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
hey @jainpawan21 👋
Please make sure to verify these cases:
- Invited the existing user (registered by email) that is currently logged, the form should not be shown.
- Invited the existing user (registered by email) that is not logged in, the form should not be shown.
- Invited the existing user (registered by GH) that is currently logged, the form should not be shown.
- Invited the existing user (registered by GH) that is not logged in, the form should not be shown.
- Invited the new user register by email, the form should be shown.
- Invited the new user register by GH, the form should be shown.
The case number four is failing, attaching the video.
Screen.Recording.2024-03-21.at.11.12.17.mov
I would appreciate if you can cover the new logic with Cypress E2E tests, as far as I see we are lucking tests for the invitation logic, but it would be much appreciated if we can add them. Excluding GH flow as it's complex and requires account confirmation in E2E tests.
| @@ -45,7 +45,7 @@ export default function InvitationPage() { | |||
| useEffect(() => { | |||
| // auto accept invitation when logged in as invited user | |||
| if (isLoggedInAsInvitedUser) { | |||
| submitToken(tokensRef.current.token as string, tokensRef.current.invitationToken as string, true); | |||
| submitToken(tokensRef.current.token as string, tokensRef.current.invitationToken as string, true, false); | |||
There was a problem hiding this comment.
it's hard to understand what are there true/false arguments, please change it to the object, which will improve readability
| } | ||
| } | ||
| }, [token, navigate, isFromVercel, startVercelSetup]); | ||
|
|
||
| return () => {}; |
|
|
||
| const { isLoading, mutateAsync, error, isError } = useMutation<string, IResponseError, string>((tokenItem) => | ||
| api.post(`/v1/invites/${tokenItem}/accept`, {}) | ||
| ); | ||
|
|
||
| const submitToken = useCallback( | ||
| async (token: string, invitationToken: string, refetch = false) => { | ||
| async (token: string, invitationToken: string, refetch = false, isSignUp = true) => { |
There was a problem hiding this comment.
by default isSignUp should be false, we only have it set to true in one place - sign up form
| @@ -77,9 +77,7 @@ export function SignUpForm({ invitationToken, email }: SignUpFormProps) { | |||
| applyToken(token); | |||
|
|
|||
| if (invitationToken) { | |||
| submitToken(token, invitationToken); | |||
|
|
|||
| return true; | |||
There was a problem hiding this comment.
why did we remove this return statement?
|
@jainpawan21 can we push to merge or close this PR? 🙏 |
What change does this PR introduce?
Why was this change needed?
Other information (Screenshots)