Warn admins before deleting non-empty groups
Original task: Task #79 — Make DELETE /api/groups/:id refuse to wipe a non-empty group unless explicitly forced, surface the impacted counts in the admin UI, and log forced deletions to an audit trail. Changes: - API: DELETE /api/groups/:id now computes member/app/role counts. If any are non-zero and `?force=true` is not passed, it returns 409 with { error, memberCount, appCount, roleCount }. With `force=true` it deletes and writes an entry to the new `audit_logs` table (action='group.force_delete', target_type='group', target_id, metadata). System group guard and 404 behaviour preserved. - DB: Added `audit_logs` table (lib/db/src/schema/audit-logs.ts) with actor, action, target type/id, jsonb metadata, timestamp. Pushed via drizzle-kit. - OpenAPI: Added `force` query param, 409 response, and `GroupDeletionConflict` schema. Re-ran orval codegen for api-client-react and api-zod. - api-client-react: Re-exported `ApiError` and `ErrorType` so the UI can inspect 409 responses. - Admin UI (artifacts/teaboy-os/src/pages/admin.tsx GroupsPanel): Replaced the bare `confirm()` with a confirmation dialog that shows the group name and impacted member/app/role counts. Empty groups get a simple "Delete" confirmation; non-empty groups get a warning + "Delete anyway" button which sends `force=true`. If the server returns 409 (race), the dialog updates to show the server-reported counts. - Locales: Added en/ar strings for the new dialog (deleteTitle, deleteWarning, deleteForceHint, deleteEmptyBody, deleteConfirm, deleteAnyway). Verified end-to-end: 409 on first DELETE, dialog warning visible, force deletion succeeds, single audit_logs row recorded with correct metadata. Replit-Task-Id: 61624ce4-83b3-43be-b22b-e261662301f1
This commit is contained in:
@@ -9,6 +9,7 @@ import {
|
||||
usersTable,
|
||||
appsTable,
|
||||
rolesTable,
|
||||
auditLogsTable,
|
||||
} from "@workspace/db";
|
||||
import { requireAdmin } from "../middlewares/auth";
|
||||
import { emitAppsChangedToUsers } from "../lib/realtime";
|
||||
@@ -448,8 +449,44 @@ router.delete("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
|
||||
res.status(400).json({ error: "Cannot delete a system group" });
|
||||
return;
|
||||
}
|
||||
const force = req.query.force === "true" || req.query.force === "1";
|
||||
const memberIds = await getGroupMemberIds(id);
|
||||
const [appCountRow] = await db
|
||||
.select({ count: sql<number>`count(*)::int` })
|
||||
.from(groupAppsTable)
|
||||
.where(eq(groupAppsTable.groupId, id));
|
||||
const [roleCountRow] = await db
|
||||
.select({ count: sql<number>`count(*)::int` })
|
||||
.from(groupRolesTable)
|
||||
.where(eq(groupRolesTable.groupId, id));
|
||||
const memberCount = memberIds.length;
|
||||
const appCount = appCountRow?.count ?? 0;
|
||||
const roleCount = roleCountRow?.count ?? 0;
|
||||
const isNonEmpty = memberCount > 0 || appCount > 0 || roleCount > 0;
|
||||
if (isNonEmpty && !force) {
|
||||
res.status(409).json({
|
||||
error: "Group is not empty",
|
||||
memberCount,
|
||||
appCount,
|
||||
roleCount,
|
||||
});
|
||||
return;
|
||||
}
|
||||
await db.delete(groupsTable).where(eq(groupsTable.id, id));
|
||||
if (isNonEmpty && force) {
|
||||
await db.insert(auditLogsTable).values({
|
||||
actorUserId: req.session.userId ?? null,
|
||||
action: "group.force_delete",
|
||||
targetType: "group",
|
||||
targetId: id,
|
||||
metadata: {
|
||||
name: existing.name,
|
||||
memberCount,
|
||||
appCount,
|
||||
roleCount,
|
||||
},
|
||||
});
|
||||
}
|
||||
await emitAppsChangedToUsers(memberIds);
|
||||
res.sendStatus(204);
|
||||
});
|
||||
|
||||
@@ -395,6 +395,12 @@
|
||||
"usersHint": "أعضاء هذه المجموعة.",
|
||||
"rolesHint": "يحصل أعضاء هذه المجموعة تلقائياً على هذه الأدوار.",
|
||||
"rolesEmpty": "لا توجد أدوار متاحة.",
|
||||
"deleteTitle": "حذف المجموعة \"{{name}}\"؟",
|
||||
"deleteWarning": "هذه المجموعة لا تزال قيد الاستخدام. حذفها سيلغي الوصول لما يلي:",
|
||||
"deleteForceHint": "سيؤدي الحذف إلى فصل الأعضاء وإلغاء التطبيقات والأدوار الممنوحة عبر المجموعة. لا يمكن التراجع عن هذا الإجراء.",
|
||||
"deleteEmptyBody": "لا تحتوي هذه المجموعة على أعضاء أو تطبيقات أو أدوار. هل تريد المتابعة؟",
|
||||
"deleteConfirm": "حذف",
|
||||
"deleteAnyway": "حذف رغم ذلك",
|
||||
"tab": {
|
||||
"info": "المعلومات",
|
||||
"apps": "التطبيقات",
|
||||
|
||||
@@ -392,6 +392,12 @@
|
||||
"usersHint": "Members of this group.",
|
||||
"rolesHint": "Members of this group are automatically granted these roles.",
|
||||
"rolesEmpty": "No roles available.",
|
||||
"deleteTitle": "Delete group \"{{name}}\"?",
|
||||
"deleteWarning": "This group is still in use. Deleting it will revoke access for the following:",
|
||||
"deleteForceHint": "Deleting will detach these members and revoke their group-granted apps and roles. This cannot be undone.",
|
||||
"deleteEmptyBody": "This group has no members, apps, or roles. Continue?",
|
||||
"deleteConfirm": "Delete",
|
||||
"deleteAnyway": "Delete anyway",
|
||||
"tab": {
|
||||
"info": "Info",
|
||||
"apps": "Apps",
|
||||
|
||||
@@ -40,8 +40,10 @@ import {
|
||||
useDeleteGroup,
|
||||
useListRoles,
|
||||
getListRolesQueryKey,
|
||||
ApiError,
|
||||
} from "@workspace/api-client-react";
|
||||
import type {
|
||||
GroupDeletionConflict,
|
||||
App,
|
||||
Service,
|
||||
UserProfile,
|
||||
@@ -2417,9 +2419,45 @@ function GroupsPanel() {
|
||||
const [createOpen, setCreateOpen] = useState(false);
|
||||
const [createForm, setCreateForm] = useState({ name: "", descriptionAr: "", descriptionEn: "" });
|
||||
const [groupSearch, setGroupSearch] = useState("");
|
||||
const [deleteTarget, setDeleteTarget] = useState<Group | null>(null);
|
||||
const [conflictCounts, setConflictCounts] = useState<{
|
||||
memberCount: number;
|
||||
appCount: number;
|
||||
roleCount: number;
|
||||
} | null>(null);
|
||||
|
||||
const invalidate = () => queryClient.invalidateQueries({ queryKey: getListGroupsQueryKey() });
|
||||
|
||||
const closeDeleteDialog = () => {
|
||||
setDeleteTarget(null);
|
||||
setConflictCounts(null);
|
||||
};
|
||||
|
||||
const performDelete = (group: Group, force: boolean) => {
|
||||
deleteGroup.mutate(
|
||||
{ id: group.id, params: force ? { force: true } : undefined },
|
||||
{
|
||||
onSuccess: () => {
|
||||
invalidate();
|
||||
toast({ title: t("common.success") });
|
||||
closeDeleteDialog();
|
||||
},
|
||||
onError: (err: unknown) => {
|
||||
if (err instanceof ApiError && err.status === 409 && err.data) {
|
||||
const data = err.data as GroupDeletionConflict;
|
||||
setConflictCounts({
|
||||
memberCount: data.memberCount,
|
||||
appCount: data.appCount,
|
||||
roleCount: data.roleCount,
|
||||
});
|
||||
return;
|
||||
}
|
||||
toast({ title: t("common.error"), variant: "destructive" });
|
||||
},
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
const visibleGroups = (groups ?? []).filter((g) =>
|
||||
!groupSearch.trim() ||
|
||||
g.name.toLowerCase().includes(groupSearch.trim().toLowerCase()) ||
|
||||
@@ -2474,17 +2512,11 @@ function GroupsPanel() {
|
||||
{!g.isSystem && (
|
||||
<button
|
||||
onClick={() => {
|
||||
if (confirm(t("admin.deleteConfirm"))) {
|
||||
deleteGroup.mutate(
|
||||
{ id: g.id },
|
||||
{
|
||||
onSuccess: () => { invalidate(); toast({ title: t("common.success") }); },
|
||||
onError: () => toast({ title: t("common.error"), variant: "destructive" }),
|
||||
},
|
||||
);
|
||||
}
|
||||
setConflictCounts(null);
|
||||
setDeleteTarget(g);
|
||||
}}
|
||||
className="p-1.5 hover:bg-destructive/20 rounded-lg text-muted-foreground hover:text-destructive"
|
||||
data-testid={`delete-group-${g.id}`}
|
||||
>
|
||||
<Trash2 size={14} />
|
||||
</button>
|
||||
@@ -2557,6 +2589,66 @@ function GroupsPanel() {
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
|
||||
{deleteTarget && (() => {
|
||||
const counts = conflictCounts ?? {
|
||||
memberCount: deleteTarget.memberCount,
|
||||
appCount: deleteTarget.appCount,
|
||||
roleCount: deleteTarget.roleCount,
|
||||
};
|
||||
const isNonEmpty =
|
||||
counts.memberCount > 0 || counts.appCount > 0 || counts.roleCount > 0;
|
||||
const showWarning = conflictCounts !== null || isNonEmpty;
|
||||
return (
|
||||
<div className="fixed inset-0 bg-slate-900/30 backdrop-blur-sm z-50 flex items-center justify-center p-4">
|
||||
<div className="glass-panel rounded-3xl p-6 w-full max-w-sm space-y-3" data-testid="delete-group-dialog">
|
||||
<h2 className="font-semibold text-foreground">
|
||||
{t("admin.groups.deleteTitle", { name: deleteTarget.name })}
|
||||
</h2>
|
||||
{showWarning ? (
|
||||
<>
|
||||
<p className="text-sm text-destructive">
|
||||
{t("admin.groups.deleteWarning")}
|
||||
</p>
|
||||
<ul className="text-sm text-foreground space-y-1 ps-4 list-disc">
|
||||
<li>{t("admin.groups.members", { count: counts.memberCount })}</li>
|
||||
<li>{t("admin.groups.appsCount", { count: counts.appCount })}</li>
|
||||
<li>{t("admin.groups.rolesCount", { count: counts.roleCount })}</li>
|
||||
</ul>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t("admin.groups.deleteForceHint")}
|
||||
</p>
|
||||
</>
|
||||
) : (
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{t("admin.groups.deleteEmptyBody")}
|
||||
</p>
|
||||
)}
|
||||
<div className="flex gap-2 pt-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
className="flex-1"
|
||||
onClick={closeDeleteDialog}
|
||||
disabled={deleteGroup.isPending}
|
||||
>
|
||||
{t("common.cancel")}
|
||||
</Button>
|
||||
<Button
|
||||
className="flex-1"
|
||||
variant="destructive"
|
||||
disabled={deleteGroup.isPending}
|
||||
onClick={() => performDelete(deleteTarget, showWarning)}
|
||||
data-testid="confirm-delete-group"
|
||||
>
|
||||
{showWarning
|
||||
? t("admin.groups.deleteAnyway")
|
||||
: t("admin.groups.deleteConfirm")}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
})()}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -193,6 +193,13 @@ export interface Group {
|
||||
createdAt: string;
|
||||
}
|
||||
|
||||
export interface GroupDeletionConflict {
|
||||
error: string;
|
||||
memberCount: number;
|
||||
appCount: number;
|
||||
roleCount: number;
|
||||
}
|
||||
|
||||
export interface GroupDetail {
|
||||
id: number;
|
||||
name: string;
|
||||
@@ -768,6 +775,13 @@ export type ListGroupsParams = {
|
||||
q?: string;
|
||||
};
|
||||
|
||||
export type DeleteGroupParams = {
|
||||
/**
|
||||
* Force deletion of a non-empty group (records an audit log entry).
|
||||
*/
|
||||
force?: boolean;
|
||||
};
|
||||
|
||||
export type GetAdminStatsParams = {
|
||||
/**
|
||||
* Time range for trend stats. Use "custom" with from/to.
|
||||
|
||||
@@ -33,6 +33,7 @@ import type {
|
||||
CreateServiceBody,
|
||||
CreateServiceOrderBody,
|
||||
CreateUserBody,
|
||||
DeleteGroupParams,
|
||||
ErrorResponse,
|
||||
ForgotPasswordBody,
|
||||
ForgotPasswordResponse,
|
||||
@@ -40,6 +41,7 @@ import type {
|
||||
GetAdminAppOpensByUserParams,
|
||||
GetAdminStatsParams,
|
||||
Group,
|
||||
GroupDeletionConflict,
|
||||
GroupDetail,
|
||||
HealthStatus,
|
||||
HomeStats,
|
||||
@@ -5094,35 +5096,48 @@ export const useUpdateGroup = <
|
||||
/**
|
||||
* @summary Delete group (admin)
|
||||
*/
|
||||
export const getDeleteGroupUrl = (id: number) => {
|
||||
return `/api/groups/${id}`;
|
||||
export const getDeleteGroupUrl = (id: number, params?: DeleteGroupParams) => {
|
||||
const normalizedParams = new URLSearchParams();
|
||||
|
||||
Object.entries(params || {}).forEach(([key, value]) => {
|
||||
if (value !== undefined) {
|
||||
normalizedParams.append(key, value === null ? "null" : value.toString());
|
||||
}
|
||||
});
|
||||
|
||||
const stringifiedParams = normalizedParams.toString();
|
||||
|
||||
return stringifiedParams.length > 0
|
||||
? `/api/groups/${id}?${stringifiedParams}`
|
||||
: `/api/groups/${id}`;
|
||||
};
|
||||
|
||||
export const deleteGroup = async (
|
||||
id: number,
|
||||
params?: DeleteGroupParams,
|
||||
options?: RequestInit,
|
||||
): Promise<void> => {
|
||||
return customFetch<void>(getDeleteGroupUrl(id), {
|
||||
return customFetch<void>(getDeleteGroupUrl(id, params), {
|
||||
...options,
|
||||
method: "DELETE",
|
||||
});
|
||||
};
|
||||
|
||||
export const getDeleteGroupMutationOptions = <
|
||||
TError = ErrorType<ErrorResponse>,
|
||||
TError = ErrorType<ErrorResponse | GroupDeletionConflict>,
|
||||
TContext = unknown,
|
||||
>(options?: {
|
||||
mutation?: UseMutationOptions<
|
||||
Awaited<ReturnType<typeof deleteGroup>>,
|
||||
TError,
|
||||
{ id: number },
|
||||
{ id: number; params?: DeleteGroupParams },
|
||||
TContext
|
||||
>;
|
||||
request?: SecondParameter<typeof customFetch>;
|
||||
}): UseMutationOptions<
|
||||
Awaited<ReturnType<typeof deleteGroup>>,
|
||||
TError,
|
||||
{ id: number },
|
||||
{ id: number; params?: DeleteGroupParams },
|
||||
TContext
|
||||
> => {
|
||||
const mutationKey = ["deleteGroup"];
|
||||
@@ -5136,11 +5151,11 @@ export const getDeleteGroupMutationOptions = <
|
||||
|
||||
const mutationFn: MutationFunction<
|
||||
Awaited<ReturnType<typeof deleteGroup>>,
|
||||
{ id: number }
|
||||
{ id: number; params?: DeleteGroupParams }
|
||||
> = (props) => {
|
||||
const { id } = props ?? {};
|
||||
const { id, params } = props ?? {};
|
||||
|
||||
return deleteGroup(id, requestOptions);
|
||||
return deleteGroup(id, params, requestOptions);
|
||||
};
|
||||
|
||||
return { mutationFn, ...mutationOptions };
|
||||
@@ -5150,26 +5165,28 @@ export type DeleteGroupMutationResult = NonNullable<
|
||||
Awaited<ReturnType<typeof deleteGroup>>
|
||||
>;
|
||||
|
||||
export type DeleteGroupMutationError = ErrorType<ErrorResponse>;
|
||||
export type DeleteGroupMutationError = ErrorType<
|
||||
ErrorResponse | GroupDeletionConflict
|
||||
>;
|
||||
|
||||
/**
|
||||
* @summary Delete group (admin)
|
||||
*/
|
||||
export const useDeleteGroup = <
|
||||
TError = ErrorType<ErrorResponse>,
|
||||
TError = ErrorType<ErrorResponse | GroupDeletionConflict>,
|
||||
TContext = unknown,
|
||||
>(options?: {
|
||||
mutation?: UseMutationOptions<
|
||||
Awaited<ReturnType<typeof deleteGroup>>,
|
||||
TError,
|
||||
{ id: number },
|
||||
{ id: number; params?: DeleteGroupParams },
|
||||
TContext
|
||||
>;
|
||||
request?: SecondParameter<typeof customFetch>;
|
||||
}): UseMutationResult<
|
||||
Awaited<ReturnType<typeof deleteGroup>>,
|
||||
TError,
|
||||
{ id: number },
|
||||
{ id: number; params?: DeleteGroupParams },
|
||||
TContext
|
||||
> => {
|
||||
return useMutation(getDeleteGroupMutationOptions(options));
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
export * from "./generated/api";
|
||||
export * from "./generated/api.schemas";
|
||||
export { setBaseUrl, setAuthTokenGetter } from "./custom-fetch";
|
||||
export type { AuthTokenGetter } from "./custom-fetch";
|
||||
export { setBaseUrl, setAuthTokenGetter, ApiError } from "./custom-fetch";
|
||||
export type { AuthTokenGetter, ErrorType } from "./custom-fetch";
|
||||
|
||||
@@ -1357,6 +1357,13 @@ paths:
|
||||
required: true
|
||||
schema:
|
||||
type: integer
|
||||
- name: force
|
||||
in: query
|
||||
required: false
|
||||
description: Force deletion of a non-empty group (records an audit log entry).
|
||||
schema:
|
||||
type: boolean
|
||||
default: false
|
||||
responses:
|
||||
"204":
|
||||
description: Deleted
|
||||
@@ -1366,6 +1373,12 @@ paths:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ErrorResponse"
|
||||
"409":
|
||||
description: Group is not empty; pass force=true to confirm deletion.
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/GroupDeletionConflict"
|
||||
|
||||
# Stats
|
||||
/stats/home:
|
||||
@@ -1910,6 +1923,23 @@ components:
|
||||
- roleCount
|
||||
- createdAt
|
||||
|
||||
GroupDeletionConflict:
|
||||
type: object
|
||||
properties:
|
||||
error:
|
||||
type: string
|
||||
memberCount:
|
||||
type: integer
|
||||
appCount:
|
||||
type: integer
|
||||
roleCount:
|
||||
type: integer
|
||||
required:
|
||||
- error
|
||||
- memberCount
|
||||
- appCount
|
||||
- roleCount
|
||||
|
||||
GroupDetail:
|
||||
type: object
|
||||
properties:
|
||||
|
||||
@@ -1689,6 +1689,17 @@ export const DeleteGroupParams = zod.object({
|
||||
id: zod.coerce.number(),
|
||||
});
|
||||
|
||||
export const deleteGroupQueryForceDefault = false;
|
||||
|
||||
export const DeleteGroupQueryParams = zod.object({
|
||||
force: zod.coerce
|
||||
.boolean()
|
||||
.default(deleteGroupQueryForceDefault)
|
||||
.describe(
|
||||
"Force deletion of a non-empty group (records an audit log entry).",
|
||||
),
|
||||
});
|
||||
|
||||
/**
|
||||
* @summary Get home screen stats
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
import { pgTable, serial, timestamp, integer, varchar, jsonb } from "drizzle-orm/pg-core";
|
||||
import { usersTable } from "./users";
|
||||
|
||||
export const auditLogsTable = pgTable("audit_logs", {
|
||||
id: serial("id").primaryKey(),
|
||||
actorUserId: integer("actor_user_id").references(() => usersTable.id, { onDelete: "set null" }),
|
||||
action: varchar("action", { length: 100 }).notNull(),
|
||||
targetType: varchar("target_type", { length: 50 }).notNull(),
|
||||
targetId: integer("target_id"),
|
||||
metadata: jsonb("metadata"),
|
||||
createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(),
|
||||
});
|
||||
|
||||
export type AuditLog = typeof auditLogsTable.$inferSelect;
|
||||
@@ -11,3 +11,4 @@ export * from "./app-opens";
|
||||
export * from "./password-reset-tokens";
|
||||
export * from "./notes";
|
||||
export * from "./groups";
|
||||
export * from "./audit-logs";
|
||||
|
||||
Reference in New Issue
Block a user