Skip to content

fix: rename SettingsPermissionsGuard to SettingsPermissionGuard for consistency#15712

Merged
FelixMalfait merged 1 commit intomainfrom
fix-settings-permission-guard-naming
Nov 7, 2025
Merged

fix: rename SettingsPermissionsGuard to SettingsPermissionGuard for consistency#15712
FelixMalfait merged 1 commit intomainfrom
fix-settings-permission-guard-naming

Conversation

@FelixMalfait
Copy link
Copy Markdown
Member

Problem

The ESLint rule graphql-resolvers-should-be-guarded introduced in #15392 was failing on main because the guard SettingsPermissionsGuard had inconsistent naming.

Root Cause

The guard was named SettingsPermissionsGuard (with an 's') which was inconsistent with other permission guards:

  • CustomPermissionGuard
  • NoPermissionGuard
  • ImpersonatePermissionGuard
  • SettingsPermissionsGuard (inconsistent!)

The ESLint rule checks if guard names end with PermissionGuard, but SettingsPermissionsGuard ends with sGuard, so it wasn't recognized as a permission guard.

Solution

Renamed the guard to be consistent with the naming convention:

  1. ✅ Renamed file: settings-permissions.guard.tssettings-permission.guard.ts
  2. ✅ Renamed export: SettingsPermissionsGuardSettingsPermissionGuard
  3. ✅ Renamed internal class: SettingsPermissionsMixinSettingsPermissionMixin
  4. ✅ Updated all 122 references across 44 files in the codebase
  5. ✅ Renamed test file: settings-permissions.guard.spec.tssettings-permission.guard.spec.ts

Testing

  • npx nx run twenty-server:lint passes
  • npx nx run twenty-server:typecheck passes
  • ✅ No references to the old name remain in the codebase
  • ✅ All previously failing resolver files now pass ESLint validation

Related

Fixes issues introduced in #15392

…onsistency

The guard was named SettingsPermissionsGuard (with an 's') which was inconsistent
with other permission guards (CustomPermissionGuard, NoPermissionGuard, etc.).

This inconsistency caused the ESLint rule 'graphql-resolvers-should-be-guarded'
to fail because it checks if guard names end with 'PermissionGuard', but
'SettingsPermissionsGuard' ends with 'sGuard'.

Changes:
- Renamed guard file: settings-permissions.guard.ts → settings-permission.guard.ts
- Renamed export: SettingsPermissionsGuard → SettingsPermissionGuard
- Renamed internal class: SettingsPermissionsMixin → SettingsPermissionMixin
- Updated all 122 references across 44 files
- Renamed test file to match

This ensures consistent naming across all permission guards and fixes ESLint
validation for mutations that rely on class-level permission guards.
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR performs a straightforward refactoring to fix naming inconsistency in the permission guard system.

Key Changes:

  • Renamed SettingsPermissionsGuardSettingsPermissionGuard (removed 's' from "Permissions")
  • Renamed SettingsPermissionsMixinSettingsPermissionMixin (internal class)
  • Renamed file settings-permissions.guard.tssettings-permission.guard.ts
  • Updated all 122+ references across 45 files including resolvers, tests, and documentation comments
  • Fixed ESLint rule comment to reflect new naming

Why This Matters:
The ESLint rule graphql-resolvers-should-be-guarded validates guard names by checking if they end with PermissionGuard. The old name SettingsPermissionsGuard ended with sGuard (not PermissionGuard), causing ESLint validation to fail. This rename brings it in line with other guards: CustomPermissionGuard, NoPermissionGuard, ImpersonatePermissionGuard.

Minor Note:
i18n locale files still contain comment references to the old file path settings-permissions.guard.ts. These are auto-generated lingui comments that will update on the next i18n extraction run.

Confidence Score: 5/5

  • This PR is completely safe to merge with zero risk
  • This is a pure refactoring PR with mechanical find-and-replace changes. No logic was modified, only names. The renaming is complete and consistent across all files. Tests pass, linting passes, and the change fixes the ESLint validation issue as intended.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-server/src/engine/guards/settings-permission.guard.ts 5/5 Renamed from settings-permissions.guard.ts with class name updated from SettingsPermissionsGuard to SettingsPermissionGuard for naming consistency
packages/twenty-server/src/engine/guards/tests/settings-permission.guard.spec.ts 5/5 Test file renamed and updated to import and test the renamed SettingsPermissionGuard
packages/twenty-server/src/engine/guards/custom-permission.guard.ts 5/5 Updated documentation comment to reference SettingsPermissionGuard instead of SettingsPermissionsGuard
packages/twenty-server/src/engine/guards/no-permission.guard.ts 5/5 Updated documentation comment to reference SettingsPermissionGuard instead of SettingsPermissionsGuard
tools/eslint-rules/utils/typedTokenHelpers.ts 5/5 Updated code comment example from SettingsPermissionsGuard to SettingsPermissionGuard

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant ESLint as ESLint Rule
    participant Resolver as GraphQL Resolver
    participant Guard as SettingsPermissionGuard
    participant PermSvc as PermissionsService
    
    Note over Dev,ESLint: Before PR: Naming Inconsistency
    Dev->>Resolver: Define @UseGuards(SettingsPermissionsGuard)
    ESLint->>Resolver: Check guard name pattern
    ESLint->>ESLint: Does name end with "PermissionGuard"?
    ESLint-->>Dev: ❌ Fails (ends with "sGuard")
    
    Note over Dev,ESLint: After PR: Consistent Naming
    Dev->>Resolver: Define @UseGuards(SettingsPermissionGuard)
    ESLint->>Resolver: Check guard name pattern
    ESLint->>ESLint: Does name end with "PermissionGuard"?
    ESLint-->>Dev: ✅ Passes
    
    Note over Resolver,PermSvc: Runtime Permission Check Flow
    Resolver->>Guard: canActivate(context)
    Guard->>Guard: Check workspace activation status
    alt Workspace being created
        Guard-->>Resolver: Allow (bypass permission)
    else Active workspace
        Guard->>PermSvc: userHasWorkspaceSettingPermission()
        PermSvc-->>Guard: boolean result
        alt Has permission
            Guard-->>Resolver: Allow
        else No permission
            Guard-->>Resolver: Throw PermissionsException
        end
    end
Loading

45 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@FelixMalfait FelixMalfait merged commit b7f5445 into main Nov 7, 2025
49 checks passed
@FelixMalfait FelixMalfait deleted the fix-settings-permission-guard-naming branch November 7, 2025 17:22
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 7, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:24312

This environment will automatically shut down when the PR is closed or after 5 hours.

@greptile-apps greptile-apps bot mentioned this pull request Nov 7, 2025
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.

1 participant