fix: Reschedule URL for an org migrated/moved user's booking. (#15736)

* Fix reschedule URl

* Add e2e for org

* Add team booking reschedule test

* Add unit tests

* Remove unnecessary select

---------

Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
This commit is contained in:
Hariom Balhara
2024-07-31 14:25:46 +02:00
committed by GitHub
co-authored by Udit Takkar
parent 63c7e4b55a
commit 5b7f2a6ffc
9 changed files with 442 additions and 31 deletions
+17 -10
View File
@@ -4,8 +4,10 @@ import { URLSearchParams } from "url";
import { z } from "zod";
import { getServerSession } from "@calcom/features/auth/lib/getServerSession";
import { buildEventUrlFromBooking } from "@calcom/lib/bookings/buildEventUrlFromBooking";
import { getDefaultEvent } from "@calcom/lib/defaultEvents";
import { maybeGetBookingUidFromSeat } from "@calcom/lib/server/maybeGetBookingUidFromSeat";
import { UserRepository } from "@calcom/lib/server/repository/user";
import prisma, { bookingMinimalSelect } from "@calcom/prisma";
import { BookingStatus } from "@calcom/prisma/client";
@@ -25,6 +27,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
prisma,
bookingUid
);
const booking = await prisma.booking.findUnique({
where: {
uid,
@@ -41,6 +44,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
slug: true,
team: {
select: {
parentId: true,
slug: true,
},
},
@@ -125,16 +129,19 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
const eventType = booking.eventType ? booking.eventType : getDefaultEvent(dynamicEventSlugRef);
const eventPage = `${
eventType.team
? `team/${eventType.team.slug}`
: dynamicEventSlugRef
? booking.dynamicGroupSlugRef
: booking.user?.username || "rick" /* This shouldn't happen */
}/${eventType?.slug}`;
const destinationUrl = new URLSearchParams();
const enrichedBookingUser = booking.user
? await UserRepository.enrichUserWithItsProfile({ user: booking.user })
: null;
destinationUrl.set("rescheduleUid", seatReferenceUid || bookingUid);
const eventUrl = await buildEventUrlFromBooking({
eventType,
dynamicGroupSlugRef: booking.dynamicGroupSlugRef ?? null,
profileEnrichedBookingUser: enrichedBookingUser,
});
const destinationUrlSearchParams = new URLSearchParams();
destinationUrlSearchParams.set("rescheduleUid", seatReferenceUid || bookingUid);
// TODO: I think we should just forward all the query params here including coep flag
if (coepFlag) {
@@ -142,7 +149,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
}
return {
redirect: {
destination: `/${eventPage}?${destinationUrl.toString()}${
destination: `${eventUrl}?${destinationUrlSearchParams.toString()}${
eventType.seatsPerTimeSlot ? "&bookingUid=null" : ""
}`,
permanent: false,
+18 -3
View File
@@ -709,6 +709,13 @@ const createUserFixture = (user: UserWithIncludes, page: Page) => {
},
delete: async () => await prisma.user.delete({ where: { id: store.user.id } }),
confirmPendingPayment: async () => confirmPendingPayment(store.page),
getFirstProfile: async () => {
return prisma.profile.findFirstOrThrow({
where: {
userId: user.id,
},
});
},
};
};
@@ -738,6 +745,7 @@ type CustomUserOpts = Partial<Pick<Prisma.User, CustomUserOptsKeys>> & {
schedule?: Schedule;
password?: string | null;
emailDomain?: string;
profileUsername?: string;
};
// creates the actual user in the db.
@@ -749,11 +757,12 @@ const createUser = (
})
| null
): PrismaType.UserUncheckedCreateInput => {
const suffixToMakeUsernameUnique = `-${workerInfo.workerIndex}-${Date.now()}`;
// build a unique name for our user
const uname =
opts?.useExactUsername && opts?.username
? opts.username
: `${opts?.username || "user"}-${workerInfo.workerIndex}-${Date.now()}`;
: `${opts?.username || "user"}${suffixToMakeUsernameUnique}`;
const emailDomain = opts?.emailDomain || "example.com";
return {
@@ -772,7 +781,11 @@ const createUser = (
role: opts?.role ?? "USER",
twoFactorEnabled: opts?.twoFactorEnabled ?? false,
disableImpersonation: opts?.disableImpersonation ?? false,
...getOrganizationRelatedProps({ organizationId: opts?.organizationId, role: opts?.roleInOrganization }),
...getOrganizationRelatedProps({
organizationId: opts?.organizationId,
role: opts?.roleInOrganization,
profileUsername: opts?.profileUsername,
}),
schedules:
opts?.completedOnboarding ?? true
? {
@@ -792,9 +805,11 @@ const createUser = (
function getOrganizationRelatedProps({
organizationId,
role,
profileUsername,
}: {
organizationId: number | null | undefined;
role: MembershipRole | undefined;
profileUsername?: string;
}) {
if (!organizationId) {
return null;
@@ -807,7 +822,7 @@ const createUser = (
profiles: {
create: {
uid: ProfileRepository.generateProfileUid(),
username: uname,
username: profileUsername ? `${profileUsername}${suffixToMakeUsernameUnique}` : uname,
organization: {
connect: {
id: organizationId,
+34 -3
View File
@@ -1,5 +1,5 @@
import type { Frame, Page } from "@playwright/test";
import { expect } from "@playwright/test";
import type { Frame, Page, Request as PlaywrightRequest } from "@playwright/test";
import { createHash } from "crypto";
import EventEmitter from "events";
import type { IncomingMessage, ServerResponse } from "http";
@@ -352,20 +352,51 @@ export async function fillStripeTestCheckout(page: Page) {
await page.click(".SubmitButton--complete-Shimmer");
}
export function goToUrlWithErrorHandling({ page, url }: { page: Page; url: string }) {
return new Promise<{ success: boolean; url: string }>(async (resolve) => {
const onRequestFailed = (request: PlaywrightRequest) => {
const failedToLoadUrl = request.url();
console.log("goToUrlWithErrorHandling: Failed to load URL:", failedToLoadUrl);
resolve({ success: false, url: failedToLoadUrl });
};
page.on("requestfailed", onRequestFailed);
try {
await page.goto(url);
} catch (e) {}
page.off("requestfailed", onRequestFailed);
resolve({ success: true, url: page.url() });
});
}
/**
* Within this function's callback if a non-org domain is opened, it is considered an org domain identfied from `orgSlug`
*/
export async function doOnOrgDomain(
{ orgSlug, page }: { orgSlug: string | null; page: Page },
callback: ({ page }: { page: Page }) => Promise<void>
callback: ({
page,
}: {
page: Page;
goToUrlWithErrorHandling: (url: string) => ReturnType<typeof goToUrlWithErrorHandling>;
}) => Promise<any>
) {
if (!orgSlug) {
throw new Error("orgSlug is not available");
}
page.setExtraHTTPHeaders({
"x-cal-force-slug": orgSlug,
});
await callback({ page });
const callbackResult = await callback({
page,
goToUrlWithErrorHandling: (url: string) => {
return goToUrlWithErrorHandling({ page, url });
},
});
await page.setExtraHTTPHeaders({
"x-cal-force-slug": "",
});
return callbackResult;
}
// 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.
+114 -12
View File
@@ -2,11 +2,17 @@ import { expect } from "@playwright/test";
import dayjs from "@calcom/dayjs";
import prisma from "@calcom/prisma";
import { MembershipRole } from "@calcom/prisma/client";
import { BookingStatus } from "@calcom/prisma/enums";
import { bookingMetadataSchema } from "@calcom/prisma/zod-utils";
import { test } from "./lib/fixtures";
import { selectFirstAvailableTimeSlotNextMonth, bookTimeSlot } from "./lib/testUtils";
import {
selectFirstAvailableTimeSlotNextMonth,
bookTimeSlot,
doOnOrgDomain,
goToUrlWithErrorHandling,
} from "./lib/testUtils";
const IS_STRIPE_ENABLED = !!(
process.env.STRIPE_CLIENT_ID &&
@@ -50,11 +56,11 @@ test.describe("Reschedule Tests", async () => {
const user = await users.create();
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const booking = await bookings.create(user.id, user.username, user.eventTypes[0].id!, {
status: BookingStatus.CANCELLED,
status: BookingStatus.ACCEPTED,
rescheduled: true,
});
await page.goto(`/${user.username}/${user.eventTypes[0].slug}?rescheduleUid=${booking.uid}`);
await page.goto(`/reschedule/${booking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
@@ -82,11 +88,11 @@ test.describe("Reschedule Tests", async () => {
const user = await users.create();
const [eventType] = user.eventTypes;
const booking = await bookings.create(user.id, user.username, eventType.id, {
status: BookingStatus.CANCELLED,
status: BookingStatus.ACCEPTED,
rescheduled: true,
});
await page.goto(`/${user.username}/${eventType.slug}?rescheduleUid=${booking.uid}`);
await page.goto(`/reschedule/${booking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
@@ -124,7 +130,7 @@ test.describe("Reschedule Tests", async () => {
const eventType = user.eventTypes.find((e) => e.slug === "paid")!;
const booking = await bookings.create(user.id, user.username, eventType.id, {
rescheduled: true,
status: BookingStatus.CANCELLED,
status: BookingStatus.ACCEPTED,
paid: false,
});
await prisma.eventType.update({
@@ -144,7 +150,7 @@ test.describe("Reschedule Tests", async () => {
},
});
const payment = await payments.create(booking.id);
await page.goto(`/${user.username}/${eventType.slug}?rescheduleUid=${booking.uid}`);
await page.goto(`/reschedule/${booking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
@@ -166,12 +172,12 @@ test.describe("Reschedule Tests", async () => {
const eventType = user.eventTypes.find((e) => e.slug === "paid")!;
const booking = await bookings.create(user.id, user.username, eventType.id, {
rescheduled: true,
status: BookingStatus.CANCELLED,
status: BookingStatus.ACCEPTED,
paid: true,
});
const payment = await payments.create(booking.id);
await page.goto(`/${user?.username}/${eventType?.slug}?rescheduleUid=${booking?.uid}`);
await page.goto(`/reschedule/${booking?.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
@@ -188,7 +194,7 @@ test.describe("Reschedule Tests", async () => {
status: BookingStatus.ACCEPTED,
});
await page.goto(`/${user.username}/${eventType.slug}?rescheduleUid=${booking.uid}`);
await page.goto(`/reschedule/${booking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
@@ -210,7 +216,7 @@ test.describe("Reschedule Tests", async () => {
});
await user.apiLogin();
await page.goto(`/${user.username}/${eventType.slug}?rescheduleUid=${booking.uid}`);
await page.goto(`/reschedule/${booking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
@@ -271,6 +277,7 @@ test.describe("Reschedule Tests", async () => {
await page.locator('[data-testid="confirm-reschedule-button"]').click();
await expect(page).toHaveURL(/.*booking/);
});
test("Should load Valid Cal video url after rescheduling Opt in events", async ({
page,
users,
@@ -304,7 +311,7 @@ test.describe("Reschedule Tests", async () => {
if (currentBooking) {
await confirmBooking(currentBooking.id);
await page.goto(`/${user.username}/${eventType.slug}?rescheduleUid=${currentBooking.uid}`);
await page.goto(`/reschedule/${currentBooking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
await page.locator('[data-testid="confirm-reschedule-button"]').click();
@@ -331,4 +338,99 @@ test.describe("Reschedule Tests", async () => {
}
}
});
test("Should be able to a dynamic group booking", async () => {
// It is tested in dynamic-booking-pages.e2e.ts
});
test("Team Event Booking", () => {
// It is tested in teams.e2e.ts
});
test.describe("Organization", () => {
test("Booking should be rescheduleable for a user that was moved to an organization", async ({
users,
bookings,
orgs,
page,
}) => {
const org = await orgs.create({
name: "TestOrg",
});
const orgMember = await users.create({
username: "username-outside-org",
organizationId: org.id,
profileUsername: "username-inside-org",
roleInOrganization: MembershipRole.MEMBER,
});
const profileUsername = (await orgMember.getFirstProfile()).username;
const eventType = orgMember.eventTypes[0];
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const orgSlug = org.slug!;
const getNonOrgUrlFromOrgUrl = (url: string) => url.replace(orgSlug, "app");
await test.step("Try rescheduling through org domain", async () => {
const booking = await bookings.create(orgMember.id, orgMember.username, eventType.id);
return await doOnOrgDomain(
{
orgSlug: orgSlug,
page,
},
async ({ page, goToUrlWithErrorHandling }) => {
const result = await goToUrlWithErrorHandling(`/reschedule/${booking.uid}`);
// Verify that the reschedule page was opened on the org domain with correct username
expectUrlToBeABookingPageOnOrgForUsername({
url: result.url,
orgSlug,
username: profileUsername,
});
const rescheduleUrlToBeOpenedInOrgContext = getNonOrgUrlFromOrgUrl(result.url);
await page.goto(rescheduleUrlToBeOpenedInOrgContext);
await expectSuccessfulReschedule();
return { url: result.url };
}
);
});
await test.step("Try rescheduling through non-org domain", async () => {
const booking = await bookings.create(orgMember.id, orgMember.username, eventType.id);
// Opening the non-org URL and expecting a redirect to the org domain by reschedule endpoint
const result = await goToUrlWithErrorHandling({ url: `/reschedule/${booking.uid}`, page });
await doOnOrgDomain(
{
orgSlug: orgSlug,
page,
},
async ({ page }) => {
await page.goto(getNonOrgUrlFromOrgUrl(result.url));
await expectSuccessfulReschedule();
}
);
});
async function expectSuccessfulReschedule() {
await selectFirstAvailableTimeSlotNextMonth(page);
await page.locator("[data-testid=confirm-reschedule-button]").click();
await expect(page.locator("[data-testid=success-page]")).toBeVisible();
}
});
});
});
function expectUrlToBeABookingPageOnOrgForUsername({
url,
orgSlug,
username,
}: {
url: string;
orgSlug: string;
username: string;
}) {
expect(url).toContain(`://${orgSlug}.`);
const urlObject = new URL(url);
const usernameInUrl = urlObject.pathname.split("/")[1];
expect(usernameInUrl).toEqual(username);
}
+26 -1
View File
@@ -321,6 +321,31 @@ testBothFutureAndLegacyRoutes.describe("Teams - NonOrg", (routeVariant) => {
});
todo("Create a Round Robin with different leastRecentlyBooked hosts");
todo("Reschedule a Collective EventType booking");
test("Reschedule a Collective EventType booking", async ({ users, page, bookings }) => {
const teamMatesObj = [
{ name: "teammate-1" },
{ name: "teammate-2" },
{ name: "teammate-3" },
{ name: "teammate-4" },
];
const owner = await users.create(
{ username: "pro-user", name: "pro-user" },
{
hasTeam: true,
teammates: teamMatesObj,
schedulingType: SchedulingType.COLLECTIVE,
}
);
const { team } = await owner.getFirstTeamMembership();
const eventType = await owner.getFirstTeamEvent(team.id);
const booking = await bookings.create(owner.id, owner.username, eventType.id);
await page.goto(`/reschedule/${booking.uid}`);
await selectFirstAvailableTimeSlotNextMonth(page);
await page.locator("[data-testid=confirm-reschedule-button]").click();
await expect(page.locator("[data-testid=success-page]")).toBeVisible();
});
todo("Reschedule a Round Robin EventType booking");
});
+6
View File
@@ -5,6 +5,7 @@ import type * as constants from "@calcom/lib/constants";
const mockedConstants = {
IS_PRODUCTION: false,
IS_TEAM_BILLING_ENABLED: false,
WEBSITE_URL: "",
} as typeof constants;
vi.mock("@calcom/lib/constants", () => {
@@ -25,4 +26,9 @@ export const constantsScenarios = {
// @ts-ignore
mockedConstants.IS_TEAM_BILLING_ENABLED = true;
},
setWebsiteUrl: (url: string) => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
mockedConstants.WEBSITE_URL = url;
},
};
@@ -0,0 +1,168 @@
import { constantsScenarios } from "@calcom/lib/__mocks__/constants";
import { describe, it, vi, expect, beforeEach } from "vitest";
import { getBrand } from "@calcom/lib/server/getBrand";
import { buildEventUrlFromBooking } from "./buildEventUrlFromBooking";
vi.mock("@calcom/lib/server/getBrand", () => ({
getBrand: vi.fn(),
}));
const WEBSITE_URL = "https://buildEventTest.example";
beforeEach(() => {
constantsScenarios.setWebsiteUrl(WEBSITE_URL);
});
describe("buildEventUrlFromBooking", () => {
describe("Non Organization", () => {
it("should correctly build the event URL for a team event booking", async () => {
const booking = {
eventType: {
slug: "30min",
team: {
slug: "engineering",
parentId: 123,
},
},
profileEnrichedBookingUser: {
profile: {
organizationId: null,
username: "john",
},
},
dynamicGroupSlugRef: null,
};
const expectedUrl = `${WEBSITE_URL}/team/engineering/30min`;
const result = await buildEventUrlFromBooking(booking);
expect(result).toBe(expectedUrl);
});
it("should correctly build the event URL for a dynamic group booking", async () => {
const booking = {
eventType: {
slug: "30min",
team: null,
},
profileEnrichedBookingUser: {
profile: {
organizationId: null,
username: "john",
},
},
dynamicGroupSlugRef: "john+jane",
};
const expectedUrl = `${WEBSITE_URL}/john+jane/30min`;
const result = await buildEventUrlFromBooking(booking);
expect(result).toBe(expectedUrl);
});
it("should correctly build the event URL for a personal booking", async () => {
const booking = {
eventType: {
slug: "30min",
team: null,
},
profileEnrichedBookingUser: {
profile: {
organizationId: null,
username: "john",
},
},
dynamicGroupSlugRef: null,
};
const expectedUrl = `${WEBSITE_URL}/john/30min`;
const result = await buildEventUrlFromBooking(booking);
expect(result).toBe(expectedUrl);
});
});
describe("Organization", () => {
const organizationId = 123;
const orgOrigin = "https://acme.cal.local";
beforeEach(() => {
getBrand.mockResolvedValue({
fullDomain: orgOrigin,
});
});
it("should correctly build the event URL for a team event booking", async () => {
const booking = {
eventType: {
slug: "30min",
team: {
slug: "engineering",
parentId: 123,
},
},
profileEnrichedBookingUser: {
profile: {
organizationId,
username: "john",
},
},
dynamicGroupSlugRef: null,
};
const expectedUrl = `${orgOrigin}/team/engineering/30min`;
const result = await buildEventUrlFromBooking(booking);
expect(result).toBe(expectedUrl);
});
it("should correctly build the event URL for a dynamic group booking", async () => {
const booking = {
eventType: {
slug: "30min",
team: null,
},
profileEnrichedBookingUser: {
profile: {
organizationId,
username: "john",
},
},
dynamicGroupSlugRef: "john+jane",
};
const expectedUrl = `${orgOrigin}/john+jane/30min`;
const result = await buildEventUrlFromBooking(booking);
expect(result).toBe(expectedUrl);
});
it("should correctly build the event URL for a personal booking", async () => {
const booking = {
eventType: {
slug: "30min",
team: null,
},
profileEnrichedBookingUser: {
profile: {
organizationId,
username: "john",
},
},
dynamicGroupSlugRef: null,
};
const expectedUrl = `${orgOrigin}/john/30min`;
const result = await buildEventUrlFromBooking(booking);
expect(result).toBe(expectedUrl);
});
});
it("should throw if the username isn't set", async () => {
const booking = {
eventType: {
slug: "30min",
team: null,
},
profileEnrichedBookingUser: {
profile: {
organizationId: null,
username: null,
},
},
dynamicGroupSlugRef: null,
};
await expect(() => buildEventUrlFromBooking(booking)).rejects.toThrow(
"No username found for booking user."
);
});
});
@@ -0,0 +1,56 @@
import { getBookerBaseUrl } from "@calcom/lib/getBookerUrl/server";
import { safeStringify } from "@calcom/lib/safeStringify";
import logger from "../logger";
type BookingEventType = {
slug: string;
team: {
slug: string | null;
parentId: number | null;
} | null;
};
/**
* It has its profile always set and if organizationId is null, then username would be regular(non-org) username.
*/
type ProfileEnrichedBookingUser = {
profile: { organizationId: number | null; username: string | null };
} | null;
export function getOrganizationIdOfBooking(booking: {
eventType: BookingEventType;
profileEnrichedBookingUser: ProfileEnrichedBookingUser;
}) {
const { eventType, profileEnrichedBookingUser } = booking;
return eventType.team
? eventType.team.parentId
: profileEnrichedBookingUser?.profile.organizationId ?? null;
}
export async function buildEventUrlFromBooking(booking: {
eventType: BookingEventType;
profileEnrichedBookingUser: ProfileEnrichedBookingUser;
dynamicGroupSlugRef: string | null;
}) {
const { eventType, dynamicGroupSlugRef, profileEnrichedBookingUser } = booking;
const eventSlug = eventType.slug;
const eventTeam = eventType.team;
const bookingOrganizationId = getOrganizationIdOfBooking({ eventType, profileEnrichedBookingUser });
const bookerUrl = await getBookerBaseUrl(bookingOrganizationId);
if (dynamicGroupSlugRef) {
return `${bookerUrl}/${dynamicGroupSlugRef}/${eventSlug}`;
}
if (eventTeam?.slug) {
return `${bookerUrl}/team/${eventTeam.slug}/${eventSlug}`;
}
const username = profileEnrichedBookingUser?.profile?.username;
if (!username) {
logger.error("No username found for booking user.", safeStringify({ profileEnrichedBookingUser }));
throw new Error("No username found for booking user.");
}
return `${bookerUrl}/${username}/${eventSlug}`;
}
+3 -2
View File
@@ -311,10 +311,11 @@ export class UserRepository {
}
/**
* Use this method if you don't directly has the profileId.
* It can happen in two cases:
* Use this method instead of `enrichUserWithTheProfile` if you don't directly have the profileId.
* It can happen in following cases:
* 1. While dealing with a User that hasn't been added to any organization yet and thus have no Profile entries.
* 2. While dealing with a User that has been moved to a Profile i.e. he was invited to an organization when he was an existing user.
* 3. We haven't added profileId to all the entities, so they aren't aware of which profile they belong to. So, we still mostly use this function to enrich the user with its profile.
*/
static async enrichUserWithItsProfile<T extends { id: number; username: string | null }>({
user,