Skip to content

feat: Added more query params to successRedirect page#8271

Merged
hariombalhara merged 3 commits into
calcom:mainfrom
titanventura:main
Aug 22, 2023
Merged

feat: Added more query params to successRedirect page#8271
hariombalhara merged 3 commits into
calcom:mainfrom
titanventura:main

Conversation

@titanventura
Copy link
Copy Markdown
Contributor

@titanventura titanventura commented Apr 14, 2023

What does this PR do?

Pass extra booking params to redirect URL
Fixes #7802

Edit by @hariombalhara


Right now there is an option to enable sending these query params in this PR.
Screenshot 2023-05-02 at 1 25 51 PM
A sample redirect URL generated using the avove configuration https://www.google.com/?isSuccessBookingPage=true&email=d%40e.com&eventTypeSlug=abc&title=ABC+between+Team+Pro+Example+and+d&description=sd&startTime=2023-05-12T03%3A45%3A00.000Z&endTime=2023-05-12T04%3A00%3A00.000Z&location=integrations%3Awhereby_video

Params Sent to Custom Success Redirect Page

  • booker_email - Email of the person booking the meeting.
  • title - Booking Title
  • description - Additional Notes of the booking
  • startTime - Booking Start Time as ISO String (Sample 2023-05-12T03:45:00.000Z)
  • endTime - Booking End Time as ISO String(Sample 2023-05-12T04:00:00.000Z)
  • location - It has many possible values. For Video Apps, the value will be in format integrations:{APPNAME}_video e.g. integrations:whereby_video. See more possible values for it here

I am of the opinion that we should do it by default and that checkbox isn't required. There is a very edge case where sending a lot of query params can break the URL size limit but we can ignore that for now.
cc @PeerRich @Jaibles


Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@titanventura titanventura requested a review from a team April 14, 2023 15:39
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 14, 2023

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

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2023 0:12am

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 14, 2023

@titanventura is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

Comment thread apps/web/components/booking/pages/BookingPage.tsx Outdated
Copy link
Copy Markdown
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution :) Havent tested but lets not implement lodash - especially in our booking page

Its huge

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 14, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@titanventura
Copy link
Copy Markdown
Contributor Author

Thanks for the support ! I would like to thank @adarsh-a-tw - link who paired with me and helped me raise this PR !

@titanventura
Copy link
Copy Markdown
Contributor Author

@sean-brydon Please review. Thanks !

@github-actions
Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

Five Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load % of Budget (350 KB)
/[user]/book 189.37 KB 420.67 KB 120.19% (🟡 +0.17%)
/apps/[slug]/[...pages] 389.09 KB 620.38 KB 177.25% (🟢 -0.17%)
/auth/setup 81.58 KB 312.87 KB 89.39% (🟢 -0.17%)
/d/[link]/book 189.02 KB 420.31 KB 120.09% (🟡 +0.17%)
/team/[slug]/book 189.02 KB 420.31 KB 120.09% (🟡 +0.17%)
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored.

@github-actions
Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@titanventura
Copy link
Copy Markdown
Contributor Author

Please review !

Comment thread apps/web/components/booking/pages/BookingPage.tsx Outdated
eventTypeSlug: eventType.slug,
seatReferenceUid: "seatReferenceUid" in responseData ? responseData.seatReferenceUid : null,
...(rescheduleUid && booking?.startTime && { formerTime: booking.startTime.toString() }),
...getBookingRedirectExtraParams(responseData),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be done as part of bookingSuccessRedirect and that fn is called at multiple places as redirections happens from multiple places.

"startTime",
"endTime",
"location",
...objectQueryParamKeys,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of differentiating manually b/w object and non-object params, we should use the typeof operator to identify the value. This would allow us to maintain a single list.

Also, I would like to keep the list very strict because whatever we expose has to be considered for backward compatibility as websites would rely on those params.

Stable Params - Good to be passed as is

  • title
  • description
  • startTime
  • endTime
  • location

user and attendees might have different props depending on what we queried. So, we need to pick keys specifically from here. So, it might be better to remove these keys and wait for a requirement from users as to what they use it for.

Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Left some comments.

@titanventura
Copy link
Copy Markdown
Contributor Author

thanks for the suggestions @hariombalhara . Have made the changes. Please review !

@titanventura
Copy link
Copy Markdown
Contributor Author

Any updates on this ?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 24, 2023

Thank you for following the naming conventions! 🙏

@hariombalhara
Copy link
Copy Markdown
Member

hariombalhara commented Jul 24, 2023

@titanventura Are you there to review the changes I pushed?

I have made the changes to send email as new param booker_email so that it's clear whose email it is. Also documented, the params that are are sending.
I have also pulled in latest main and resolved some conflicts.

I have also ensured that no other query param is sent that isn't in documentation.

price: true,
slug: true,
length: true,
offsetStart: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are these changes made in managedEventTypes ?

@github-actions github-actions Bot removed the Stale label Jul 25, 2023
@github-actions
Copy link
Copy Markdown
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions Bot added the Stale label Aug 10, 2023
@PeerRich
Copy link
Copy Markdown
Member

/bonus 20 @titanventura can you take another look at this PR and fix the merge conflicts? <3

@algora-pbc
Copy link
Copy Markdown

algora-pbc Bot commented Aug 14, 2023

A bonus of $20 has been added by PeerRich.
@titanventura: You will receive $20 once you implement the requested changes and your PR is merged.

@hariombalhara hariombalhara changed the title feat: Added option to pass extra booking query params to redirect url feat: Added more query params to successRedirect page Aug 22, 2023
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

LGTM now !! Thanks @titanventura

@hariombalhara hariombalhara dismissed stale reviews from zomars and sean-brydon August 22, 2023 12:10

Resolved

@hariombalhara hariombalhara enabled auto-merge (squash) August 22, 2023 12:10
@hariombalhara hariombalhara merged commit 16bb5c3 into calcom:main Aug 22, 2023
@algora-pbc
Copy link
Copy Markdown

algora-pbc Bot commented Aug 22, 2023

@titanventura: Your claim has been rewarded! 👉 Complete your Algora onboarding to collect the bounty.

@algora-pbc
Copy link
Copy Markdown

algora-pbc Bot commented Aug 22, 2023

🎉🎈 @titanventura has been awarded $20! 🎈🎊

@algora-pbc algora-pbc Bot added the 💰 Rewarded Rewarded bounties on Algora.io label Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs area: docs, documentation, cal.com/docs ✨ feature New feature or request Low priority Created by Linear-GitHub Sync 💰 Rewarded Rewarded bounties on Algora.io

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

Pass meeting details in query or body to redirection URL

6 participants