feat(roles): ✨ add delete role functionality with confirmation dialog#725
feat(roles): ✨ add delete role functionality with confirmation dialog#725aXenDeveloper wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 784ad813ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const deleteRoleAdminRoute = buildRoute({ | ||
| pluginId: CONFIG_PLUGIN.pluginId, |
There was a problem hiding this comment.
Any authenticated admin can currently invoke this destructive endpoint, including restricted admins who lack the declared @vitnode/core:roles:can_manage permission, because the route omits adminStaffPermission. The global admin middleware only validates the admin session, so add the roles can_manage authorization guard before allowing reassignment and deletion.
Useful? React with 👍 / 👎.
| await tx.delete(core_roles).where(eq(core_roles.id, roleId)); | ||
| }); | ||
|
|
||
| await c.get("events").emit("role.deleted", { roleId }); |
There was a problem hiding this comment.
This turns role.deleted into a real built-in event, but apps/docs/content/docs/dev/events/built-in-events.mdx still describes it as “Declared only” with no deletion flow. Update that event documentation so plugin authors can reliably discover and use the new behavior.
AGENTS.md reference: AGENTS.md:L46-L47
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
|
|
||
| toast.success(t("success")); |
There was a problem hiding this comment.
After a successful deletion, this toast supplies only a title, so the new flow does not meet the repository requirement that create/edit/delete notifications include a description. Pass a localized description to toast.success while retaining the existing table revalidation.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| import { core_roles } from "@/database/roles"; | ||
| import { core_users, core_users_secondary_roles } from "@/database/users"; | ||
|
|
||
| export const deleteRoleAdminRoute = buildRoute({ |
There was a problem hiding this comment.
The commit adds no tests for this destructive route despite the package having Vitest configured. Cover at least protected-role rejection, missing or invalid reassignment targets, user reassignment with duplicate secondary roles, permission enforcement, and successful deletion so these data-integrity and authorization branches cannot regress unnoticed.
AGENTS.md reference: AGENTS.md:L87-L90
Useful? React with 👍 / 👎.
1 participant
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?