Make the tire hotel's audit rows findable and readable (#257)

Twenty of the module's twenty-five audit rows set no entity. Every other
feature sets one, so the audit log's entity filter silently skips all of
them and the detail drawer shows the field blank. Nothing errors; the rows
are just unfindable by the filter meant to find them. Each now names the
model its entityId points at, which is the only version of that field that
cannot mislead: readings and prep-work rows point at the set, because that
is the id they actually carry.

Two of the messages needed the same care. "Marked wash_tires as done" put a
column value in the middle of an English sentence, and three others said
"file(s)". Audit text is composed once and stored, so unlike everything
else in the app it can never be corrected at read time.

Four deletes now return the id they just removed, so their row can point at
it the way the other features' deletes do.

src/__tests__/features/tire-hotel/audit-entries.test.ts

@@ -0,0 +1,86 @@
+/**
+ * What the tire hotel writes into the audit trail.
+ *
+ * Two failures here are invisible until somebody goes looking months later.
+ *
+ * An audit row with no `entity` is dropped by the audit log's entity filter
+ * and shows no entity in the detail drawer. Nothing errors, the row is simply
+ * unfindable by the one filter meant to find it. Every other feature sets it,
+ * so the omission looks deliberate rather than forgotten.
+ *
+ * And the message is composed once and stored, so it can never be corrected
+ * at read time the way a translated string can. A database enum or a `(s)`
+ * that reaches it is there for good.
+ */
+
+import { describe, it, expect } from 'vitest'
+import { readFileSync, readdirSync } from 'node:fs'
+import path from 'node:path'
+import { plural, humanise } from '@/features/tire-hotel/Lib/auditText'
+import { TREATMENT_TYPES } from '@/features/tire-hotel/Lib/treatments'
+
+const ACTIONS_DIR = 'src/features/tire-hotel/Actions'
+
+/** Every audit block in the tire hotel, with the action it records. */
+function auditBlocks(): { file: string; action: string; body: string }[] {
+  const found: { file: string; action: string; body: string }[] = []
+  for (const name of readdirSync(ACTIONS_DIR)) {
+    if (!name.endsWith('.ts')) continue
+    const file = path.join(ACTIONS_DIR, name)
+    const source = readFileSync(file, 'utf-8')
+    for (const match of source.matchAll(
+      /audit:\s*(?:\([^)]*\)\s*=>\s*)?\(?\{((?:[^{}]|\{[^{}]*\})*)\}/g
+    )) {
+      const body = match[1]
+      const action = body.match(/action:\s*(.+)/)?.[1]?.trim()
+      if (!action) continue
+      found.push({ file, action, body })
+    }
+  }
+  return found
+}
+
+describe('every tire hotel audit row', () => {
+  it('finds the audit blocks at all', () => {
+    // Without this the sweeps below pass by inspecting nothing.
+    expect(auditBlocks().length).toBeGreaterThan(20)
+  })
+
+  it('names the thing it happened to', () => {
+    const anonymous = auditBlocks()
+      .filter(({ body }) => !body.includes('entity:'))
+      .map(({ file, action }) => `${file}  ${action}`)
+
+    expect(anonymous, `these write no entity:\n${anonymous.join('\n')}`).toEqual([])
+  })
+
+  it('carries no unfinished plural into stored text', () => {
+    const sloppy = auditBlocks()
+      .filter(({ body }) => /\(s\)/.test(body))
+      .map(({ file, action }) => `${file}  ${action}`)
+
+    expect(sloppy, `these store "(s)":\n${sloppy.join('\n')}`).toEqual([])
+  })
+})
+
+describe('the wording helpers', () => {
+  it('counts one thing and several things differently', () => {
+    expect(plural(1, 'file')).toBe('1 file')
+    expect(plural(3, 'file')).toBe('3 files')
+    expect(plural(0, 'file')).toBe('0 files')
+  })
+
+  it('takes an irregular plural when the noun needs one', () => {
+    expect(plural(2, 'entry', 'entries')).toBe('2 entries')
+  })
+
+  it('turns every treatment type into words', () => {
+    // The reported symptom: "Marked wash_tires as done", a column value
+    // sitting in the middle of an English sentence.
+    for (const type of TREATMENT_TYPES) {
+      expect(humanise(type)).not.toContain('_')
+    }
+    expect(humanise('wash_tires')).toBe('wash tires')
+    expect(humanise('tpms_service')).toBe('tpms service')
+  })
+})

