Task #195: Audit-log every service deletion, not only forced ones
Background
----------
`DELETE /api/services/:id` previously wrote an audit_logs row only when
hasDeps && force was true. A clean delete (no orders, or ?force=false on a
service with no dependents) left no trace, so admins reviewing the Audit
Log could not tell who removed a service or when, and the new
"Target: service #… · Name" line never appeared for routine deletions.
Implementation status (already in tree from prior work in this branch)
----------------------------------------------------------------------
- artifacts/api-server/src/routes/services.ts wraps the delete + audit
insert in a single db.transaction:
* hasDeps && force -> service.force_delete with { nameEn, nameAr,
orderCount } (unchanged on-the-wire shape, so existing forced-only
filter and target-filter behavior is preserved).
* otherwise (no deps, or force=true with no deps) -> new service.delete
row with { nameEn, nameAr } and the actor.
- artifacts/tx-os/src/lib/audit-summary.ts already has a service.delete
case rendering admin.audit.summary.service.delete (with name) or
service.deleteId (id-only fallback) in both EN and AR.
- delete-force-warnings.test.mjs already covers the route-level no-deps
and ?force=true-with-no-deps paths.
This commit
-----------
Adds the canonical audit-log coverage tests requested by the task to
artifacts/api-server/tests/audit-log-coverage.test.mjs:
- "DELETE /api/services/:id (no deps, no force)" -> exactly one
service.delete row with nameEn + nameAr, no orderCount, and no
service.force_delete companion.
- "DELETE /api/services/:id without force on a service with deps" ->
409 + service still present + zero audit rows of either action.
- "DELETE /api/services/:id?force=true (with deps)" -> exactly one
service.force_delete row with orderCount=2 and no plain service.delete
companion.
Also adds an insertService helper, a createdServiceIds bucket, and a
service_orders/services teardown block to keep the suite self-cleaning.
Verification
------------
- node --test tests/audit-log-coverage.test.mjs -> 29/29 pass (3 new).
- node --test tests/delete-force-warnings.test.mjs tests/service-orders.test.mjs
tests/audit-logs-forced-only-filter.test.mjs
tests/audit-logs-target-filter.test.mjs -> 34/34 pass (no regressions).
Deviations
----------
None. Scope matches the task brief exactly.
Replit-Task-Id: b12a13e6-32dd-4707-8a46-ea7a4e6336d9
This commit is contained in:
@@ -32,6 +32,7 @@ const createdUserIds = [];
|
||||
const createdGroupIds = [];
|
||||
const createdRoleIds = [];
|
||||
const createdAppIds = [];
|
||||
const createdServiceIds = [];
|
||||
|
||||
let originalSettings = null;
|
||||
|
||||
@@ -95,6 +96,19 @@ async function insertGroup(prefix) {
|
||||
return { id, name };
|
||||
}
|
||||
|
||||
async function insertService(prefix) {
|
||||
const nameEn = uniqueName(prefix);
|
||||
const nameAr = `${nameEn}_ar`;
|
||||
const r = await pool.query(
|
||||
`INSERT INTO services (name_ar, name_en, price, is_available)
|
||||
VALUES ($1, $2, 1.00, true) RETURNING id`,
|
||||
[nameAr, nameEn],
|
||||
);
|
||||
const id = r.rows[0].id;
|
||||
createdServiceIds.push(id);
|
||||
return { id, nameEn, nameAr };
|
||||
}
|
||||
|
||||
async function insertRole(prefix) {
|
||||
const name = uniqueName(prefix).toLowerCase();
|
||||
const r = await pool.query(
|
||||
@@ -190,6 +204,15 @@ after(async () => {
|
||||
createdGroupIds,
|
||||
]);
|
||||
}
|
||||
if (createdServiceIds.length) {
|
||||
await pool.query(
|
||||
`DELETE FROM service_orders WHERE service_id = ANY($1::int[])`,
|
||||
[createdServiceIds],
|
||||
);
|
||||
await pool.query(`DELETE FROM services WHERE id = ANY($1::int[])`, [
|
||||
createdServiceIds,
|
||||
]);
|
||||
}
|
||||
if (createdAppIds.length) {
|
||||
await pool.query(`DELETE FROM app_opens WHERE app_id = ANY($1::int[])`, [
|
||||
createdAppIds,
|
||||
@@ -915,6 +938,113 @@ test("DELETE /api/apps/:id?force=true (with deps) writes app.delete with force=t
|
||||
assert.equal(rows[0].metadata.openCount, 1);
|
||||
});
|
||||
|
||||
// ---------- services ----------
|
||||
|
||||
test("DELETE /api/services/:id (no deps, no force) writes one service.delete row with nameEn and nameAr", async () => {
|
||||
// #195: a clean delete (no orders) used to leave no audit trail at all.
|
||||
// It must now write a `service.delete` row carrying both names so the
|
||||
// audit log can render the service name in either locale after deletion.
|
||||
const svc = await insertService("aud_svc_d_nf");
|
||||
|
||||
const res = await fetch(`${API_BASE}/api/services/${svc.id}`, {
|
||||
method: "DELETE",
|
||||
headers: { Cookie: adminCookie },
|
||||
});
|
||||
assert.equal(res.status, 204);
|
||||
|
||||
const rows = await fetchAuditRows({
|
||||
action: "service.delete",
|
||||
targetId: svc.id,
|
||||
targetType: "service",
|
||||
});
|
||||
assert.equal(rows.length, 1, "expected exactly one service.delete audit row");
|
||||
const row = rows[0];
|
||||
assert.equal(row.actor_user_id, adminId);
|
||||
assert.equal(row.metadata.nameEn, svc.nameEn);
|
||||
assert.equal(row.metadata.nameAr, svc.nameAr);
|
||||
// No-deps path must NOT include the dependency counts that
|
||||
// `service.force_delete` carries.
|
||||
assert.equal(row.metadata.orderCount, undefined);
|
||||
|
||||
// The forced action must not be emitted on the clean path.
|
||||
const forced = await fetchAuditRows({
|
||||
action: "service.force_delete",
|
||||
targetId: svc.id,
|
||||
targetType: "service",
|
||||
});
|
||||
assert.equal(forced.length, 0);
|
||||
});
|
||||
|
||||
test("DELETE /api/services/:id without force on a service with deps returns 409 and writes NO audit row", async () => {
|
||||
const svc = await insertService("aud_svc_409");
|
||||
await pool.query(
|
||||
`INSERT INTO service_orders (user_id, service_id, status) VALUES ($1, $2, 'pending')`,
|
||||
[adminId, svc.id],
|
||||
);
|
||||
|
||||
const res = await fetch(`${API_BASE}/api/services/${svc.id}`, {
|
||||
method: "DELETE",
|
||||
headers: { Cookie: adminCookie },
|
||||
});
|
||||
assert.equal(res.status, 409);
|
||||
|
||||
const stillThere = await pool.query(
|
||||
`SELECT id FROM services WHERE id = $1`,
|
||||
[svc.id],
|
||||
);
|
||||
assert.equal(stillThere.rowCount, 1, "service must still exist after 409");
|
||||
|
||||
const deleteRows = await fetchAuditRows({
|
||||
action: "service.delete",
|
||||
targetId: svc.id,
|
||||
targetType: "service",
|
||||
});
|
||||
const forceRows = await fetchAuditRows({
|
||||
action: "service.force_delete",
|
||||
targetId: svc.id,
|
||||
targetType: "service",
|
||||
});
|
||||
assert.equal(
|
||||
deleteRows.length + forceRows.length,
|
||||
0,
|
||||
"denied delete (409) must NOT emit any service audit row",
|
||||
);
|
||||
});
|
||||
|
||||
test("DELETE /api/services/:id?force=true (with deps) writes one service.force_delete row with orderCount", async () => {
|
||||
const svc = await insertService("aud_svc_d_f");
|
||||
await pool.query(
|
||||
`INSERT INTO service_orders (user_id, service_id, status) VALUES ($1, $2, 'pending'), ($1, $2, 'completed')`,
|
||||
[adminId, svc.id],
|
||||
);
|
||||
|
||||
const res = await fetch(`${API_BASE}/api/services/${svc.id}?force=true`, {
|
||||
method: "DELETE",
|
||||
headers: { Cookie: adminCookie },
|
||||
});
|
||||
assert.equal(res.status, 204);
|
||||
|
||||
const rows = await fetchAuditRows({
|
||||
action: "service.force_delete",
|
||||
targetId: svc.id,
|
||||
targetType: "service",
|
||||
});
|
||||
assert.equal(rows.length, 1);
|
||||
const row = rows[0];
|
||||
assert.equal(row.actor_user_id, adminId);
|
||||
assert.equal(row.metadata.nameEn, svc.nameEn);
|
||||
assert.equal(row.metadata.nameAr, svc.nameAr);
|
||||
assert.equal(row.metadata.orderCount, 2);
|
||||
|
||||
// Forced path must not also emit the plain `service.delete` row.
|
||||
const plain = await fetchAuditRows({
|
||||
action: "service.delete",
|
||||
targetId: svc.id,
|
||||
targetType: "service",
|
||||
});
|
||||
assert.equal(plain.length, 0);
|
||||
});
|
||||
|
||||
// ---------- auth: password reset link ----------
|
||||
|
||||
test("POST /api/auth/admin/users/:id/issue-reset-link writes one auth.issue_reset_link row", async () => {
|
||||
|
||||
Reference in New Issue
Block a user