DEVPROD-4021 Redirect project ids to identifiers (#35)

apps/spruce/cypress/integration/projectSettings/project_settings.ts

@@ -40,3 +40,16 @@ describe("A project that has GitHub webhooks disabled", () => {
     cy.get("input").should("be.disabled");
   });
 });
+
+describe("A project id should redirect to the project identifier", () => {
+  const projectId = "602d70a2b2373672ee493189";
+  const origin = getGeneralRoute(projectId);
+
+  beforeEach(() => {
+    cy.visit(origin);
+  });
+
+  it("Redirects to the project identifier", () => {
+    cy.url().should("include", getGeneralRoute("parsley"));
+  });
+});

apps/spruce/src/analytics/projectSettings/useProjectSettingsAnalytics.ts

@@ -10,7 +10,12 @@ type Action =
   | { name: "Detach project from repo"; repoOwner: string; repoName: string }
   | { name: "Move project to new repo"; repoOwner: string; repoName: string }
   | { name: "Create new project" }
-  | { name: "Duplicate project"; projectIdToCopy: string };
+  | { name: "Duplicate project"; projectIdToCopy: string }
+  | {
+      name: "Redirect to project identifier";
+      projectId: string;
+      projectIdentifier: string;
+    };
 
 export const useProjectSettingsAnalytics = () => {
   const { [slugs.projectIdentifier]: projectIdentifier } = useParams();

apps/spruce/src/hooks/useProjectRedirect/index.ts

@@ -1,3 +1,4 @@
+import { useState } from "react";
 import { useQuery } from "@apollo/client";
 import { useParams, useLocation, useNavigate } from "react-router-dom";
 import { slugs } from "constants/routes";
@@ -9,36 +10,58 @@ const { validateObjectId } = validators;
 
 interface UseProjectRedirectProps {
   sendAnalyticsEvent: (projectId: string, projectIdentifier: string) => void;
+  shouldRedirect?: boolean;
+  onError?: (e: Error) => 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
+ * @param props.shouldRedirect - boolean to indicate if a redirect should be attempted
+ * @param props.onError - function to call if an error occurs during the redirect
  * @returns isRedirecting - boolean to indicate if a redirect is in progress
  */
 export const useProjectRedirect = ({
+  onError,
   sendAnalyticsEvent = () => {},
+  shouldRedirect,
 }: UseProjectRedirectProps) => {
-  const { [slugs.projectIdentifier]: project } = useParams();
+  const { [slugs.projectIdentifier]: projectIdentifier } = useParams();
   const navigate = useNavigate();
   const location = useLocation();
+  const needsRedirect = validateObjectId(projectIdentifier) && shouldRedirect;
 
-  const needsRedirect = validateObjectId(project);
+  const [attemptedRedirect, setAttemptedRedirect] = useState(false);
 
   const { loading } = useQuery<ProjectQuery, ProjectQueryVariables>(PROJECT, {
     skip: !needsRedirect,
     variables: {
-      idOrIdentifier: project,
+      idOrIdentifier: projectIdentifier,
     },
     onCompleted: (projectData) => {
       const { identifier } = projectData.project;
       const currentUrl = location.pathname.concat(location.search);
-      const redirectPathname = currentUrl.replace(project, identifier);
-      sendAnalyticsEvent(project, identifier);
-      navigate(redirectPathname);
+      const redirectPathname = currentUrl.replace(
+        projectIdentifier,
+        identifier,
+      );
+      sendAnalyticsEvent(projectIdentifier, identifier);
+      navigate(redirectPathname, { replace: true });
+      setAttemptedRedirect(true);
+    },
+    onError: (e) => {
+      setAttemptedRedirect(true);
+      onError?.(
+        Error(
+          `There was an error loading the project ${projectIdentifier}: ${e.message}`,
+        ),
+      );
     },
   });
 
-  return { isRedirecting: needsRedirect && loading };
+  return {
+    isRedirecting: needsRedirect && loading,
+    attemptedRedirect,
+  };
 };

apps/spruce/src/hooks/useProjectRedirect/useProjectRedirect.test.tsx

@@ -7,14 +7,10 @@ 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 useJointHook = (props: Parameters<typeof useProjectRedirect>[0]) => {
+  const { attemptedRedirect, isRedirecting } = useProjectRedirect({ ...props });
   const { pathname, search } = useLocation();
-  return { isRedirecting, pathname, search };
+  return { isRedirecting, pathname, search, attemptedRedirect };
 };
 
 const ProviderWrapper: React.FC<{
@@ -33,12 +29,17 @@ const ProviderWrapper: React.FC<{
 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" }),
-    });
+    const onError = jest.fn();
+    const { result } = renderHook(
+      () => useJointHook({ sendAnalyticsEvent, onError, shouldRedirect: true }),
+      {
+        wrapper: ({ children }) =>
+          ProviderWrapper({ children, location: "/commits/my-project" }),
+      },
+    );
     expect(result.current).toMatchObject({
       isRedirecting: false,
+      attemptedRedirect: false,
       pathname: "/commits/my-project",
       search: "",
     });
@@ -47,18 +48,25 @@ describe("useProjectRedirect", () => {
 
   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}` }),
-    });
+    const onError = jest.fn();
+
+    const { result } = renderHook(
+      () => useJointHook({ sendAnalyticsEvent, onError, shouldRedirect: true }),
+      {
+        wrapper: ({ children }) =>
+          ProviderWrapper({ children, location: `/commits/${projectId}` }),
+      },
+    );
     expect(result.current).toMatchObject({
       isRedirecting: true,
+      attemptedRedirect: false,
       pathname: "/commits/5f74d99ab2373627c047c5e5",
       search: "",
     });
     await waitFor(() => {
       expect(result.current).toMatchObject({
         isRedirecting: false,
+        attemptedRedirect: true,
         pathname: "/commits/my-project",
         search: "",
       });
@@ -70,23 +78,59 @@ describe("useProjectRedirect", () => {
     );
   });
 
-  it("should preserve query params when redirecting", async () => {
+  it("should not redirect if shouldRedirect is disabled", async () => {
     const sendAnalyticsEvent = jest.fn();
-    const { result } = renderHook(() => useJointHook({ sendAnalyticsEvent }), {
-      wrapper: ({ children }) =>
-        ProviderWrapper({
-          children,
-          location: `/commits/${projectId}?taskName=thirdparty`,
-        }),
+    const onError = jest.fn();
+
+    const { result } = renderHook(
+      () =>
+        useJointHook({ sendAnalyticsEvent, onError, shouldRedirect: false }),
+      {
+        wrapper: ({ children }) =>
+          ProviderWrapper({ children, location: `/commits/${projectId}` }),
+      },
+    );
+    expect(result.current).toMatchObject({
+      isRedirecting: false,
+      attemptedRedirect: false,
+      pathname: "/commits/5f74d99ab2373627c047c5e5",
+      search: "",
     });
+    await waitFor(() => {
+      expect(result.current).toMatchObject({
+        isRedirecting: false,
+        attemptedRedirect: false,
+        pathname: "/commits/5f74d99ab2373627c047c5e5",
+        search: "",
+      });
+    });
+    expect(sendAnalyticsEvent).toHaveBeenCalledTimes(0);
+  });
+
+  it("should preserve query params when redirecting", async () => {
+    const sendAnalyticsEvent = jest.fn();
+    const onError = jest.fn();
+
+    const { result } = renderHook(
+      () => useJointHook({ sendAnalyticsEvent, onError, shouldRedirect: true }),
+      {
+        wrapper: ({ children }) =>
+          ProviderWrapper({
+            children,
+            location: `/commits/${projectId}?taskName=thirdparty`,
+          }),
+      },
+    );
     expect(result.current).toMatchObject({
       isRedirecting: true,
+      attemptedRedirect: false,
       pathname: "/commits/5f74d99ab2373627c047c5e5",
       search: "?taskName=thirdparty",
     });
     await waitFor(() => {
       expect(result.current).toMatchObject({
         isRedirecting: false,
+        attemptedRedirect: true,
         pathname: "/commits/my-project",
         search: "?taskName=thirdparty",
       });
@@ -100,23 +144,36 @@ describe("useProjectRedirect", () => {
 
   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}` }),
-    });
+    const onError = jest.fn();
+
+    const { result } = renderHook(
+      () => useJointHook({ sendAnalyticsEvent, onError, shouldRedirect: true }),
+      {
+        wrapper: ({ children }) =>
+          ProviderWrapper({ children, location: `/commits/${repoId}` }),
+      },
+    );
     expect(result.current).toMatchObject({
       isRedirecting: true,
+      attemptedRedirect: false,
       pathname: "/commits/5e6bb9e23066155a993e0f1a",
       search: "",
     });
     await waitFor(() => {
       expect(result.current).toMatchObject({
         isRedirecting: false,
+        attemptedRedirect: true,
         pathname: "/commits/5e6bb9e23066155a993e0f1a",
         search: "",
       });
     });
     expect(sendAnalyticsEvent).toHaveBeenCalledTimes(0);