src/features/tire-hotel/Actions/attachmentActions.ts

@@ -6,6 +6,7 @@ import { db } from '@/lib/db'
 import { withAuth } from '@/lib/with-auth'
 import { PermissionAction, PermissionSubject } from '@/lib/permissions'
 import { requireTireHotel } from '../Lib/tireHotelSettings'
+import { plural } from '../Lib/auditText'
 
 const READ = [{ action: PermissionAction.READ, subject: PermissionSubject.TIRE_HOTEL }]
 const UPDATE = [{ action: PermissionAction.UPDATE, subject: PermissionSubject.TIRE_HOTEL }]
@@ -110,8 +111,9 @@ export async function addTireSetAttachments(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.attach',
-        message: `Added ${result.added} file(s) to tire set ${result.reference ?? result.tireSetId}`,
-        metadata: { tireSetId: result.tireSetId },
+        entity: 'TireSet',
+        entityId: result.tireSetId,
+        message: `Added ${plural(result.added, 'file')} to tire set ${result.reference ?? result.tireSetId}`,
       }),
     }
   )
@@ -173,8 +175,9 @@ export async function deleteTireSetAttachment(id: string) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.detach',
+        entity: 'TireSet',
+        entityId: result.tireSetId,
         message: `Removed ${result.fileName} from a tire set`,
-        metadata: { tireSetId: result.tireSetId },
       }),
     }
   )

src/features/tire-hotel/Actions/storageActions.ts

@@ -14,6 +14,7 @@ import {
 import { buildLocationCode } from '../Lib/tireConstants'
 import { locationCapacity, warehouseCapacity } from '../Lib/capacity'
 import { requireTireHotel, getTireHotelSettings } from '../Lib/tireHotelSettings'
+import { plural } from '../Lib/auditText'
 
 const READ = [{ action: PermissionAction.READ, subject: PermissionSubject.TIRE_HOTEL }]
 const MANAGE = [{ action: PermissionAction.MANAGE, subject: PermissionSubject.TIRE_HOTEL }]
@@ -144,8 +145,9 @@ export async function createWarehouse(input: unknown) {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: 'tire_warehouse.create',
+        entity: 'TireWarehouse',
+        entityId: result.id,
         message: `Created tire warehouse ${result.name}`,
-        metadata: { warehouseId: result.id },
       }),
     }
   )
@@ -184,8 +186,9 @@ export async function updateWarehouse(input: unknown) {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: 'tire_warehouse.update',
+        entity: 'TireWarehouse',
+        entityId: result.id,
         message: `Updated tire warehouse ${result.name}`,
-        metadata: { warehouseId: result.id },
       }),
     }
   )
@@ -216,17 +219,19 @@ export async function deleteWarehouse(id: string) {
       if (storedCount > 0) {
         await db.tireWarehouse.update({ where: { id }, data: { isArchived: true } })
         revalidateStorage()
-        return { archived: true, name: warehouse.name, storedCount }
+        return { id, archived: true, name: warehouse.name, storedCount }
       }
 
       await db.tireWarehouse.delete({ where: { id } })
       revalidateStorage()
-      return { archived: false, name: warehouse.name, storedCount: 0 }
+      return { id, archived: false, name: warehouse.name, storedCount: 0 }
     },
     {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: result.archived ? 'tire_warehouse.archive' : 'tire_warehouse.delete',
+        entity: 'TireWarehouse',
+        entityId: result.id,
         message: `${result.archived ? 'Archived' : 'Deleted'} tire warehouse ${result.name}`,
       }),
     }
