feat(web): make every table column sortable

apps/api/src/audit/__tests__/audit.controller.test.ts

@@ -8,7 +8,7 @@ import { beforeEach, describe, expect, it } from 'vitest';
 import { AuditController } from '../audit.controller';
 import { AuditService } from '../audit.service';
 
-const query: $AuditLogsQueryParams = { limit: AUDIT_LOGS_PAGE_SIZE, page: 2, sortOrder: 'asc' };
+const query: $AuditLogsQueryParams = { limit: AUDIT_LOGS_PAGE_SIZE, page: 2, sortBy: 'timestamp', sortOrder: 'asc' };
 
 describe('AuditController', () => {
   let auditController: AuditController;

apps/api/src/audit/__tests__/audit.service.test.ts

@@ -9,7 +9,12 @@ import { beforeEach, describe, expect, it } from 'vitest';
 
 import { AuditService } from '../audit.service';
 
-const defaults: $AuditLogsQueryParams = { limit: AUDIT_LOGS_PAGE_SIZE, page: 1, sortOrder: 'desc' };
+const defaults: $AuditLogsQueryParams = {
+  limit: AUDIT_LOGS_PAGE_SIZE,
+  page: 1,
+  sortBy: 'timestamp',
+  sortOrder: 'desc'
+};
 
 describe('AuditService', () => {
   let auditService: AuditService;
@@ -41,11 +46,28 @@ describe('AuditService', () => {
 
     it('should sort in the database rather than leaving it to the client', async () => {
       await auditService.find(defaults);
-      expect(auditLogModel.findMany.mock.lastCall?.[0]).toMatchObject({ orderBy: { timestamp: 'desc' } });
+      expect(auditLogModel.findMany.mock.lastCall?.[0]).toMatchObject({
+        orderBy: [{ timestamp: 'desc' }, { id: 'desc' }]
+      });
       await auditService.find({ ...defaults, sortOrder: 'asc' });
-      expect(auditLogModel.findMany.mock.lastCall?.[0]).toMatchObject({ orderBy: { timestamp: 'asc' } });
+      expect(auditLogModel.findMany.mock.lastCall?.[0]).toMatchObject({
+        orderBy: [{ timestamp: 'asc' }, { id: 'desc' }]
+      });
     });
 
+    it.each([
+      ['action', { action: 'asc' }],
+      ['entity', { entity: 'asc' }],
+      ['group', { group: { name: 'asc' } }],
+      ['user', { user: { username: 'asc' } }]
+    ] as const)(
+      'should order by %s, breaking ties on the id so pages neither repeat nor skip',
+      async (sortBy, orderBy) => {
+        await auditService.find({ ...defaults, sortBy, sortOrder: 'asc' });
+        expect(auditLogModel.findMany.mock.lastCall?.[0]).toMatchObject({ orderBy: [orderBy, { id: 'desc' }] });
+      }
+    );
+
     it('should apply the filters to both the page and the count', async () => {
       await auditService.find({ ...defaults, action: 'LOGIN', groupId: 'group-1' });
       const where = { action: 'LOGIN', entity: undefined, groupId: 'group-1', userId: undefined };

apps/api/src/audit/audit.service.ts

@@ -1,7 +1,24 @@
 import { InjectModel } from '@douglasneuroinformatics/libnest';
 import type { Model } from '@douglasneuroinformatics/libnest';
 import { Injectable } from '@nestjs/common';
-import type { $AuditLogsPage, $AuditLogsQueryParams } from '@opendatacapture/schemas/audit';
+import type { $AuditLogsPage, $AuditLogsQueryParams, $AuditLogsSortBy } from '@opendatacapture/schemas/audit';
+
+type SortOrder = $AuditLogsQueryParams['sortOrder'];
+
+/**
+ * The trailing id makes the order total, since Mongo may otherwise return tied rows in a different
+ * order per page, repeating some rows and skipping others.
+ */
+const getOrderBy = (sortBy: $AuditLogsSortBy, sortOrder: SortOrder) => {
+  const orderBy = {
+    action: { action: sortOrder },
+    entity: { entity: sortOrder },
+    group: { group: { name: sortOrder } },
+    timestamp: { timestamp: sortOrder },
+    user: { user: { username: sortOrder } }
+  }[sortBy];
+  return [orderBy, { id: 'desc' as const }];
+};
 
 @Injectable()
 export class AuditService {
@@ -13,13 +30,14 @@ export class AuditService {
     groupId,
     limit,
     page,
+    sortBy,
     sortOrder,
     userId
   }: $AuditLogsQueryParams): Promise<$AuditLogsPage> {
     const where = { action, entity, groupId, userId };
     const [data, total] = await Promise.all([
       this.auditLogModel.findMany({
-        orderBy: { timestamp: sortOrder },
+        orderBy: getOrderBy(sortBy, sortOrder),
         select: {
           action: true,
           entity: true,

apps/api/src/instrument-repos/__tests__/instrument-repos.controller.test.ts

@@ -58,7 +58,7 @@ describe('InstrumentReposController', () => {
   });
 
   it("should scope the activity log to the current user's ability, and pass the page asked for", async () => {
-    const query = { limit: 10, page: 2, sortOrder: 'desc' } as const;
+    const query = { limit: 10, page: 2, sortBy: 'createdAt', sortOrder: 'desc' } as const;
     const result = { data: [], pageCount: 0, total: 0 };
     service.findSyncLogs.mockResolvedValueOnce(result);
     await expect(controller.findSyncLogs(query, ability)).resolves.toBe(result);

apps/api/src/instrument-repos/__tests__/instrument-repos.service.test.ts

@@ -398,7 +398,7 @@ describe('InstrumentReposService', () => {
   });
 
   describe('findSyncLogs', () => {
-    const query = { limit: 10, page: 1, sortOrder: 'desc' } as const;
+    const query = { limit: 10, page: 1, sortBy: 'createdAt', sortOrder: 'desc' } as const;
 
     it('should return the requested page of the history, with how many pages there are', async () => {
       instrumentRepoSyncLogModel.findMany.mockResolvedValueOnce([{ id: 'log-1' }]);
@@ -410,7 +410,7 @@ describe('InstrumentReposService', () => {
         total: 25
       });
       expect(instrumentRepoSyncLogModel.findMany).toHaveBeenCalledWith({
-        orderBy: { createdAt: 'desc' },
+        orderBy: [{ createdAt: 'desc' }, { id: 'desc' }],
         skip: 0,
         take: 10,
         where: { AND: [{}], repoId: undefined }
@@ -442,9 +442,30 @@ describe('InstrumentReposService', () => {
       await service.findSyncLogs({ ...query, sortOrder: 'asc' });
 
       expect(instrumentRepoSyncLogModel.findMany.mock.lastCall?.[0]).toMatchObject({
-        orderBy: { createdAt: 'asc' }
+        orderBy: [{ createdAt: 'asc' }, { id: 'desc' }]
       });
     });
+
+    it.each([
+      ['action', [{ action: 'asc' }]],
+      ['failedCount', [{ failures: { _count: 'asc' } }]],
+      ['importedCount', [{ importedCount: 'asc' }]],
+      ['repoName', [{ repoName: 'asc' }]],
+      // A null message sorts lowest, so this runs success, partial, failed, matching the derived status.
+      ['status', [{ message: 'asc' }, { failures: { _count: 'asc' } }]]
+    ] as const)(
+      'should order by %s, breaking ties on the id so pages neither repeat nor skip',
+      async (sortBy, orderBy) => {
+        instrumentRepoSyncLogModel.findMany.mockResolvedValueOnce([]);
+        instrumentRepoSyncLogModel.count.mockResolvedValueOnce(0);
+
+        await service.findSyncLogs({ ...query, sortBy, sortOrder: 'asc' });
+
+        expect(instrumentRepoSyncLogModel.findMany.mock.lastCall?.[0]).toMatchObject({
+          orderBy: [...orderBy, { id: 'desc' }]
+        });
+      }
+    );
   });
 
   describe('findLatestSyncLogs', () => {

apps/api/src/instrument-repos/instrument-repos.service.ts

@@ -44,6 +44,26 @@ type InstrumentRepoRecord = Awaited<ReturnType<Model<'InstrumentRepo'>['findFirs
  * The detail Nest exceptions thrown by `InstrumentsService.create` carry beyond their message: a VM
  * evaluation error as `cause`, schema validation errors as `issues`.
  */
+/**
+ * The trailing id makes the order total, since Mongo may otherwise return tied rows in a different
+ * order per page, repeating some rows and skipping others. Status is derived, not stored: Mongo sorts
+ * a null `message` lowest, so ordering by it and then by failure count runs SUCCESS, PARTIAL, FAILED.
+ */
+const getSyncLogOrderBy = (
+  sortBy: InstrumentRepoSyncLogsQueryParams['sortBy'],
+  sortOrder: InstrumentRepoSyncLogsQueryParams['sortOrder']
+) => {
+  const orderBy = {
+    action: [{ action: sortOrder }],
+    createdAt: [{ createdAt: sortOrder }],
+    failedCount: [{ failures: { _count: sortOrder } }],
+    importedCount: [{ importedCount: sortOrder }],
+    repoName: [{ repoName: sortOrder }],
+    status: [{ message: sortOrder }, { failures: { _count: sortOrder } }]
+  }[sortBy];
+  return [...orderBy, { id: 'desc' as const }];
+};
+
 const $ExceptionDetails = z.object({
   cause: z.object({ message: z.string() }).optional(),
   issues: z.array(z.object({ message: z.string(), path: z.array(z.union([z.string(), z.number()])) })).optional()
@@ -199,13 +219,13 @@ export class InstrumentReposService implements OnModuleInit {
    * returned whole: the client asks for the slice it is showing.
    */
   async findSyncLogs(
-    { limit, page, repoId, sortOrder }: InstrumentRepoSyncLogsQueryParams,
+    { limit, page, repoId, sortBy, sortOrder }: InstrumentRepoSyncLogsQueryParams,
     { ability }: EntityOperationOptions = {}
   ): Promise<InstrumentRepoSyncLogsPage> {
     const where = { AND: [accessibleQuery(ability, 'read', 'InstrumentRepoSyncLog')], repoId };
     const [data, total] = await Promise.all([
       this.instrumentRepoSyncLogModel.findMany({
-        orderBy: { createdAt: sortOrder },
+        orderBy: getSyncLogOrderBy(sortBy, sortOrder),
         skip: (page - 1) * limit,
         take: limit,
         where

apps/web/src/components/InstrumentRepoSyncLogTable/InstrumentRepoSyncLogTable.tsx

@@ -2,34 +2,47 @@ import { Fragment, useState } from 'react';
 
 import { Badge, Button, DataTable, Dialog } from '@douglasneuroinformatics/libui/components';
 import { useTheme, useTranslation } from '@douglasneuroinformatics/libui/hooks';
-import type { InstrumentRepoSyncLog, InstrumentRepoSyncStage } from '@opendatacapture/schemas/instrument-repo';
+import { $InstrumentRepoSyncLogsSortBy } from '@opendatacapture/schemas/instrument-repo';
+import type {
+  InstrumentRepoSyncLog,
+  InstrumentRepoSyncLogsSortBy,
+  InstrumentRepoSyncStage
+} from '@opendatacapture/schemas/instrument-repo';
 
 import { ColorTagCell } from '@/components/ColorTagCell';
 import { SortableHeader } from '@/components/SortableHeader';
 import { TruncatedCell } from '@/components/TruncatedCell';
-import { MIN_TABLE_BODY_HEIGHT } from '@/utils/data-table';
+import { MIN_TABLE_BODY_HEIGHT, readServerSort } from '@/utils/data-table';
 import { getSyncStatus } from '@/utils/instrument-repo-sync';
 import type { InstrumentRepoSyncStatus } from '@/utils/instrument-repo-sync';
 import { getSyncStatusColor } from '@/utils/tag-colors';
 
+type InstrumentRepoSyncLogSort = {
+  sortBy: InstrumentRepoSyncLogsSortBy;
+  sortOrder: 'asc' | 'desc';
+};
+
 type InstrumentRepoSyncLogTableProps = {
   logs: InstrumentRepoSyncLog[];
   onPageChange: (page: number) => void;
-  onSortOrderChange: (sortOrder: 'asc' | 'desc') => void;
+  onSortChange: (sort: InstrumentRepoSyncLogSort) => void;
   pageCount: number;
-  sortOrder: 'asc' | 'desc';
+  sort: InstrumentRepoSyncLogSort;
 };
 
-export const InstrumentRepoSyncLogTable = ({
+const InstrumentRepoSyncLogTable = ({
   logs,
   onPageChange,
-  onSortOrderChange,
+  onSortChange,
   pageCount,
-  sortOrder
+  sort
 }: InstrumentRepoSyncLogTableProps) => {
   const { t } = useTranslation();
   const [theme] = useTheme();
   const [logToView, setLogToView] = useState<InstrumentRepoSyncLog | null>(null);
+  // In server mode the table mounts with no sorting state of its own, so the column it is ordered by
+  // is told its direction, or it would read as unsorted.
+  const sortOrderOf = (sortBy: InstrumentRepoSyncLogsSortBy) => (sort.sortBy === sortBy ? sort.sortOrder : undefined);
 
   const statusLabels = {
     FAILED: t({ en: 'Failed', es: 'Fallido', fr: 'Échec' }),
@@ -74,6 +87,7 @@ export const InstrumentRepoSyncLogTable = ({
           disableSearch
           columns={[
             {
+              accessorKey: 'createdAt',
               cell: (ctx) => (
                 <TruncatedCell
                   title={ctx.row.original.createdAt.toLocaleString()}
@@ -83,15 +97,13 @@ export const InstrumentRepoSyncLogTable = ({
                   })}
                 />
               ),
-              enableSorting: true,
               header: ({ column }) => (
                 <SortableHeader
                   column={column}
                   label={t({ en: 'Date', es: 'Fecha', fr: 'Date' })}
-                  sortOrder={sortOrder}
+                  sortOrder={sortOrderOf('createdAt')}
                 />
               ),
-              id: 'date',
               size: 150,
               // The rows already arrive newest first, so the table's first toggle of its unsorted
               // column must go to ascending, or the first click would request what is on screen.
@@ -106,20 +118,31 @@ export const InstrumentRepoSyncLogTable = ({
                   value={ctx.row.original.repoName}
                 />
               ),
-              enableSorting: false,
-              header: t({ en: 'Repository', es: 'Repositorio', fr: 'Dépôt' })
+              header: ({ column }) => (
+                <SortableHeader
+                  column={column}
+                  label={t({ en: 'Repository', es: 'Repositorio', fr: 'Dépôt' })}
+                  sortOrder={sortOrderOf('repoName')}
+                />
+              )
             },
             {
+              accessorKey: 'action',
               cell: (ctx) => {
                 const label = actionLabels[ctx.row.original.action];
                 return <TruncatedCell title={label} value={label} />;
               },
-              enableSorting: false,
-              header: t({ en: 'Action', es: 'Acción', fr: 'Action' }),
-              id: 'action',
+              header: ({ column }) => (
+                <SortableHeader
+                  column={column}
+                  label={t({ en: 'Action', es: 'Acción', fr: 'Action' })}
+                  sortOrder={sortOrderOf('action')}
+                />
+              ),
               size: 95
             },
             {
+              accessorFn: getSyncStatus,
               cell: (ctx) => {
                 const status = getSyncStatus(ctx.row.original);
                 return (
@@ -130,8 +153,13 @@ export const InstrumentRepoSyncLogTable = ({
                   />
                 );
               },
-              enableSorting: false,
-              header: t({ en: 'Status', es: 'Estado', fr: 'Statut' }),
+              header: ({ column }) => (
+                <SortableHeader
+                  column={column}
+                  label={t({ en: 'Status', es: 'Estado', fr: 'Statut' })}
+                  sortOrder={sortOrderOf('status')}
+                />
+              ),
               id: 'status',
               size: 140
             },
@@ -141,19 +169,30 @@ export const InstrumentRepoSyncLogTable = ({
                 const count = String(ctx.row.original.importedCount);
                 return <TruncatedCell title={count} value={count} />;
               },
-              enableSorting: false,
-              header: t({ en: 'Imported', es: 'Importados', fr: 'Importés' }),
-              size: 105
+              header: ({ column }) => (
+                <SortableHeader
+                  column={column}
+                  label={t({ en: 'Imported', es: 'Importados', fr: 'Importés' })}
+                  sortOrder={sortOrderOf('importedCount')}
+                />
+              ),
+              size: 114
             },
             {
+              accessorFn: (log) => log.failures.length,
               cell: (ctx) => {
                 const count = String(ctx.row.original.failures.length);
                 return <TruncatedCell title={count} value={count} />;
               },
-              enableSorting: false,
-              header: t({ en: 'Failed', es: 'Fallidos', fr: 'Échoués' }),
+              header: ({ column }) => (
+                <SortableHeader
+                  column={column}
+                  label={t({ en: 'Failed', es: 'Fallidos', fr: 'Échoués' })}
+                  sortOrder={sortOrderOf('failedCount')}
+                />
+              ),
               id: 'failedCount',
-              size: 85
+              size: 92
             },
             {
               cell: (ctx) => {
@@ -185,13 +224,15 @@ export const InstrumentRepoSyncLogTable = ({
           // rest, so pinning every column but the repository leaves the repository the room a long
           // owner/name needs.
           initialState={{
-            columnPinning: { left: ['date'], right: ['action', 'status', 'importedCount', 'failedCount', 'messages'] }
+            columnPinning: {
+              left: ['createdAt'],
+              right: ['action', 'status', 'importedCount', 'failedCount', 'messages']
+            }
           }}
           mode="server"
           pageCount={pageCount}
           onPaginationChange={({ pageIndex }) => onPageChange(pageIndex + 1)}
-          // Date is the only sortable column, so the state describes it or nothing.
-          onSortingChange={(state) => onSortOrderChange(state.some(({ desc }) => desc) ? 'desc' : 'asc')}
+          onSortingChange={(state) => onSortChange(readServerSort(state, $InstrumentRepoSyncLogsSortBy.options))}
         />
       </div>
       <Dialog open={logToView !== null} onOpenChange={() => setLogToView(null)}>
@@ -227,3 +268,5 @@ export const InstrumentRepoSyncLogTable = ({
     </Fragment>
   );
 };
+
+export { type InstrumentRepoSyncLogSort, InstrumentRepoSyncLogTable };