perf: add paginated host endpoints and repository methods for large teams#28156
perf: add paginated host endpoints and repository methods for large teams#28156joeauyeung wants to merge 6 commits intomainfrom
Conversation
…vent type editor - New cursor-paginated tRPC endpoints: getHostsForAssignment, getHostsForAvailability, searchTeamMembers, getChildrenForAssignment, exportHostsForWeights, getHostsWithLocationOptions - Delta-based host update support in update.handler.ts (pendingHostChanges, pendingChildrenChanges) - Repository additions: EventTypeRepository.findChildrenByParentId, HostRepository pagination, MembershipRepository.searchMembers, UserRepository.findByIdsWithPagination - Remove teamMembers from getEventTypeById initial load - Shared types: PendingHostChangesInput, PendingChildrenChangesInput, HostUpdateInput Co-Authored-By: unknown <>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: unknown <>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 25 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/features/host/repositories/HostRepository.ts">
<violation number="1" location="packages/features/host/repositories/HostRepository.ts:206">
P2: `memberUserIds` being an empty array currently disables the filter and returns all hosts. Treat an empty array as a valid filter so callers requesting no members get an empty result set instead of all hosts.</violation>
</file>
<file name="packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts:1000">
P1: Performance regression and behavioral change: `findChildrenByParentId` DB query is executed on **every** event type update where `children` is not explicitly provided, even when `pendingChildrenChanges` is also undefined. This adds an unnecessary DB round-trip for non-managed event types and changes the semantics for `updateChildrenEventTypes` (previously received `undefined` meaning "no changes", now receives a fully reconstructed children array).
Guard this block to only execute when there are actually pending children changes to process.</violation>
</file>
<file name="packages/features/users/repositories/UserRepository.ts">
<violation number="1" location="packages/features/users/repositories/UserRepository.ts:471">
P2: `total` is computed with the cursor filter applied, so pagination pages after the first report a shrinking total. If `total` is meant to represent the full match count, compute it without the cursor constraint.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts
Outdated
Show resolved
Hide resolved
| const userIdFilter = memberUserIds?.length | ||
| ? cursor | ||
| ? { in: memberUserIds, gt: cursor } | ||
| : { in: memberUserIds } | ||
| : cursor | ||
| ? { gt: cursor } | ||
| : undefined; |
There was a problem hiding this comment.
P2: memberUserIds being an empty array currently disables the filter and returns all hosts. Treat an empty array as a valid filter so callers requesting no members get an empty result set instead of all hosts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/host/repositories/HostRepository.ts, line 206:
<comment>`memberUserIds` being an empty array currently disables the filter and returns all hosts. Treat an empty array as a valid filter so callers requesting no members get an empty result set instead of all hosts.</comment>
<file context>
@@ -142,6 +142,188 @@ export class HostRepository {
+ search?: string;
+ memberUserIds?: number[];
+ }) {
+ const userIdFilter = memberUserIds?.length
+ ? cursor
+ ? { in: memberUserIds, gt: cursor }
</file context>
| const userIdFilter = memberUserIds?.length | |
| ? cursor | |
| ? { in: memberUserIds, gt: cursor } | |
| : { in: memberUserIds } | |
| : cursor | |
| ? { gt: cursor } | |
| : undefined; | |
| const userIdFilter = memberUserIds | |
| ? cursor | |
| ? { in: memberUserIds, gt: cursor } | |
| : { in: memberUserIds } | |
| : cursor | |
| ? { gt: cursor } | |
| : undefined; |
| const items = hasMore ? users.slice(0, limit) : users; | ||
| const nextCursor = hasMore ? items[items.length - 1].id : undefined; | ||
|
|
||
| const total = await this.prismaClient.user.count({ where }); |
There was a problem hiding this comment.
P2: total is computed with the cursor filter applied, so pagination pages after the first report a shrinking total. If total is meant to represent the full match count, compute it without the cursor constraint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/users/repositories/UserRepository.ts, line 471:
<comment>`total` is computed with the cursor filter applied, so pagination pages after the first report a shrinking total. If `total` is meant to represent the full match count, compute it without the cursor constraint.</comment>
<file context>
@@ -427,6 +427,52 @@ export class UserRepository {
+ const items = hasMore ? users.slice(0, limit) : users;
+ const nextCursor = hasMore ? items[items.length - 1].id : undefined;
+
+ const total = await this.prismaClient.user.count({ where });
+
+ return { users: items, nextCursor, total };
</file context>
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. ✅ Pushed commit |
…nges exists Co-Authored-By: unknown <>
Review annotations from original PR #27371These are the review annotations from @joeauyeung on the original PR, carried over to this split PR for reviewer context.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
|
|
||
| const { locations, metadata, ...restEventType } = rawEventType; | ||
| const newMetadata = eventTypeMetaDataSchemaWithTypedApps.parse(metadata || {}) || {}; |
There was a problem hiding this comment.
Making async calls in a loop wrecks performance. Especially for large event types. Since we're paginating team members we don't need to make these calls.
| } | ||
| : [] | ||
| ), | ||
| childrenCount: rawEventType._count?.children ?? 0, |
There was a problem hiding this comment.
Since we're paginating the children event types we don't need to fetch this data and iterate through it
| const currentUserMembership = eventTypeObject.team?.members.find((el) => el.user.id === userId) ?? null; | ||
| // Find the current user's membership role to enable/disable deletion in the UI. | ||
| // Null when not a team event type. | ||
| const membershipRepo = new MembershipRepository(prisma); |
There was a problem hiding this comment.
Fetch the current user's membership to determine if they have permission to delete the event type
| }); | ||
| } | ||
|
|
||
| export const getEventTypeByIdWithTeamMembers = async (props: getEventTypeByIdProps) => { |
There was a problem hiding this comment.
This wrapper around getEventTypeById still fetches memberships since atoms expect membership data along with the event type data.
| }, | ||
| }, | ||
| hosts: { | ||
| _count: { |
There was a problem hiding this comment.
The changes in this file only affect getEventTypeById
| eventTypeColor, | ||
| users, | ||
| children, | ||
| pendingChildrenChanges, |
There was a problem hiding this comment.
@eunjae-lee's review comment from PR #27371: can we use a repository instead of direct prisma usage
| hasMore: boolean; | ||
| }; | ||
|
|
||
| export const searchTeamMembersHandler = async ({ |
There was a problem hiding this comment.
@eunjae-lee's review comment from PR #27371: let's move this to a repository
| const matchingTeamMembers = await new UserRepository(ctx.prisma).findByIds({ ids: matchingTeamMembersIds }); | ||
| const userRepo = new UserRepository(ctx.prisma); | ||
|
|
||
| const { users, nextCursor, total } = await userRepo.findByIdsWithPagination({ |
There was a problem hiding this comment.
@eunjae-lee's review comment from PR #27371: can we update findByIds or create a new method that supports search & limit & cursor out of the box, so that we don't fetch everything and filter at the app level?
|
@Udit-takkar @eunjae-lee @volnei here's the 1st PR in the stack for improving the event type page. I had Devin move all my inline comments here |
Move pendingHostChanges/pendingChildrenChanges processing out of backend PR. These changes belong in the frontend PR since they are tightly coupled to the new frontend delta tracking components. Backend PR now contains only read-side optimizations: - Paginated host/children/member endpoints - Repository methods - getEventTypeById optimizations Co-Authored-By: unknown <>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts">
<violation number="1">
P2: O(n×m) membership check: `teamMemberIds` is an array, and `Array.includes()` is O(m) per host. With 700+ team members (the target scale of this PR), convert to a `Set` for O(1) lookups.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. ✅ No changes pushed |
devin-ai-integration
bot
changed the title
Reverts getEventTypeById.ts, eventTypeRepository.ts, API v2 atom service, and platform libraries to main. The backend PR now only adds new infrastructure (paginated endpoints, repository methods, findChildrenByParentId) without changing existing return types. The getEventTypeById optimizations will be in the frontend PR instead. Co-Authored-By: unknown <>
…ontend PR The handler's return type change (adding nextCursor/total) breaks frontend files on main that expect the old shape. Moving these changes to the frontend PR keeps the backend PR purely additive. Co-Authored-By: unknown <>
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/features/eventtypes/lib/getEventTypeById.ts">
<violation number="1">
P2: This performs a linear search through team members for every child, which can become O(n²) on large teams. Precompute a memberRoleByUserId map once and use O(1) lookups inside the child mapping.</violation>
<violation number="2">
P2: This loop awaits each owner enrichment sequentially. For many managed children, this adds unnecessary latency; use Promise.all to run enrichments concurrently.</violation>
<violation number="3">
P2: This per-member profile enrichment runs sequentially in a loop, which adds avoidable latency for large teams. Parallelize with Promise.all to keep the per-member lookups concurrent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
|
The three Cubic violations are all in |
devin-ai-integration
bot
changed the title
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/server/routers/viewer/attributes/findTeamMembersMatchingAttributeLogic.handler.ts">
<violation number="1">
P2: This handler now loads full `userSelect` records even though it only returns `id/name/email`, which pulls sensitive/large columns unnecessarily. Use a minimal select for this lookup to avoid extra data exposure and DB overhead.</violation>
<violation number="2">
P2: Pagination/search support is removed from this handler, so it now always loads all matching users and returns an unbounded payload. This is a behavior regression and can hurt performance on large teams.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
|
Both Cubic violations on ✅ No changes pushed |
1 participant
What does this PR do?
This is the backend half of #27371, split for easier review. It adds the server-side infrastructure for paginated host fetching to dramatically improve event type page performance for teams with many hosts (tested with 700+).
This PR is purely additive — it introduces new paginated tRPC endpoints and repository methods without modifying any existing return types or queries. The
getEventTypeByIdoptimizations, delta-based saving logic, andfindTeamMembersMatchingAttributeLogicpagination all live in the frontend PR #28155.Key changes
New paginated tRPC endpoints:
getHostsForAssignment/getHostsForAvailability– cursor-paginated host queriessearchTeamMembers– paginated team member search with text filteringgetChildrenForAssignment– paginated managed event childrenexportHostsForWeights– CSV download of host weightsRepository additions:
HostRepositorypagination methods (findHostsForAssignmentPaginated,findHostsForAvailabilityPaginated,findAllRoundRobinHosts,findChildrenForAssignmentPaginated)MembershipRepository.searchMembers(),findRoleByUserIdAndTeamId(),findMembershipsWithUserByTeamId()UserRepository.findByIdsWithPagination()EventTypeRepository.findChildrenByParentId()Minor fix:
getHostsWithLocationOptionscursor schema changed from.optional()to.nullish()for consistency with other paginated endpointsMandatory Tasks (DO NOT REMOVE)
How should this be tested?
Prerequisites:
Manual testing:
Human Review Checklist
hasMembershipbehavioral change –searchTeamMembershandler useshasMembership()which checksaccepted: true, whereas the original handler didn't check membership acceptance. Confirm this is intended (more secure – only accepted members can search).MembershipRepository.searchMembersPrisma query – When bothmemberUserIdsandcursorare provided, it uses{ in: memberUserIds, gt: cursor }on the same field. Confirm Prisma handles this correctly (should filter to IDs in array AND greater than cursor).UserRepository.findByIdsWithPaginationtotal count – Thetotalis computed with the cursor filter applied, so pagination pages after the first report a shrinking total. Confirm whether callers need the full match count or if per-page count is acceptable.CI Status
✅ All checks passing:
ready-for-e2elabel. Therequiredcheck intentionally fails when E2E tests are skipped to prevent merging without E2E validation.Checklist
Link to Devin run: https://app.devin.ai/sessions/91be6db4b8794a4e8f11e2fa2649a2d5
Requested by: @joeauyeung