@@ -281,8 +286,9 @@ export async function createLocation(input: unknown) {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: 'tire_location.create',
+        entity: 'TireLocation',
+        entityId: result.id,
         message: `Created tire storage location ${result.code}`,
-        metadata: { locationId: result.id },
       }),
     }
   )
@@ -352,7 +358,8 @@ export async function createLocationsBulk(input: unknown) {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: 'tire_location.bulk_create',
-        message: `Created ${result.created} tire storage locations`,
+        entity: 'TireLocation',
+        message: `Created ${plural(result.created, 'tire storage location')}`,
         metadata: { skipped: result.skipped },
       }),
     }
@@ -421,8 +428,9 @@ export async function updateLocation(input: unknown) {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: 'tire_location.update',
+        entity: 'TireLocation',
+        entityId: result.id,
         message: `Updated tire storage location ${result.code}`,
-        metadata: { locationId: result.id },
       }),
     }
   )
@@ -447,12 +455,14 @@ export async function deleteLocation(id: string) {
 
       await db.tireLocation.delete({ where: { id } })
       revalidateStorage()
-      return { code: location.code }
+      return { id, code: location.code }
     },
     {
       requiredPermissions: MANAGE,
       audit: ({ result }) => ({
         action: 'tire_location.delete',
+        entity: 'TireLocation',
+        entityId: result.id,
         message: `Deleted tire storage location ${result.code}`,
       }),
     }

src/features/tire-hotel/Actions/tireSetActions.ts

@@ -15,6 +15,7 @@ import {
 } from '../Schema/tireHotelSchema'
 import type { MeasurementInput } from '../Schema/tireHotelSchema'
 import { requireTireHotel } from '../Lib/tireHotelSettings'
+import { plural } from '../Lib/auditText'
 
 const READ = [{ action: PermissionAction.READ, subject: PermissionSubject.TIRE_HOTEL }]
 const CREATE = [{ action: PermissionAction.CREATE, subject: PermissionSubject.TIRE_HOTEL }]
@@ -384,8 +385,10 @@ export async function checkInTireSet(input: unknown) {
       requiredPermissions: CREATE,
       audit: ({ result }) => ({
         action: 'tire_set.check_in',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Checked in tire set ${result.reference} to ${result.locationCode}`,
-        metadata: { tireSetId: result.id, quantity: result.quantity },
+        metadata: { quantity: result.quantity },
       }),
     }
   )
@@ -613,8 +616,10 @@ export async function returnTireSet(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.return',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Stored tire set ${result.reference} again, on ${result.locationCode}`,
-        metadata: { tireSetId: result.id, quantity: result.quantity },
+        metadata: { quantity: result.quantity },
       }),
     }
   )
@@ -675,8 +680,9 @@ export async function disposeTireSet(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.dispose',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Wrote off tire set ${result.reference}`,
-        metadata: { tireSetId: result.id },
       }),
     }
   )
@@ -729,8 +735,9 @@ export async function checkOutTireSet(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.check_out',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Checked out tire set ${result.reference}`,
-        metadata: { tireSetId: result.id },
       }),
     }
   )
@@ -782,8 +789,9 @@ export async function relocateTireSet(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.relocate',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Moved tire set ${result.reference} from ${result.fromCode ?? 'unassigned'} to ${result.toCode}`,
-        metadata: { tireSetId: result.id },
       }),
     }
   )
@@ -879,8 +887,9 @@ export async function updateTireSet(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_set.update',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Updated tire set ${result.reference}`,
-        metadata: { tireSetId: result.id },
       }),
     }
   )
