Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Support adding a non-existent email as the owner of the organization #14569

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
aebad2e
fix: Support adding an non-existent email as the owner of the organiz…
hariombalhara Apr 13, 2024
f88758f
Merge remote-tracking branch 'origin/main' into 04-13-fix_support_add…
hariombalhara Apr 13, 2024
719d1c4
Merge remote-tracking branch 'origin/main' into 04-13-fix_support_add…
hariombalhara Apr 14, 2024
0f8a776
Remove dead code
hariombalhara Apr 27, 2024
e504d5e
Merge remote-tracking branch 'origin/main' into 04-13-fix_support_add…
hariombalhara Apr 27, 2024
054c0cd
fixes
hariombalhara Apr 27, 2024
af26c07
Merge remote-tracking branch 'origin/main' into 04-13-fix_support_add…
hariombalhara Apr 30, 2024
6e98912
add test
hariombalhara Apr 30, 2024
4727be2
Merge remote-tracking branch 'origin/main' into 04-13-fix_support_add…
hariombalhara Apr 30, 2024
163f152
self review changes
hariombalhara May 1, 2024
969e104
Merge branch 'main' into 04-13-fix_support_adding_an_non-existent_ema…
hariombalhara May 2, 2024
8ed5991
Merge branch 'main' into 04-13-fix_support_adding_an_non-existent_ema…
joeauyeung May 2, 2024
ecef7fb
Merge branch 'main' into 04-13-fix_support_adding_an_non-existent_ema…
joeauyeung May 2, 2024
49408c9
Update packages/trpc/server/routers/viewer/organizations/create.handl…
hariombalhara May 3, 2024
eb4cd2e
Fix typo everywhere
hariombalhara May 3, 2024
9f9a7df
Merge remote-tracking branch 'origin/04-13-fix_support_adding_an_non-…
hariombalhara May 3, 2024
7eb4d8f
Merge branch 'main' into 04-13-fix_support_adding_an_non-existent_ema…
joeauyeung May 3, 2024
87757eb
Merge branch 'main' into 04-13-fix_support_adding_an_non-existent_ema…
PeerRich May 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 4 additions & 40 deletions apps/web/playwright/organization/booking.e2e.ts
Expand Up @@ -2,7 +2,6 @@
import { expect } from "@playwright/test";

import { getOrgUsernameFromEmail } from "@calcom/features/auth/signup/utils/getOrgUsernameFromEmail";
import { getOrgFullOrigin } from "@calcom/features/ee/organizations/lib/orgDomains";
import { MembershipRole, SchedulingType } from "@calcom/prisma/enums";

import { test } from "../lib/fixtures";
Expand All @@ -14,6 +13,7 @@
testName,
} from "../lib/testUtils";
import { expectExistingUserToBeInvitedToOrganization } from "../team/expects";
import { gotoPathAndExpectRedirectToOrgDomain } from "./lib/gotoPathAndExpectRedirectToOrgDomain";
import { acceptTeamOrOrgInvite, inviteExistingUserToOrganization } from "./lib/inviteUser";

test.describe("Bookings", () => {
Expand Down Expand Up @@ -346,9 +346,9 @@
});

const inviteLink = await expectExistingUserToBeInvitedToOrganization(page, emails, invitedUserEmail);
if (!inviteLink) {
throw new Error("Invite link not found");
}

Check warning on line 351 in apps/web/playwright/organization/booking.e2e.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

apps/web/playwright/organization/booking.e2e.ts#L349-L351

[playwright/no-conditional-in-test] Avoid having conditionals in tests
const usernameInOrg = getOrgUsernameFromEmail(
invitedUserEmail,
org.organizationSettings?.orgAutoAcceptEmail ?? null
Expand Down Expand Up @@ -380,11 +380,11 @@

await test.step("Booking through old link redirects to new link on org domain", async () => {
const event = await userOutsideOrganization.getFirstEventAsOwner();
await expectRedirectToOrgDomain({
await gotoPathAndExpectRedirectToOrgDomain({
page,
org,
eventSlug: `/${usernameOutsideOrg}/${event.slug}`,
expectedEventSlug: `/${usernameInOrg}/${event.slug}`,
path: `/${usernameOutsideOrg}/${event.slug}`,
expectedPath: `/${usernameInOrg}/${event.slug}`,
});
// As the redirection correctly happens, the booking would work too which we have verified in previous step. But we can't test that with org domain as that domain doesn't exist.
});
Expand Down Expand Up @@ -450,7 +450,7 @@
const BookingTitle = `${event.title} between ${teamMate.name} and ${testName}`;
return BookingTitle === bookingTitle;
})
).toBe(true);

