perf: use repository for webhooks list query & caching in /settings/developer/webhooks/... RSC #21781
Conversation
…eveloper/webhooks/... RSC
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
…che-settings-developers-webhooks
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (06/12/25)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
cubic found 3 issues across 11 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| }); | ||
|
|
||
| if (Array.isArray(where.AND)) { | ||
| if (input?.eventTypeId) { |
There was a problem hiding this comment.
When eventTypeId is provided, the query omits any filter based on userId or team membership, allowing the caller to fetch webhooks that belong to other users or teams. This breaks access control and can expose private data.
| } | ||
| } else { | ||
| where.AND?.push({ | ||
| OR: [{ userId }, { teamId: { in: user?.teams.map((membership) => membership.teamId) } }], |
There was a problem hiding this comment.
user?.teams may be undefined when the user is not found, causing .map to be called on undefined and throw at runtime. Use optional chaining or a fallback empty array before mapping.
| OR: [{ userId }, { teamId: { in: user?.teams.map((membership) => membership.teamId) } }], | |
| OR: [{ userId }, { teamId: { in: (user?.teams?.map((membership) => membership.teamId) ?? []) } }], |
| @@ -15,12 +16,24 @@ export const generateMetadata = async ({ params }: { params: Promise<{ id: strin | |||
| `/settings/developer/webhooks/${(await params).id}` | |||
| ); | |||
|
|
|||
| const getCachedWebhook = (id?: string) => { | |||
| const fn = unstable_cache( | |||
There was a problem hiding this comment.
unstable_cache is instantiated inside getCachedWebhook on every call, so caching cannot persist between calls because a new cache instance is created each time and no explicit cache key is provided; this largely defeats the purpose of caching and wastes memory.
E2E results are ready! |
| const getCachedWebhook = (id?: string) => { | ||
| const fn = unstable_cache( |
There was a problem hiding this comment.
Wouldn't we want to only revalidate cache of only a particular webhook? We cannot pass id directly to tags when using unstable_cache, so we need to wrap it
| return await prisma.webhook.findMany({ | ||
| where, | ||
| }); | ||
| return WebhookRepository.findWebhooksByFilters({ userId: ctx.user.id, input }); |
…che-settings-developers-webhooks
…hub.com/calcom/cal.com into perf/cache-settings-developers-webhooks
There was a problem hiding this comment.
cubic found 1 issue across 11 files. Review it in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| async () => { | ||
| return await WebhookRepository.findByWebhookId(id); | ||
| }, | ||
| undefined, |
There was a problem hiding this comment.
Passing undefined as the keyParts argument means a new cached function (and therefore a new cache entry) is created every time getCachedWebhook is called, which largely negates the purpose of unstable_cache. Provide a stable key (e.g. the webhook id) or hoist the cached function outside the wrapper so that the same cache entry is reused.
| undefined, | |
| [`webhook:${id}`], |
There was a problem hiding this comment.
cubic found 3 issues across 11 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| async () => { | ||
| return await WebhookRepository.findByWebhookId(id); | ||
| }, | ||
| undefined, |
There was a problem hiding this comment.
Passing undefined as the keyParts argument means a new cached function (and therefore a new cache entry) is created every time getCachedWebhook is called, which largely negates the purpose of unstable_cache. Provide a stable key (e.g. the webhook id) or hoist the cached function outside the wrapper so that the same cache entry is reused.
| undefined, | |
| [`webhook:${id}`], |
| } | ||
| } else { | ||
| where.AND?.push({ | ||
| OR: [{ userId }, { teamId: { in: user?.teams.map((membership) => membership.teamId) } }], |
There was a problem hiding this comment.
user can be null when the lookup fails, causing in: undefined and a Prisma runtime error. Provide a fallback array or ensure the user exists before constructing the filter.
| OR: [{ userId }, { teamId: { in: user?.teams.map((membership) => membership.teamId) } }], | |
| OR: [{ userId }, { teamId: { in: (user?.teams ?? []).map((membership) => membership.teamId) } }], |
| id: userId, | ||
| }, | ||
| select: { | ||
| teams: true, |
There was a problem hiding this comment.
Selecting the entire teams relation fetches all membership fields and nested team data even though only teamId is subsequently used. This increases query payload size and may expose unnecessary data.
| teams: true, | |
| teams: { select: { teamId: true } }, |
|
This PR is being marked as stale due to inactivity. |
…ttings/developer/webhooks/... RSC (calcom#21781)" (calcom#22963)
What does this PR do?
Caches
Video Demo (if applicable):
BEFORE
Screencast.from.2025-06-13.02-40-55.webm
AFTER
Screencast.from.2025-06-13.02-44-45.webm
Mandatory Tasks (DO NOT REMOVE)
Summary by cubic
Improved performance of the webhooks settings pages by switching to repository-based queries and adding caching for webhook lists and details.