From 7d37691424cb3695753a3714e0d23995a6bb8a08 Mon Sep 17 00:00:00 2001 From: Scott Cazan Date: Thu, 22 Jan 2026 21:38:22 +0100 Subject: [PATCH 1/6] Add sorting to listUsers, extract standard sorting params --- .../src/services/profile/listProfileUsers.ts | 34 +++++++++++++++++-- packages/common/src/utils/db.ts | 3 ++ .../src/routers/profile/users/listUsers.ts | 15 ++++++-- services/api/src/utils/index.ts | 28 ++++++++++++++- 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/packages/common/src/services/profile/listProfileUsers.ts b/packages/common/src/services/profile/listProfileUsers.ts index f508d32de..95a2c68c4 100644 --- a/packages/common/src/services/profile/listProfileUsers.ts +++ b/packages/common/src/services/profile/listProfileUsers.ts @@ -1,4 +1,4 @@ -import { db, eq } from '@op/db/client'; +import { db, eq, sql } from '@op/db/client'; import { profileUsers } from '@op/db/schema'; import type { User } from '@op/supabase/lib'; import { assertAccess, permission } from 'access-zones'; @@ -11,15 +11,23 @@ import type { ProfileUserWithRelations, } from './getProfileUserWithRelations'; +import type { SortDir } from '../../utils/db'; + +export type ProfileUserOrderBy = 'name' | 'email' | 'role'; + /** * List all members of a profile */ export const listProfileUsers = async ({ profileId, user, + orderBy = 'name', + dir = 'asc', }: { profileId: string; user: User; + orderBy?: ProfileUserOrderBy; + dir?: SortDir; }): Promise => { const [profileAccessUser] = await Promise.all([ getProfileAccessUser({ user, profileId }), @@ -51,7 +59,29 @@ export const listProfileUsers = async ({ }, }, }, - orderBy: (table, { asc }) => [asc(table.name), asc(table.email)], + orderBy: (table, { asc, desc }) => { + const orderFn = dir === 'desc' ? desc : asc; + + if (orderBy === 'role') { + // Use a subquery to get the first role name for sorting + // Note: Using raw SQL strings because Drizzle's sql template uses outer query aliases + const roleNameSubquery = sql`( + SELECT ar.name + FROM "profileUser_to_access_roles" pur + INNER JOIN "access_roles" ar ON ar.id = pur.access_role_id + WHERE pur.profile_user_id = ${table.id} + LIMIT 1 + )`; + return [orderFn(roleNameSubquery)]; + } + + if (orderBy === 'email') { + return [orderFn(table.email)]; + } + + // Default to name + return [orderFn(table.name)]; + }, }); return profileUserResults.map((result) => { diff --git a/packages/common/src/utils/db.ts b/packages/common/src/utils/db.ts index dc08c60e2..8392dad43 100644 --- a/packages/common/src/utils/db.ts +++ b/packages/common/src/utils/db.ts @@ -3,6 +3,9 @@ import { PgColumn } from 'drizzle-orm/pg-core'; import { CommonError } from './error'; +/** Standard sort direction type for database queries */ +export type SortDir = 'asc' | 'desc'; + // Cursor utilities type GenericCursor = { date: Date; diff --git a/services/api/src/routers/profile/users/listUsers.ts b/services/api/src/routers/profile/users/listUsers.ts index b35850246..7a5561ae3 100644 --- a/services/api/src/routers/profile/users/listUsers.ts +++ b/services/api/src/routers/profile/users/listUsers.ts @@ -3,18 +3,29 @@ import { z } from 'zod'; import { profileUserEncoder } from '../../../encoders/profiles'; import { commonAuthedProcedure, router } from '../../../trpcFactory'; +import { createSortable } from '../../../utils'; + +const profileUserSortable = createSortable(['name', 'email', 'role'] as const); export const listUsersRouter = router({ listUsers: commonAuthedProcedure() - .input(z.object({ profileId: z.uuid() })) + .input( + z + .object({ + profileId: z.uuid(), + }) + .merge(profileUserSortable), + ) .output(z.array(profileUserEncoder)) .query(async ({ ctx, input }) => { const { user } = ctx; - const { profileId } = input; + const { profileId, orderBy, dir } = input; return listProfileUsers({ profileId, user, + orderBy, + dir, }); }), }); diff --git a/services/api/src/utils/index.ts b/services/api/src/utils/index.ts index 8bbfe52df..73c571f67 100644 --- a/services/api/src/utils/index.ts +++ b/services/api/src/utils/index.ts @@ -1,11 +1,37 @@ +import type { SortDir } from '@op/common'; import sanitizeForS3 from 'sanitize-s3-objectkey'; import { z } from 'zod'; +/** Standard sort direction schema */ +export const sortDir = z.enum(['asc', 'desc']) satisfies z.ZodType; +export type { SortDir }; + +/** + * Creates a type-safe sortable schema for a given set of columns + * @example + * const userSortable = createSortable(['name', 'email', 'createdAt']); + * // Results in: { orderBy?: 'name' | 'email' | 'createdAt', dir?: 'asc' | 'desc' } + */ +export const createSortable = ( + columns: T, +) => + z.object({ + orderBy: z.enum(columns).optional(), + dir: sortDir.optional(), + }); + +/** Generic sortable schema when column types aren't constrained */ +export const sortable = z.object({ + orderBy: z.string().optional(), + dir: sortDir.optional(), +}); +export type Sortable = z.infer; + export const dbFilter = z.object({ limit: z.number().optional(), cursor: z.string().nullish(), orderBy: z.string().optional(), - dir: z.enum(['asc', 'desc']).optional(), + dir: sortDir.optional(), }); export function sanitizeS3Filename(filename: string) { From e888c385687ff0c0c23514d1f2781c229938cde2 Mon Sep 17 00:00:00 2001 From: Scott Cazan Date: Thu, 22 Jan 2026 21:57:00 +0100 Subject: [PATCH 2/6] Update sorting in tests --- .../routers/profile/users/listUsers.test.ts | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/services/api/src/routers/profile/users/listUsers.test.ts b/services/api/src/routers/profile/users/listUsers.test.ts index d31b5813c..fe82a7e89 100644 --- a/services/api/src/routers/profile/users/listUsers.test.ts +++ b/services/api/src/routers/profile/users/listUsers.test.ts @@ -107,4 +107,106 @@ describe.concurrent('profile.users.listUsers', () => { }), ).rejects.toThrow(/not found/i); }); + + describe('sorting', () => { + it('should sort users by name with admin first in asc and last in desc', async ({ + task, + onTestFinished, + }) => { + const testData = new TestProfileUserDataManager(task.id, onTestFinished); + const { profile, adminUser } = await testData.createProfile({ + users: { admin: 1, member: 2 }, + }); + + const { session } = await createIsolatedSession(adminUser.email); + const caller = createCaller(await createTestContextWithSession(session)); + + const resultAsc = await caller.listUsers({ + profileId: profile.id, + orderBy: 'name', + dir: 'asc', + }); + + const resultDesc = await caller.listUsers({ + profileId: profile.id, + orderBy: 'name', + dir: 'desc', + }); + + expect(resultAsc).toHaveLength(3); + expect(resultDesc).toHaveLength(3); + + // Test data creates names like "Test Admin User" and "Test Member User" + // "Test Admin User" < "Test Member User" alphabetically (A < M after "Test ") + // So admin should be first in ASC order and last in DESC order + expect(resultAsc[0]?.email).toBe(adminUser.email); + expect(resultDesc[resultDesc.length - 1]?.email).toBe(adminUser.email); + }); + + it('should reverse order when switching between asc and desc for email', async ({ + task, + onTestFinished, + }) => { + const testData = new TestProfileUserDataManager(task.id, onTestFinished); + const { profile, adminUser } = await testData.createProfile({ + users: { admin: 1, member: 2 }, + }); + + const { session } = await createIsolatedSession(adminUser.email); + const caller = createCaller(await createTestContextWithSession(session)); + + const resultAsc = await caller.listUsers({ + profileId: profile.id, + orderBy: 'email', + dir: 'asc', + }); + + const resultDesc = await caller.listUsers({ + profileId: profile.id, + orderBy: 'email', + dir: 'desc', + }); + + expect(resultAsc).toHaveLength(3); + expect(resultDesc).toHaveLength(3); + + // Emails are unique, so ascending and descending should be exact reverses + const ascEmails = resultAsc.map((u) => u.email); + const descEmails = resultDesc.map((u) => u.email); + expect(ascEmails).toEqual([...descEmails].reverse()); + }); + + it('should sort users by role with admin first in asc and last in desc', async ({ + task, + onTestFinished, + }) => { + const testData = new TestProfileUserDataManager(task.id, onTestFinished); + const { profile, adminUser } = await testData.createProfile({ + users: { admin: 1, member: 2 }, + }); + + const { session } = await createIsolatedSession(adminUser.email); + const caller = createCaller(await createTestContextWithSession(session)); + + const resultAsc = await caller.listUsers({ + profileId: profile.id, + orderBy: 'role', + dir: 'asc', + }); + + const resultDesc = await caller.listUsers({ + profileId: profile.id, + orderBy: 'role', + dir: 'desc', + }); + + expect(resultAsc).toHaveLength(3); + expect(resultDesc).toHaveLength(3); + + // "Admin" comes before "Member" alphabetically + // So admin should be first in ASC order and last in DESC order + expect(resultAsc[0]?.email).toBe(adminUser.email); + expect(resultDesc[resultDesc.length - 1]?.email).toBe(adminUser.email); + }); + }); }); From 02d23c0717073fbf5b0c118e27646a747695b068 Mon Sep 17 00:00:00 2001 From: Scott Cazan Date: Thu, 22 Jan 2026 21:59:16 +0100 Subject: [PATCH 3/6] format --- packages/common/src/services/profile/listProfileUsers.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/common/src/services/profile/listProfileUsers.ts b/packages/common/src/services/profile/listProfileUsers.ts index 95a2c68c4..4484e243a 100644 --- a/packages/common/src/services/profile/listProfileUsers.ts +++ b/packages/common/src/services/profile/listProfileUsers.ts @@ -3,6 +3,7 @@ import { profileUsers } from '@op/db/schema'; import type { User } from '@op/supabase/lib'; import { assertAccess, permission } from 'access-zones'; +import type { SortDir } from '../../utils/db'; import { UnauthorizedError } from '../../utils/error'; import { getProfileAccessUser } from '../access'; import { assertProfile } from '../assert'; @@ -11,8 +12,6 @@ import type { ProfileUserWithRelations, } from './getProfileUserWithRelations'; -import type { SortDir } from '../../utils/db'; - export type ProfileUserOrderBy = 'name' | 'email' | 'role'; /** From 72b5c2dc2fccc9fa3f0ad0c4c5b2bc01fc871157 Mon Sep 17 00:00:00 2001 From: Scott Cazan Date: Fri, 23 Jan 2026 19:12:27 +0100 Subject: [PATCH 4/6] Add ORDER BY name in sorting --- packages/common/src/services/profile/listProfileUsers.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/common/src/services/profile/listProfileUsers.ts b/packages/common/src/services/profile/listProfileUsers.ts index 4484e243a..4930fde5b 100644 --- a/packages/common/src/services/profile/listProfileUsers.ts +++ b/packages/common/src/services/profile/listProfileUsers.ts @@ -69,6 +69,7 @@ export const listProfileUsers = async ({ FROM "profileUser_to_access_roles" pur INNER JOIN "access_roles" ar ON ar.id = pur.access_role_id WHERE pur.profile_user_id = ${table.id} + ORDER BY ar.name LIMIT 1 )`; return [orderFn(roleNameSubquery)]; From 99e4b8b1a7d9d59bea98c0182ac1bf2705e98431 Mon Sep 17 00:00:00 2001 From: Scott Cazan Date: Fri, 23 Jan 2026 19:16:23 +0100 Subject: [PATCH 5/6] PR comments --- services/api/src/utils/index.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/services/api/src/utils/index.ts b/services/api/src/utils/index.ts index 73c571f67..e30437ba4 100644 --- a/services/api/src/utils/index.ts +++ b/services/api/src/utils/index.ts @@ -4,7 +4,6 @@ import { z } from 'zod'; /** Standard sort direction schema */ export const sortDir = z.enum(['asc', 'desc']) satisfies z.ZodType; -export type { SortDir }; /** * Creates a type-safe sortable schema for a given set of columns @@ -21,17 +20,15 @@ export const createSortable = ( }); /** Generic sortable schema when column types aren't constrained */ -export const sortable = z.object({ +const sortableSchema = z.object({ orderBy: z.string().optional(), dir: sortDir.optional(), }); -export type Sortable = z.infer; +export type Sortable = z.infer; -export const dbFilter = z.object({ +export const dbFilter = sortableSchema.extend({ limit: z.number().optional(), cursor: z.string().nullish(), - orderBy: z.string().optional(), - dir: sortDir.optional(), }); export function sanitizeS3Filename(filename: string) { From 440fe8773635b5f2dba6346019c918e25f859ad8 Mon Sep 17 00:00:00 2001 From: Scott Cazan Date: Fri, 23 Jan 2026 20:02:57 +0100 Subject: [PATCH 6/6] Add in comment with correlated subquery for ordering --- .../src/services/organization/getOrganizationUsers.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/common/src/services/organization/getOrganizationUsers.ts b/packages/common/src/services/organization/getOrganizationUsers.ts index e51e64b81..4b516a039 100644 --- a/packages/common/src/services/organization/getOrganizationUsers.ts +++ b/packages/common/src/services/organization/getOrganizationUsers.ts @@ -1,4 +1,4 @@ -import { db } from '@op/db/client'; +import { db, sql } from '@op/db/client'; import { User } from '@op/supabase/lib'; import { assertAccess, permission } from 'access-zones'; @@ -35,6 +35,15 @@ export const getOrganizationUsers = async ({ with: { accessRole: true, }, + // Correlated subquery to sort by role name. Drizzle's relational queries + // don't support ordering nested relations by a joined table's column directly. + // Performance is acceptable here: roles per user are few (1-3), and the + // subquery hits the primary key index on access_roles. + orderBy: (table, { asc }) => [ + asc( + sql`(SELECT name FROM access_roles WHERE id = ${table.accessRoleId})`, + ), + ], }, serviceUser: { with: {