+    expect(onError).toHaveBeenCalledTimes(1);
+    expect(onError).toHaveBeenCalledWith(
+      Error(
+        `There was an error loading the project ${repoId}: Error finding project by id ${repoId}: 404 (Not Found): project '${repoId}' not found`,
+      ),
+    );
   });
 });
 

apps/spruce/src/pages/commits/index.tsx

@@ -77,7 +77,10 @@ const Commits = () => {
       projectIdentifier: identifier,
     });
   };
-  const { isRedirecting } = useProjectRedirect({ sendAnalyticsEvent });
+  const { isRedirecting } = useProjectRedirect({
+    sendAnalyticsEvent,
+    shouldRedirect: true,
+  });
 
   const recentlySelectedProject = Cookies.get(CURRENT_PROJECT);
   // Push default project to URL if there isn't a project in

apps/spruce/src/pages/projectSettings/index.tsx

@@ -1,7 +1,9 @@
+import { useState } from "react";
 import { useQuery } from "@apollo/client";
 import styled from "@emotion/styled";
 import { Skeleton } from "antd";
 import { useParams, Link, Navigate } from "react-router-dom";
+import { useProjectSettingsAnalytics } from "analytics";
 import { ProjectBanner } from "components/Banners";
 import { ProjectSelect } from "components/ProjectSelect";
 import {
@@ -25,6 +27,7 @@ import {
 } from "gql/generated/types";
 import { PROJECT_SETTINGS, REPO_SETTINGS } from "gql/queries";
 import { usePageTitle } from "hooks";
+import { useProjectRedirect } from "hooks/useProjectRedirect";
 import { validators } from "utils";
 import { ProjectSettingsProvider } from "./Context";
 import { CreateDuplicateProjectButton } from "./CreateDuplicateProjectButton";
@@ -42,14 +45,31 @@ const ProjectSettings: React.FC = () => {
       [slugs.projectIdentifier]: string | null;
       [slugs.tab]: ProjectSettingsTabRoutes;
     }>();
-  // If the path includes an Object ID, this page represents a repo and we should not attempt to fetch a project.
-  const isRepo = validateObjectId(identifier);
+  // If the path includes an Object ID, this page could either be a project or a repo if it is a project we should redirect the user so that they use the identifier.
+  const identifierIsObjectId = validateObjectId(identifier);
+  const [isRepo, setIsRepo] = useState<boolean>(false);
+
+  const { sendEvent } = useProjectSettingsAnalytics();
+
+  useProjectRedirect({
+    shouldRedirect: identifierIsObjectId,
+    onError: () => {
+      setIsRepo(true);
+    },
+    sendAnalyticsEvent: (projectId: string, projectIdentifier: string) => {
+      sendEvent({
+        name: "Redirect to project identifier",
+        projectId,
+        projectIdentifier,
+      });
+    },
+  });
 
   const { data: projectData, loading: projectLoading } = useQuery<
     ProjectSettingsQuery,
     ProjectSettingsQueryVariables
   >(PROJECT_SETTINGS, {
-    skip: isRepo,
+    skip: identifierIsObjectId,
     variables: { identifier },
     onError: (e) => {
       dispatchToast.error(