Skip to content

Commit

Permalink
fix: 404 collisions (#15249)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
keithwillcode and zomars committed May 31, 2024
1 parent ac3fb0a commit 5d01eb2
Show file tree
Hide file tree
Showing 10 changed files with 17 additions and 110 deletions.
16 changes: 1 addition & 15 deletions apps/web/pages/404.tsx → apps/web/app/404/page.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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",
Expand Down Expand Up @@ -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,
},
};
};
64 changes: 0 additions & 64 deletions apps/web/app/error.tsx

This file was deleted.

17 changes: 0 additions & 17 deletions apps/web/app/global-error.tsx

This file was deleted.

5 changes: 3 additions & 2 deletions apps/web/app/not-found.tsx
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
7 changes: 7 additions & 0 deletions apps/web/playwright/booking-pages.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions apps/web/playwright/lib/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
3 changes: 0 additions & 3 deletions apps/web/playwright/login.2fa.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
8 changes: 2 additions & 6 deletions apps/web/playwright/organization/booking.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { test } from "../lib/fixtures";
import {
bookTimeSlot,
doOnOrgDomain,
NotFoundPageTextAppDir,
selectFirstAvailableTimeSlotNextMonth,
testName,
} from "../lib/testUtils";
Expand Down Expand Up @@ -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(
{
Expand Down Expand Up @@ -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();
}
2 changes: 1 addition & 1 deletion apps/web/playwright/teams.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {

Check warning on line 152 in apps/web/playwright/teams.e2e.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

apps/web/playwright/teams.e2e.ts#L152

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
const BookingTitle = `${teamEventTitle} between ${teamMate.name} and ${testName}`;
return BookingTitle === bookingTitle;
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 }) => {
Expand Down

0 comments on commit 5d01eb2

Please sign in to comment.