Skip to content

Commit a162311

Browse files
authored
FE-1097: Fix post-signup loop possibility (#8918)
1 parent 1cb7ffd commit a162311

5 files changed

Lines changed: 112 additions & 14 deletions

File tree

apps/hash-frontend/src/pages/_app.page.tsx

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,17 +127,28 @@ const App: FunctionComponent<AppProps> = ({
127127
const awaitingEmailVerificationStatus =
128128
!!authenticatedUser && !emailVerificationStatusKnown && !aal2Required;
129129

130+
/**
131+
* A `redirectTo` that points at the page we're already on is a no-op we must
132+
* ignore. `getInitialProps` re-runs on every navigation — including the
133+
* same-URL `router.replace`s this effect performs — and `useRouter()` returns
134+
* a fresh object each time, so honouring such a redirect spins forever in a
135+
* `replace` -> `getInitialProps` -> `replace` loop (e.g. landing on `/` after
136+
* accepting an org invite, where a stale `redirectTo: "/"` kept re-firing).
137+
*/
138+
const pendingRedirect =
139+
!!redirectTo && redirectTo !== router.asPath ? redirectTo : undefined;
140+
130141
/**
131142
* Handle client-side redirects that were determined in getInitialProps.
132143
* On the server these are HTTP 307s; on the client getInitialProps returns
133144
* a `redirectTo` prop instead, and this effect performs the navigation after
134145
* the current route transition completes (avoiding NProgress stalls).
135146
*/
136147
useEffect(() => {
137-
if (redirectTo) {
138-
void router.replace(redirectTo);
148+
if (pendingRedirect) {
149+
void router.replace(pendingRedirect);
139150
}
140-
}, [redirectTo, router]);
151+
}, [pendingRedirect, router]);
141152

142153
useEffect(() => {
143154
setSentryUser({ authenticatedUser });
@@ -167,8 +178,15 @@ const App: FunctionComponent<AppProps> = ({
167178
// router.query is empty during server-side rendering for pages that don’t use
168179
// getServerSideProps. By showing app skeleton on the server, we avoid UI
169180
// mismatches during rehydration and improve type-safety of param extraction.
170-
// We also gate on `redirectTo` so the page doesn't flash before navigating.
171-
if (ssr || !router.isReady || awaitingEmailVerificationStatus || redirectTo) {
181+
// We also gate on a pending redirect so the page doesn't flash before
182+
// navigating. A `redirectTo` matching the current path isn't pending (see
183+
// `pendingRedirect`), so we render rather than stall on the loading state.
184+
if (
185+
ssr ||
186+
!router.isReady ||
187+
awaitingEmailVerificationStatus ||
188+
pendingRedirect
189+
) {
172190
return <Suspense />; // Replace with app skeleton
173191
}
174192

apps/hash-frontend/src/pages/index.page.tsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { useQuery } from "@apollo/client";
22
import { Stack } from "@mui/material";
33
import { useRouter } from "next/router";
4-
import { useMemo } from "react";
4+
import { useEffect, useMemo } from "react";
55

66
import { hasAccessToHashQuery } from "../graphql/queries/user.queries";
77
import { getLayoutWithSidebar } from "../shared/layout";
@@ -30,9 +30,20 @@ const Page: NextPageWithLayout = () => {
3030
}
3131
}, [authenticatedUser, hasAccessToHashResponse]);
3232

33+
const shouldCompleteSignup =
34+
!authenticatedUser?.accountSignupComplete && !!hasAccessToHash;
35+
36+
/**
37+
* Send users who have access but haven't finished signup to `/signup`.
38+
*/
39+
useEffect(() => {
40+
if (shouldCompleteSignup) {
41+
void push("/signup");
42+
}
43+
}, [shouldCompleteSignup, push]);
44+
3345
if (!authenticatedUser?.accountSignupComplete) {
3446
if (hasAccessToHash) {
35-
void push("/signup");
3647
return null;
3748
}
3849

apps/hash-frontend/src/pages/shared/auth-info-context.tsx

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import {
1313
getOutgoingLinksForEntity,
1414
getRoots,
15+
intervalCompareWithInterval,
1516
intervalForTimestamp,
1617
} from "@blockprotocol/graph/stdlib";
1718
import {
@@ -53,6 +54,29 @@ export const AuthInfoContext = createContext<AuthInfoContextValue | undefined>(
5354
undefined,
5455
);
5556

57+
/**
58+
* Returns `true` if `candidate` contains a newer edition of the user (by
59+
* transaction time) than `existing`.
60+
*/
61+
const subgraphHasNewerUser = (
62+
candidate: Subgraph<EntityRootType<HashEntity>>,
63+
existing: Subgraph<EntityRootType<HashEntity>>,
64+
): boolean => {
65+
const candidateUser = getRoots(candidate)[0];
66+
const existingUser = getRoots(existing)[0];
67+
68+
if (!candidateUser || !existingUser) {
69+
return true;
70+
}
71+
72+
return (
73+
intervalCompareWithInterval(
74+
existingUser.metadata.temporalVersioning.transactionTime,
75+
candidateUser.metadata.temporalVersioning.transactionTime,
76+
) < 0
77+
);
78+
};
79+
5680
type AuthInfoProviderProps = {
5781
initialAuthenticatedUserSubgraph?: Subgraph<EntityRootType<HashEntity>>;
5882
children: ReactElement;
@@ -72,6 +96,24 @@ export const AuthInfoProvider: FunctionComponent<AuthInfoProviderProps> = ({
7296
const [emailVerificationStatusKnown, setEmailVerificationStatusKnown] =
7397
useState(false);
7498

99+
/**
100+
* `getInitialProps` re-fetches the authenticated user on every navigation and
101+
* passes it as `initialAuthenticatedUserSubgraph`. Adopt that server-provided
102+
* data whenever it is newer than what the client currently holds.
103+
*/
104+
useEffect(() => {
105+
if (!initialAuthenticatedUserSubgraph) {
106+
return;
107+
}
108+
109+
setAuthenticatedUserSubgraph((current) =>
110+
!current ||
111+
subgraphHasNewerUser(initialAuthenticatedUserSubgraph, current)
112+
? initialAuthenticatedUserSubgraph
113+
: current,
114+
);
115+
}, [initialAuthenticatedUserSubgraph]);
116+
75117
const userMemberOfLinks = useMemo(() => {
76118
if (!authenticatedUserSubgraph) {
77119
return undefined;

apps/hash-frontend/src/pages/shared/workspace-context.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,17 @@ export const WorkspaceContextProvider: FunctionComponent<{
7979
}
8080
}, [activeWorkspaceWebId, updateActiveWorkspaceWebId, authenticatedUser]);
8181

82-
const workspaceContextValue = useMemo<WorkspaceContextValue>(() => {
83-
const activeWorkspace =
82+
const activeWorkspace = useMemo(
83+
() =>
8484
authenticatedUser && authenticatedUser.accountId === activeWorkspaceWebId
8585
? authenticatedUser
8686
: authenticatedUser?.memberOf.find(
8787
({ org: { webId } }) => webId === activeWorkspaceWebId,
88-
)?.org;
88+
)?.org,
89+
[authenticatedUser, activeWorkspaceWebId],
90+
);
8991

92+
useEffect(() => {
9093
/**
9194
* If there is an `activeWorkspaceWebId` and an `authenticatedUser`, but
9295
* `activeWorkspace` is not defined, reset `activeWorkspaceWebId` to the
@@ -98,15 +101,22 @@ export const WorkspaceContextProvider: FunctionComponent<{
98101
(authenticatedUser.accountId as WebId),
99102
);
100103
}
104+
}, [
105+
activeWorkspace,
106+
activeWorkspaceWebId,
107+
authenticatedUser,
108+
updateActiveWorkspaceWebId,
109+
]);
101110

111+
const workspaceContextValue = useMemo<WorkspaceContextValue>(() => {
102112
return {
103113
activeWorkspace,
104114
activeWorkspaceWebId,
105115
updateActiveWorkspaceWebId,
106116
refetchActiveWorkspace: () => refetch().then(() => undefined),
107117
};
108118
}, [
109-
authenticatedUser,
119+
activeWorkspace,
110120
activeWorkspaceWebId,
111121
updateActiveWorkspaceWebId,
112122
refetch,

apps/hash-frontend/src/pages/signup.page.tsx

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,16 @@ const SignupPage: NextPageWithLayout = () => {
242242
refetchInvites();
243243
await refetchAuthenticatedUser();
244244
updateActiveWorkspaceWebId(invitation.org.webId);
245-
void router.replace("/");
245+
/**
246+
* Hard navigation rather than `router.replace`: a client-side
247+
* transition here can leave this component mounted on `/` (Next
248+
* doesn't reliably re-resolve the route component on these
249+
* `getInitialProps`-driven transitions). A full load guarantees a
250+
* clean home render with fresh auth state.
251+
*
252+
* @todo sort out signup redirecting logic generally
253+
*/
254+
window.location.assign("/");
246255
return;
247256
}
248257

@@ -368,7 +377,16 @@ const SignupPage: NextPageWithLayout = () => {
368377
updateActiveWorkspaceWebId(invitation.org.webId);
369378
}
370379

371-
void router.push("/");
380+
/**
381+
* Hard navigation rather than `router.push`: a client-side transition
382+
* here can leave this component mounted on `/` (Next doesn't reliably
383+
* re-resolve the route component on these `getInitialProps`-driven
384+
* transitions), stranding the user on a loading state. A full load
385+
* guarantees a clean home render with fresh auth state.
386+
*
387+
* @todo sort out signup redirecting logic generally
388+
*/
389+
window.location.assign("/");
372390
},
373391
[
374392
acceptInvitationOnce,
@@ -378,7 +396,6 @@ const SignupPage: NextPageWithLayout = () => {
378396
refetchAuthenticatedUser,
379397
updateAuthenticatedUser,
380398
updateActiveWorkspaceWebId,
381-
router,
382399
],
383400
);
384401

0 commit comments

Comments
 (0)