Skip to content

fix: Routing Form and other org related fixes#10320

Merged
hariombalhara merged 8 commits into
mainfrom
fix/wrong-url-org-domain
Jul 25, 2023
Merged

fix: Routing Form and other org related fixes#10320
hariombalhara merged 8 commits into
mainfrom
fix/wrong-url-org-domain

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Jul 23, 2023

What does this PR do?

Fixes #9821 by fixing the last pending item
Screenshot 2023-07-24 at 7 18 53 PM

  • Make Routing Forms public pages(/router and /forms/{FORMID}) org context aware
  • Fix duplicate / in org URLs avoiding too many warnings in console about it

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works

@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 23, 2023

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

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 8:27pm
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 8:27pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 8:27pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 8:27pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jul 24, 2023 8:27pm
qa ⬜️ Ignored (Inspect) Jul 24, 2023 8:27pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 23, 2023

Thank you for following the naming conventions! 🙏

@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented Jul 23, 2023

🤖 Meticulous spotted visual differences in 41 of 234 screens tested: view and approve differences detected.

Last updated for commit 1be908f. This comment will update as new commits are pushed.

Base automatically changed from fix/nameless-in-sidebar to main July 24, 2023 10:25
@PeerRich PeerRich changed the title Removed duplicate / fix: removed duplicate / Jul 24, 2023
Comment thread apps/web/pages/event-types/index.tsx Outdated
<Link href={`${CAL_URL}/${profile.slug}`} className="text-subtle block text-xs">
{orgBranding
? `${orgBranding.fullDomain.replace("https://", "").replace("http://", "")}${profile.slug}`
? `${orgBranding.fullDomain.replace("https://", "").replace("http://", "")}/${profile.slug}`
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because fullDomain doesn't have an ending slash now, we need to add it manually here. this is now consistent with WEBAPP and CAL_URL

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 24, 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! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Jul 24, 2023

Current Playwright Test Results Summary

✅ 96 Passing - ❌ 1 Failing - ⚠️ 4 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/24/2023 08:32:10pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 1be908f

Started: 07/24/2023 08:28:33pm UTC

❌ Failures

📄   packages/embeds/embed-core/playwright/tests/embed-pages.e2e.ts • 1 Failure

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Embed Pages Event Type Page: should not have margin top on embed page
Retry 2Retry 1Initial Attempt
Error: expect(received).toBe(expected) // Object.is equality...
expect(received).toBe(expected) // Object.is equality

Expected: 0
Received: null
3.93% (7) 7 / 178 runs
failed over last 7 days
0.56% (1) 1 / 178 run
flaked over last 7 days

⚠️ Flakes

📄   apps/web/playwright/manage-booking-questions.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Manage Booking Questions For User EventType Do a booking with a user added question and verify a few thing in b/w
Retry 1Initial Attempt
1.82% (6) 6 / 329 runs
failed over last 7 days
1.52% (5) 5 / 329 runs
flaked over last 7 days
Manage Booking Questions For Team EventType Do a booking with a user added question and verify a few thing in b/w
Retry 1Initial Attempt
1.82% (6) 6 / 329 runs
failed over last 7 days
1.22% (4) 4 / 329 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 1Initial Attempt
1.63% (3) 3 / 184 runs
failed over last 7 days
97.28% (179) 179 / 184 runs
flaked over last 7 days

📄   apps/web/playwright/webhook.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
FORM_SUBMITTED can submit a form and get a submission event
Retry 1Initial Attempt
0.62% (2) 2 / 325 runs
failed over last 7 days
3.69% (12) 12 / 325 runs
flaked over last 7 days

View Detailed Build Results


{orgBranding
? `${orgBranding.fullDomain.replace("https://", "").replace("http://", "")}${profile.slug}`
: `${CAL_URL?.replace("https://", "").replace("http://", "")}/${profile.slug}`}
{`${bookerUrl.replace("https://", "").replace("http://", "")}/${profile.slug}`}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because fullDomain doesn't have an ending slash now which is consistent with WEBAPP and CAL_URL. We can simplify the logic using bookerUrl

const form = await prisma.app_RoutingForms_Form.findUnique({
const { currentOrgDomain, isValidOrgDomain } = orgDomainConfig(context.req.headers.host ?? "");

const form = await prisma.app_RoutingForms_Form.findFirst({
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed to findFirst because you can have user has in where clause for findUnique

export function getOrgFullDomain(slug: string, options: { protocol: boolean } = { protocol: true }) {
// TODO: It is a replacement for WEBAPP_URL and that doesn't have / in the end. Remove / after ensuring that it works reliably everywhere
return `${options.protocol ? `${new URL(WEBAPP_URL).protocol}//` : ""}${slug}.${subdomainSuffix()}/`;
return `${options.protocol ? `${new URL(WEBAPP_URL).protocol}//` : ""}${slug}.${subdomainSuffix()}`;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment has the reason why this change was made.

<Button
target="_blank"
href={"/" + props.member.username}
href={`${bookerUrl}/${props.member.username}`}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This link would have given 404 for org member.

export const useBookerUrl = () => {
const orgBranding = useOrgBranding();
return orgBranding?.fullDomain ?? WEBAPP_URL;
return orgBranding?.fullDomain ?? CAL_URL ?? WEBAPP_URL;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Prefer CAL_URL for public links, if not available , go for WEBAPP_URL.

@hariombalhara hariombalhara changed the title fix: removed duplicate / fix: Routing Form and other org related fixes Jul 24, 2023
@hariombalhara hariombalhara marked this pull request as ready for review July 24, 2023 14:04
@hariombalhara hariombalhara added High priority Created by Linear-GitHub Sync organizations area: organizations, orgs labels Jul 24, 2023
@hariombalhara hariombalhara self-assigned this Jul 24, 2023
@hariombalhara hariombalhara requested review from a team July 24, 2023 14:05
Copy link
Copy Markdown
Contributor

@leog leog left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you for taking care of this @hariombalhara.

@hariombalhara hariombalhara merged commit 35be418 into main Jul 25, 2023
@hariombalhara hariombalhara deleted the fix/wrong-url-org-domain branch July 25, 2023 09:28
fritterhoff pushed a commit to hm-edu/cal.com that referenced this pull request Jul 25, 2023
Co-authored-by: Leo Giovanetti <hello@leog.me>
joeauyeung pushed a commit that referenced this pull request Jul 26, 2023
Co-authored-by: Leo Giovanetti <hello@leog.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync organizations area: organizations, orgs

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

[CAL-2059] Orgs Testing - List of bugs[Will keep on updating]

3 participants