Skip to content

feat: rbac permissions#3310

Merged
ogzhanolguncu merged 53 commits intomainfrom
rbac-permissions
Jun 17, 2025
Merged

feat: rbac permissions#3310
ogzhanolguncu merged 53 commits intomainfrom
rbac-permissions

Conversation

@ogzhanolguncu
Copy link
Contributor

@ogzhanolguncu ogzhanolguncu commented Jun 10, 2025

What does this PR do?

This PR refactors /authorization/permissions page.

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test Permissions page's CRUD operations.

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • New Features

    • Introduced a redesigned permissions management interface with enhanced filtering, search (including AI-powered natural language search), and sorting capabilities.

    • Added a virtualized permissions table with improved selection controls, batch actions, and real-time status indicators.

    • Provided dialogs for creating, editing, and deleting permissions, with validation and confirmation flows.

    • Enabled detailed permission and role assignment views, including visual indicators and skeleton loaders for loading states.

  • Bug Fixes

    • Improved error handling and user feedback for permission creation, editing, and deletion actions.
  • Refactor

    • Simplified navigation and page components for permissions, removing server-side data fetching in favor of client-side rendering.

    • Streamlined empty state and modal management for a more intuitive user experience.

  • Chores

    • Updated backend APIs to support advanced querying, batch deletion, and AI-driven search for permissions and roles.

    • Enhanced schema validation and filtering logic for robust data handling and consistency.

Summary by CodeRabbit

  • New Features

    • Added a comprehensive permissions management table with virtualized lists, pagination, and multi-selection.
    • Introduced advanced filter controls and LLM-powered natural language search for permissions.
    • Enabled creation and editing of permissions via a dialog with form validation and persisted state.
    • Provided detailed permission metadata display, including assigned roles, keys, and last updated timestamps.
    • Added individual and bulk deletion workflows with confirmation dialogs and user feedback.
    • Included animated selection controls and skeleton loaders for improved UX.
  • Refactor

    • Consolidated permission detail views and empty states into an interactive table interface.
    • Streamlined navigation and permission creation flows by replacing modal forms with dialog components.
    • Removed legacy permission detail and deletion pages.
  • Bug Fixes

    • Enhanced validation schemas and error handling for permission operations and filtering mechanisms.
  • Chores

    • Improved audit logging and error reporting for permission creation, update, and deletion actions.

Copy link
Collaborator

@mcstepp mcstepp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing blocking, just a couple nitpicks for optimizations and simplifications

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (1)

55-58: Still hard-coded, should reflect the type prop

Previous feedback flagged this; the string is now “permissions…”, but it’s still not dynamic and will be wrong when type === "slug".

-            {totalCount - items.length} more permissions...
+            {totalCount - items.length} more {type}...
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e6f0a6 and 39500f3.

📒 Files selected for processing (2)
  • apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (1 hunks)
  • apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (2)
internal/icons/src/icons/tag.tsx (1)
  • Tag (15-39)
internal/icons/src/icons/hand-holding-key.tsx (1)
  • HandHoldingKey (15-97)
⏰ Context from checks skipped due to timeout of 90000ms (8)
  • GitHub Check: Test Packages / Test ./packages/cache
  • GitHub Check: Test Packages / Test ./packages/nextjs
  • GitHub Check: Test Packages / Test ./apps/dashboard
  • GitHub Check: Test Packages / Test ./packages/hono
  • GitHub Check: Test Packages / Test ./internal/clickhouse
  • GitHub Check: Test Packages / Test ./internal/hash
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/dashboard/lib/trpc/routers/index.ts (1)

175-180: ```shell
#!/bin/bash
set -e

echo "Routers directory contents:"
ls apps/dashboard/lib/trpc/routers

echo -e "\nShowing apps/dashboard/lib/trpc/routers/index.ts around the permissions block (lines 160-200):"
sed -n '160,200p' apps/dashboard/lib/trpc/routers/index.ts

echo -e "\nFinding any occurrences of 'rbac' in routers folder:"
rg "rbac" -n apps/dashboard/lib/trpc/routers

echo -e "\nChecking for middleware chaining in routers folder:"
rg ".middleware" -n apps/dashboard/lib/trpc/routers


</details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (1)
apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/index.tsx (1)

113-115: Replace console.error with user-facing feedback (comment still unresolved)

The earlier review pointed this out; the raw console log remains. Show a toast or field error instead so the user understands why the action failed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd3f4ed and c4f98b4.

📒 Files selected for processing (3)
  • apps/dashboard/app/(app)/authorization/permissions/components/table/components/actions/components/delete-permission.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/index.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/upsert-permission.schema.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (25)
  • GitHub Check: Test Go API Local / Test (Shard 7/8)
  • GitHub Check: Test Go API Local / Test (Shard 3/8)
  • GitHub Check: Test Go API Local / Test (Shard 8/8)
  • GitHub Check: Test Go API Local / Test (Shard 6/8)
  • GitHub Check: Test Go API Local / Test (Shard 5/8)
  • GitHub Check: Test Go API Local / Test (Shard 4/8)
  • GitHub Check: Test Go API Local / Test (Shard 2/8)
  • GitHub Check: Test Go API Local / Test (Shard 1/8)
  • GitHub Check: Build / Build
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Packages / Test ./packages/hono
  • GitHub Check: Test Packages / Test ./internal/resend
  • GitHub Check: Test Packages / Test ./internal/clickhouse
  • GitHub Check: Test Packages / Test ./internal/keys
  • GitHub Check: Test Packages / Test ./internal/billing
  • GitHub Check: Test Packages / Test ./internal/encryption
  • GitHub Check: Test Packages / Test ./packages/cache
  • GitHub Check: Test Packages / Test ./internal/hash
  • GitHub Check: Test Packages / Test ./packages/api
  • GitHub Check: Test Packages / Test ./packages/rbac
  • GitHub Check: Test Packages / Test ./apps/dashboard
  • GitHub Check: Test Packages / Test ./packages/nextjs
  • GitHub Check: Test Packages / Test ./internal/id
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/dashboard/app/(app)/authorization/permissions/components/table/components/actions/components/delete-permission.tsx (1)

52-56: 👍 Previous nested‐conditional concern resolved

You applied the suggested early-exit pattern (if (!open && !isConfirmPopoverOpen)) which removes the previous indentation level and keeps the callback tidy.

apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/upsert-permission.schema.ts (1)

7-12: trim() would make this refine unreachable

Because the slug schema below uses .trim(), consider also trimming the name or removing this refine; otherwise the UX between the two fields is inconsistent.

Copy link
Contributor Author

@mcstepp we are good to go 🫡

Copy link
Collaborator

@mcstepp mcstepp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mcstepp
Copy link
Collaborator

mcstepp commented Jun 17, 2025

Some tests are failing in dashboard

@ogzhanolguncu ogzhanolguncu added this pull request to the merge queue Jun 17, 2025
@ogzhanolguncu
Copy link
Contributor Author

Some tests are failing in dashboard

I'll fix that in another PR.

Merged via the queue into main with commit c2615ea Jun 17, 2025
31 of 37 checks passed
@ogzhanolguncu ogzhanolguncu deleted the rbac-permissions branch June 17, 2025 20:23
Copy link
Collaborator

I’ll fix that in another PR.
I recall you said that a week ago and we had failing tests since 😄

Copy link
Contributor Author

Shit you got me. I promise I’ll fix it tomorrow 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants