fix: Allow install on teams for concurrent meeting apps #23429
Conversation
WalkthroughClient-side handlers in AppPage.tsx and AppCard.tsx now require both isConferencing(categories) and !concurrentMeetings to trigger the Event Types onboarding step; if concurrentMeetings is true (or the app is not conferencing) the flow falls back to the existing install mutation or the Accounts onboarding path based on team availability. On the server (getServerSideProps), installableOnTeams is computed as !!appMetadata?.concurrentMeetings || !isConferencing — i.e., conferencing apps are installable on teams only when appMetadata.concurrentMeetings is truthy, while non-conferencing apps remain installable. No exported/public declarations changed. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
keithwillcode
added
core
dosubot
bot
added
the
teams
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (3)
179-197: OR with {} matches everything; also over-fetches credentials (potentially sensitive)The empty object inside OR will return all credentials. Also, only id/teamId/userId are needed—avoid fetching other fields.
-const getAppInstallsBySlug = async (appSlug: string, userId: number, teamIds?: number[]) => { - const appInstalls = await prisma.credential.findMany({ - where: { - OR: [ - { - appId: appSlug, - userId: userId, - }, - teamIds && Boolean(teamIds.length) - ? { - appId: appSlug, - teamId: { in: teamIds }, - } - : {}, - ], - }, - }); - return appInstalls; -}; +const getAppInstallsBySlug = async (appSlug: string, userId: number, teamIds?: number[]) => { + const or: Prisma.CredentialWhereInput[] = [{ appId: appSlug, userId }]; + if (teamIds?.length) { + or.push({ appId: appSlug, teamId: { in: teamIds } }); + } + return prisma.credential.findMany({ + where: { OR: or }, + select: { id: true, teamId: true, userId: true }, // minimal set; never fetch credential.key + }); +};
68-74: Avoid sending app.keys to the clientapp.keys is selected and the whole app object is returned via props. Remove keys from the select to prevent leaking metadata/secrets.
const getAppBySlug = async (appSlug: string) => { const app = await prisma.app.findUnique({ where: { slug: appSlug, enabled: true }, - select: { slug: true, keys: true, enabled: true, dirName: true }, + select: { slug: true, enabled: true, dirName: true }, }); return app; };Also applies to: 306-309
216-218: Gate Event Types step for concurrent-meeting apps
In apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (around line 217), update the showEventTypesStep assignment to skip conferencing apps that support concurrent meetings:- const showEventTypesStep = extendsEventType || isConferencing; + const showEventTypesStep = + extendsEventType || + (isConferencing && !appMetadata.concurrentMeetings); + // Skip Event Types step for conferencing apps with concurrentMeetings support
🧹 Nitpick comments (4)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (3)
213-218: Null-safety on appMetadata.categoriesGuard against undefined metadata to avoid runtime crashes.
- const isConferencing = isConferencingApp(appMetadata.categories); + const isConferencing = isConferencingApp(appMetadata?.categories ?? []);
300-301: Fix typo in comment- // dont allow app installation without cretendialId + // don't allow app installation without credentialId
199-323: Add test coverage for new team-install behaviorPlease add SSR tests for:
- conferencing + concurrentMeetings=true → installableOnTeams=true
- conferencing + concurrentMeetings=false → installableOnTeams=false
- non-conferencing → installableOnTeams=true
I can draft Playwright/SSR tests targeting getServerSideProps and the Accounts/Event Types routing if helpful.
packages/features/apps/components/AppCard.tsx (1)
59-81: Correct gating for Event Types; avoid recomputing doesAppSupportTeamInstallCondition aligns with the new flow (skip Event Types when concurrentMeetings=true). Reuse enabledOnTeams instead of recomputing.
- } else if ( - !doesAppSupportTeamInstall({ - appCategories: app.categories, - concurrentMeetings: app.concurrentMeetings, - isPaid: !!app.paid, - }) - ) { + } else if (!enabledOnTeams) {Note: This depends on the SSR fix to allow team installs when concurrentMeetings=true.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/components/apps/AppPage.tsx(1 hunks)apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts(1 hunks)packages/features/apps/components/AppCard.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/components/apps/AppPage.tsxpackages/features/apps/components/AppCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/components/apps/AppPage.tsxapps/web/lib/apps/installation/[[...step]]/getServerSideProps.tspackages/features/apps/components/AppCard.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/components/apps/AppPage.tsxapps/web/lib/apps/installation/[[...step]]/getServerSideProps.tspackages/features/apps/components/AppCard.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts
🧬 Code graph analysis (3)
apps/web/components/apps/AppPage.tsx (1)
packages/app-store/utils.ts (1)
isConferencing(168-170)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (1)
packages/app-store/utils.ts (1)
isConferencing(168-170)
packages/features/apps/components/AppCard.tsx (1)
packages/app-store/utils.ts (1)
isConferencing(168-170)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
🔇 Additional comments (1)
apps/web/components/apps/AppPage.tsx (1)
114-133: Gating looks right; ensure server-side installability matchesThis correctly routes conferencing apps with concurrent meetings to Accounts. Ensure SSR sets installableOnTeams=true for that case (currently inverted in getServerSideProps).
Consider adding a component test covering both branches (concurrentMeetings true/false) to verify navigation targets.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (4)
216-218: Align server gating with client: skip Event Types for conferencing apps with concurrentMeetings=trueClient handlers only show Event Types when isConferencing && !concurrentMeetings. Server still uses extendsEventType || isConferencing, enabling manual navigation to the step. Align to avoid inconsistent flows.
Apply:
- const showEventTypesStep = extendsEventType || isConferencing; + const showEventTypesStep = + extendsEventType || (isConferencing && !appMetadata?.concurrentMeetings);
179-197: Bug:{}inside OR broadens query to all credentials; also overfetches sensitive fieldsWhen
teamIdsis falsy/empty, the{}branch makesOR: [{...}, {}]which matches everything. Additionally, noselectis used, risking retrieval ofcredential.key. Build OR conditionally and select only needed fields.-const getAppInstallsBySlug = async (appSlug: string, userId: number, teamIds?: number[]) => { - const appInstalls = await prisma.credential.findMany({ - where: { - OR: [ - { - appId: appSlug, - userId: userId, - }, - teamIds && Boolean(teamIds.length) - ? { - appId: appSlug, - teamId: { in: teamIds }, - } - : {}, - ], - }, - }); - return appInstalls; -}; +const getAppInstallsBySlug = async (appSlug: string, userId: number, teamIds?: number[]) => { + const orFilters: Prisma.CredentialWhereInput[] = [{ appId: appSlug, userId }]; + if (teamIds && teamIds.length) { + orFilters.push({ appId: appSlug, teamId: { in: teamIds } }); + } + return prisma.credential.findMany({ + where: { OR: orFilters }, + // Avoid fetching credential.key and other unused fields + select: { id: true, teamId: true, userId: true }, + }); +};
254-260: Limit destinationCalendar fields to the minimum requiredCurrently fetches the full record and later ships it in props via eventTypeGroups, which may include credentialId. Select only needed fields.
- const destinationCalendar = await prisma.destinationCalendar.findFirst({ + const destinationCalendar = await prisma.destinationCalendar.findFirst({ where: { userId: user.id, eventTypeId: null, - }, + }, + select: { id: true }, // expand if UI needs more, but avoid credentialId });
76-99: Constrain EventType.destinationCalendar selection to avoid leaking credential linkage
destinationCalendar: truepulls the full relation, potentially includingcredentialId. Select a safe subset.- destinationCalendar: true, + destinationCalendar: { select: { id: true } },
♻️ Duplicate comments (1)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (1)
319-320: Update stale comment to match logicComment says conferencing apps don’t support team install; logic now allows it when
concurrentMeetingsis true.- // conferencing apps dont support team install + // Conferencing apps are installable on teams only when appMetadata.concurrentMeetings is true
🧹 Nitpick comments (2)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (2)
300-301: Fix typos in commentMinor polish.
- // dont allow app installation without cretendialId + // Don't allow app installation without credentialId
204-208: Be strict with [[...step]] parsing
params.stepcan be string|string[].z.coerce.string()on an array yields a comma-joined string and may failstepsEnum.parse. Normalize first element.- const parsedStepParam = z.coerce.string().parse(params?.step); - const _ = stepsEnum.parse(parsedStepParam); + const rawStep = params?.step; + const parsedStepParam = Array.isArray(rawStep) ? rawStep[0] : rawStep ?? ""; + stepsEnum.parse(parsedStepParam);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts
🧬 Code graph analysis (1)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (1)
packages/app-store/utils.ts (1)
isConferencing(168-170)
🔇 Additional comments (1)
apps/web/lib/apps/installation/[[...step]]/getServerSideProps.ts (1)
320-321: LGTM: installableOnTeams logic matches PR objective
!!appMetadata?.concurrentMeetings || !isConferencingcorrectly enables team installs for conferencing apps only when concurrent meetings are supported; non‑conferencing apps remain installable.
E2E results are ready! |
What does this PR do?
Allows conferencing apps to be installed on a team when the
concurrentMeetingproperty is true in the app's metadataMandatory Tasks (DO NOT REMOVE)
How should this be tested?
concurrentMeetingsto a conferencing app's metadata