Skip to content

Comments

feat: add availability and ooo permissions to PBAC registry#24081

Merged
eunjae-lee merged 3 commits intomainfrom
devin/availability-ooo-permissions-1758807829
Sep 25, 2025
Merged

feat: add availability and ooo permissions to PBAC registry#24081
eunjae-lee merged 3 commits intomainfrom
devin/availability-ooo-permissions-1758807829

Conversation

@eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Sep 25, 2025

What does this PR do?

This PR adds new availability and ooo (Out of Office) permissions to the PBAC (Permission-Based Access Control) system registry and creates a database migration to seed these permissions for the default roles.

Changes:

  • Adds Availability and OutOfOffice resources to the permission registry with full CRUD operations
  • Creates database migration to seed admin_role with all CRUD permissions and member_role with read-only permissions for both resources
  • Adds corresponding i18n entries for all permission descriptions

Link to Devin run: https://app.devin.ai/sessions/fce187a8677f429e8489d2de5520aa94
Requested by: @eunjae-lee

How should this be tested?

Database Migration Testing:

  1. Run the migration: yarn workspace @calcom/prisma db-migrate
  2. Verify the RolePermission table contains the new entries:
    SELECT * FROM "RolePermission" WHERE resource IN ('availability', 'ooo');
  3. Confirm admin_role has create/read/update/delete permissions for both resources
  4. Confirm member_role has only read permissions for both resources

Permission Registry Testing:

  1. Verify the new resources appear in permission management UI
  2. Test that permission scopes are empty arrays as intended
  3. Verify i18n strings display correctly (no missing translation keys)

Environment variables: Standard Cal.com database setup required

Important Review Notes

⚠️ Critical areas for review:

  1. Permission Scopes: Both resources use scope: [] as specifically requested - verify this is intentional
  2. Resource Naming: Using "ooo" as resource key vs OutOfOffice enum name - confirm this follows existing patterns
  3. Migration Safety: Uses ON CONFLICT DO NOTHING for safe re-runs, but verify SQL syntax and logic
  4. i18n Alignment: Ensure all i18nKey and descriptionI18nKey values in permission registry have matching entries in common.json

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked if my changes generate no new warnings
  • I have made corresponding changes to the documentation

- Add Availability and OutOfOffice resources to Resource enum
- Add CRUD permissions for both resources with empty scope arrays
- Create migration to seed admin_role with all CRUD permissions
- Create migration to seed member_role with read-only permissions

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

This change introduces two new PBAC resources: Availability and OutOfOffice. It adds corresponding CRUD permission entries to PERMISSION_REGISTRY, including i18n keys, categories, scopes, and dependsOn references. The Resource enum gains Availability and OutOfOffice. English locale strings are added for the resources and each CRUD description. A Prisma migration seeds default RolePermission rows: admin_role receives create/read/update/delete for both resources; member_role receives read for both. An inline ESLint comment is added without altering behavior.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly indicates the addition of availability and ooo permissions to the PBAC registry, which aligns with the main change of the pull request. It is concise, uses the conventional “feat:” prefix, and specifically mentions the resources being introduced. This phrasing makes it immediately clear to reviewers what functionality is being added.
Description Check ✅ Passed The description provides a detailed account of changes including adding new resources, creating a migration, and updating i18n entries, which directly corresponds to the modifications in the pull request. It also outlines testing steps and important review notes, confirming relevance and clarity. Therefore, it is clearly related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/availability-ooo-permissions-1758807829

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8ed0fb6 and 7a3e3e6.

📒 Files selected for processing (3)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • packages/features/pbac/domain/types/permission-registry.ts (3 hunks)
  • packages/prisma/migrations/20250925134226_add_availability_ooo_permissions_default_roles/migration.sql (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Detect changes
🔇 Additional comments (3)
packages/prisma/migrations/20250925134226_add_availability_ooo_permissions_default_roles/migration.sql (1)

1-45: Seeding logic looks solid.

The inserts cover the expected CRUD/read combinations for both roles, use gen_random_uuid()/NOW(), and the ON CONFLICT predicate keeps the migration idempotent. The resource strings (availability, ooo) line up with the new enum values, so this should apply cleanly.

packages/features/pbac/domain/types/permission-registry.ts (1)

13-633: Registry additions are coherent.

Resource.Availability / Resource.OutOfOffice mirror the migration payload, the CRUD entries wire up the correct i18n keys, and the empty scopes are consistent with the existing “hide from UI” convention. Depends-on chains (*.read) also align with the rest of the registry.

apps/web/public/static/locales/en/common.json (1)

3513-3526: Strings line up with the registry keys.

The new pbac_resource_* and pbac_desc_* entries match the keys referenced in the permission registry, so the UI will pick them up without missing translations.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the ❗️ migrations contains migration files label Sep 25, 2025
@keithwillcode keithwillcode added consumer core area: core, team members only labels Sep 25, 2025
- Add pbac_resource_availability and pbac_resource_out_of_office resource names
- Add description entries for all CRUD operations on both resources
- Follow existing PBAC i18n pattern for consistency

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@vercel
Copy link

vercel bot commented Sep 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 25, 2025 3:35pm
cal-eu Ignored Ignored Sep 25, 2025 3:35pm

@eunjae-lee eunjae-lee marked this pull request as ready for review September 25, 2025 14:26
@eunjae-lee eunjae-lee requested a review from a team as a code owner September 25, 2025 14:26
@graphite-app graphite-app bot requested a review from a team September 25, 2025 14:27
@dosubot dosubot bot added the ✨ feature New feature or request label Sep 25, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 25, 2025

E2E results are ready!

@eunjae-lee eunjae-lee merged commit d1c5576 into main Sep 25, 2025
61 of 63 checks passed
@eunjae-lee eunjae-lee deleted the devin/availability-ooo-permissions-1758807829 branch September 25, 2025 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only ✨ feature New feature or request ❗️ migrations contains migration files ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants