From c94a5935c8dfa4fbd178bc9bc7e64ff2161ca114 Mon Sep 17 00:00:00 2001 From: minnakt <47064971+minnakt@users.noreply.github.com> Date: Mon, 18 Mar 2024 19:44:36 -0400 Subject: [PATCH] DEVPROD-4193: Redirect to project identifier on Project Health page (#2290) --- .../useProjectHealthAnalytics.ts | 7 +- src/components/Header/Navbar.tsx | 17 +- src/gql/generated/types.ts | 36 ++++ src/gql/queries/index.ts | 2 + src/gql/queries/project.graphql | 6 + src/hooks/useProjectRedirect/index.ts | 45 +++++ .../useProjectRedirect.test.tsx | 157 ++++++++++++++++++ src/pages/commits/index.tsx | 19 ++- 8 files changed, 280 insertions(+), 9 deletions(-) create mode 100644 src/gql/queries/project.graphql create mode 100644 src/hooks/useProjectRedirect/index.ts create mode 100644 src/hooks/useProjectRedirect/useProjectRedirect.test.tsx diff --git a/src/analytics/projectHealth/useProjectHealthAnalytics.ts b/src/analytics/projectHealth/useProjectHealthAnalytics.ts index 6ec3a4f01d..1a4a04069c 100644 --- a/src/analytics/projectHealth/useProjectHealthAnalytics.ts +++ b/src/analytics/projectHealth/useProjectHealthAnalytics.ts @@ -44,7 +44,12 @@ type Action = name: "Add Notification"; subscription: SaveSubscriptionForUserMutationVariables["subscription"]; } - | { name: "Toggle view"; toggle: ProjectHealthView }; + | { name: "Toggle view"; toggle: ProjectHealthView } + | { + name: "Redirect to project identifier"; + projectId: string; + projectIdentifier: string; + }; export const useProjectHealthAnalytics = (p: { page: pageType }) => useAnalyticsRoot("ProjectHealthPages", { page: p.page }); diff --git a/src/components/Header/Navbar.tsx b/src/components/Header/Navbar.tsx index d02ecccee6..d6a45cb846 100644 --- a/src/components/Header/Navbar.tsx +++ b/src/components/Header/Navbar.tsx @@ -16,9 +16,12 @@ import { useAuthStateContext } from "context/Auth"; import { UserQuery, SpruceConfigQuery } from "gql/generated/types"; import { USER, SPRUCE_CONFIG } from "gql/queries"; import { useLegacyUIURL } from "hooks"; +import { validators } from "utils"; import { AuxiliaryDropdown } from "./AuxiliaryDropdown"; import { UserDropdown } from "./UserDropdown"; +const { validateObjectId } = validators; + const { blue, gray, white } = palette; export const Navbar: React.FC = () => { @@ -33,16 +36,20 @@ export const Navbar: React.FC = () => { const { projectIdentifier: projectFromUrl } = useParams<{ projectIdentifier: string; }>(); + const currProject = Cookies.get(CURRENT_PROJECT); - // Update current project cookie if the project in the URL does not equal the cookie value. + // Update current project cookie if the project in the URL is not an objectId and is not equal + // to the current project. // This will inform future navigations to the /commits page. useEffect(() => { - if (projectFromUrl && projectFromUrl !== Cookies.get(CURRENT_PROJECT)) { + if ( + projectFromUrl && + !validateObjectId(projectFromUrl) && + projectFromUrl !== currProject + ) { Cookies.set(CURRENT_PROJECT, projectFromUrl); } - }, [projectFromUrl]); - - const currProject = projectFromUrl ?? Cookies.get(CURRENT_PROJECT); + }, [currProject, projectFromUrl]); const { data: configData } = useQuery(SPRUCE_CONFIG, { skip: currProject !== undefined, diff --git a/src/gql/generated/types.ts b/src/gql/generated/types.ts index 769810448e..2c912cd4ad 100644 --- a/src/gql/generated/types.ts +++ b/src/gql/generated/types.ts @@ -699,6 +699,7 @@ export type Host = { instanceType?: Maybe; lastCommunicationTime?: Maybe; noExpiration: Scalars["Boolean"]["output"]; + persistentDnsName: Scalars["String"]["output"]; provider: Scalars["String"]["output"]; runningTask?: Maybe; startedBy: Scalars["String"]["output"]; @@ -1057,6 +1058,7 @@ export type Mutation = { unschedulePatchTasks?: Maybe; unscheduleTask: Task; updateHostStatus: Scalars["Int"]["output"]; + updateParsleySettings?: Maybe; updatePublicKey: Array; updateSpawnHostStatus: Host; updateUserSettings: Scalars["Boolean"]["output"]; @@ -1315,6 +1317,10 @@ export type MutationUpdateHostStatusArgs = { status: Scalars["String"]["input"]; }; +export type MutationUpdateParsleySettingsArgs = { + opts: UpdateParsleySettingsInput; +}; + export type MutationUpdatePublicKeyArgs = { targetKeyName: Scalars["String"]["input"]; updateInfo: PublicKeyInput; @@ -1406,6 +1412,16 @@ export type ParsleyFilterInput = { expression: Scalars["String"]["input"]; }; +/** ParsleySettings contains information about a user's settings for Parsley. */ +export type ParsleySettings = { + __typename?: "ParsleySettings"; + sectionsEnabled: Scalars["Boolean"]["output"]; +}; + +export type ParsleySettingsInput = { + sectionsEnabled?: InputMaybe; +}; + /** Patch is a manually initiated version submitted to test local code changes. */ export type Patch = { __typename?: "Patch"; @@ -2532,6 +2548,7 @@ export type Task = { startTime?: Maybe; status: Scalars["String"]["output"]; stepbackInfo?: Maybe; + tags: Array; /** @deprecated Use files instead */ taskFiles: TaskFiles; taskGroup?: Maybe; @@ -2865,6 +2882,15 @@ export type UiConfig = { userVoice?: Maybe; }; +export type UpdateParsleySettingsInput = { + parsleySettings: ParsleySettingsInput; +}; + +export type UpdateParsleySettingsPayload = { + __typename?: "UpdateParsleySettingsPayload"; + parsleySettings?: Maybe; +}; + /** * UpdateVolumeInput is the input to the updateVolume mutation. * Its fields determine how a given volume will be modified. @@ -2911,6 +2937,7 @@ export type User = { displayName: Scalars["String"]["output"]; emailAddress: Scalars["String"]["output"]; parsleyFilters: Array; + parsleySettings: ParsleySettings; patches: Patches; permissions: Permissions; subscriptions?: Maybe>; @@ -7389,6 +7416,15 @@ export type ProjectSettingsQuery = { }; }; +export type ProjectQueryVariables = Exact<{ + idOrIdentifier: Scalars["String"]["input"]; +}>; + +export type ProjectQuery = { + __typename?: "Query"; + project: { __typename?: "Project"; id: string; identifier: string }; +}; + export type ProjectsQueryVariables = Exact<{ [key: string]: never }>; export type ProjectsQuery = { diff --git a/src/gql/queries/index.ts b/src/gql/queries/index.ts index c1e97bad63..b62c6dba1e 100644 --- a/src/gql/queries/index.ts +++ b/src/gql/queries/index.ts @@ -46,6 +46,7 @@ import PROJECT_EVENT_LOGS from "./project-event-logs.graphql"; import PROJECT_HEALTH_VIEW from "./project-health-view.graphql"; import PROJECT_PATCHES from "./project-patches.graphql"; import PROJECT_SETTINGS from "./project-settings.graphql"; +import PROJECT from "./project.graphql"; import PROJECTS from "./projects.graphql"; import MY_PUBLIC_KEYS from "./public-keys.graphql"; import REPO_EVENT_LOGS from "./repo-event-logs.graphql"; @@ -124,6 +125,7 @@ export { PATCH, POD_EVENTS, POD, + PROJECT, PROJECT_BANNER, PROJECT_EVENT_LOGS, PROJECT_HEALTH_VIEW, diff --git a/src/gql/queries/project.graphql b/src/gql/queries/project.graphql new file mode 100644 index 0000000000..9c50d685c1 --- /dev/null +++ b/src/gql/queries/project.graphql @@ -0,0 +1,6 @@ +query Project($idOrIdentifier: String!) { + project(projectIdentifier: $idOrIdentifier) { + id + identifier + } +} diff --git a/src/hooks/useProjectRedirect/index.ts b/src/hooks/useProjectRedirect/index.ts new file mode 100644 index 0000000000..a3b2450977 --- /dev/null +++ b/src/hooks/useProjectRedirect/index.ts @@ -0,0 +1,45 @@ +import { useQuery } from "@apollo/client"; +import { useParams, useLocation, useNavigate } from "react-router-dom"; +import { ProjectQuery, ProjectQueryVariables } from "gql/generated/types"; +import { PROJECT } from "gql/queries"; +import { validators } from "utils"; + +const { validateObjectId } = validators; + +interface UseProjectRedirectProps { + sendAnalyticsEvent: (projectId: string, projectIdentifier: string) => void; +} + +/** + * useProjectRedirect will replace the project id with the project identifier in the URL. + * @param props - Object containing the following: + * @param props.sendAnalyticsEvent - analytics event to send upon redirect + * @returns isRedirecting - boolean to indicate if a redirect is in progress + */ +export const useProjectRedirect = ({ + sendAnalyticsEvent = () => {}, +}: UseProjectRedirectProps) => { + const { projectIdentifier: project } = useParams<{ + projectIdentifier: string; + }>(); + const navigate = useNavigate(); + const location = useLocation(); + + const needsRedirect = validateObjectId(project); + + const { loading } = useQuery(PROJECT, { + skip: !needsRedirect, + variables: { + idOrIdentifier: project, + }, + onCompleted: (projectData) => { + const { identifier } = projectData.project; + const currentUrl = location.pathname.concat(location.search); + const redirectPathname = currentUrl.replace(project, identifier); + sendAnalyticsEvent(project, identifier); + navigate(redirectPathname); + }, + }); + + return { isRedirecting: needsRedirect && loading }; +}; diff --git a/src/hooks/useProjectRedirect/useProjectRedirect.test.tsx b/src/hooks/useProjectRedirect/useProjectRedirect.test.tsx new file mode 100644 index 0000000000..ff5b8241d8 --- /dev/null +++ b/src/hooks/useProjectRedirect/useProjectRedirect.test.tsx @@ -0,0 +1,157 @@ +import { MockedProvider } from "@apollo/client/testing"; +import { GraphQLError } from "graphql"; +import { MemoryRouter, Routes, Route, useLocation } from "react-router-dom"; +import { ProjectQuery, ProjectQueryVariables } from "gql/generated/types"; +import { PROJECT } from "gql/queries"; +import { renderHook, waitFor } from "test_utils"; +import { ApolloMock } from "types/gql"; +import { useProjectRedirect } from "."; + +const useJointHook = ({ + sendAnalyticsEvent, +}: { + sendAnalyticsEvent: (projectId: string, projectIdentifier: string) => void; +}) => { + const { isRedirecting } = useProjectRedirect({ sendAnalyticsEvent }); + const { pathname, search } = useLocation(); + return { isRedirecting, pathname, search }; +}; + +const ProviderWrapper: React.FC<{ + children: React.ReactNode; + location: string; +}> = ({ children, location }) => ( + + + + + + + +); + +describe("useProjectRedirect", () => { + it("should not redirect if URL has project identifier", async () => { + const sendAnalyticsEvent = jest.fn(); + const { result } = renderHook(() => useJointHook({ sendAnalyticsEvent }), { + wrapper: ({ children }) => + ProviderWrapper({ children, location: "/commits/my-project" }), + }); + expect(result.current).toMatchObject({ + isRedirecting: false, + pathname: "/commits/my-project", + search: "", + }); + expect(sendAnalyticsEvent).toHaveBeenCalledTimes(0); + }); + + it("should redirect if URL has project ID", async () => { + const sendAnalyticsEvent = jest.fn(); + const { result } = renderHook(() => useJointHook({ sendAnalyticsEvent }), { + wrapper: ({ children }) => + ProviderWrapper({ children, location: `/commits/${projectId}` }), + }); + expect(result.current).toMatchObject({ + isRedirecting: true, + pathname: "/commits/5f74d99ab2373627c047c5e5", + search: "", + }); + await waitFor(() => { + expect(result.current).toMatchObject({ + isRedirecting: false, + pathname: "/commits/my-project", + search: "", + }); + }); + expect(sendAnalyticsEvent).toHaveBeenCalledTimes(1); + expect(sendAnalyticsEvent).toHaveBeenCalledWith( + "5f74d99ab2373627c047c5e5", + "my-project", + ); + }); + + it("should preserve query params when redirecting", async () => { + const sendAnalyticsEvent = jest.fn(); + const { result } = renderHook(() => useJointHook({ sendAnalyticsEvent }), { + wrapper: ({ children }) => + ProviderWrapper({ + children, + location: `/commits/${projectId}?taskName=thirdparty`, + }), + }); + expect(result.current).toMatchObject({ + isRedirecting: true, + pathname: "/commits/5f74d99ab2373627c047c5e5", + search: "?taskName=thirdparty", + }); + await waitFor(() => { + expect(result.current).toMatchObject({ + isRedirecting: false, + pathname: "/commits/my-project", + search: "?taskName=thirdparty", + }); + }); + expect(sendAnalyticsEvent).toHaveBeenCalledTimes(1); + expect(sendAnalyticsEvent).toHaveBeenCalledWith( + "5f74d99ab2373627c047c5e5", + "my-project", + ); + }); + + it("should attempt redirect if URL has repo ID but stop attempting after query", async () => { + const sendAnalyticsEvent = jest.fn(); + const { result } = renderHook(() => useJointHook({ sendAnalyticsEvent }), { + wrapper: ({ children }) => + ProviderWrapper({ children, location: `/commits/${repoId}` }), + }); + expect(result.current).toMatchObject({ + isRedirecting: true, + pathname: "/commits/5e6bb9e23066155a993e0f1a", + search: "", + }); + await waitFor(() => { + expect(result.current).toMatchObject({ + isRedirecting: false, + pathname: "/commits/5e6bb9e23066155a993e0f1a", + search: "", + }); + }); + expect(sendAnalyticsEvent).toHaveBeenCalledTimes(0); + }); +}); + +const projectId = "5f74d99ab2373627c047c5e5"; +const projectMock: ApolloMock = { + request: { + query: PROJECT, + variables: { + idOrIdentifier: projectId, + }, + }, + result: { + data: { + project: { + __typename: "Project", + id: projectId, + identifier: "my-project", + }, + }, + }, +}; + +const repoId = "5e6bb9e23066155a993e0f1a"; +const repoMock: ApolloMock = { + request: { + query: PROJECT, + variables: { + idOrIdentifier: repoId, + }, + }, + result: { + errors: [ + new GraphQLError( + `Error finding project by id ${repoId}: 404 (Not Found): project '${repoId}' not found`, + ), + ], + }, +}; diff --git a/src/pages/commits/index.tsx b/src/pages/commits/index.tsx index 04ae662229..680bf7dd25 100644 --- a/src/pages/commits/index.tsx +++ b/src/pages/commits/index.tsx @@ -36,6 +36,7 @@ import { useUpsertQueryParams, useUserSettings, } from "hooks"; +import { useProjectRedirect } from "hooks/useProjectRedirect"; import { useQueryParam } from "hooks/useQueryParam"; import { ProjectFilterOptions, MainlineCommitQueryParams } from "types/commits"; import { array, queryString, validators } from "utils"; @@ -69,8 +70,17 @@ const Commits = () => { const { projectIdentifier } = useParams<{ projectIdentifier: string; }>(); - usePageTitle(`Project Health | ${projectIdentifier}`); + + const sendAnalyticsEvent = (id: string, identifier: string) => { + sendEvent({ + name: "Redirect to project identifier", + projectId: id, + projectIdentifier: identifier, + }); + }; + const { isRedirecting } = useProjectRedirect({ sendAnalyticsEvent }); + const recentlySelectedProject = Cookies.get(CURRENT_PROJECT); // Push default project to URL if there isn't a project in // the URL already and an mci-project-cookie does not exist. @@ -139,7 +149,7 @@ const Commits = () => { MainlineCommitsQuery, MainlineCommitsQueryVariables >(MAINLINE_COMMITS, { - skip: !projectIdentifier || isResizing, + skip: !projectIdentifier || isRedirecting || isResizing, errorPolicy: "all", fetchPolicy: "cache-and-network", variables, @@ -233,7 +243,10 @@ const Commits = () => { versions={versions} revision={revision} isLoading={ - (loading && !versions) || !projectIdentifier || isResizing + (loading && !versions) || + !projectIdentifier || + isRedirecting || + isResizing } hasTaskFilter={hasTasks} hasFilters={hasFilters}