perf: remove trpc basecamp3 router
#23785
Conversation
WalkthroughThis change removes the Basecamp3 TRPC router and its Next.js TRPC API route, deletes the TRPC input schema, and removes the appBasecamp3 entry from the viewer router. The Basecamp3 handlers (projects and projectMutation) were converted from TRPC procedures to Next.js HTTP API handlers, switching from TRPC context to direct Prisma and session-based auth and replacing TRPC errors with explicit HTTP responses. The Basecamp3 UI now calls REST endpoints under /api/integrations/basecamp3. Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
hbjORbj
changed the title
basecamp3 router
dosubot
bot
added
app-store
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
E2E results are ready! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsx (1)
62-64: Text should use localization.According to coding guidelines, all text in TSX files should use the
t()function for localization.<div className="mt-2"> - Please note that as of now you can only link <span className="italic">one</span> of your projects to - cal.com + {t("basecamp_note_single_project", "Please note that as of now you can only link")} <span className="italic">{t("one")}</span> {t("basecamp_note_single_project_suffix", "of your projects to cal.com")} </div>Also applies to line 50:
- <p className="py-2">Link a Basecamp project to this event:</p> + <p className="py-2">{t("basecamp_link_project", "Link a Basecamp project to this event:")}</p>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/web/pages/api/trpc/appBasecamp3/[trpc].ts(0 hunks)packages/app-store/basecamp3/api/projectMutation.ts(2 hunks)packages/app-store/basecamp3/api/projects.ts(1 hunks)packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsx(2 hunks)packages/trpc/server/routers/apps/basecamp3/_router.ts(0 hunks)packages/trpc/server/routers/apps/basecamp3/projectMutation.schema.ts(0 hunks)packages/trpc/server/routers/viewer/_router.tsx(0 hunks)
💤 Files with no reviewable changes (4)
- packages/trpc/server/routers/apps/basecamp3/projectMutation.schema.ts
- packages/trpc/server/routers/viewer/_router.tsx
- packages/trpc/server/routers/apps/basecamp3/_router.ts
- apps/web/pages/api/trpc/appBasecamp3/[trpc].ts
🧰 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:
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.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:
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsxpackages/app-store/basecamp3/api/projects.tspackages/app-store/basecamp3/api/projectMutation.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:
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsxpackages/app-store/basecamp3/api/projects.tspackages/app-store/basecamp3/api/projectMutation.ts
**/*.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:
packages/app-store/basecamp3/api/projects.tspackages/app-store/basecamp3/api/projectMutation.ts
🧬 Code graph analysis (3)
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsx (1)
packages/app-store/types.d.ts (1)
EventTypeAppSettingsComponent(89-89)
packages/app-store/basecamp3/api/projects.ts (2)
packages/lib/server/defaultHandler.ts (1)
defaultHandler(8-24)packages/lib/server/defaultResponder.ts (1)
defaultResponder(11-42)
packages/app-store/basecamp3/api/projectMutation.ts (2)
packages/lib/server/defaultHandler.ts (1)
defaultHandler(8-24)packages/lib/server/defaultResponder.ts (1)
defaultResponder(11-42)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
packages/app-store/basecamp3/api/projectMutation.ts (1)
69-72: Ensure credential.key field is never exposed.While updating the credential, ensure that the key field with sensitive token information is properly handled and never exposed in responses.
The update operation correctly stores the credential data without exposing it in the response.
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsx
Show resolved
Hide resolved
packages/app-store/basecamp3/components/EventTypeAppSettingsInterface.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/app-store/basecamp3/api/projects.ts (3)
44-46: Preserve upstream status and context when Basecamp responds with an error.Surface the provider’s HTTP status (and optionally body) for better debugging and accurate client handling.
- if (!resp.ok) { - throw new HttpError({ statusCode: 400, message: "Failed to fetch Basecamp projects" }); - } + if (!resp.ok) { + const errorBody = await resp.text().catch(() => undefined); + throw new HttpError({ + statusCode: resp.status, + message: "Failed to fetch Basecamp projects", + url, + method: "GET", + data: errorBody, + }); + }
1-1: Align handler signature with expected type to avoid bivariant-parameter pitfalls.Include
NextApiResponseand accept it in the handler to matchdefaultResponder’s handle type.-import type { NextApiRequest } from "next"; +import type { NextApiRequest, NextApiResponse } from "next"; @@ -async function handler(req: NextApiRequest) { +async function handler(req: NextApiRequest, _res: NextApiResponse) {Also applies to: 12-12
52-54: Add endpointRoute to defaultResponder for better tracing/metrics labels.This improves Sentry and perf marks naming.
-export default defaultHandler({ - GET: Promise.resolve({ default: defaultResponder(handler) }), -}); +export default defaultHandler({ + GET: Promise.resolve({ default: defaultResponder(handler, "/api/integrations/basecamp3/projects") }), +});
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/app-store/basecamp3/api/projectMutation.ts(2 hunks)packages/app-store/basecamp3/api/projects.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app-store/basecamp3/api/projectMutation.ts
🧰 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:
packages/app-store/basecamp3/api/projects.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:
packages/app-store/basecamp3/api/projects.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:
packages/app-store/basecamp3/api/projects.ts
🧠 Learnings (2)
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/app-store/basecamp3/api/projects.ts
📚 Learning: 2025-09-01T08:56:14.071Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/lib/tokenManager.ts:25-31
Timestamp: 2025-09-01T08:56:14.071Z
Learning: In token refresh utilities like tokenManager.ts, accessing credential.key from Prisma is legitimate and necessary for OAuth token refresh flows. These internal utilities need stored credentials to refresh tokens and don't expose them in API responses.
Applied to files:
packages/app-store/basecamp3/api/projects.ts
🧬 Code graph analysis (1)
packages/app-store/basecamp3/api/projects.ts (2)
packages/lib/server/defaultHandler.ts (1)
defaultHandler(8-24)packages/lib/server/defaultResponder.ts (1)
defaultResponder(11-42)
⏰ 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). (9)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Production builds / Build API v2
- GitHub Check: Production builds / Build API v1
- GitHub Check: Tests / Unit
- GitHub Check: Production builds / Build Web App
- GitHub Check: Type check / check-types
- GitHub Check: Production builds / Build Docs
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Linters / lint
🔇 Additional comments (2)
packages/app-store/basecamp3/api/projects.ts (2)
10-16: Nice: standardized error handling with HttpError and defaultResponder.The migration away from ad-hoc
{status, body}responses is correct and consistent with our handler conventions.Also applies to: 44-46, 48-50
35-37: Expires_at is milliseconds — no change needed.BasecampToken in packages/app-store/basecamp3/lib/types.ts declares expires_at: number, and other Basecamp3 code (e.g., CalendarService) already compares expires_at to Date.now(), implying expires_at is milliseconds since epoch. No normalization required; the current comparison is correct.
| const credential = await prisma.credential.findFirst({ | ||
| where: { | ||
| userId: user?.id, | ||
| }, | ||
| where: { userId }, | ||
| select: credentialForCalendarServiceSelect, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Filter credential by Basecamp3 app to avoid cross-app leakage.
Without appId, this may pick an arbitrary credential for the user. Filter to the Basecamp3 credential.
- const credential = await prisma.credential.findFirst({
- where: { userId },
- select: credentialForCalendarServiceSelect,
- });
+ const credential = await prisma.credential.findFirst({
+ where: { userId, appId: "basecamp3" },
+ select: credentialForCalendarServiceSelect,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const credential = await prisma.credential.findFirst({ | |
| where: { | |
| userId: user?.id, | |
| }, | |
| where: { userId }, | |
| select: credentialForCalendarServiceSelect, | |
| }); | |
| const credential = await prisma.credential.findFirst({ | |
| where: { userId, appId: "basecamp3" }, | |
| select: credentialForCalendarServiceSelect, | |
| }); |
🤖 Prompt for AI Agents
In packages/app-store/basecamp3/api/projects.ts around lines 20 to 23, the
Prisma query that fetches a credential only filters by userId which can return a
credential for a different app; update the where clause to also filter for the
Basecamp3 app (e.g. add appId: 'basecamp3' or the appropriate constant/enum
value used in the codebase) while keeping the same select
(credentialForCalendarServiceSelect) so only the Basecamp3 credential for that
user is returned.
What does this PR do?
appBasecamp3trpc router is only consumed by appstore package. It makes more sense for the APIs to live inpackages/app-store/basecamp3/api/. This also fixes the fact thatapp-store/basecamp3/EventTypeAppSettingsInterfacewas importingtrpcclient.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?