Task #98: Warn admins before renaming a role used in code references
- Added GET /api/roles/:id/usage endpoint returning {userCount, groupCount},
guarded by requireAdmin and validating the role id.
- Updated PATCH /api/roles/:id audit metadata to record renamed=true,
renamedFrom, and renamedTo when the role name changes (in addition to
existing name fields), satisfying the traceability requirement.
- Added RoleUsage schema + /roles/{id}/usage operation to openapi.yaml,
regenerated lib/api-zod via orval. Codegen script in lib/api-spec was
updated intentionally to preserve `export * from './manual'` in
api-zod's index, which orval was overwriting.
- RolesPanel edit dialog (admin.tsx) now tracks the original role name
on open and shows an amber warning banner with from/to text plus a
usage line (users + groups counts) whenever the trimmed name input
differs from the original. Warning is suppressed for system or
protected roles since their name input is disabled.
- Added en/ar locale strings: renameWarningTitle, renameWarningBody,
renameUsage.
- Reverted accidental opengraph.jpg change picked up earlier in the
session — it is unrelated to this task.
Verification: e2e test created a non-system role, assigned the admin
user to it, opened the edit dialog, observed the warning + usage line,
saved the rename, and confirmed both the role name persisted and the
audit log captured renamed/renamedFrom/renamedTo.
Note: The pnpm `test` workflow shows pre-existing failures in
executive-meetings.ts that are unrelated to this task (verified via
git stash before changes).
Replit-Task-Id: c1ee048d-50c6-4439-a430-ee0b7ec09959
This commit is contained in:
@@ -55,6 +55,31 @@ router.get("/permissions", requireAdmin, async (_req, res): Promise<void> => {
|
||||
res.json(rows.map(serializePermission));
|
||||
});
|
||||
|
||||
router.get("/roles/:id/usage", requireAdmin, async (req, res): Promise<void> => {
|
||||
const id = Number(req.params.id);
|
||||
if (!Number.isInteger(id)) {
|
||||
res.status(400).json({ error: "Invalid id" });
|
||||
return;
|
||||
}
|
||||
const [role] = await db.select({ id: rolesTable.id }).from(rolesTable).where(eq(rolesTable.id, id));
|
||||
if (!role) {
|
||||
res.status(404).json({ error: "Role not found" });
|
||||
return;
|
||||
}
|
||||
const [userCountRow] = await db
|
||||
.select({ count: sql<number>`count(*)::int` })
|
||||
.from(userRolesTable)
|
||||
.where(eq(userRolesTable.roleId, id));
|
||||
const [groupCountRow] = await db
|
||||
.select({ count: sql<number>`count(*)::int` })
|
||||
.from(groupRolesTable)
|
||||
.where(eq(groupRolesTable.roleId, id));
|
||||
res.json({
|
||||
userCount: userCountRow?.count ?? 0,
|
||||
groupCount: groupCountRow?.count ?? 0,
|
||||
});
|
||||
});
|
||||
|
||||
router.post("/roles", requireAdmin, async (req, res): Promise<void> => {
|
||||
const parsed = CreateRoleBody.safeParse(req.body);
|
||||
if (!parsed.success) {
|
||||
@@ -141,6 +166,7 @@ router.patch("/roles/:id", requireAdmin, async (req, res): Promise<void> => {
|
||||
if (parsed.data.descriptionEn !== undefined) updates.descriptionEn = parsed.data.descriptionEn;
|
||||
if (Object.keys(updates).length > 0) {
|
||||
await db.update(rolesTable).set(updates).where(eq(rolesTable.id, id));
|
||||
const renamed = updates.name !== undefined && updates.name !== existing.name;
|
||||
await db.insert(auditLogsTable).values({
|
||||
actorUserId: req.session.userId ?? null,
|
||||
action: "role.update",
|
||||
@@ -149,6 +175,10 @@ router.patch("/roles/:id", requireAdmin, async (req, res): Promise<void> => {
|
||||
metadata: {
|
||||
previousName: existing.name,
|
||||
name: updates.name ?? existing.name,
|
||||
renamed,
|
||||
...(renamed
|
||||
? { renamedFrom: existing.name, renamedTo: updates.name }
|
||||
: {}),
|
||||
changes: updates,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -440,6 +440,9 @@
|
||||
"errorNameTaken": "يوجد دور بنفس الاسم.",
|
||||
"errorInvalidName": "يجب أن يبدأ اسم الدور بحرف ويحتوي على حروف وأرقام وشرطات سفلية فقط.",
|
||||
"errorSystemRename": "لا يمكن تغيير اسم الأدوار النظامية.",
|
||||
"renameWarningTitle": "إعادة تسمية \"{{from}}\" إلى \"{{to}}\"",
|
||||
"renameWarningBody": "قد يُشار إلى اسم هذا الدور في الشيفرة أو التكاملات أو استعلامات سجل التدقيق. إعادة التسمية قد تُعطّل هذه التدفقات بصمت. حدّث جميع المراجع قبل الحفظ.",
|
||||
"renameUsage": "مستخدم حالياً من قِبل {{users}} مستخدم و{{groups}} مجموعة.",
|
||||
"permissions": "الصلاحيات",
|
||||
"permissionsHint": "اختر الصلاحيات التي يمنحها هذا الدور.",
|
||||
"permissionsSystemHint": "صلاحيات الأدوار النظامية للقراءة فقط.",
|
||||
|
||||
@@ -437,6 +437,9 @@
|
||||
"errorNameTaken": "A role with that name already exists.",
|
||||
"errorInvalidName": "Role name must start with a letter and contain only letters, numbers, or underscores.",
|
||||
"errorSystemRename": "System role names cannot be changed.",
|
||||
"renameWarningTitle": "Renaming \"{{from}}\" to \"{{to}}\"",
|
||||
"renameWarningBody": "This role's name may be referenced by code, integrations, or audit log queries. Renaming can quietly break those flows. Update any callers before saving.",
|
||||
"renameUsage": "Currently used by {{users}} users and {{groups}} groups.",
|
||||
"permissions": "Permissions",
|
||||
"permissionsHint": "Select which permissions this role grants.",
|
||||
"permissionsSystemHint": "System role permissions are read-only.",
|
||||
|
||||
@@ -45,6 +45,8 @@ import {
|
||||
getListPermissionsQueryKey,
|
||||
useGetRolePermissions,
|
||||
getGetRolePermissionsQueryKey,
|
||||
useGetRoleUsage,
|
||||
getGetRoleUsageQueryKey,
|
||||
useReplaceRolePermissions,
|
||||
useListAuditLogs,
|
||||
getListAuditLogsQueryKey,
|
||||
@@ -3226,6 +3228,7 @@ function RolesPanel() {
|
||||
const [editingId, setEditingId] = useState<number | null>(null);
|
||||
const [editingIsSystem, setEditingIsSystem] = useState(false);
|
||||
const [editForm, setEditForm] = useState({ name: "", descriptionAr: "", descriptionEn: "" });
|
||||
const [originalEditName, setOriginalEditName] = useState("");
|
||||
const [editPermissionIds, setEditPermissionIds] = useState<Set<number>>(new Set());
|
||||
const [initialPermissionIds, setInitialPermissionIds] = useState<Set<number>>(new Set());
|
||||
const [deleteTarget, setDeleteTarget] = useState<{ id: number; name: string } | null>(null);
|
||||
@@ -3241,6 +3244,12 @@ function RolesPanel() {
|
||||
},
|
||||
},
|
||||
);
|
||||
const { data: editingRoleUsage } = useGetRoleUsage(editingPermissionsId, {
|
||||
query: {
|
||||
queryKey: getGetRoleUsageQueryKey(editingPermissionsId),
|
||||
enabled: editingId != null && !editingIsSystem,
|
||||
},
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
if (editingId == null) return;
|
||||
@@ -3309,14 +3318,23 @@ function RolesPanel() {
|
||||
descriptionAr: role.descriptionAr ?? "",
|
||||
descriptionEn: role.descriptionEn ?? "",
|
||||
});
|
||||
setOriginalEditName(role.name);
|
||||
};
|
||||
|
||||
const closeEditDialog = () => {
|
||||
setEditingId(null);
|
||||
setEditPermissionIds(new Set());
|
||||
setInitialPermissionIds(new Set());
|
||||
setOriginalEditName("");
|
||||
};
|
||||
|
||||
const editNameTrimmed = editForm.name.trim();
|
||||
const isRenaming =
|
||||
!editingIsSystem &&
|
||||
originalEditName.length > 0 &&
|
||||
editNameTrimmed.length > 0 &&
|
||||
editNameTrimmed !== originalEditName;
|
||||
|
||||
const permissionsChanged = (() => {
|
||||
if (editPermissionIds.size !== initialPermissionIds.size) return true;
|
||||
for (const id of editPermissionIds) {
|
||||
@@ -3585,6 +3603,32 @@ function RolesPanel() {
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{editingIsSystem ? t("admin.roles.editSystemNameHint") : t("admin.roles.nameHint")}
|
||||
</p>
|
||||
{isRenaming && (
|
||||
<div
|
||||
className="mt-2 rounded-xl border border-amber-300 bg-amber-50 p-3 text-xs text-amber-900 space-y-1"
|
||||
data-testid="role-rename-warning"
|
||||
role="alert"
|
||||
>
|
||||
<p className="font-semibold">
|
||||
{t("admin.roles.renameWarningTitle", {
|
||||
from: originalEditName,
|
||||
to: editNameTrimmed,
|
||||
})}
|
||||
</p>
|
||||
<p>{t("admin.roles.renameWarningBody")}</p>
|
||||
{editingRoleUsage && (
|
||||
<p
|
||||
className="text-amber-800"
|
||||
data-testid="role-rename-usage"
|
||||
>
|
||||
{t("admin.roles.renameUsage", {
|
||||
users: editingRoleUsage.userCount,
|
||||
groups: editingRoleUsage.groupCount,
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
<div className="space-y-1">
|
||||
<Label>{t("admin.roles.descriptionAr")}</Label>
|
||||
|
||||
@@ -215,6 +215,11 @@ export interface UpdateRoleBody {
|
||||
descriptionEn?: string | null;
|
||||
}
|
||||
|
||||
export interface RoleUsage {
|
||||
userCount: number;
|
||||
groupCount: number;
|
||||
}
|
||||
|
||||
export interface RoleDeletionConflict {
|
||||
error: string;
|
||||
userCount: number;
|
||||
|
||||
@@ -67,6 +67,7 @@ import type {
|
||||
ResetPasswordBody,
|
||||
Role,
|
||||
RoleDeletionConflict,
|
||||
RoleUsage,
|
||||
SendMessageBody,
|
||||
Service,
|
||||
ServiceCategory,
|
||||
@@ -5131,6 +5132,93 @@ export const useDeleteRole = <
|
||||
return useMutation(getDeleteRoleMutationOptions(options));
|
||||
};
|
||||
|
||||
/**
|
||||
* @summary Count users and groups currently assigned to a role (admin)
|
||||
*/
|
||||
export const getGetRoleUsageUrl = (id: number) => {
|
||||
return `/api/roles/${id}/usage`;
|
||||
};
|
||||
|
||||
export const getRoleUsage = async (
|
||||
id: number,
|
||||
options?: RequestInit,
|
||||
): Promise<RoleUsage> => {
|
||||
return customFetch<RoleUsage>(getGetRoleUsageUrl(id), {
|
||||
...options,
|
||||
method: "GET",
|
||||
});
|
||||
};
|
||||
|
||||
export const getGetRoleUsageQueryKey = (id: number) => {
|
||||
return [`/api/roles/${id}/usage`] as const;
|
||||
};
|
||||
|
||||
export const getGetRoleUsageQueryOptions = <
|
||||
TData = Awaited<ReturnType<typeof getRoleUsage>>,
|
||||
TError = ErrorType<ErrorResponse>,
|
||||
>(
|
||||
id: number,
|
||||
options?: {
|
||||
query?: UseQueryOptions<
|
||||
Awaited<ReturnType<typeof getRoleUsage>>,
|
||||
TError,
|
||||
TData
|
||||
>;
|
||||
request?: SecondParameter<typeof customFetch>;
|
||||
},
|
||||
) => {
|
||||
const { query: queryOptions, request: requestOptions } = options ?? {};
|
||||
|
||||
const queryKey = queryOptions?.queryKey ?? getGetRoleUsageQueryKey(id);
|
||||
|
||||
const queryFn: QueryFunction<Awaited<ReturnType<typeof getRoleUsage>>> = ({
|
||||
signal,
|
||||
}) => getRoleUsage(id, { signal, ...requestOptions });
|
||||
|
||||
return {
|
||||
queryKey,
|
||||
queryFn,
|
||||
enabled: !!id,
|
||||
...queryOptions,
|
||||
} as UseQueryOptions<
|
||||
Awaited<ReturnType<typeof getRoleUsage>>,
|
||||
TError,
|
||||
TData
|
||||
> & { queryKey: QueryKey };
|
||||
};
|
||||
|
||||
export type GetRoleUsageQueryResult = NonNullable<
|
||||
Awaited<ReturnType<typeof getRoleUsage>>
|
||||
>;
|
||||
export type GetRoleUsageQueryError = ErrorType<ErrorResponse>;
|
||||
|
||||
/**
|
||||
* @summary Count users and groups currently assigned to a role (admin)
|
||||
*/
|
||||
|
||||
export function useGetRoleUsage<
|
||||
TData = Awaited<ReturnType<typeof getRoleUsage>>,
|
||||
TError = ErrorType<ErrorResponse>,
|
||||
>(
|
||||
id: number,
|
||||
options?: {
|
||||
query?: UseQueryOptions<
|
||||
Awaited<ReturnType<typeof getRoleUsage>>,
|
||||
TError,
|
||||
TData
|
||||
>;
|
||||
request?: SecondParameter<typeof customFetch>;
|
||||
},
|
||||
): UseQueryResult<TData, TError> & { queryKey: QueryKey } {
|
||||
const queryOptions = getGetRoleUsageQueryOptions(id, options);
|
||||
|
||||
const query = useQuery(queryOptions) as UseQueryResult<TData, TError> & {
|
||||
queryKey: QueryKey;
|
||||
};
|
||||
|
||||
return { ...query, queryKey: queryOptions.queryKey };
|
||||
}
|
||||
|
||||
/**
|
||||
* @summary List permissions assigned to a role (admin)
|
||||
*/
|
||||
|
||||
@@ -1425,6 +1425,31 @@ paths:
|
||||
schema:
|
||||
$ref: "#/components/schemas/RoleDeletionConflict"
|
||||
|
||||
/roles/{id}/usage:
|
||||
get:
|
||||
operationId: getRoleUsage
|
||||
tags: [roles]
|
||||
summary: Count users and groups currently assigned to a role (admin)
|
||||
parameters:
|
||||
- name: id
|
||||
in: path
|
||||
required: true
|
||||
schema:
|
||||
type: integer
|
||||
responses:
|
||||
"200":
|
||||
description: Counts of users and groups using the role
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/RoleUsage"
|
||||
"404":
|
||||
description: Role not found
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ErrorResponse"
|
||||
|
||||
/roles/{id}/permissions:
|
||||
get:
|
||||
operationId: getRolePermissions
|
||||
@@ -2282,6 +2307,17 @@ components:
|
||||
descriptionEn:
|
||||
type: ["string", "null"]
|
||||
|
||||
RoleUsage:
|
||||
type: object
|
||||
properties:
|
||||
userCount:
|
||||
type: integer
|
||||
groupCount:
|
||||
type: integer
|
||||
required:
|
||||
- userCount
|
||||
- groupCount
|
||||
|
||||
RoleDeletionConflict:
|
||||
type: object
|
||||
properties:
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"version": "0.0.0",
|
||||
"private": true,
|
||||
"scripts": {
|
||||
"codegen": "orval --config ./orval.config.ts && echo \"export * from './generated/api';\" > ../api-zod/src/index.ts && pnpm -w run typecheck:libs"
|
||||
"codegen": "orval --config ./orval.config.ts && printf \"export * from './generated/api';\\nexport * from './manual';\\n\" > ../api-zod/src/index.ts && pnpm -w run typecheck:libs"
|
||||
},
|
||||
"devDependencies": {
|
||||
"orval": "^8.5.2"
|
||||
|
||||
@@ -1896,6 +1896,18 @@ export const DeleteRoleParams = zod.object({
|
||||
id: zod.coerce.number(),
|
||||
});
|
||||
|
||||
/**
|
||||
* @summary Count users and groups currently assigned to a role (admin)
|
||||
*/
|
||||
export const GetRoleUsageParams = zod.object({
|
||||
id: zod.coerce.number(),
|
||||
});
|
||||
|
||||
export const GetRoleUsageResponse = zod.object({
|
||||
userCount: zod.number(),
|
||||
groupCount: zod.number(),
|
||||
});
|
||||
|
||||
/**
|
||||
* @summary List permissions assigned to a role (admin)
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user