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 - /bookings status page #18183

Merged
merged 12 commits into from
Dec 23, 2024
Merged

Conversation

hbjORbj
Copy link
Contributor

@hbjORbj hbjORbj commented Dec 14, 2024

What does this PR do?

  • Fixes CAL-4894

Tested

Screen.Recording.2024-12-20.at.15.37.04.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?

  • Please use the latest Vercel preview and test please 🙏

Copy link

linear bot commented Dec 14, 2024

@graphite-app graphite-app bot requested a review from a team December 14, 2024 18:09
@github-actions github-actions bot added the ❗️ .env changes contains changes to env variables label Dec 14, 2024
@dosubot dosubot bot added the bookings area: bookings, availability, timezones, double booking 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
Contributor

github-actions bot commented Dec 14, 2024

E2E results are ready!

@hbjORbj hbjORbj marked this pull request as ready for review December 17, 2024 16:54
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 21, 2024 4:28pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Dec 21, 2024 4:28pm

@eunjae-lee
Copy link
Contributor

e2e broken

@hbjORbj hbjORbj marked this pull request as draft December 20, 2024 17:24
auto-merge was automatically disabled December 20, 2024 17:24

Pull request was converted to draft

@hbjORbj
Copy link
Contributor Author

hbjORbj commented Dec 20, 2024

@eunjae-lee addressed!

const Page = async ({ params, searchParams }: PageProps) => {
const parsed = querySchema.safeParse({ ...params, ...searchParams });
if (!parsed.success) {
redirect("/bookings/upcoming");
Copy link
Contributor Author

@hbjORbj hbjORbj Dec 20, 2024

Choose a reason for hiding this comment

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

  • In Production, we currently take user to 404 if they go to URL like /bookings/123 because 123 is not a valid status (upcoming, cancelled, etc).
  • With this change, we redirect user to /bookings/upcoming instead
  • To achieve this, we should remove export const dynamic = "force-static"; from this route. So I removed it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we don't have dynamic = "force-static", does this mean this page is server rendered every time? Or Next.js still builds it statically because of generateStaticParams? I'd appreciate your explaination as I'm not familiar enough with these to understand the behavior 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or Next.js still builds it statically because of generateStaticParams?

This is absolutely correct. With dynamic = "force-static", user will get routed to 404 if status is not one of those returned by generateStaticParams before we have chance to redirect them to /bookings/upcoming.

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.

The code change is much cleaner ✨

const Page = async ({ params, searchParams }: PageProps) => {
const parsed = querySchema.safeParse({ ...params, ...searchParams });
if (!parsed.success) {
redirect("/bookings/upcoming");
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we don't have dynamic = "force-static", does this mean this page is server rendered every time? Or Next.js still builds it statically because of generateStaticParams? I'd appreciate your explaination as I'm not familiar enough with these to understand the behavior 👀

@hbjORbj hbjORbj merged commit 542799e into main Dec 23, 2024
38 checks passed
@hbjORbj hbjORbj deleted the chore/bookings-app-router branch December 23, 2024 09:12
nizzyabi pushed a commit that referenced this pull request Dec 26, 2024
* chore: app router - /bookings page

* remove env vars

* fix

* Update middleware.ts

* revert unneeded change

* refactor for the better

* fix
emrysal added a commit that referenced this pull request Dec 26, 2024
* plain custom desin

* No display on card

* dynamic plain chat component and hmac hash

* re-route users to plain.com chat instead of intercom chat when going to /support

* provider errors

* yarn lock fix

* plain chat removed unneeded hmac

* fix ts error

* error handling improved

* remove intercome provider from app-dir

* Create getting-started.mdx (#18342)

* Delete help directory (#18343)

* chore: moved docs/help to /help (#18345)

* chore: Delete unused guides directory (#18346)

* feat: booking filters (#18303)

Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>

* chore: Remove `HeadSeo` components where no longer needed + improve app router metadata logic (#18348)

* remove HeadSeo for already migrated pages and refactor prepareMetadata

* create _generateMetadataWithoutImage and refactor _generateMetadata

* chore: app router - /bookings status page (#18183)

* chore: app router - /bookings page

* remove env vars

* fix

* Update middleware.ts

* revert unneeded change

* refactor for the better

* fix

* cache i18n instances (#18309)

* feat: virtual queues tab in insights (#18260)

Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: sean-brydon <sean@cal.com>
Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Co-authored-by: Omar López <zomars@me.com>

* feat: update translations via @replexica (#18361)

Co-authored-by: Replexica <support@replexica.com>

* update OOO e2e tests to remove flakiness (#18367)

* fix: metadata is overwritten for child managed eventType when updating parent (#18059)

* fix: metadata is overwirten

* Update

* type error

* Update

* fix test

* fix type error

* chore: added routing support link (#18369)

* Added routing form support link

* small change

* Type fix

---------

Co-authored-by: Joe Au-Yeung <j.auyeung419@gmail.com>

* chore: refactor handling logic for embeds in app router (#18362)

* refactor handling logic for embeds in app router

* fix type checks

* add test for withEmbedSsrAppDir

* fix

* review changes

* yarn lock

---------

Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Co-authored-by: Calcom Bot <109866826+calcom-bot@users.noreply.github.com>
Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
Co-authored-by: Benny Joo <sldisek783@gmail.com>
Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com>
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: sean-brydon <sean@cal.com>
Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com>
Co-authored-by: Omar López <zomars@me.com>
Co-authored-by: Replexica <support@replexica.com>
Co-authored-by: Vijay <vijayraghav22@gmail.com>
Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
Co-authored-by: Joe Au-Yeung <j.auyeung419@gmail.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking 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.

3 participants