Skip to content
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

fix(web): Hubspot onboarding invite member flow #5674

Merged
merged 7 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions apps/web/src/pages/auth/InvitationPage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useParams, Link } from 'react-router-dom';
import { useParams, Link, useNavigate } from 'react-router-dom';
import { useQuery, useQueryClient } from '@tanstack/react-query';
import { useEffect } from 'react';
import { Center, LoadingOverlay } from '@mantine/core';
Expand All @@ -8,7 +8,7 @@ import { getInviteTokenData } from '../../api/invitation';
import AuthLayout from '../../components/layout/components/AuthLayout';
import { SignUpForm } from './components/SignUpForm';
import { colors, Text, Button } from '@novu/design-system';
import { useAuth } from '@novu/shared-web';
import { ROUTES, useAuth } from '@novu/shared-web';
import { useAcceptInvite } from './components/useAcceptInvite';
import { LoginForm } from './components/LoginForm';

Expand All @@ -17,6 +17,7 @@ export default function InvitationPage() {
const { currentUser, logout } = useAuth();
const { token: invitationToken } = useParams<{ token: string }>();
const { isLoading: isAcceptingInvite, acceptInvite } = useAcceptInvite();
const navigate = useNavigate();
const { data, isLoading: isInviteTokenDataLoading } = useQuery<IGetInviteResponseDto, IGetInviteResponseDto>(
['getInviteTokenData'],
() => getInviteTokenData(invitationToken || ''),
Expand All @@ -34,8 +35,10 @@ export default function InvitationPage() {
useEffect(() => {
if (invitationToken && isLoggedInAsInvitedUser) {
acceptInvite(invitationToken);
navigate(ROUTES.WORKFLOWS);
}
}, [isLoggedInAsInvitedUser, acceptInvite, invitationToken]);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [acceptInvite, invitationToken, isLoggedInAsInvitedUser]);

useEffect(() => {
return () => {
Expand Down
12 changes: 5 additions & 7 deletions apps/web/src/pages/auth/components/HubspotSignupForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ export function HubspotSignupForm() {

return;
}
navigate(ROUTES.HOME);
}
}
}, [navigate, isFromVercel, startVercelSetup, currentUser, environmentId]);
}, [isFromVercel, startVercelSetup, currentUser, environmentId]);