Check failure on line 453 in apps/web/playwright/organization/booking.e2e.ts

View workflow job for this annotation

GitHub Actions / E2E tests / E2E tests (5/7)

[@calcom/web] › apps/web/playwright/organization/booking.e2e.ts:71:9 › Bookings › Team Event › Can create a booking for Round Robin EventType

1) [@***com/web] › apps/web/playwright/organization/booking.e2e.ts:71:9 › Bookings › Team Event › Can create a booking for Round Robin EventType Error: expect(received).toBe(expected) // Object.is equality Expected: true Received: false 451 | return BookingTitle === bookingTitle; 452 | }) > 453 | ).toBe(true); | ^ 454 | } else { 455 | const BookingTitle = `${event.title} between ${team.name} and ${testName}`; 456 | await expect(page.getByTestId("booking-title")).toHaveText(BookingTitle); at bookTeamEvent (/home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/organization/booking.e2e.ts:453:7) at /home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/organization/booking.e2e.ts:106:11 at doOnOrgDomain (/home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/lib/testUtils.ts:365:3) at /home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/organization/booking.e2e.ts:100:7

Check failure on line 453 in apps/web/playwright/organization/booking.e2e.ts

View workflow job for this annotation

GitHub Actions / E2E tests / E2E tests (5/7)

[@calcom/web] › apps/web/playwright/organization/booking.e2e.ts:71:9 › Bookings › Team Event › Can create a booking for Round Robin EventType

1) [@***com/web] › apps/web/playwright/organization/booking.e2e.ts:71:9 › Bookings › Team Event › Can create a booking for Round Robin EventType Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(received).toBe(expected) // Object.is equality Expected: true Received: false 451 | return BookingTitle === bookingTitle; 452 | }) > 453 | ).toBe(true); | ^ 454 | } else { 455 | const BookingTitle = `${event.title} between ${team.name} and ${testName}`; 456 | await expect(page.getByTestId("booking-title")).toHaveText(BookingTitle); at bookTeamEvent (/home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/organization/booking.e2e.ts:453:7) at /home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/organization/booking.e2e.ts:106:11 at doOnOrgDomain (/home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/lib/testUtils.ts:365:3) at /home/runner/actions-runner/_work/***.com/***.com/apps/web/playwright/organization/booking.e2e.ts:100:7
} else {
const BookingTitle = `${event.title} between ${team.name} and ${testName}`;
await expect(page.getByTestId("booking-title")).toHaveText(BookingTitle);
Expand All @@ -463,39 +463,3 @@
await page.goto(`${url}`);
await expect(page.locator(`text=${NotFoundPageTextAppDir}`)).toBeVisible();
}

async function expectRedirectToOrgDomain({
page,
org,
eventSlug,
expectedEventSlug,
}: {
page: Page;
org: { slug: string | null };
eventSlug: string;
expectedEventSlug: string;
}) {
if (!org.slug) {
throw new Error("Org slug is not defined");
}
page.goto(eventSlug).catch((e) => {
console.log("Expected navigation error to happen");
});

const orgSlug = org.slug;

const orgRedirectUrl = await new Promise(async (resolve) => {
page.on("request", (request) => {
if (request.isNavigationRequest()) {
const requestedUrl = request.url();
console.log("Requested navigation to", requestedUrl);
// Resolve on redirection to org domain
if (requestedUrl.includes(orgSlug)) {
resolve(requestedUrl);
}
}
});
});

expect(orgRedirectUrl).toContain(`${getOrgFullOrigin(org.slug)}${expectedEventSlug}`);
}
@@ -0,0 +1,40 @@
import type { Page } from "@playwright/test";
import { expect } from "@playwright/test";

import { getOrgFullOrigin } from "@calcom/features/ee/organizations/lib/orgDomains";

export async function gotoPathAndExpectRedirectToOrgDomain({
page,
org,
path,
expectedPath,
}: {
page: Page;
org: { slug: string | null };
path: string;
expectedPath: string;
}) {
if (!org.slug) {
throw new Error("Org slug is not defined");
}
page.goto(path).catch((e) => {
console.log("Expected navigation error to happen");
});

const orgSlug = org.slug;

const orgRedirectUrl = await new Promise(async (resolve) => {
page.on("request", (request) => {
if (request.isNavigationRequest()) {
const requestedUrl = request.url();
console.log("Requested navigation to", requestedUrl);
// Resolve on redirection to org domain
if (requestedUrl.includes(orgSlug)) {
resolve(requestedUrl);
}
}
});
});

expect(orgRedirectUrl).toContain(`${getOrgFullOrigin(org.slug)}${expectedPath}`);
}
196 changes: 178 additions & 18 deletions apps/web/playwright/organization/organization-creation.e2e.ts
@@ -1,11 +1,94 @@
import type { Page } from "@playwright/test";
import { expect } from "@playwright/test";
import { JSDOM } from "jsdom";
import type { Messages } from "mailhog";
import path from "path";
import { uuid } from "short-uuid";

import { IS_TEAM_BILLING_ENABLED } from "@calcom/lib/constants";

import type { createEmailsFixture } from "../fixtures/emails";
import { test } from "../lib/fixtures";
import { fillStripeTestCheckout } from "../lib/testUtils";
import { getEmailsReceivedByUser } from "../lib/testUtils";
import { gotoPathAndExpectRedirectToOrgDomain } from "./lib/gotoPathAndExpectRedirectToOrgDomain";

async function expectEmailWithSubject(
page: Page,
emails: ReturnType<typeof createEmailsFixture>,
userEmail: string,
subject: string
) {
if (!emails) return null;

// eslint-disable-next-line playwright/no-wait-for-timeout
await page.waitForTimeout(2000);
const receivedEmails = await getEmailsReceivedByUser({ emails, userEmail });

const allEmails = (receivedEmails as Messages).items;
const email = allEmails.find((email) => email.subject === subject);
if (!email) {
throw new Error(`Email with subject ${subject} not found`);
}
const dom = new JSDOM(email.html);
return dom;
}

export async function expectOrganizationCreationEmailToBeSent({
page,
emails,
userEmail,
orgSlug,
}: {
page: Page;
emails: ReturnType<typeof createEmailsFixture>;
userEmail: string;
orgSlug: string;
}) {
const dom = await expectEmailWithSubject(page, emails, userEmail, "Your organization has been created");
const document = dom?.window?.document;
expect(document?.querySelector(`[href*=${orgSlug}]`)).toBeTruthy();
return dom;
}

async function expectOrganizationCreationEmailToBeSentWithLinks({
page,
emails,
userEmail,
oldUsername,
newUsername,
orgSlug,
}: {
page: Page;
emails: ReturnType<typeof createEmailsFixture>;
userEmail: string;
oldUsername: string;
newUsername: string;
orgSlug: string;
}) {
const dom = await expectOrganizationCreationEmailToBeSent({
page,
emails,
userEmail,
orgSlug,
});
const document = dom?.window.document;
const links = document?.querySelectorAll(`[data-testid="organization-link-info"] [href]`);
if (!links) {
throw new Error(`data-testid="organization-link-info doesn't have links`);
}
expect((links[0] as unknown as HTMLAnchorElement).href).toContain(oldUsername);
expect((links[1] as unknown as HTMLAnchorElement).href).toContain(newUsername);
}

export async function expectEmailVerificationEmailToBeSent(
page: Page,
emails: ReturnType<typeof createEmailsFixture>,
userEmail: string
) {
const subject = "Cal.com: Verify your account";
return expectEmailWithSubject(page, emails, userEmail, subject);
}

