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: migrate /apps/[slug] page #18848

Merged
merged 2 commits into from
Jan 24, 2025
Merged

chore: migrate /apps/[slug] page #18848

merged 2 commits into from
Jan 24, 2025

Conversation

hbjORbj
Copy link
Contributor

@hbjORbj hbjORbj commented Jan 23, 2025

What does this PR do?

  • Migrate /apps/[slug] to App Router

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 /apps/[slug] pages

@graphite-app graphite-app bot requested a review from a team January 23, 2025 22:25
@github-actions github-actions bot added the ❗️ .env changes contains changes to env variables label Jan 23, 2025
Copy link

vercel bot commented Jan 23, 2025

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 Jan 23, 2025 11:01pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Jan 23, 2025 11:01pm

@keithwillcode keithwillcode added consumer core area: core, team members only labels Jan 23, 2025
@hbjORbj hbjORbj changed the title chore: migrate apps slug page chore: migrate /apps/[slug] page Jan 23, 2025
@@ -10,8 +8,6 @@ import { getAppAssetFullPath } from "@calcom/app-store/getAppAssetFullPath";
import { IS_PRODUCTION } from "@calcom/lib/constants";
import prisma from "@calcom/prisma";

const md = new MarkdownIt("default", { html: true, breaks: true });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wasn't being used

Copy link

graphite-app bot commented Jan 23, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (01/23/25)

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

Copy link
Contributor

github-actions bot commented Jan 23, 2025

E2E results are ready!

Comment on lines +28 to +32
return await generateAppMetadata(
{ slug: logo, name, description },
() => name,
() => description
);
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you passing logo to slug property? there is already slug property in props.data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2025-01-24 at 9 28 17 AM
  • We are replacing HeadSeo with generateMetadata
  • we need it for generating OG image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eunjae-lee
Copy link
Contributor

Looks good but waiting for Benny's reply to Udit's comment.

@hbjORbj hbjORbj enabled auto-merge (squash) January 24, 2025 14:30
Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

Great. Let's go!

@hbjORbj hbjORbj merged commit beb542e into main Jan 24, 2025
42 checks passed
@hbjORbj hbjORbj deleted the chore/migrate-apps-slug-page branch January 24, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consumer core area: core, team members only ❗️ .env changes contains changes to env variables ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants