Skip to content

[permissions V2] Remove feature flag #12790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 23, 2025
Merged

Conversation

ijreilly
Copy link
Collaborator

No description provided.

Copy link
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.

PR Summary

Large-scale feature deployment that removes Permissions V2 feature flag, making it the permanent, default permission system across front and back end.

  • Restructures security by making permission validation mandatory in WorkspaceEntityManager and Repository layers, removing conditional permission checks
  • Cleans up legacy permission system code, including removal of UserWorkspaceRoleEntity and deprecated validation methods
  • Streamlines role management by removing feature flag checks in RoleService and associated components
  • Reconfigures workspace initialization to always use Permissions V2 system during setup
  • Requires attention to deployment planning as this removes backward compatibility with the V1 permission system

22 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

Comment on lines +256 to +269
if (
standardId &&
[
STANDARD_OBJECT_IDS.workflow,
STANDARD_OBJECT_IDS.workflowRun,
STANDARD_OBJECT_IDS.workflowVersion,
].includes(standardId)
) {
const hasWorkflowsPermissions = this.hasWorkflowsPermissions(role);

canRead = hasWorkflowsPermissions;
canUpdate = hasWorkflowsPermissions;
canSoftDelete = hasWorkflowsPermissions;
canDestroy = hasWorkflowsPermissions;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider extracting workflow-related standardIds to a constant to improve maintainability

Copy link
Contributor

github-actions bot commented Jun 23, 2025

🚀 Preview Environment Ready!

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

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

Copy link
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, let's launch this!

@ijreilly ijreilly enabled auto-merge (squash) June 23, 2025 14:14
Copy link
Contributor

📊 API Changes Report

GraphQL Schema Changes

GraphQL Schema Changes

[log]
Detected the following changes (1) between schemas:

[log] ✖ Enum value IS_PERMISSIONS_V2_ENABLED was removed from enum FeatureFlagKey
[error] Detected 1 breaking change
⚠️ Breaking changes or errors detected in GraphQL schema

[log] 
Detected the following changes (1) between schemas:

[log] ✖  Enum value IS_PERMISSIONS_V2_ENABLED was removed from enum FeatureFlagKey
[error] Detected 1 breaking change
Error generating diff

GraphQL Metadata Schema Changes

GraphQL Metadata Schema Changes

[log]
Detected the following changes (1) between schemas:

[log] ✖ Enum value IS_PERMISSIONS_V2_ENABLED was removed from enum FeatureFlagKey
[error] Detected 1 breaking change
⚠️ Breaking changes or errors detected in GraphQL metadata schema

[log] 
Detected the following changes (1) between schemas:

[log] ✖  Enum value IS_PERMISSIONS_V2_ENABLED was removed from enum FeatureFlagKey
[error] Detected 1 breaking change
Error generating diff

⚠️ Please review these API changes carefully before merging.

⚠️ Breaking Change Protocol

Breaking changes detected but PR title does not contain "breaking" - CI will pass but action needed.

🔄 Options:

  1. If this IS a breaking change: Add "breaking" to your PR title and add BREAKING CHANGE: to your commit message
  2. If this is NOT a breaking change: The API diff tool may have false positives - please review carefully

For breaking changes, add to commit message:

feat: add new API endpoint

BREAKING CHANGE: removed deprecated field from User schema

@ijreilly ijreilly merged commit 4c94fc2 into main Jun 23, 2025
57 checks passed
@ijreilly ijreilly deleted the permissions--remove-feature-flag branch June 23, 2025 15:22
Copy link

sentry-io bot commented Jun 27, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

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