fix: 404 collisions (#15249)

* fix: 404 collisions * Added E2E test * Removed await * Attempting to fix E2E * fingers crossed Signed-off-by: zomars <zomars@me.com> * fix: e2e tests * fix: missing navigation to 404 page * fix: don't interrupt post login navigation * fix: Expect owner as possible host for RR events --------- Signed-off-by: zomars <zomars@me.com> Co-authored-by: Omar López <zomars@me.com>
2024-05-31 22:28:43 +00:00
parent ac3fb0a1cd
commit 5d01eb2f38
10 changed files with 17 additions and 110 deletions
@@ -1,6 +1,5 @@
 "use client";

-import type { GetStaticPropsContext } from "next";
 import Link from "next/link";
 import { usePathname } from "next/navigation";
 import { useEffect, useState } from "react";
@@ -11,14 +10,11 @@ import {
 } from "@calcom/features/ee/organizations/lib/orgDomains";
 import { DOCS_URL, IS_CALCOM, JOIN_DISCORD, WEBSITE_URL } from "@calcom/lib/constants";
 import { useLocale } from "@calcom/lib/hooks/useLocale";
-import { HeadSeo } from "@calcom/ui";
-import { Icon } from "@calcom/ui";
+import { HeadSeo, Icon } from "@calcom/ui";
 import { Discord } from "@calcom/ui/components/icon/Discord";

 import PageWrapper from "@components/PageWrapper";

-import { getTranslations } from "@server/lib/getTranslations";
-
 enum pageType {
  ORG = "org",
  TEAM = "team",
@@ -271,13 +267,3 @@ export default function Custom404() {
 }

 Custom404.PageWrapper = PageWrapper;
-
-export const getStaticProps = async (context: GetStaticPropsContext) => {
-  const i18n = await getTranslations(context);
-
-  return {
-    props: {
-      i18n,
-    },
-  };
-};
@@ -1,64 +0,0 @@
-"use client";
-
-/**
- * Typescript class based component for custom-error
- * @link https://nextjs.org/docs/advanced-features/custom-error-page
- */
-import type { NextPage } from "next";
-import type { ErrorProps } from "next/error";
-import React from "react";
-
-import { HttpError } from "@calcom/lib/http-error";
-import logger from "@calcom/lib/logger";
-import { redactError } from "@calcom/lib/redactError";
-
-import { ErrorPage } from "@components/error/error-page";
-
-type NextError = Error & { digest?: string };
-
-// Ref: https://nextjs.org/docs/app/api-reference/file-conventions/error#props
-export type DefaultErrorProps = {
-  error: NextError;
-  reset: () => void; // A function to reset the error boundary
-};
-
-type AugmentedError = NextError | HttpError | null;
-
-type CustomErrorProps = {
-  err?: AugmentedError;
-  statusCode?: number;
-  message?: string;
-} & Omit<ErrorProps, "err" | "statusCode">;
-
-const log = logger.getSubLogger({ prefix: ["[error]"] });
-
-const CustomError: NextPage<DefaultErrorProps> = (props) => {
-  const { error } = props;
-  let errorObject: CustomErrorProps = {
-    message: error.message,
-    err: error,
-  };
-
-  if (error instanceof HttpError) {
-    const redactedError = redactError(error);
-    errorObject = {
-      statusCode: error.statusCode,
-      title: redactedError.name,
-      message: redactedError.message,
-      err: {
-        ...redactedError,
-        ...error,
-      },
-    };
-  }
-
-  // `error.digest` property contains an automatically generated hash of the error that can be used to match the corresponding error in server-side logs
-  log.debug(`${error?.toString() ?? JSON.stringify(error)}`);
-  log.info("errorObject: ", errorObject);
-
-  return (
-    <ErrorPage statusCode={errorObject.statusCode} error={errorObject.err} message={errorObject.message} />
-  );
-};
-
-export default CustomError;
@@ -1,17 +0,0 @@
-"use client";
-
-import { type NextPage } from "next";
-
-import CustomError, { type DefaultErrorProps } from "./error";
-
-export const GlobalError: NextPage<DefaultErrorProps> = (props) => {
-  return (
-    <html>
-      <body>
-        <CustomError {...props} />
-      </body>
-    </html>
-  );
-};
-
-export default GlobalError;
@@ -1,9 +1,10 @@
-import NotFoundPage from "@pages/404";
-import { WithLayout } from "app/layoutHOC";
 import type { GetStaticPropsContext } from "next";

 import { getTranslations } from "@server/lib/getTranslations";

+import NotFoundPage from "./404/page";
+import { WithLayout } from "./layoutHOC";
+
 const getData = async (context: GetStaticPropsContext) => {
  const i18n = await getTranslations(context);

@@ -153,6 +153,13 @@ testBothFutureAndLegacyRoutes.describe("pro user", () => {
    await expect(page).toHaveURL(new RegExp(`${pro.username}/${eventType.slug}`));
  });

+  test("it returns a 404 when a requested event type does not exist", async ({ page, users }) => {
+    const [pro] = users.get();
+    const unexistingPageUrl = new URL(`${pro.username}/invalid-event-type`, WEBAPP_URL);
+    const response = await page.goto(unexistingPageUrl.href);
+    expect(response?.status()).toBe(404);
+  });
+
  test("Can cancel the recently created booking and rebook the same timeslot", async ({
    page,
    users,
@@ -370,4 +370,5 @@ export async function doOnOrgDomain(

 // When App directory is there, this is the 404 page text. We should work on fixing the 404 page as it changed due to app directory.
 export const NotFoundPageTextAppDir = "This page does not exist.";
+export const NotFoundPageTextPages = "404: This page could not be found.";
 // export const NotFoundPageText = "ERROR 404";
@@ -83,9 +83,6 @@ test.describe("2FA Tests", async () => {
        page.waitForResponse("**/api/auth/callback/credentials**"),
      ]);
      const shellLocator = page.locator(`[data-testid=dashboard-shell]`);
-
-      // expects the home page for an authorized user
-      await page.goto("/");
      await expect(shellLocator).toBeVisible();
    });
  });
@@ -10,7 +10,6 @@ import { test } from "../lib/fixtures";
 import {
  bookTimeSlot,
  doOnOrgDomain,
-  NotFoundPageTextAppDir,
  selectFirstAvailableTimeSlotNextMonth,
  testName,
 } from "../lib/testUtils";
@@ -184,10 +183,7 @@ test.describe("Bookings", () => {
      });

      const event = await user.getFirstEventAsOwner();
-      await page.goto(`/${user.username}/${event.slug}`);
-
-      // Shouldn't be servable on the non-org domain
-      await expect(page.locator(`text=${NotFoundPageTextAppDir}`)).toBeVisible();
+      await expectPageToBeNotFound({ page, url: `/${user.username}/${event.slug}` });

      await doOnOrgDomain(
        {
@@ -523,5 +519,5 @@ async function bookTeamEvent({

 async function expectPageToBeNotFound({ page, url }: { page: Page; url: string }) {
  await page.goto(`${url}`);
-  await expect(page.locator(`text=${NotFoundPageTextAppDir}`)).toBeVisible();
+  await expect(page.getByTestId(`404-page`)).toBeVisible();
 }
@@ -149,7 +149,7 @@ testBothFutureAndLegacyRoutes.describe("Teams - NonOrg", (routeVariant) => {
    // The title of the booking
    const bookingTitle = await page.getByTestId("booking-title").textContent();
    expect(
-      teamMatesObj?.some((teamMate) => {
+      teamMatesObj.concat([{ name: owner.name! }]).some((teamMate) => {
        const BookingTitle = `${teamEventTitle} between ${teamMate.name} and ${testName}`;
        return BookingTitle === bookingTitle;
      })
@@ -3,7 +3,7 @@ import { expect } from "@playwright/test";

 import type { Fixtures } from "@calcom/web/playwright/lib/fixtures";
 import { test } from "@calcom/web/playwright/lib/fixtures";
-import { NotFoundPageTextAppDir, gotoRoutingLink } from "@calcom/web/playwright/lib/testUtils";
+import { gotoRoutingLink } from "@calcom/web/playwright/lib/testUtils";

 import {
  addForm,
@@ -36,7 +36,7 @@ test.describe("Routing Forms", () => {
      await page.goto(`apps/routing-forms/route-builder/${formId}`);
      await disableForm(page);
      await gotoRoutingLink({ page, formId });
-      await expect(page.locator(`text=${NotFoundPageTextAppDir}`)).toBeVisible();
+      await expect(page.getByTestId(`404-page`)).toBeVisible();
    });

    test("should be able to edit the form", async ({ page }) => {