test.afterAll(({ users, orgs }) => {
users.deleteAll();
Expand All @@ -20,26 +103,33 @@ function capitalize(text: string) {
}

test.describe("Organization", () => {
test("Admin should be able to create an org for a target user", async ({ page, users, emails }) => {
test("Admin should be able to create an org where an existing user is made an owner", async ({
page,
users,
emails,
}) => {
const appLevelAdmin = await users.create({
role: "ADMIN",
});
await appLevelAdmin.apiLogin();
const stringUUID = uuid();

const orgOwnerUsername = `owner-${stringUUID}`;
const orgOwnerUsernamePrefix = "owner";

const targetOrgEmail = users.trackEmail({
username: orgOwnerUsername,
const orgOwnerEmail = users.trackEmail({
username: orgOwnerUsernamePrefix,
domain: `example.com`,
});

const orgOwnerUser = await users.create({
username: orgOwnerUsername,
email: targetOrgEmail,
username: orgOwnerUsernamePrefix,
email: orgOwnerEmail,
role: "ADMIN",
});

const orgName = capitalize(`${orgOwnerUsername}`);
const orgOwnerUsernameOutsideOrg = orgOwnerUser.username;
const orgOwnerUsernameInOrg = orgOwnerEmail.split("@")[0];
const orgName = capitalize(`${orgOwnerUser.username}`);
const orgSlug = `myOrg-${uuid()}`.toLowerCase();
await page.goto("/settings/organizations/new");
await page.waitForLoadState("networkidle");

Expand All @@ -49,17 +139,16 @@ test.describe("Organization", () => {
await expect(page.locator(".text-red-700")).toHaveCount(3);

// Happy path
await page.locator("input[name=orgOwnerEmail]").fill(targetOrgEmail);
// Since we are admin fill in this infomation instead of deriving it
await page.locator("input[name=name]").fill(orgName);
await page.locator("input[name=slug]").fill(orgOwnerUsername);

// Fill in seat infomation
await page.locator("input[name=seats]").fill("30");
await page.locator("input[name=pricePerSeat]").fill("30");
await fillAndSubmitFirstStepAsAdmin(page, orgOwnerEmail, orgName, orgSlug);
});

await page.locator("button[type=submit]").click();
await page.waitForLoadState("networkidle");
await expectOrganizationCreationEmailToBeSentWithLinks({
page,
emails,
userEmail: orgOwnerEmail,
oldUsername: orgOwnerUsernameOutsideOrg || "",
newUsername: orgOwnerUsernameInOrg,
orgSlug,
});

await test.step("About the organization", async () => {
Expand Down Expand Up @@ -149,6 +238,56 @@ test.describe("Organization", () => {

await expect(upgradeButtonHidden).toBeHidden();
});

// Verify that the owner's old username redirect is properly set
await gotoPathAndExpectRedirectToOrgDomain({
page,
org: {
slug: orgSlug,
},
path: `/${orgOwnerUsernameOutsideOrg}`,
expectedPath: `/${orgOwnerUsernameInOrg}`,
});
});

test("Admin should be able to create an org where the owner doesn't exist yet", async ({
page,
users,
emails,
}) => {
const appLevelAdmin = await users.create({
role: "ADMIN",
});
await appLevelAdmin.apiLogin();
const orgOwnerUsername = `owner`;
const orgName = capitalize(`${orgOwnerUsername}`);
const orgSlug = `myOrg-${uuid()}`.toLowerCase();
const orgOwnerEmail = users.trackEmail({
username: orgOwnerUsername,
domain: `example.com`,
});

await page.goto("/settings/organizations/new");
await page.waitForLoadState("networkidle");

await test.step("Basic info", async () => {
// Check required fields
await page.locator("button[type=submit]").click();
await expect(page.locator(".text-red-700")).toHaveCount(3);

// Happy path
await fillAndSubmitFirstStepAsAdmin(page, orgOwnerEmail, orgName, orgSlug);
});

const dom = await expectOrganizationCreationEmailToBeSent({
page,
emails,
userEmail: orgOwnerEmail,
orgSlug,
});
expect(dom?.window.document.querySelector(`[href*=${orgSlug}]`)).toBeTruthy();
await expectEmailVerificationEmailToBeSent(page, emails, orgOwnerEmail);
// Rest of the steps remain same as org creation with existing user as owner. So skipping them
});

test("User can create and upgrade a org", async ({ page, users, emails }) => {
Expand Down Expand Up @@ -426,3 +565,24 @@ test.describe("Organization", () => {
});
});
});

async function fillAndSubmitFirstStepAsAdmin(
page: Page,
targetOrgEmail: string,
orgName: string,
orgSlug: string
) {
await page.locator("input[name=orgOwnerEmail]").fill(targetOrgEmail);
// Since we are admin fill in this infomation instead of deriving it
await page.locator("input[name=name]").fill(orgName);
await page.locator("input[name=slug]").fill(orgSlug);

// Fill in seat infomation
await page.locator("input[name=seats]").fill("30");
await page.locator("input[name=pricePerSeat]").fill("30");

await Promise.all([
page.waitForResponse("**/api/trpc/organizations/create**"),
page.locator("button[type=submit]").click(),
]);
}