From 97ca786d516a9707b57a2236c3c3dc8e5809b3ef Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Tue, 10 Dec 2024 15:29:44 -0500 Subject: [PATCH 1/6] client login implementation wip from acouch/issue-2654-login-callback --- .../dev/feature-flags/FeatureFlagsTable.tsx | 5 ++ .../app/[locale]/dev/feature-flags/page.tsx | 3 +- frontend/src/app/[locale]/dev/layout.tsx | 14 +++++ frontend/src/app/[locale]/search/page.tsx | 2 +- .../src/hoc/{search => }/withFeatureFlag.tsx | 0 frontend/src/services/auth/UserProvider.tsx | 51 ++++++++++++++++++ frontend/src/services/auth/useUser.tsx | 54 +++++++++++++++++++ frontend/src/services/auth/utils.tsx | 21 ++++++++ 8 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 frontend/src/app/[locale]/dev/layout.tsx rename frontend/src/hoc/{search => }/withFeatureFlag.tsx (100%) create mode 100644 frontend/src/services/auth/UserProvider.tsx create mode 100644 frontend/src/services/auth/useUser.tsx create mode 100644 frontend/src/services/auth/utils.tsx diff --git a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx index 934b6b952..b2477acb3 100644 --- a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx +++ b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx @@ -1,6 +1,7 @@ "use client"; import { useFeatureFlags } from "src/hooks/useFeatureFlags"; +import { useUser } from "src/services/auth/useUser"; import React from "react"; import { Button, Table } from "@trussworks/react-uswds"; @@ -10,12 +11,15 @@ import { Button, Table } from "@trussworks/react-uswds"; */ export default function FeatureFlagsTable() { const { featureFlagsManager, mounted, setFeatureFlag } = useFeatureFlags(); + const { user, isLoading, error } = useUser(); if (!mounted) { return null; } return ( + <> +

User with token {user?.token} can see this

@@ -58,5 +62,6 @@ export default function FeatureFlagsTable() { )}
+ ); } diff --git a/frontend/src/app/[locale]/dev/feature-flags/page.tsx b/frontend/src/app/[locale]/dev/feature-flags/page.tsx index 88986ad97..611e54a93 100644 --- a/frontend/src/app/[locale]/dev/feature-flags/page.tsx +++ b/frontend/src/app/[locale]/dev/feature-flags/page.tsx @@ -2,7 +2,6 @@ import { Metadata } from "next"; import Head from "next/head"; import React from "react"; - import FeatureFlagsTable from "./FeatureFlagsTable"; export function generateMetadata() { @@ -17,12 +16,14 @@ export function generateMetadata() { * View for managing feature flags */ export default function FeatureFlags() { + return ( <> Manage Feature Flags
+

Manage Feature Flags

diff --git a/frontend/src/app/[locale]/dev/layout.tsx b/frontend/src/app/[locale]/dev/layout.tsx new file mode 100644 index 000000000..38df29c32 --- /dev/null +++ b/frontend/src/app/[locale]/dev/layout.tsx @@ -0,0 +1,14 @@ +import React from 'react'; +import UserProvider from "src/services/auth/UserProvider"; + +export default async function Layout({ + children, locale, + }: { + children: React.ReactNode; + locale: string; + }) { + console.log(UserProvider) + return ( + {children} + ); +} diff --git a/frontend/src/app/[locale]/search/page.tsx b/frontend/src/app/[locale]/search/page.tsx index d006a905b..05314e83a 100644 --- a/frontend/src/app/[locale]/search/page.tsx +++ b/frontend/src/app/[locale]/search/page.tsx @@ -1,6 +1,6 @@ import { Metadata } from "next"; import QueryProvider from "src/app/[locale]/search/QueryProvider"; -import withFeatureFlag from "src/hoc/search/withFeatureFlag"; +import withFeatureFlag from "src/hoc/withFeatureFlag"; import { LocalizedPageProps } from "src/types/intl"; import { SearchParamsTypes } from "src/types/search/searchRequestTypes"; import { Breakpoints } from "src/types/uiTypes"; diff --git a/frontend/src/hoc/search/withFeatureFlag.tsx b/frontend/src/hoc/withFeatureFlag.tsx similarity index 100% rename from frontend/src/hoc/search/withFeatureFlag.tsx rename to frontend/src/hoc/withFeatureFlag.tsx diff --git a/frontend/src/services/auth/UserProvider.tsx b/frontend/src/services/auth/UserProvider.tsx new file mode 100644 index 000000000..c4b6b21ed --- /dev/null +++ b/frontend/src/services/auth/UserProvider.tsx @@ -0,0 +1,51 @@ +'use client'; +import React, { ReactElement, useState, useEffect, useCallback, useContext, createContext, useMemo } from 'react'; +import { UserContext } from './useUser'; +import { RequestError } from "./utils"; +import { Session, UserFetcher, UserContextType, UserProviderProps, UserProviderState } from "./types"; + +/** + * @ignore + */ +const userFetcher: UserFetcher = async (url) => { + let response; + try { + response = await fetch(url); + console.log(response); + } catch { + throw new RequestError(0); // Network error + } + if (response.status == 204) return undefined; + if (response.ok) return response.json(); + throw new RequestError(response.status); +}; + +export default ({ + children, + user: Session, +}: UserProviderProps): ReactElement => { + const [state, setState] = useState({ user: Session, isLoading: !Session }); + const checkSession = useCallback(async (): Promise => { + try { + const user = await userFetcher('/api/auth/session') as Session; + setState((previous) => ({ ...previous, user, isLoading: false, error: undefined })); + } catch (error) { + setState((previous) => ({ ...previous, error: error as Error })); + } + },[children]); + + useEffect((): void => { + if (state.user) return; + (async (): Promise => { + await checkSession(); + setState((previous) => ({ ...previous, isLoading: false })); + })(); + }, [state.user]); + + const { user, error, isLoading } = state; + const value = useMemo(() => ({ user, error, isLoading, checkSession }), [user, error, isLoading, checkSession]); + console.log(value); + return ( + {children} + ); +}; diff --git a/frontend/src/services/auth/useUser.tsx b/frontend/src/services/auth/useUser.tsx new file mode 100644 index 000000000..365b09839 --- /dev/null +++ b/frontend/src/services/auth/useUser.tsx @@ -0,0 +1,54 @@ +'use client'; +import React, { ReactElement, useState, useEffect, useCallback, useContext, createContext, useMemo } from 'react'; + +import { UserContextType } from './types'; + +/** + * @ignore + */ +const missingUserProvider = 'You forgot to wrap your app in '; + +/** + * @ignore + */ +export const UserContext = createContext({ + get user(): never { + throw new Error(missingUserProvider); + }, + get error(): never { + throw new Error(missingUserProvider); + }, + get isLoading(): never { + throw new Error(missingUserProvider); + }, + checkSession: (): never => { + throw new Error(missingUserProvider); + } +}); + +/** + * @ignore + */ +export type UseUser = () => UserContextType; + +/** + * The `useUser` hook, which will get you the {@link UserProfile} object from the server-side session by fetching it + * from the {@link HandleProfile} API route. + * + * ```js + * import Link from 'next/link'; + * import { useUser } from 'src/services/auth/useUser'; + * + * export default function Profile() { + * const { user, error, isLoading } = useUser(); + * + * if (isLoading) return
Loading...
; + * if (error) return
{error.message}
; + * if (!user) return Login; + * return
Hello {user.name}, Logout
; + * } + * ``` + * + * @category Client + */ +export const useUser: UseUser = () => useContext(UserContext); diff --git a/frontend/src/services/auth/utils.tsx b/frontend/src/services/auth/utils.tsx new file mode 100644 index 000000000..646f9e2e1 --- /dev/null +++ b/frontend/src/services/auth/utils.tsx @@ -0,0 +1,21 @@ +/** + * The error thrown by the default {@link UserFetcher}. + * + * The `status` property contains the status code of the response. It is `0` when the request + * fails, for example due to being offline. + * + * This error is not thrown when the status code of the response is `204`, because that means the + * user is not authenticated. + * + * @category Client + */ +export class RequestError extends Error { + public status: number; + + constructor(status: number) { + /* c8 ignore next */ + super(); + this.status = status; + Object.setPrototypeOf(this, RequestError.prototype); + } +} From 778afa44093f3a466192bd04fc2cfdb91236b9a0 Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Fri, 13 Dec 2024 15:59:28 -0500 Subject: [PATCH 2/6] basically working --- .../dev/feature-flags/FeatureFlagsTable.tsx | 88 ++++++++--------- .../app/[locale]/dev/feature-flags/page.tsx | 4 +- frontend/src/app/[locale]/dev/layout.tsx | 15 +-- frontend/src/app/api/auth/session/route.ts | 14 +++ frontend/src/services/auth/UserProvider.tsx | 94 +++++++++++-------- frontend/src/services/auth/useUser.tsx | 33 ++----- frontend/src/utils/authUtil.ts | 10 ++ 7 files changed, 139 insertions(+), 119 deletions(-) create mode 100644 frontend/src/app/api/auth/session/route.ts create mode 100644 frontend/src/utils/authUtil.ts diff --git a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx index b2477acb3..e2dd1efe3 100644 --- a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx +++ b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx @@ -13,55 +13,59 @@ export default function FeatureFlagsTable() { const { featureFlagsManager, mounted, setFeatureFlag } = useFeatureFlags(); const { user, isLoading, error } = useUser(); + console.log("~~~ user ~~~", user); + console.log("~~~ error ~~~", error); + console.log("~~~ isLoading ~~~", isLoading); + if (!mounted) { return null; } return ( <> -

User with token {user?.token} can see this

- - - - - - - - - - {Object.entries(featureFlagsManager.featureFlags).map( - ([featureName, enabled]) => ( - - - - + + + + ), + )} + +
StatusFeature FlagActions
- {enabled ? "Enabled" : "Disabled"} - {featureName} - - {featureName} + + +
); } diff --git a/frontend/src/app/[locale]/dev/feature-flags/page.tsx b/frontend/src/app/[locale]/dev/feature-flags/page.tsx index 611e54a93..18ed8e909 100644 --- a/frontend/src/app/[locale]/dev/feature-flags/page.tsx +++ b/frontend/src/app/[locale]/dev/feature-flags/page.tsx @@ -1,8 +1,8 @@ import { Metadata } from "next"; +import FeatureFlagsTable from "src/app/[locale]/dev/feature-flags/FeatureFlagsTable"; import Head from "next/head"; import React from "react"; -import FeatureFlagsTable from "./FeatureFlagsTable"; export function generateMetadata() { const meta: Metadata = { @@ -16,14 +16,12 @@ export function generateMetadata() { * View for managing feature flags */ export default function FeatureFlags() { - return ( <> Manage Feature Flags
-

Manage Feature Flags

diff --git a/frontend/src/app/[locale]/dev/layout.tsx b/frontend/src/app/[locale]/dev/layout.tsx index 38df29c32..05c95d2fc 100644 --- a/frontend/src/app/[locale]/dev/layout.tsx +++ b/frontend/src/app/[locale]/dev/layout.tsx @@ -1,14 +1,7 @@ -import React from 'react'; import UserProvider from "src/services/auth/UserProvider"; -export default async function Layout({ - children, locale, - }: { - children: React.ReactNode; - locale: string; - }) { - console.log(UserProvider) - return ( - {children} - ); +import React from "react"; + +export default function Layout({ children }: { children: React.ReactNode }) { + return {children}; } diff --git a/frontend/src/app/api/auth/session/route.ts b/frontend/src/app/api/auth/session/route.ts new file mode 100644 index 000000000..668eae8bb --- /dev/null +++ b/frontend/src/app/api/auth/session/route.ts @@ -0,0 +1,14 @@ +import { getSession } from "src/services/auth/session"; + +import { NextResponse } from "next/server"; + +export async function GET() { + const currentSession = await getSession(); + if (currentSession) { + return NextResponse.json({ + token: currentSession.token, + }); + } else { + return NextResponse.json({ token: "" }); + } +} diff --git a/frontend/src/services/auth/UserProvider.tsx b/frontend/src/services/auth/UserProvider.tsx index c4b6b21ed..768fdc224 100644 --- a/frontend/src/services/auth/UserProvider.tsx +++ b/frontend/src/services/auth/UserProvider.tsx @@ -1,51 +1,71 @@ -'use client'; -import React, { ReactElement, useState, useEffect, useCallback, useContext, createContext, useMemo } from 'react'; -import { UserContext } from './useUser'; -import { RequestError } from "./utils"; -import { Session, UserFetcher, UserContextType, UserProviderProps, UserProviderState } from "./types"; - -/** - * @ignore - */ +"use client"; + +import { noop } from "lodash"; +import { + UserContextType, + UserFetcher, + UserProviderProps, + UserSession, +} from "src/services/auth/types"; +import { UserContext } from "src/services/auth/useUser"; +import { RequestError } from "src/services/auth/utils"; +import { isSessionExpired } from "src/utils/authUtil"; + +import React, { + ReactElement, + useCallback, + useEffect, + useMemo, + useState, +} from "react"; + const userFetcher: UserFetcher = async (url) => { let response; try { response = await fetch(url); - console.log(response); - } catch { + } catch (e) { + console.error("User session fetch network error", e); + // not sure we need this error type throw new RequestError(0); // Network error } - if (response.status == 204) return undefined; - if (response.ok) return response.json(); + if (response.status === 204) return undefined; + if (response.ok) return (await response.json()) as UserSession; throw new RequestError(response.status); }; -export default ({ +export default function UserProvider({ children, - user: Session, -}: UserProviderProps): ReactElement => { - const [state, setState] = useState({ user: Session, isLoading: !Session }); - const checkSession = useCallback(async (): Promise => { +}: UserProviderProps): ReactElement { + const [localUser, setLocalUser] = useState(null); + const [isLoading, setIsLoading] = useState(!localUser); + const [userFetchError, setUserFetchError] = useState(); + + const getUserSession = useCallback(async (): Promise => { try { - const user = await userFetcher('/api/auth/session') as Session; - setState((previous) => ({ ...previous, user, isLoading: false, error: undefined })); + setIsLoading(true); + const fetchedUser = await userFetcher("/api/auth/session"); + if (fetchedUser) { + setLocalUser(fetchedUser); + setUserFetchError(undefined); + setIsLoading(false); + return; + } + throw new Error("received empty user session"); } catch (error) { - setState((previous) => ({ ...previous, error: error as Error })); + setIsLoading(false); + setUserFetchError(error as Error); } - },[children]); - - useEffect((): void => { - if (state.user) return; - (async (): Promise => { - await checkSession(); - setState((previous) => ({ ...previous, isLoading: false })); - })(); - }, [state.user]); - - const { user, error, isLoading } = state; - const value = useMemo(() => ({ user, error, isLoading, checkSession }), [user, error, isLoading, checkSession]); - console.log(value); - return ( - {children} + }, []); + + useEffect(() => { + if (localUser && !isSessionExpired(localUser)) return; + getUserSession().then(noop).catch(noop); + }, [localUser, getUserSession]); + + const value = useMemo( + () => ({ user: localUser, error: userFetchError, isLoading }), + [localUser, userFetchError, isLoading], ); -}; + + return {children}; +} diff --git a/frontend/src/services/auth/useUser.tsx b/frontend/src/services/auth/useUser.tsx index 365b09839..7c54bf29d 100644 --- a/frontend/src/services/auth/useUser.tsx +++ b/frontend/src/services/auth/useUser.tsx @@ -1,35 +1,15 @@ -'use client'; -import React, { ReactElement, useState, useEffect, useCallback, useContext, createContext, useMemo } from 'react'; +"use client"; -import { UserContextType } from './types'; +import { UserContextType } from "src/services/auth/types"; -/** - * @ignore - */ -const missingUserProvider = 'You forgot to wrap your app in '; +import { createContext, useContext } from "react"; -/** - * @ignore - */ -export const UserContext = createContext({ - get user(): never { - throw new Error(missingUserProvider); - }, - get error(): never { - throw new Error(missingUserProvider); - }, - get isLoading(): never { - throw new Error(missingUserProvider); - }, - checkSession: (): never => { - throw new Error(missingUserProvider); - } -}); +export const UserContext = createContext({} as UserContextType); /** * @ignore */ -export type UseUser = () => UserContextType; +export type UserContextHook = () => UserContextType; /** * The `useUser` hook, which will get you the {@link UserProfile} object from the server-side session by fetching it @@ -51,4 +31,5 @@ export type UseUser = () => UserContextType; * * @category Client */ -export const useUser: UseUser = () => useContext(UserContext); +export const useUser: UserContextHook = () => + useContext(UserContext); diff --git a/frontend/src/utils/authUtil.ts b/frontend/src/utils/authUtil.ts new file mode 100644 index 000000000..405012542 --- /dev/null +++ b/frontend/src/utils/authUtil.ts @@ -0,0 +1,10 @@ +import { UserSession } from "src/services/auth/types"; + +export const isSessionExpired = (userSession: UserSession): boolean => { + // if we haven't implemented expiration yet + // TODO: remove this once expiration is implemented in the token + if (!userSession?.expiresAt) { + return false; + } + return userSession.expiresAt > new Date(Date.now()); +}; From 354dda9cc443d427f03b13de78db2c30b0815473 Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Fri, 13 Dec 2024 16:24:29 -0500 Subject: [PATCH 3/6] some cleanup and further proof of concept --- .../dev/feature-flags/FeatureFlagsTable.tsx | 20 ++++++++-- .../app/[locale]/opportunity/[id]/page.tsx | 2 +- .../loading.tsx => components/Loading.tsx} | 0 .../src/components/search/SearchResults.tsx | 2 +- frontend/src/services/auth/UserProvider.tsx | 7 ++-- frontend/src/services/auth/session.ts | 2 +- frontend/src/services/auth/utils.tsx | 40 +++++++++---------- frontend/stories/pages/loading.stories.tsx | 3 +- frontend/tests/pages/search/page.test.tsx | 2 +- .../tests/services/withFeatureFlag.test.tsx | 2 +- frontend/tests/utils/getRoutes.test.ts | 1 - 11 files changed, 46 insertions(+), 35 deletions(-) rename frontend/src/{app/[locale]/search/loading.tsx => components/Loading.tsx} (100%) diff --git a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx index e2dd1efe3..ce9d789e6 100644 --- a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx +++ b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx @@ -6,6 +6,8 @@ import { useUser } from "src/services/auth/useUser"; import React from "react"; import { Button, Table } from "@trussworks/react-uswds"; +import Loading from "src/components/Loading"; + /** * View for managing feature flags */ @@ -13,14 +15,24 @@ export default function FeatureFlagsTable() { const { featureFlagsManager, mounted, setFeatureFlag } = useFeatureFlags(); const { user, isLoading, error } = useUser(); - console.log("~~~ user ~~~", user); - console.log("~~~ error ~~~", error); - console.log("~~~ isLoading ~~~", isLoading); - if (!mounted) { return null; } + if (isLoading) { + return ; + } + + if (error) { + // there's no error page within this tree, should we make a top level error? + return ( + <> +

Error

+ {error.message} + + ); + } + return ( <>

User with token {user?.token} can see this

diff --git a/frontend/src/app/[locale]/opportunity/[id]/page.tsx b/frontend/src/app/[locale]/opportunity/[id]/page.tsx index 4ed492676..a9699fbe7 100644 --- a/frontend/src/app/[locale]/opportunity/[id]/page.tsx +++ b/frontend/src/app/[locale]/opportunity/[id]/page.tsx @@ -3,7 +3,7 @@ import NotFound from "src/app/[locale]/not-found"; import { fetchOpportunity } from "src/app/api/fetchers"; import { OPPORTUNITY_CRUMBS } from "src/constants/breadcrumbs"; import { ApiRequestError, parseErrorStatus } from "src/errors"; -import withFeatureFlag from "src/hoc/search/withFeatureFlag"; +import withFeatureFlag from "src/hoc/withFeatureFlag"; import { Opportunity } from "src/types/opportunity/opportunityResponseTypes"; import { WithFeatureFlagProps } from "src/types/uiTypes"; diff --git a/frontend/src/app/[locale]/search/loading.tsx b/frontend/src/components/Loading.tsx similarity index 100% rename from frontend/src/app/[locale]/search/loading.tsx rename to frontend/src/components/Loading.tsx diff --git a/frontend/src/components/search/SearchResults.tsx b/frontend/src/components/search/SearchResults.tsx index c98da36b7..c4d35d2eb 100644 --- a/frontend/src/components/search/SearchResults.tsx +++ b/frontend/src/components/search/SearchResults.tsx @@ -1,9 +1,9 @@ -import Loading from "src/app/[locale]/search/loading"; import { searchForOpportunities } from "src/app/api/searchFetcher"; import { QueryParamData } from "src/types/search/searchRequestTypes"; import { Suspense } from "react"; +import Loading from "src/components/Loading"; import SearchPagination from "src/components/search/SearchPagination"; import SearchPaginationFetch from "src/components/search/SearchPaginationFetch"; import SearchResultsHeader from "src/components/search/SearchResultsHeader"; diff --git a/frontend/src/services/auth/UserProvider.tsx b/frontend/src/services/auth/UserProvider.tsx index 768fdc224..b802ba426 100644 --- a/frontend/src/services/auth/UserProvider.tsx +++ b/frontend/src/services/auth/UserProvider.tsx @@ -1,6 +1,7 @@ "use client"; import { noop } from "lodash"; +import { ApiRequestError } from "src/errors"; import { UserContextType, UserFetcher, @@ -8,7 +9,6 @@ import { UserSession, } from "src/services/auth/types"; import { UserContext } from "src/services/auth/useUser"; -import { RequestError } from "src/services/auth/utils"; import { isSessionExpired } from "src/utils/authUtil"; import React, { @@ -25,12 +25,11 @@ const userFetcher: UserFetcher = async (url) => { response = await fetch(url); } catch (e) { console.error("User session fetch network error", e); - // not sure we need this error type - throw new RequestError(0); // Network error + throw new ApiRequestError(0); // Network error } if (response.status === 204) return undefined; if (response.ok) return (await response.json()) as UserSession; - throw new RequestError(response.status); + throw new ApiRequestError(response.status); }; export default function UserProvider({ diff --git a/frontend/src/services/auth/session.ts b/frontend/src/services/auth/session.ts index 28b48eae4..ba57cddce 100644 --- a/frontend/src/services/auth/session.ts +++ b/frontend/src/services/auth/session.ts @@ -21,7 +21,7 @@ export async function encrypt({ const jwt = await new SignJWT({ token }) .setProtectedHeader({ alg: "HS256" }) .setIssuedAt() - .setExpirationTime(expiresAt) + .setExpirationTime(expiresAt || "") .sign(encodedKey); return jwt; } diff --git a/frontend/src/services/auth/utils.tsx b/frontend/src/services/auth/utils.tsx index 646f9e2e1..6dd887caf 100644 --- a/frontend/src/services/auth/utils.tsx +++ b/frontend/src/services/auth/utils.tsx @@ -1,21 +1,21 @@ -/** - * The error thrown by the default {@link UserFetcher}. - * - * The `status` property contains the status code of the response. It is `0` when the request - * fails, for example due to being offline. - * - * This error is not thrown when the status code of the response is `204`, because that means the - * user is not authenticated. - * - * @category Client - */ -export class RequestError extends Error { - public status: number; +// /** +// * The error thrown by the default {@link UserFetcher}. +// * +// * The `status` property contains the status code of the response. It is `0` when the request +// * fails, for example due to being offline. +// * +// * This error is not thrown when the status code of the response is `204`, because that means the +// * user is not authenticated. +// * +// * @category Client +// */ +// export class RequestError extends Error { +// public status: number; - constructor(status: number) { - /* c8 ignore next */ - super(); - this.status = status; - Object.setPrototypeOf(this, RequestError.prototype); - } -} +// constructor(status: number) { +// /* c8 ignore next */ +// super(); +// this.status = status; +// Object.setPrototypeOf(this, RequestError.prototype); +// } +// } diff --git a/frontend/stories/pages/loading.stories.tsx b/frontend/stories/pages/loading.stories.tsx index 1ee7e1995..adcd65e3e 100644 --- a/frontend/stories/pages/loading.stories.tsx +++ b/frontend/stories/pages/loading.stories.tsx @@ -1,5 +1,6 @@ import { Meta } from "@storybook/react"; -import Loading from "src/app/[locale]/search/loading"; + +import Loading from "src/components/Loading"; const meta: Meta = { component: Loading, diff --git a/frontend/tests/pages/search/page.test.tsx b/frontend/tests/pages/search/page.test.tsx index 62cd9d00e..f3da02137 100644 --- a/frontend/tests/pages/search/page.test.tsx +++ b/frontend/tests/pages/search/page.test.tsx @@ -7,7 +7,7 @@ import { useTranslationsMock } from "src/utils/testing/intlMocks"; import { ReadonlyURLSearchParams } from "next/navigation"; // test without feature flag functionality -jest.mock("src/hoc/search/withFeatureFlag", () => +jest.mock("src/hoc/withFeatureFlag", () => jest.fn((Component: React.Component) => Component), ); diff --git a/frontend/tests/services/withFeatureFlag.test.tsx b/frontend/tests/services/withFeatureFlag.test.tsx index ead6d50b7..54ba9e7d5 100644 --- a/frontend/tests/services/withFeatureFlag.test.tsx +++ b/frontend/tests/services/withFeatureFlag.test.tsx @@ -1,7 +1,7 @@ // import Cookies from "js-cookie"; import Cookies from "js-cookie"; import { identity } from "lodash"; -import withFeatureFlag from "src/hoc/search/withFeatureFlag"; +import withFeatureFlag from "src/hoc/withFeatureFlag"; import { render } from "tests/react-utils"; let enableFeature = false; diff --git a/frontend/tests/utils/getRoutes.test.ts b/frontend/tests/utils/getRoutes.test.ts index 307cadb89..34b128c33 100644 --- a/frontend/tests/utils/getRoutes.test.ts +++ b/frontend/tests/utils/getRoutes.test.ts @@ -61,7 +61,6 @@ function getPaths() { "src/app/[locale]/search/SearchForm.tsx", "src/app/[locale]/search/actions.ts", "src/app/[locale]/search/error.tsx", - "src/app/[locale]/search/loading.tsx", "src/app/[locale]/search/page.tsx", "src/app/[locale]/subscribe/SubscriptionForm.tsx", "src/app/[locale]/subscribe/confirmation/page.tsx", From 4579ba0f7e90df1d2fde96f383042131906bcb11 Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Mon, 16 Dec 2024 14:46:59 -0500 Subject: [PATCH 4/6] fix issue with overrequesting, some type fixes --- .../dev/feature-flags/FeatureFlagsTable.tsx | 4 ++- frontend/src/services/auth/UserProvider.tsx | 33 +++++++++++-------- frontend/src/services/auth/types.tsx | 6 +--- frontend/src/services/auth/useUser.tsx | 8 ++--- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx index ce9d789e6..b0c2fe1ae 100644 --- a/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx +++ b/frontend/src/app/[locale]/dev/feature-flags/FeatureFlagsTable.tsx @@ -35,7 +35,9 @@ export default function FeatureFlagsTable() { return ( <> -

User with token {user?.token} can see this

+

+ {user?.token ? `Logged in with token: ${user.token}` : "Not logged in"} +

diff --git a/frontend/src/services/auth/UserProvider.tsx b/frontend/src/services/auth/UserProvider.tsx index b802ba426..5921f62d9 100644 --- a/frontend/src/services/auth/UserProvider.tsx +++ b/frontend/src/services/auth/UserProvider.tsx @@ -1,23 +1,16 @@ "use client"; -import { noop } from "lodash"; +import { debounce, noop } from "lodash"; import { ApiRequestError } from "src/errors"; import { - UserContextType, + SessionPayload, UserFetcher, - UserProviderProps, UserSession, } from "src/services/auth/types"; import { UserContext } from "src/services/auth/useUser"; import { isSessionExpired } from "src/utils/authUtil"; -import React, { - ReactElement, - useCallback, - useEffect, - useMemo, - useState, -} from "react"; +import React, { useCallback, useEffect, useMemo, useState } from "react"; const userFetcher: UserFetcher = async (url) => { let response; @@ -28,21 +21,33 @@ const userFetcher: UserFetcher = async (url) => { throw new ApiRequestError(0); // Network error } if (response.status === 204) return undefined; - if (response.ok) return (await response.json()) as UserSession; + if (response.ok) return (await response.json()) as SessionPayload; throw new ApiRequestError(response.status); }; +// if we don't debounce this call we get multiple requests going out on page load +const debouncedUserFetcher = debounce( + () => userFetcher("/api/auth/session"), + 500, + { + leading: true, + trailing: false, + }, +); + export default function UserProvider({ children, -}: UserProviderProps): ReactElement { +}: { + children: React.ReactNode; +}) { const [localUser, setLocalUser] = useState(null); - const [isLoading, setIsLoading] = useState(!localUser); + const [isLoading, setIsLoading] = useState(false); const [userFetchError, setUserFetchError] = useState(); const getUserSession = useCallback(async (): Promise => { try { setIsLoading(true); - const fetchedUser = await userFetcher("/api/auth/session"); + const fetchedUser = await debouncedUserFetcher(); if (fetchedUser) { setLocalUser(fetchedUser); setUserFetchError(undefined); diff --git a/frontend/src/services/auth/types.tsx b/frontend/src/services/auth/types.tsx index 18c544807..a9c46a0d7 100644 --- a/frontend/src/services/auth/types.tsx +++ b/frontend/src/services/auth/types.tsx @@ -55,11 +55,7 @@ export type SessionPayload = { /** * Fetches the user from the profile API route to fill the useUser hook with the * UserProfile object. - * - * If needed, you can pass a custom fetcher to the UserProvider component via the - * UserProviderProps.fetcher prop. - * - * @throws {@link RequestError} + */ export type UserFetcher = (url: string) => Promise; diff --git a/frontend/src/services/auth/useUser.tsx b/frontend/src/services/auth/useUser.tsx index 7c54bf29d..e99bb7bd3 100644 --- a/frontend/src/services/auth/useUser.tsx +++ b/frontend/src/services/auth/useUser.tsx @@ -1,15 +1,15 @@ "use client"; -import { UserContextType } from "src/services/auth/types"; +import { UserProviderState } from "src/services/auth/types"; import { createContext, useContext } from "react"; -export const UserContext = createContext({} as UserContextType); +export const UserContext = createContext({} as UserProviderState); /** * @ignore */ -export type UserContextHook = () => UserContextType; +export type UserContextHook = () => UserProviderState; /** * The `useUser` hook, which will get you the {@link UserProfile} object from the server-side session by fetching it @@ -32,4 +32,4 @@ export type UserContextHook = () => UserContextType; * @category Client */ export const useUser: UserContextHook = () => - useContext(UserContext); + useContext(UserContext); From c8896069ad7a871ddca9eadb83b6b14c5e95822c Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Mon, 16 Dec 2024 16:24:32 -0500 Subject: [PATCH 5/6] provider tests, and some code reorganization --- frontend/src/app/api/userFetcher.ts | 19 ++++++ frontend/src/services/auth/UserProvider.tsx | 25 ++------ frontend/tests/api/auth/session/route.test.ts | 45 +++++++++++++ frontend/tests/services/auth/useUser.test.tsx | 64 +++++++++++++++++++ 4 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 frontend/src/app/api/userFetcher.ts create mode 100644 frontend/tests/api/auth/session/route.test.ts create mode 100644 frontend/tests/services/auth/useUser.test.tsx diff --git a/frontend/src/app/api/userFetcher.ts b/frontend/src/app/api/userFetcher.ts new file mode 100644 index 000000000..2093f4e06 --- /dev/null +++ b/frontend/src/app/api/userFetcher.ts @@ -0,0 +1,19 @@ +"use client"; + +import { ApiRequestError } from "src/errors"; +import { SessionPayload, UserFetcher } from "src/services/auth/types"; + +// this fetcher is a one off for now, since the request is made from the client to the +// NextJS Node server. We will need to build out a fetcher pattern to accomodate this usage in the future +export const userFetcher: UserFetcher = async (url) => { + let response; + try { + response = await fetch(url); + } catch (e) { + console.error("User session fetch network error", e); + throw new ApiRequestError(0); // Network error + } + if (response.status === 204) return undefined; + if (response.ok) return (await response.json()) as SessionPayload; + throw new ApiRequestError(response.status); +}; diff --git a/frontend/src/services/auth/UserProvider.tsx b/frontend/src/services/auth/UserProvider.tsx index 5921f62d9..092b3f328 100644 --- a/frontend/src/services/auth/UserProvider.tsx +++ b/frontend/src/services/auth/UserProvider.tsx @@ -1,30 +1,15 @@ "use client"; -import { debounce, noop } from "lodash"; -import { ApiRequestError } from "src/errors"; -import { - SessionPayload, - UserFetcher, - UserSession, -} from "src/services/auth/types"; +// note that importing these individually allows us to mock them, otherwise mocks don't work :shrug: +import debounce from "lodash/debounce"; +import noop from "lodash/noop"; +import { userFetcher } from "src/app/api/userFetcher"; +import { UserSession } from "src/services/auth/types"; import { UserContext } from "src/services/auth/useUser"; import { isSessionExpired } from "src/utils/authUtil"; import React, { useCallback, useEffect, useMemo, useState } from "react"; -const userFetcher: UserFetcher = async (url) => { - let response; - try { - response = await fetch(url); - } catch (e) { - console.error("User session fetch network error", e); - throw new ApiRequestError(0); // Network error - } - if (response.status === 204) return undefined; - if (response.ok) return (await response.json()) as SessionPayload; - throw new ApiRequestError(response.status); -}; - // if we don't debounce this call we get multiple requests going out on page load const debouncedUserFetcher = debounce( () => userFetcher("/api/auth/session"), diff --git a/frontend/tests/api/auth/session/route.test.ts b/frontend/tests/api/auth/session/route.test.ts new file mode 100644 index 000000000..f20551705 --- /dev/null +++ b/frontend/tests/api/auth/session/route.test.ts @@ -0,0 +1,45 @@ +/** + * @jest-environment node + */ + +import { identity } from "lodash"; +import { GET } from "src/app/api/auth/session/route"; + +const getSessionMock = jest.fn(); +const responseJsonMock = jest.fn(identity); + +jest.mock("src/services/auth/session", () => ({ + getSession: (): unknown => getSessionMock(), +})); + +jest.mock("next/server", () => ({ + NextResponse: { + json: (any: object) => responseJsonMock(any), + }, +})); + +// note that all calls to the GET endpoint need to be caught here since the behavior of the Next redirect +// is to throw an error +describe("GET request", () => { + afterEach(() => jest.clearAllMocks()); + it("returns the current session token when one exists", async () => { + getSessionMock.mockImplementation(() => ({ + token: "fakeToken", + })); + + await GET(); + + expect(getSessionMock).toHaveBeenCalledTimes(1); + expect(responseJsonMock).toHaveBeenCalledTimes(1); + expect(responseJsonMock).toHaveBeenCalledWith({ token: "fakeToken" }); + }); + + it("returns a resopnse with an empty token if no session token exists", async () => { + getSessionMock.mockImplementation(() => null); + await GET(); + + expect(getSessionMock).toHaveBeenCalledTimes(1); + expect(responseJsonMock).toHaveBeenCalledTimes(1); + expect(responseJsonMock).toHaveBeenCalledWith({ token: "" }); + }); +}); diff --git a/frontend/tests/services/auth/useUser.test.tsx b/frontend/tests/services/auth/useUser.test.tsx new file mode 100644 index 000000000..e0a1b6dd9 --- /dev/null +++ b/frontend/tests/services/auth/useUser.test.tsx @@ -0,0 +1,64 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import UserProvider from "src/services/auth/UserProvider"; +import { useUser } from "src/services/auth/useUser"; + +const userFetcherMock = jest.fn(); + +jest.mock("src/app/api/userFetcher", () => ({ + userFetcher: () => userFetcherMock(), +})); + +jest.mock("lodash/debounce", () => (fn) => fn); + +const UseUserConsumer = () => { + const { error, isLoading, user } = useUser(); + return ( + <> +
{error?.toString() || ""}
+
{isLoading.toString()}
+
{user?.toString() || ""}
+ + ); +}; + +describe("useUser", () => { + afterEach(() => jest.clearAllMocks()); + it("renders with the expected state on successful fetch", async () => { + userFetcherMock.mockResolvedValue("this is where a user would be"); + + render( + + + , + ); + + const errorDisplay = await screen.findByTestId("error"); + const userDisplay = await screen.findByTestId("user"); + + await waitFor(() => { + expect(userDisplay).toHaveTextContent("this is where a user would be"); + }); + + expect(errorDisplay).toBeEmptyDOMElement(); + expect(userFetcherMock).toHaveBeenCalledTimes(1); + }); + + it("renders with the expected state on error", async () => { + userFetcherMock.mockResolvedValue(null); + render( + + + , + ); + + const errorDisplay = await screen.findByTestId("error"); + const userDisplay = await screen.findByTestId("user"); + + await waitFor(() => { + expect(errorDisplay).not.toBeEmptyDOMElement(); + }); + + expect(userDisplay).toBeEmptyDOMElement(); + expect(userFetcherMock).toHaveBeenCalledTimes(1); + }); +}); From 12f3234ea850c3d04b75073af8a02c4bcb6828fe Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Mon, 16 Dec 2024 16:30:57 -0500 Subject: [PATCH 6/6] cleanup --- frontend/src/services/auth/utils.tsx | 21 ------------------- frontend/tests/api/auth/session/route.test.ts | 3 +-- frontend/tests/services/auth/useUser.test.tsx | 5 +++-- 3 files changed, 4 insertions(+), 25 deletions(-) delete mode 100644 frontend/src/services/auth/utils.tsx diff --git a/frontend/src/services/auth/utils.tsx b/frontend/src/services/auth/utils.tsx deleted file mode 100644 index 6dd887caf..000000000 --- a/frontend/src/services/auth/utils.tsx +++ /dev/null @@ -1,21 +0,0 @@ -// /** -// * The error thrown by the default {@link UserFetcher}. -// * -// * The `status` property contains the status code of the response. It is `0` when the request -// * fails, for example due to being offline. -// * -// * This error is not thrown when the status code of the response is `204`, because that means the -// * user is not authenticated. -// * -// * @category Client -// */ -// export class RequestError extends Error { -// public status: number; - -// constructor(status: number) { -// /* c8 ignore next */ -// super(); -// this.status = status; -// Object.setPrototypeOf(this, RequestError.prototype); -// } -// } diff --git a/frontend/tests/api/auth/session/route.test.ts b/frontend/tests/api/auth/session/route.test.ts index f20551705..1db694619 100644 --- a/frontend/tests/api/auth/session/route.test.ts +++ b/frontend/tests/api/auth/session/route.test.ts @@ -2,11 +2,10 @@ * @jest-environment node */ -import { identity } from "lodash"; import { GET } from "src/app/api/auth/session/route"; const getSessionMock = jest.fn(); -const responseJsonMock = jest.fn(identity); +const responseJsonMock = jest.fn((something: unknown) => something); jest.mock("src/services/auth/session", () => ({ getSession: (): unknown => getSessionMock(), diff --git a/frontend/tests/services/auth/useUser.test.tsx b/frontend/tests/services/auth/useUser.test.tsx index e0a1b6dd9..17e2ee5b4 100644 --- a/frontend/tests/services/auth/useUser.test.tsx +++ b/frontend/tests/services/auth/useUser.test.tsx @@ -1,14 +1,15 @@ import { render, screen, waitFor } from "@testing-library/react"; +import identity from "lodash/identity"; import UserProvider from "src/services/auth/UserProvider"; import { useUser } from "src/services/auth/useUser"; const userFetcherMock = jest.fn(); jest.mock("src/app/api/userFetcher", () => ({ - userFetcher: () => userFetcherMock(), + userFetcher: () => userFetcherMock() as unknown, })); -jest.mock("lodash/debounce", () => (fn) => fn); +jest.mock("lodash/debounce", () => identity); const UseUserConsumer = () => { const { error, isLoading, user } = useUser();