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

chore: app router - /team, /org, /[user] booking pages (excl. embeds) #18186

Open
wants to merge 61 commits into
base: main
Choose a base branch
from

Conversation

hbjORbj
Copy link
Contributor

@hbjORbj hbjORbj commented Dec 14, 2024

What does this PR do?

  • Fixes CAL-4898
  • Left a self review and comments

Tested /team sub-pages

test.mov

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A - I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Test every booking page (user, team, org)

Copy link

linear bot commented Dec 14, 2024

@hbjORbj hbjORbj marked this pull request as draft December 14, 2024 18:52
@graphite-app graphite-app bot requested a review from a team December 14, 2024 18:52
@github-actions github-actions bot added the ❗️ .env changes contains changes to env variables label Dec 14, 2024
@dosubot dosubot bot added the teams area: teams, round robin, collective, managed event-types label Dec 14, 2024
@keithwillcode keithwillcode added consumer core area: core, team members only labels Dec 14, 2024
Copy link

graphite-app bot commented Dec 14, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/14/24)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link

vercel bot commented Dec 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Dec 31, 2024 6:40pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Dec 31, 2024 6:40pm

@hbjORbj hbjORbj changed the title chore: app router - /team sub-pages chore: app router - /team and /org sub-pages Dec 18, 2024
@hbjORbj hbjORbj requested a review from a team December 27, 2024 20:16
@@ -103,3 +104,22 @@ export const _generateMetadata = async (
},
};
};

export const generateMeetingMetadata = async (
Copy link
Contributor Author

@hbjORbj hbjORbj Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key difference is const image = SEO_IMG_OGIMG + constructMeetingImage(meeting);. Using constructMeetingImage is important to generate correct OG images.

Comment on lines +53 to +54
follow: !(event?.hidden || !isSEOIndexable),
index: !(event?.hidden || !isSEOIndexable),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • noindex and nofollow are not supported in App Router. We need to use follow and index instead.

@@ -249,7 +249,6 @@
"APP_ROUTER_AUTH_ERROR_ENABLED",
"APP_ROUTER_AUTH_PLATFORM_ENABLED",
"APP_ROUTER_AUTH_OAUTH2_ENABLED",
"APP_ROUTER_TEAM_ENABLED",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that we do not have env variables for user or org

@@ -20,13 +19,6 @@ export default function Type({
}: PageProps) {
return (
<main className={getBookerWrapperClasses({ isEmbed: !!isEmbed })}>
<BookerSeo
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BookerSeo is not needed anymore as we handle the metadata using app router metadata API (by exporting generateMetadata funtion)

const searchParams = useSearchParams();
const { profile, users, hidden, title } = eventData;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer used

@@ -11,6 +11,8 @@ import {
selectSecondAvailableTimeSlotNextMonth,
} from "./lib/testUtils";

test.describe.configure({ mode: "parallel" });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small optimization

@@ -66,7 +66,7 @@ async function verifyRobotsMetaTag({ page, orgSlug, urls, expectedContent }: Ver
await doOnOrgDomain({ orgSlug, page }, async ({ page, goToUrlWithErrorHandling }) => {
for (const relativeUrl of urls) {
const { url } = await goToUrlWithErrorHandling(relativeUrl);
const metaTag = await page.locator('head > meta[name="robots"]');
const metaTag = await page.locator('head > meta[name="robots"]').first();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we didn't completely deprecate NextSeo yet (we can completely deprecate once we are 100% done with migration), we can have two metadata tags with robots in the html.

@@ -132,6 +133,7 @@ export const getServerSideProps: GetServerSideProps<UserPageProps> = async (cont
const [user] = usersInOrgContext; //to be used when dealing with single user, not dynamic group

const profile = {
id: user.id,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing id here to get user's avatar url (needed for OG image generation in generateMetadata function)

@hbjORbj hbjORbj added organizations area: organizations, orgs booking-page area: booking page, public booking page, booker labels Dec 27, 2024
@hbjORbj hbjORbj force-pushed the chore/app-router-team-pages branch from a55294c to 876ea41 Compare December 28, 2024 00:36
@hbjORbj
Copy link
Contributor Author

hbjORbj commented Dec 28, 2024

Some notes to make your review easier: @zomars @emrysal

  • All pages in /pages/team, /pages/org and /pages/[user] except embeds are migrated to App Router. Embed pages will still be rendered by Pages Router because the embed pages in App Router are still inside /future directory (We plan to migrate embed pages separately).
  • Since metadata is handled by App Router's built-in Metadata API (generateMetadata function), I removed (A) HeadSeo from booking client components (d-type-view, /team/team-view, /team/type-view, etc) and (B) BookerSeo entirely.
  • Don't be confused: d/[link]/[slug] is already migrated to App Router in Production. In this PR, I simply remove HeadSeo from its client component as this is now handled by App router's Metadata API.

Comment on lines +29 to 33
const isEmbedSnippetGeneratorPath = pathname.startsWith("/event-types");
const isEmbed =
(pathname.endsWith("/embed") || (searchParams?.get("embedType") ?? null) !== null) &&
!isEmbedSnippetGeneratorPath;
const embedColorScheme = searchParams?.get("ui.color-scheme");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I synced this file with _document.tsx in pages router

@hbjORbj hbjORbj changed the title chore: app router - /team, /org, /[user] booking pages chore: app router - /team, /org, /[user] booking pages except embed pages Dec 28, 2024
@hbjORbj hbjORbj changed the title chore: app router - /team, /org, /[user] booking pages except embed pages chore: app router - /team, /org, /[user] booking pages (excl. embed pages) Dec 28, 2024
@hbjORbj hbjORbj changed the title chore: app router - /team, /org, /[user] booking pages (excl. embed pages) chore: app router - /team, /org, /[user] booking pages (excl. embeds) Dec 28, 2024
@hbjORbj hbjORbj marked this pull request as ready for review December 30, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
booking-page area: booking page, public booking page, booker consumer core area: core, team members only ❗️ .env changes contains changes to env variables high-risk Requires approval by Foundation team organizations area: organizations, orgs ready-for-e2e teams area: teams, round robin, collective, managed event-types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants