-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix: Embed: Get correct snippet for Org events #12380
fix: Embed: Get correct snippet for Org events #12380
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes! |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
Current Playwright Test Results Summary✅ 318 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 11/16/2023 12:15:52pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: f88bd5b Started: 11/16/2023 12:09:52pm UTC
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Stripe integration Change stripe presented currency Should be able to change currency
Retry 1 • Initial Attempt |
0% (0)0 / 290 runsfailed over last 7 days |
1.72% (5)5 / 290 runsflaked over last 7 days |
📄 packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
2 Test Cases Affected |
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when no theme is configured through Embed API
Retry 1 • Initial Attempt |
1.29% (4)4 / 310 runsfailed over last 7 days |
56.77% (176)176 / 310 runsflaked over last 7 days |
Popup Tests should be able to reschedule
Retry 1 • Initial Attempt |
17.42% (54)54 / 310 runsfailed over last 7 days |
77.74% (241)241 / 310 runsflaked over last 7 days |
605c46b
to
1df00cb
Compare
1df00cb
to
c0c9687
Compare
c0c9687
to
bc4361a
Compare
bc4361a
to
436f357
Compare
436f357
to
35e9eba
Compare
35e9eba
to
caf63f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self review done
}); | ||
}); | ||
|
||
test.describe("Organization", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New tests to verify the code for organization events.
@@ -427,7 +427,7 @@ const createUserFixture = (user: UserWithIncludes, page: Page) => { | |||
} | |||
return membership; | |||
}, | |||
getOrg: async () => { | |||
getOrgMembership: async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to better convey what it does.
page.locator(`[data-testid=${embedType}]`).click(); | ||
} | ||
|
||
async function gotToPreviewTab(page: Page) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to goToPreviewTab
- Typo fix
return `./${path.replace(/\\/g, "/")}/${moduleName | ||
.replace(/\/index\.ts|\/index\.tsx/, "") | ||
.replace(/\.tsx$|\.ts$/, "")}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prettier autofix
@@ -53,7 +55,7 @@ export const Codes = { | |||
return code` | |||
import { getCalApi } from "@calcom/embed-react"; | |||
import { useEffect } from "react"; | |||
export default function App() { | |||
export default function MyApp() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to make it same as inline
embed code
caf63f0
to
bcd14a0
Compare
bcd14a0
to
bea4131
Compare
bea4131
to
a04b62f
Compare
a04b62f
to
f88bd5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @hariombalhara looks good enough to merge
What does this PR do?
This makes the embed snippet now work with org events without any manual change's requirement.
E2E Tests
orgSlug
is present in code and bookerUrl for all types of embeds.Fixes #11644
Type of change
How should this be tested?
See loom - https://www.loom.com/share/4e6696a6da704ae1b2d8146dd02c2a5a
Mandatory Tasks