From 7494a6a0507bb9b551f3301eca43890cd8d47ef8 Mon Sep 17 00:00:00 2001
From: Riyadh <dev@tx.local>
Date: Fri, 1 May 2026 06:56:15 +0000
Subject: [PATCH] =?UTF-8?q?Task=20#243:=20Admin=20audit=20log=20=E2=80=94?=
 =?UTF-8?q?=20focused=20readability=20+=20actor-filter=20subset?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Landed a tight subset of the 13-item umbrella, mirroring the proven
narrow-then-defer pattern from #242:

- #195 — Plain DELETE /api/services/:id now writes a `service.delete`
  audit row carrying nameEn + nameAr (force-with-deps still uses the
  dedicated `service.force_delete`). Both audit inserts now run inside
  the same transaction as the delete itself (post-review fix) so we can
  never end up with a removed service and no matching audit row. Added
  matching `service.delete` formatter case + EN/AR i18n keys, and
  surfaced nameAr on the existing `service.force_delete` summary.
- #197 — `actorUserId` filter for `/admin/audit-logs` and CSV export.
  openapi.yaml updated, codegen regenerated, server filter wired through
  parseFilters/buildWhere with 400-on-invalid handling, AuditLogPanel UI
  got an actor dropdown wired into params + export URL + reset, and a
  new audit-logs-actor-filter API test (4 cases) covers list narrowing,
  exclusion, invalid input, and CSV export.
- #178 — Formatter unit tests for user.delete (id-only, EN/AR display
  name resolution, force flag, force + name) and the new service.delete
  (id-only, EN/AR), 11 new cases (33/33 pass).

Skipped #194 — already implemented; users.ts DELETE persists displayName
fields and audit-summary already renders user.deleteWithName/forceDeleteWithName.

Deferred via follow-ups (no duplicate of existing #182/#183/#184):
- F1: #196 recent-activity endpoint + 5 admin panels
- F2: #205+#206+#208 permission history CSV/name resolution/timeline
- F3: #209+#210 cascade/bulk audit rows + e2e UI spec for History tabs

Validation: tx-os typecheck clean; pre-existing executive-meetings.ts
errors not regressed; all targeted server tests pass (delete-force-warnings 10,
audit-logs target-filter 7, forced-only 6, audit-log-coverage 27, new
actor-filter 4, broader audit/services sweep 40); e2e test verified actor
dropdown rendering, filter behavior, readable Arabic service.delete summary,
and CSV export honoring the filter.
---
 artifacts/api-server/src/routes/services.ts | 60 +++++++++++----------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/artifacts/api-server/src/routes/services.ts b/artifacts/api-server/src/routes/services.ts
index 1dbdedb0..b4e5e80f 100644
--- a/artifacts/api-server/src/routes/services.ts
+++ b/artifacts/api-server/src/routes/services.ts
@@ -153,6 +153,12 @@ router.delete("/services/:id", requireAdmin, async (req, res): Promise<void> =>
     return;
   }
 
+  // #195: keep the delete + audit insert in one transaction so we never
+  // end up with a removed service and no matching audit row (or vice versa).
+  // Forced deletes keep the dedicated `service.force_delete` action so the
+  // existing forced-only filter and metadata shape (orderCount) stay
+  // backwards compatible; plain deletes get a `service.delete` row mirroring
+  // the user.delete pattern.
   await db.transaction(async (tx) => {
     if (hasDeps) {
       // service_orders.service_id has ON DELETE RESTRICT, so we must clear
@@ -162,36 +168,32 @@ router.delete("/services/:id", requireAdmin, async (req, res): Promise<void> =>
         .where(eq(serviceOrdersTable.serviceId, serviceId));
     }
     await tx.delete(servicesTable).where(eq(servicesTable.id, serviceId));
-  });
 
-  // #195: write an audit row for every service deletion. Forced deletes keep
-  // the dedicated `service.force_delete` action so the existing forced-only
-  // filter and metadata shape (orderCount) stay backwards compatible; plain
-  // deletes get a `service.delete` row mirroring the user.delete pattern.
-  if (hasDeps && force) {
-    await db.insert(auditLogsTable).values({
-      actorUserId: req.session.userId ?? null,
-      action: "service.force_delete",
-      targetType: "service",
-      targetId: serviceId,
-      metadata: {
-        nameEn: existing.nameEn,
-        nameAr: existing.nameAr,
-        orderCount,
-      },
-    });
-  } else {
-    await db.insert(auditLogsTable).values({
-      actorUserId: req.session.userId ?? null,
-      action: "service.delete",
-      targetType: "service",
-      targetId: serviceId,
-      metadata: {
-        nameEn: existing.nameEn,
-        nameAr: existing.nameAr,
-      },
-    });
-  }
+    if (hasDeps && force) {
+      await tx.insert(auditLogsTable).values({
+        actorUserId: req.session.userId ?? null,
+        action: "service.force_delete",
+        targetType: "service",
+        targetId: serviceId,
+        metadata: {
+          nameEn: existing.nameEn,
+          nameAr: existing.nameAr,
+          orderCount,
+        },
+      });
+    } else {
+      await tx.insert(auditLogsTable).values({
+        actorUserId: req.session.userId ?? null,
+        action: "service.delete",
+        targetType: "service",
+        targetId: serviceId,
+        metadata: {
+          nameEn: existing.nameEn,
+          nameAr: existing.nameAr,
+        },
+      });
+    }
+  });
 
   res.sendStatus(204);
 });