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: { diff --git a/packages/common/src/services/profile/listProfileUsers.ts b/packages/common/src/services/profile/listProfileUsers.ts index f508d32de..4930fde5b 100644 --- a/packages/common/src/services/profile/listProfileUsers.ts +++ b/packages/common/src/services/profile/listProfileUsers.ts @@ -1,8 +1,9 @@ -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'; +import type { SortDir } from '../../utils/db'; import { UnauthorizedError } from '../../utils/error'; import { getProfileAccessUser } from '../access'; import { assertProfile } from '../assert'; @@ -11,15 +12,21 @@ import type { ProfileUserWithRelations, } from './getProfileUserWithRelations'; +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 +58,30 @@ 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} + ORDER BY ar.name + 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.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); + }); + }); }); 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..e30437ba4 100644 --- a/services/api/src/utils/index.ts +++ b/services/api/src/utils/index.ts @@ -1,11 +1,34 @@ +import type { SortDir } from '@op/common'; import sanitizeForS3 from 'sanitize-s3-objectkey'; import { z } from 'zod'; -export const dbFilter = z.object({ +/** Standard sort direction schema */ +export const sortDir = z.enum(['asc', 'desc']) satisfies z.ZodType; + +/** + * 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 */ +const sortableSchema = z.object({ + orderBy: z.string().optional(), + dir: sortDir.optional(), +}); +export type Sortable = z.infer; + +export const dbFilter = sortableSchema.extend({ limit: z.number().optional(), cursor: z.string().nullish(), - orderBy: z.string().optional(), - dir: z.enum(['asc', 'desc']).optional(), }); export function sanitizeS3Filename(filename: string) {