feat: Temporary role assignment (#128)#208
Conversation
- Add temp_roles DB table (migration 004) - Add tempRoleHandler module with assign/revoke/list/scheduler - Add /temprole slash command (assign | revoke | list subcommands) - Add /temp-roles REST API routes (GET list, POST assign, DELETE revoke) - Wire tempRoleScheduler into bot startup/shutdown lifecycle
- Add /api/temp-roles Next.js proxy route (GET list, POST assign) - Add /api/temp-roles/[id] proxy route (DELETE revoke) - Add /dashboard/temp-roles page with active role table + revoke UI - Add Clock icon + 'Temp Roles' nav entry to sidebar - Add unit tests: 9 module tests + 11 command tests (20 total, all passing) - Fix lint: biome format + import sort across all new files
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds first-class “temporary role” support across the bot, REST API, and web dashboard, enabling moderators/admins to assign roles that automatically expire and can be managed from both Discord and the dashboard.
Changes:
- Introduces
temp_rolespersistence + core handler logic with a 60s expiry polling scheduler. - Adds
/temproleslash command (assign/revoke/list) and new REST endpoints for listing/assigning/revoking temp roles. - Adds a dashboard page + sidebar navigation and Next.js API proxy routes for managing temp roles via the web UI.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
migrations/004_temp_roles.cjs |
Creates temp_roles table + indexes for expiry polling and queries. |
src/modules/tempRoleHandler.js |
Core DB operations + expiry poller/scheduler lifecycle. |
src/commands/temprole.js |
Discord slash command interface for managing temp roles. |
src/api/routes/tempRoles.js |
Express REST API for listing/assigning/revoking temp roles. |
src/api/index.js |
Mounts the temp roles router under /api/v1/temp-roles. |
src/index.js |
Starts/stops the temp role scheduler with the bot lifecycle. |
tests/modules/tempRoleHandler.test.js |
Unit tests for handler DB logic and scheduler start/stop. |
tests/commands/temprole.test.js |
Unit tests for /temprole command behavior. |
web/src/app/api/temp-roles/route.ts |
Next.js proxy for list/assign temp roles to bot API. |
web/src/app/api/temp-roles/[id]/route.ts |
Next.js proxy for revoking temp roles by id. |
web/src/app/dashboard/temp-roles/page.tsx |
Dashboard UI for viewing/revoking active temp roles. |
web/src/components/layout/sidebar.tsx |
Adds “Temp Roles” entry in dashboard navigation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| Filename | Overview |
|---|---|
| src/api/index.js | Missing import causes ReferenceError - ticketsRouter used on line 52 but import was removed |
| migrations/004_temp_roles.cjs | Clean migration with proper indexes for pending roles, guild queries, and compound lookups |
| src/modules/tempRoleHandler.js | Solid implementation with proper error handling, optimistic locking, and Winston logging |
| src/commands/temprole.js | Well-structured command with hierarchy checks, comprehensive error handling, and user-friendly responses |
| src/api/routes/tempRoles.js | Good validation and error handling; missing middleware already flagged in previous review |
| web/src/app/dashboard/temp-roles/page.tsx | Clean React component with proper state management, pagination, and user-friendly interface |
Sequence Diagram
sequenceDiagram
participant Mod as Moderator
participant Bot as Discord Bot
participant DB as PostgreSQL
participant Sched as Scheduler
participant User as Target User
Note over Mod,User: Assign Temp Role Flow
Mod->>Bot: /temprole assign @user @role 7d
Bot->>Bot: Validate hierarchy<br/>(bot & mod roles)
Bot->>User: Add role via Discord API
Bot->>DB: INSERT temp_roles record<br/>(expires_at = now + 7d)
Bot->>Mod: ✅ Role assigned for 7 days
Note over Sched,User: Automatic Expiry Flow
loop Every 60s
Sched->>DB: SELECT expired roles<br/>(removed=FALSE, expires_at <= NOW)
DB-->>Sched: Return expired records
Sched->>DB: UPDATE removed=TRUE<br/>(optimistic lock)
Sched->>User: Remove role via Discord API
end
Note over Mod,DB: Dashboard Revoke Flow
Mod->>Bot: DELETE /api/temp-roles/:id
Bot->>DB: UPDATE removed=TRUE<br/>WHERE id=:id
Bot->>User: Remove role via Discord API
Bot->>Mod: ✅ Role revoked
Last reviewed commit: cf20c84
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Fix adaptGuildIdParam middleware collision: scope to GET only, not DELETE - Fix POST auth: add adaptBodyGuildId middleware to read guildId from req.body - Fix DELETE revoke: use revokeTempRoleById to revoke by specific record ID - Add guildId validation: return 400 if guildId missing/not a string in GET/DELETE - Wrap DB operations in try/catch: add error handling in tempRoleHandler.js - Remove empty test: removed redundant scheduler test with no assertions - Use proxyToBotApi helper: refactor web routes to use shared helper instead of raw fetch
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| router.delete('/:id', async (req, res) => { | ||
| try { | ||
| const guildId = req.query.guildId; |
There was a problem hiding this comment.
The DELETE endpoint is not protected by requireGuildModerator, unlike the GET/POST routes. This enables any authenticated user (with access to the API) to revoke temp roles by guessing IDs + guildId. Add adaptGuildIdParam (or equivalent) and requireGuildModerator middleware to the DELETE route to enforce guild-level authorization.
| guildId: row.guild_id, | ||
| userId: row.user_id, | ||
| roleId: row.role_id, | ||
| }); |
There was a problem hiding this comment.
The scheduler marks records as removed = TRUE before attempting Discord role removal. If Discord removal fails (permissions outage, transient API errors, rate limits), the record will never be retried and the user may keep the role indefinitely. Consider a separate claim/processing state (e.g., claimed_at, claimed_by, processing = true) and only set removed = TRUE after successful removal (or after confirming the member is gone); alternatively reset removed back to FALSE on failure so it can be retried.
| }); | |
| }); | |
| // Reset removal flags so this record can be retried on a future poll | |
| try { | |
| await pool.query( | |
| `UPDATE temp_roles | |
| SET removed = FALSE, | |
| removed_at = NULL | |
| WHERE id = $1`, | |
| [row.id], | |
| ); | |
| } catch (resetErr) { | |
| logError('Failed to reset temp role removal state after error', { | |
| error: resetErr.message, | |
| id: row.id, | |
| }); | |
| } |
| { params }: { params: Promise<{ id: string }> }, | ||
| ): Promise<NextResponse> { | ||
| const { id } = await params; |
There was a problem hiding this comment.
The route handler context typing is off: params is typically a plain object ({ params: { id: string } }) rather than a Promise, and await params is unnecessary. Using the correct type here prevents masking real type errors and keeps the handler aligned with Next.js route handler conventions.
| { params }: { params: Promise<{ id: string }> }, | |
| ): Promise<NextResponse> { | |
| const { id } = await params; | |
| { params }: { params: { id: string } }, | |
| ): Promise<NextResponse> { | |
| const { id } = params; |
| const upstream = buildUpstreamUrl( | ||
| config.baseUrl, | ||
| `/temp-roles/${encodeURIComponent(id)}`, | ||
| LOG_PREFIX, | ||
| ); |
There was a problem hiding this comment.
The id path segment is forwarded upstream without validation. Since the upstream endpoint expects a numeric record ID, validate id (e.g., positive integer) and return a 400 with a clear error when invalid, instead of proxying malformed requests.
| const [error, setError] = useState<string | null>(null); | ||
| const [page, setPage] = useState(1); | ||
| const [revoking, setRevoking] = useState<number | null>(null); | ||
| const abortRef = useRef<AbortController | null>(null); |
There was a problem hiding this comment.
Requests are aborted when a new fetch starts, but there is no unmount cleanup to abort an in-flight request. Add a useEffect cleanup (or equivalent) to call abortRef.current?.abort() on unmount to avoid updates after navigation and reduce unnecessary work.
|
|
||
| useEffect(() => { | ||
| if (!guildId) return; | ||
| void fetchTempRoles(guildId, page); |
There was a problem hiding this comment.
Requests are aborted when a new fetch starts, but there is no unmount cleanup to abort an in-flight request. Add a useEffect cleanup (or equivalent) to call abortRef.current?.abort() on unmount to avoid updates after navigation and reduce unnecessary work.
| void fetchTempRoles(guildId, page); | |
| void fetchTempRoles(guildId, page); | |
| return () => { | |
| abortRef.current?.abort(); | |
| }; |
| if (!role) { | ||
| return res.status(400).json({ error: 'Role not found' }); | ||
| } | ||
|
|
There was a problem hiding this comment.
The dashboard assignment path does not perform role hierarchy checks before attempting member.roles.add(...). When the role is above the bot/moderator, this predictably fails and returns a generic 500. Mirror the command’s hierarchy validation and return a 400/403 with a specific error (e.g., role is higher than bot/mod) to make failures actionable.
| // Role hierarchy checks | |
| let botMember = guild.members.me; | |
| if (!botMember && client.user?.id) { | |
| try { | |
| botMember = await guild.members.fetch(client.user.id); | |
| } catch { | |
| // fall through and handle below | |
| } | |
| } | |
| if (!botMember) { | |
| return res.status(503).json({ error: 'Bot member not available in guild for role assignment' }); | |
| } | |
| if (role.position >= botMember.roles.highest.position) { | |
| return res.status(403).json({ error: 'Cannot assign a role higher than or equal to the bot’s highest role' }); | |
| } | |
| if (req.user?.id) { | |
| try { | |
| const moderatorMember = await guild.members.fetch(req.user.id); | |
| if (moderatorMember && role.position >= moderatorMember.roles.highest.position) { | |
| return res.status(403).json({ error: 'Cannot assign a role higher than or equal to the moderator’s highest role' }); | |
| } | |
| } catch { | |
| // If the moderator is not a guild member, skip moderator hierarchy check | |
| } | |
| } |
Additional Comments (1)
Add this import back on line 22 after the |
Summary
Implements issue #128 — allow assigning roles that expire after a set time.
Features
🤖 Discord Command:
/temprole/temprole assign @user @role <duration> [reason]— Assigns a role that expires after the given duration (supports1h,7d,2w, etc.)/temprole revoke @user @role— Removes a temp role before it expires/temprole list [@user]— Lists active temp role assignments (server-wide or per user)⏱ Automatic Expiry Scheduler
🗄️ Database
temp_rolestable (migration004_temp_roles.cjs)🌐 REST API
GET /api/v1/temp-roles?guildId=...— List active assignments (paginated)POST /api/v1/temp-roles— Assign via dashboardDELETE /api/v1/temp-roles/:id?guildId=...— Revoke via dashboard📊 Web Dashboard
/dashboard/temp-rolespageTests
tempRoleHandler)temprole)Files Changed
Closes #128