-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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: Add the 'cal-video' location only if it doesn't already exist #15038
Conversation
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/15/24)1 reviewer was added to this PR based on Keith Williams's automation. |
📦 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✅ 34 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 05/17/2024 09:13:05am UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 900bb32 Started: 05/17/2024 09:12:19am UTC
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Update Profile Can verify the newly added secondary email
Retry 1 • Initial Attempt |
2.44% (6)6 / 246 runsfailed over last 7 days |
24.39% (60)60 / 246 runsflaked over last 7 days |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
const integrationQuery = getRemovedIntegrationNameFromAppSlug(credential.app?.slug ?? ""); | ||
|
||
// Check if the event type uses the deleted integration | ||
|
||
// To avoid type errors, need to stringify and parse JSON to use array methods | ||
const locations = locationsSchema.parse(eventType.locations); | ||
|
||
const doesDailyVideoAlreadyExists = locations.some((location) => | ||
location.type.includes(DailyLocationType) | ||
); | ||
|
||
const updatedLocations: TlocationsSchema = locations.reduce((acc: TlocationsSchema, location) => { | ||
if (location.type.includes(integrationQuery)) { | ||
if (!doesDailyVideoAlreadyExists) acc.push({ type: DailyLocationType }); | ||
} else { | ||
acc.push(location); | ||
} | ||
return acc; | ||
}, []); | ||
|
||
await prisma.eventType.update({ | ||
where: { | ||
id: eventType.id, | ||
}, | ||
data: { | ||
locations: updatedLocations, | ||
}, | ||
}); |
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.
You code worked fine but it wasn't easily readable/maintainable so i pushed these changes. (Nothing major just extracted logic to some functions and reduced nested conditionals and changed variable names)
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.
Do let me know if you find anything wrong
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.
Thank you!!!!, looks good 👍
What does this PR do?
Removing a conferencing app adds 'cal video' option to the location dropdown, even if it already exist.
Before
LOOM
After
LOOM
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
checkout loom