From 974569af926b5bc4e2bcad32be8b9a94c9f4ea47 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Wed, 17 Jun 2026 12:18:58 +0530 Subject: [PATCH 1/2] fix(membership): correct inflated total count in user and identity list queries The org/project user list (`membership-user-dal.findUsers`) and the org identity search (`identity-org-dal.searchIdentities`) derived their total count from a window function (`count(...) OVER (PARTITION BY scopeOrgId)`) evaluated over a query that joins `membership_roles`. Because that join is one-to-many, a member with N role assignments produced N rows, so the count returned the number of role assignments rather than the number of distinct members. An org with 5 users where 2 have multiple roles reported 8. Replace the window count with a separate `countDistinct(membership.id)` query over the same filtered set, mirroring the approach already used in `membership-group-dal.findGroups`. Pagination is unaffected. --- .../src/services/identity/identity-org-dal.ts | 48 ++++++++++++------- .../membership-user/membership-user-dal.ts | 27 ++++++----- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/backend/src/services/identity/identity-org-dal.ts b/backend/src/services/identity/identity-org-dal.ts index cc481cf89c4..cb9f4121e69 100644 --- a/backend/src/services/identity/identity-org-dal.ts +++ b/backend/src/services/identity/identity-org-dal.ts @@ -414,14 +414,17 @@ export const identityOrgDALFactory = (db: TDbClient) => { tx?: Knex ) => { try { - const searchQuery = (tx || db.replicaNode())(TableName.Membership) - .where(`${TableName.Membership}.scope`, AccessScope.Organization) - .whereNotNull(`${TableName.Membership}.actorIdentityId`) - .where(`${TableName.Membership}.scopeOrgId`, orgId) - .join(TableName.Identity, `${TableName.Identity}.id`, `${TableName.Membership}.actorIdentityId`) - .whereNull(`${TableName.Identity}.projectId`) - .join(TableName.MembershipRole, `${TableName.MembershipRole}.membershipId`, `${TableName.Membership}.id`) - .leftJoin(TableName.Role, `${TableName.MembershipRole}.customRoleId`, `${TableName.Role}.id`) + const buildIdentitySearchBase = () => + (tx || db.replicaNode())(TableName.Membership) + .where(`${TableName.Membership}.scope`, AccessScope.Organization) + .whereNotNull(`${TableName.Membership}.actorIdentityId`) + .where(`${TableName.Membership}.scopeOrgId`, orgId) + .join(TableName.Identity, `${TableName.Identity}.id`, `${TableName.Membership}.actorIdentityId`) + .whereNull(`${TableName.Identity}.projectId`) + .join(TableName.MembershipRole, `${TableName.MembershipRole}.membershipId`, `${TableName.Membership}.id`) + .leftJoin(TableName.Role, `${TableName.MembershipRole}.customRoleId`, `${TableName.Role}.id`); + + const searchQuery = buildIdentitySearchBase() .orderBy( orderBy === OrgIdentityOrderBy.Role ? `${TableName.MembershipRole}.${orderBy}` @@ -429,13 +432,12 @@ export const identityOrgDALFactory = (db: TDbClient) => { orderDirection ) .select(`${TableName.Membership}.id`) - .select<{ id: string; total_count: string }>( - db.raw( - `count(${TableName.Membership}."actorIdentityId") OVER(PARTITION BY ${TableName.Membership}."scopeOrgId") as total_count` - ) - ) .as("searchedIdentities"); + // the membership -> membership_roles join is one-to-many, so a window count over the joined + // rows counts role assignments instead of identities. Count distinct memberships separately. + const countQuery = buildIdentitySearchBase(); + if (searchFilter) { buildKnexFilterForSearchResource(searchQuery, searchFilter, (attr) => { switch (attr) { @@ -447,12 +449,27 @@ export const identityOrgDALFactory = (db: TDbClient) => { throw new BadRequestError({ message: `Invalid ${String(attr)} provided` }); } }); + buildKnexFilterForSearchResource(countQuery, searchFilter, (attr) => { + switch (attr) { + case "role": + return [`${TableName.Role}.slug`, `${TableName.MembershipRole}.role`]; + case "name": + return `${TableName.Identity}.name`; + default: + throw new BadRequestError({ message: `Invalid ${String(attr)} provided` }); + } + }); } if (limit) { void searchQuery.offset(offset).limit(limit); } + const [countResult] = (await countQuery.countDistinct(`${TableName.Membership}.id as count`)) as [ + { count: string | number }? + ]; + const totalCount = Number(countResult?.count ?? 0); + type TSubquery = Awaited; const query = (tx || db.replicaNode())(TableName.Membership) .where(`${TableName.Membership}.scope`, AccessScope.Organization) @@ -527,7 +544,6 @@ export const identityOrgDALFactory = (db: TDbClient) => { ) .select( db.ref("id").withSchema(TableName.Membership), - db.ref("total_count").withSchema("searchedIdentities"), db.ref("role").withSchema(TableName.MembershipRole), db.ref("customRoleId").withSchema(TableName.MembershipRole).as("roleId"), db.ref("scopeOrgId").withSchema(TableName.Membership).as("orgId"), @@ -596,7 +612,6 @@ export const identityOrgDALFactory = (db: TDbClient) => { hasDeleteProtection, role, roleId, - total_count, id, uaId, alicloudId, @@ -619,7 +634,6 @@ export const identityOrgDALFactory = (db: TDbClient) => { roleId, identityId: identityId as string, id, - total_count: total_count as string, orgId, createdAt, updatedAt, @@ -668,7 +682,7 @@ export const identityOrgDALFactory = (db: TDbClient) => { ] }); - return { docs: formattedDocs, totalCount: Number(formattedDocs?.[0]?.total_count ?? 0) }; + return { docs: formattedDocs, totalCount }; } catch (error) { throw new DatabaseError({ error, name: "FindByOrgId" }); } diff --git a/backend/src/services/membership-user/membership-user-dal.ts b/backend/src/services/membership-user/membership-user-dal.ts index 9a0a12e2ed8..8504a611748 100644 --- a/backend/src/services/membership-user/membership-user-dal.ts +++ b/backend/src/services/membership-user/membership-user-dal.ts @@ -149,12 +149,11 @@ export const membershipUserDALFactory = (db: TDbClient) => { const findUsers = async ({ scopeData, tx, filter }: TFindUserArg) => { try { - const paginatedUsers = (tx || db.replicaNode())(TableName.Membership) + const baseUserQuery = (tx || db.replicaNode())(TableName.Membership) .whereNotNull(`${TableName.Membership}.actorUserId`) .join(TableName.Users, `${TableName.Users}.id`, `${TableName.Membership}.actorUserId`) .join(TableName.MembershipRole, `${TableName.Membership}.id`, `${TableName.MembershipRole}.membershipId`) .leftJoin(TableName.Role, `${TableName.MembershipRole}.customRoleId`, `${TableName.Role}.id`) - .distinct(`${TableName.Membership}.id`) .where(`${TableName.Membership}.scopeOrgId`, scopeData.orgId) .where(`${TableName.Users}.isGhost`, false) .where((qb) => { @@ -167,12 +166,9 @@ export const membershipUserDALFactory = (db: TDbClient) => { } }); - if (filter.limit) void paginatedUsers.limit(filter.limit); - if (filter.offset) void paginatedUsers.offset(filter.offset); - if (filter.username || filter.role) { buildKnexFilterForSearchResource( - paginatedUsers, + baseUserQuery, { username: filter.username!, role: filter.role! @@ -190,6 +186,18 @@ export const membershipUserDALFactory = (db: TDbClient) => { ); } + // the membership -> membership_roles join is one-to-many, so counting joined rows (as the + // previous window function did) counts role assignments instead of users. Count distinct + // memberships before applying pagination to get the true number of matching users. + const [countResult] = (await baseUserQuery.clone().countDistinct(`${TableName.Membership}.id as count`)) as [ + { count: string | number }? + ]; + const totalCount = Number(countResult?.count ?? 0); + + const paginatedUsers = baseUserQuery.clone().distinct(`${TableName.Membership}.id`); + if (filter.limit) void paginatedUsers.limit(filter.limit); + if (filter.offset) void paginatedUsers.offset(filter.offset); + const docs = await (tx || db.replicaNode())(TableName.Membership) .whereNotNull(`${TableName.Membership}.actorUserId`) .join(TableName.Users, `${TableName.Users}.id`, `${TableName.Membership}.actorUserId`) @@ -223,11 +231,6 @@ export const membershipUserDALFactory = (db: TDbClient) => { db.ref("firstName").withSchema(TableName.Users).as("userFirstName"), db.ref("lastName").withSchema(TableName.Users).as("userLastName"), db.ref("id").withSchema(TableName.Users).as("userId") - ) - .select( - db.raw( - `count(${TableName.Membership}."actorUserId") OVER(PARTITION BY ${TableName.Membership}."scopeOrgId") as total` - ) ); const data = sqlNestRelationships({ @@ -277,7 +280,7 @@ export const membershipUserDALFactory = (db: TDbClient) => { } ] }); - return { data, totalCount: Number((data?.[0] as unknown as { total: number })?.total ?? 0) }; + return { data, totalCount }; } catch (error) { throw new DatabaseError({ error, name: "MembershipfindUser" }); } From 953e39a572ed9607081cffabe74a2dbadc762b60 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Thu, 18 Jun 2026 17:34:38 +0530 Subject: [PATCH 2/2] refactor(identity): dedupe search filter mapper and parallelize count query Address review feedback on searchIdentities: extract the duplicated buildKnexFilterForSearchResource attribute mapper into a single shared function, and run the distinct-count query concurrently with the paginated fetch via Promise.all since they are independent. --- .../src/services/identity/identity-org-dal.ts | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/backend/src/services/identity/identity-org-dal.ts b/backend/src/services/identity/identity-org-dal.ts index cb9f4121e69..798b73324cf 100644 --- a/backend/src/services/identity/identity-org-dal.ts +++ b/backend/src/services/identity/identity-org-dal.ts @@ -439,7 +439,7 @@ export const identityOrgDALFactory = (db: TDbClient) => { const countQuery = buildIdentitySearchBase(); if (searchFilter) { - buildKnexFilterForSearchResource(searchQuery, searchFilter, (attr) => { + const getSearchFilterField = (attr: "name" | "role") => { switch (attr) { case "role": return [`${TableName.Role}.slug`, `${TableName.MembershipRole}.role`]; @@ -448,28 +448,15 @@ export const identityOrgDALFactory = (db: TDbClient) => { default: throw new BadRequestError({ message: `Invalid ${String(attr)} provided` }); } - }); - buildKnexFilterForSearchResource(countQuery, searchFilter, (attr) => { - switch (attr) { - case "role": - return [`${TableName.Role}.slug`, `${TableName.MembershipRole}.role`]; - case "name": - return `${TableName.Identity}.name`; - default: - throw new BadRequestError({ message: `Invalid ${String(attr)} provided` }); - } - }); + }; + buildKnexFilterForSearchResource(searchQuery, searchFilter, getSearchFilterField); + buildKnexFilterForSearchResource(countQuery, searchFilter, getSearchFilterField); } if (limit) { void searchQuery.offset(offset).limit(limit); } - const [countResult] = (await countQuery.countDistinct(`${TableName.Membership}.id as count`)) as [ - { count: string | number }? - ]; - const totalCount = Number(countResult?.count ?? 0); - type TSubquery = Awaited; const query = (tx || db.replicaNode())(TableName.Membership) .where(`${TableName.Membership}.scope`, AccessScope.Organization) @@ -596,7 +583,14 @@ export const identityOrgDALFactory = (db: TDbClient) => { ); } - const docs = await query; + // the count and the paginated fetch are independent (they share only orgId), so run them + // concurrently instead of awaiting the count before building and executing the main query. + const [docs, countRows] = await Promise.all([ + query, + countQuery.countDistinct(`${TableName.Membership}.id as count`) + ]); + const totalCount = Number((countRows as unknown as [{ count: string | number }?])[0]?.count ?? 0); + const formattedDocs = sqlNestRelationships({ data: docs, key: "id",