async function createOrganization(data: IOrganizationCreateForm) {
const { organizationName, jobTitle, ...rest } = data;
Expand All @@ -53,7 +52,7 @@ export function HubspotSignupForm() {
// TODO: Move this into useAuth
const organizationResponseToken = await api.post(`/v1/auth/organizations/${organization._id}/switch`, {});

login(organizationResponseToken);
login(organizationResponseToken, ROUTES.GET_STARTED);
}

const handleCreateOrganization = async (data: IOrganizationCreateForm) => {
Expand All @@ -73,7 +72,6 @@ export function HubspotSignupForm() {

return;
}

navigate(ROUTES.GET_STARTED);
};

Expand All @@ -88,15 +86,15 @@ export function HubspotSignupForm() {
lastname: currentUser?.lastName as string,
email: currentUser?.email as string,

company: '',
company: (currentOrganization?.name as string) || '',
role___onboarding: '',
heard_about_novu: '',
use_case___onboarding: '',
role___onboarding__other_: '',
heard_about_novu__other_: '',
}}
readonlyProperties={['email']}
focussedProperty="company"
readonlyProperties={currentOrganization ? ['email', 'company'] : ['email']}
focussedProperty={currentOrganization ? 'role___onboarding' : 'company'}
onFormSubmitted={($form, values) => {
const submissionValues = values?.submissionValues as unknown as {
company: string;
Expand Down
74 changes: 48 additions & 26 deletions apps/web/src/pages/auth/components/LoginForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ export interface LocationState {

export function LoginForm({ email, invitationToken }: LoginFormProps) {
const segment = useSegment();
const { login, currentUser, organizationId, environmentId } = useAuth();
const { login, currentUser, organizations } = useAuth();
const { startVercelSetup } = useVercelIntegration();
const { isFromVercel, params: vercelParams } = useVercelParams();
const [params] = useSearchParams();
const tokenInQuery = params.get('token');
const source = params.get('source');
const sourceWidget = params.get('source_widget');
const invitationTokenFromGithub = params.get('invitationToken') as string;
const isRedirectedFromLoginPage = params.get('isLoginPage') as string;

const { isLoading: isLoadingAcceptInvite, acceptInvite } = useAcceptInvite();
const navigate = useNavigate();
const location = useLocation();
Expand All @@ -48,40 +51,59 @@ export function LoginForm({ email, invitationToken }: LoginFormProps) {
}
>((data) => api.post('/v1/auth/login', data));

useEffect(() => {
(async () => {
if (!tokenInQuery) {
return;
}
const handleLoginInUseEffect = async () => {
// if currentUser is true, it means user exists, then while accepting invitation, InvitationPage will handle accept this case
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indicates a routing issue as the invitation handling should be done in one place, not between pages. It's OK for now but we will have to get back to this.

if (currentUser) {
return;
}

if (!invitationToken && (!organizationId || !environmentId)) {
await login(tokenInQuery, ROUTES.AUTH_APPLICATION);
// if token from github is not present
jainpawan21 marked this conversation as resolved.
Show resolved Hide resolved
if (!tokenInQuery) {
return;
}

return;
}
// handle github login after invitation
if (invitationTokenFromGithub) {
await login(tokenInQuery);
const updatedToken = await acceptInvite(invitationTokenFromGithub);

if (isFromVercel) {
await login(tokenInQuery);
startVercelSetup();
if (updatedToken) {
await login(updatedToken, isRedirectedFromLoginPage === 'true' ? ROUTES.WORKFLOWS : ROUTES.AUTH_APPLICATION);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch. This has to be done this way as the accept invitation endpoint is authenticated but it shouldn't.

Removing the UserGuard is a breaking change, but I'd say the current need for an authentication token is wrong, so I feel confident making the change in a follow-up PR.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, the user should exist before accepting invitation. So current UserGuard is correct


return;
}
}

if (source === 'cli') {
segment.track('Dashboard Visit', {
widget: sourceWidget || 'unknown',
source: 'cli',
});
await login(tokenInQuery, ROUTES.GET_STARTED);

return;
}
if (organizations) {
navigate(ROUTES.WORKFLOWS);
} else {
await login(tokenInQuery, ROUTES.AUTH_APPLICATION);
}

if (isFromVercel) {
await login(tokenInQuery);
navigate(ROUTES.WORKFLOWS);
})();
startVercelSetup();

return;
}

if (source === 'cli') {
segment.track('Dashboard Visit', {
widget: sourceWidget || 'unknown',
source: 'cli',
});
await login(tokenInQuery, ROUTES.GET_STARTED);

return;
}

await login(tokenInQuery, ROUTES.WORKFLOWS);
};

useEffect(() => {
handleLoginInUseEffect();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [login, navigate, currentUser, tokenInQuery, segment, organizationId, environmentId]);
}, [login]);

const signupLink = isFromVercel ? `${ROUTES.AUTH_SIGNUP}?${params.toString()}` : ROUTES.AUTH_SIGNUP;
const resetPasswordLink = isFromVercel
Expand Down Expand Up @@ -135,7 +157,7 @@ export function LoginForm({ email, invitationToken }: LoginFormProps) {

return (
<>
<OAuth />
<OAuth invitationToken={invitationToken} isLoginPage={true} />
<form noValidate onSubmit={handleSubmit(onLogin)}>
<Input
error={emailClientError || emailServerError}
Expand Down
10 changes: 8 additions & 2 deletions apps/web/src/pages/auth/components/OAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ import { buildGithubLink, buildGoogleLink, buildVercelGithubLink } from './gitHu
import { useVercelParams } from '../../../hooks';
import { PropsWithChildren } from 'react';

export function OAuth({ invitationToken }: { invitationToken?: string | undefined }) {
export function OAuth({
invitationToken,
isLoginPage = false,
}: {
invitationToken?: string | undefined;
isLoginPage?: boolean;
}) {
const { isFromVercel, code, next, configurationId } = useVercelParams();

const githubLink = isFromVercel
? buildVercelGithubLink({ code, next, configurationId })
: buildGithubLink({ invitationToken });
: buildGithubLink({ invitationToken, isLoginPage });
const googleLink = buildGoogleLink({ invitationToken });

return (
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/pages/auth/components/SignUpForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export function SignUpForm({ invitationToken, email }: SignUpFormProps) {
const updatedToken = await acceptInvite(invitationToken);
if (updatedToken) {
await login(updatedToken);
navigate(ROUTES.WORKFLOWS);
}
navigate(ROUTES.AUTH_APPLICATION);
} else {
navigate(isFromVercel ? `${ROUTES.AUTH_APPLICATION}?${params.toString()}` : ROUTES.AUTH_APPLICATION);
}
Expand Down Expand Up @@ -104,7 +104,7 @@ export function SignUpForm({ invitationToken, email }: SignUpFormProps) {

return (
<>
<OAuth />
<OAuth invitationToken={invitationToken} />
<form noValidate name="login-form" onSubmit={handleSubmit(onSubmit)}>
<Input
error={errors.fullName?.message}
Expand Down
11 changes: 10 additions & 1 deletion apps/web/src/pages/auth/components/gitHubUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@ import { SignUpOriginEnum } from '@novu/shared';

import { API_ROOT } from '../../../config';

export const buildGithubLink = ({ invitationToken }: { invitationToken?: string }) => {
export const buildGithubLink = ({
invitationToken,
isLoginPage,
}: {
invitationToken?: string;
isLoginPage?: boolean;
}) => {
const queryParams = new URLSearchParams();
queryParams.append('source', SignUpOriginEnum.WEB);
if (invitationToken) {
queryParams.append('invitationToken', invitationToken);
}
if (isLoginPage) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto about redirect_url

queryParams.append('isLoginPage', 'true');
}

return `${API_ROOT}/v1/auth/github?${queryParams.toString()}`;
};
Expand Down
4 changes: 4 additions & 0 deletions libs/application-generic/src/services/auth/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ export const buildOauthRedirectUrl = (request): string => {
if (invitationToken) {
url += `&invitationToken=${invitationToken}`;
}
const isLoginPage = JSON.parse(request.query.state).isLoginPage;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of marking whether OAuth comes from loginPage, we can add a redirect_url parameter set from the front end, which would simplify the FE logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SokratisVidros which means instead of hardcoed /auth/login at this line, redirect_url query param value should be used?

if (isLoginPage) {
url += `&isLoginPage=${isLoginPage}`;
}

return url;
};
Loading