Skip to content

[Fix] fix getRoles for demo#15666

Merged
ijreilly merged 1 commit intomainfrom
fix--demo
Nov 6, 2025
Merged

[Fix] fix getRoles for demo#15666
ijreilly merged 1 commit intomainfrom
fix--demo

Conversation

@ijreilly
Copy link
Copy Markdown
Contributor

@ijreilly ijreilly commented Nov 6, 2025

following #15547

Copy link
Copy Markdown
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM

@ijreilly ijreilly enabled auto-merge (squash) November 6, 2025 10:14
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 completes the work started in PR #15547 by adding shouldBypassPermissionChecks: true to two remaining workspaceMember repository calls that were missed in the initial implementation.

Changes:

  • microsoft-apis.service.ts:161 - Added bypass flag to workspaceMember repository call in the reconnection flow
  • user-role.service.ts:156 - Added bypass flag to workspaceMember repository call when retrieving members assigned to a role

Context:
PR #15547 introduced the ability to work with workspaceMember as a non-system object for demo purposes. This requires bypassing permission checks for all workspaceMember repository calls. This PR ensures consistency across the codebase by adding the flag to two locations that were overlooked.

The changes are safe because:

  1. workspaceMember was previously a system object with permissions already bypassed
  2. System-level permissions for workspace member operations are checked elsewhere
  3. This maintains functional parity with the previous behavior while supporting the demo workspace configuration

Confidence Score: 5/5

  • This PR is safe to merge with no risk - it completes a pattern established in PR #15547
  • Score reflects that these are minimal, mechanical changes following an established and reviewed pattern from PR #15547. The changes add explicit permission bypass flags to workspaceMember repository calls, which maintains the same effective behavior as before (since workspaceMember was a system object). No logic changes, no new functionality, just consistency improvements.
  • No files require special attention - both changes are straightforward parameter additions

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-server/src/engine/core-modules/auth/services/microsoft-apis.service.ts 5/5 Added shouldBypassPermissionChecks: true to workspaceMember repository call on line 161, consistent with PR #15547 pattern
packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts 5/5 Added shouldBypassPermissionChecks: true to workspaceMember repository call on line 156, consistent with PR #15547 pattern

Sequence Diagram

sequenceDiagram
    participant Service as MicrosoftAPIsService/UserRoleService
    participant ORM as TwentyORMGlobalManager
    participant Repo as WorkspaceMemberRepository
    participant DB as Database

    Note over Service,DB: PR #15666: Adding shouldBypassPermissionChecks

    Service->>ORM: getRepositoryForWorkspace(workspaceId, 'workspaceMember', {shouldBypassPermissionChecks: true})
    Note over ORM: Before: No third parameter<br/>After: Added bypass flag
    ORM->>Repo: Create repository with bypass flag
    Repo-->>Service: WorkspaceMemberRepository
    Service->>Repo: findOneOrFail() / find()
    Note over Repo: Bypasses permission checks<br/>for demo workspace support
    Repo->>DB: Query workspaceMember
    DB-->>Repo: WorkspaceMember data
    Repo-->>Service: WorkspaceMember entity
Loading

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ijreilly ijreilly merged commit 3bb0ecc into main Nov 6, 2025
50 checks passed
@ijreilly ijreilly deleted the fix--demo branch November 6, 2025 10:22
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.

2 participants