@@ -914,8 +923,9 @@ export async function addMeasurements(input: { tireSetId: string; measurements:
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_measurement.create',
-        message: `Recorded ${result.count} tire measurement(s) on set ${result.reference}`,
-        metadata: { tireSetId: result.id },
+        entity: 'TireSet',
+        entityId: result.id,
+        message: `Recorded ${plural(result.count, 'reading')} on tire set ${result.reference}`,
       }),
     }
   )
@@ -937,12 +947,14 @@ export async function deleteTireSet(id: string) {
 
       await db.tireSet.delete({ where: { id } })
       revalidateTireHotel()
-      return { reference: set.reference }
+      return { id, reference: set.reference }
     },
     {
       requiredPermissions: DELETE,
       audit: ({ result }) => ({
         action: 'tire_set.delete',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Deleted tire set ${result.reference}`,
       }),
     }

src/features/tire-hotel/Actions/treatmentActions.ts

@@ -7,6 +7,7 @@ import { withAuth } from '@/lib/with-auth'
 import { PermissionAction, PermissionSubject } from '@/lib/permissions'
 import { TREATMENT_STATUSES, TREATMENT_TYPES } from '../Lib/treatments'
 import { requireTireHotel } from '../Lib/tireHotelSettings'
+import { humanise, plural } from '../Lib/auditText'
 
 const UPDATE = [{ action: PermissionAction.UPDATE, subject: PermissionSubject.TIRE_HOTEL }]
 
@@ -88,6 +89,8 @@ export async function setTreatments(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_treatment.update',
+        entity: 'TireSet',
+        entityId: result.id,
         message: `Updated prep work on tire set ${result.reference ?? result.id}`,
         metadata: { added: result.added, removed: result.removed },
       }),
@@ -132,8 +135,10 @@ export async function markTreatment(input: unknown) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_treatment.mark',
-        message: `Marked ${result.type} as ${result.status} on tire set ${result.reference ?? result.tireSetId}`,
-        metadata: { treatmentId: result.id },
+        entity: 'TireTreatment',
+        entityId: result.id,
+        message: `Marked ${humanise(result.type)} as ${result.status} on tire set ${result.reference ?? result.tireSetId}`,
+        metadata: { tireSetId: result.tireSetId },
       }),
     }
   )
@@ -163,7 +168,9 @@ export async function completeAllTreatments(tireSetId: string) {
       requiredPermissions: UPDATE,
       audit: ({ result }) => ({
         action: 'tire_treatment.complete_all',
-        message: `Completed ${result.count} prep job(s) on tire set ${result.reference ?? result.id}`,
+        entity: 'TireSet',
+        entityId: result.id,
+        message: `Completed ${plural(result.count, 'prep job')} on tire set ${result.reference ?? result.id}`,
       }),
     }
   )

src/features/tire-hotel/Lib/auditText.ts

@@ -0,0 +1,29 @@
+/**
+ * Wording for the audit trail.
+ *
+ * Audit messages are composed once and stored, so unlike the rest of the app
+ * they cannot be translated at read time. They are English by design, which
+ * makes it all the more important that they read as English: a stored line is
+ * the one thing nobody can fix later without a migration.
+ */
+
+/**
+ * "1 file", "3 files".
+ *
+ * Replaces the `file(s)` shorthand, which reads as a placeholder somebody
+ * forgot to finish and is wrong in the singular either way.
+ */
+export function plural(count: number, noun: string, pluralForm?: string): string {
+  return `${count} ${count === 1 ? noun : (pluralForm ?? `${noun}s`)}`
+}
+
+/**
+ * Turns a stored enum into something readable: `wash_tires` to `wash tires`.
+ *
+ * Not translated, and deliberately not a lookup table. A table would have to
+ * be kept in step with TREATMENT_TYPES by hand, and the failure when it drifts
+ * is silent: a new treatment type prints as nothing rather than as itself.
+ */
+export function humanise(value: string): string {
+  return value.replaceAll('_', ' ')
+}