-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fixed Align all field-level permissions #14695 #15046
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
base: main
Are you sure you want to change the base?
fixed Align all field-level permissions #14695 #15046
Conversation
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
There was a problem hiding this 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 attempts to fix field-level permissions alignment but contains critical infrastructure failures that will break the application. The most severe issue is that multiple Yarn package manager binary files have been completely emptied across the codebase, including the root .yarn/releases/yarn-4.9.2.cjs
and application-specific instances. This will prevent dependency management and break the build system entirely.
The PR includes some legitimate formatting changes (line ending normalization in TypeScript files) and a CSS syntax fix in the permissions UI component. However, several critical migration services for database event triggers and serverless functions have been completely deleted, which will break workspace migration functionality. Additionally, there's a problematic CSS syntax error (padding-0
instead of padding-left
) that will cause styling issues.
The changes also include script modifications for database initialization commands and various formatting updates across Docker and shell scripts, but these are overshadowed by the infrastructure failures.
Important Files Changed
Changed Files
Filename | Score | Overview |
---|---|---|
.yarn/releases/yarn-4.9.2.cjs |
1/5 | Critical: Yarn binary completely emptied, breaking dependency management |
packages/twenty-apps/hello-world/.yarn/releases/yarn-4.9.2.cjs |
1/5 | Critical: Yarn binary emptied, breaking hello-world app package management |
packages/twenty-cli/src/constants/base-application-project/.yarn/releases/yarn-4.9.2.cjs |
1/5 | Critical: Yarn binary emptied, breaking CLI template functionality |
packages/twenty-server/src/engine/core-modules/serverless/drivers/layers/engine/.yarn/releases/yarn-4.9.2.cjs |
1/5 | Critical: Yarn binary emptied, breaking serverless layer functionality |
packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/field-permissions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTableAllHeaderRow.tsx |
1/5 | Critical CSS syntax error: padding0 should be padding-left |
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/database-event-trigger/services/create-database-event-trigger-action-handler.service.ts |
1/5 | Critical: Service completely deleted, breaking database event trigger creation |
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/database-event-trigger/services/delete-database-event-trigger-action-handler.service.ts |
1/5 | Critical: Service completely deleted, breaking database event trigger deletion |
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/database-event-trigger/services/update-database-event-trigger-action-handler.service.ts |
1/5 | Critical: Service completely deleted, breaking database event trigger updates |
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-builder-v2/builders/serverless-function/workspace-migration-v2-serverless-function-actions-builder.service.ts |
1/5 | Critical: Service completely deleted, breaking serverless function migrations |
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-builder-v2/builders/database-event-trigger/workspace-migration-v2-database-event-trigger-actions-builder.service.ts |
1/5 | Critical: Service completely deleted, breaking database event trigger migration building |
.yarn/patches/react-phone-number-input-npm-3.4.5-dc2895c306.patch |
2/5 | Phone number input patch with coding style issues and potential logic conflicts |
packages/twenty-server/scripts/render-run.sh |
3/5 | Database command changed from database:setup:prod to database:init:prod |
packages/twenty-front/src/modules/ui/layout/selectable-list/states/selectedItemIdComponentState.ts |
4/5 | Line ending normalization, functionally identical |
packages/twenty-docker/scripts/install.sh |
4/5 | Minor formatting changes to Docker installation script |
packages/twenty-server/scripts/ssl-generation/script.sh |
4/5 | Minor formatting changes to SSL certificate generation script |
packages/twenty-front/src/modules/ui/layout/selectable-list/states/isSelectedItemIdComponentFamilyState.ts |
5/5 | Line ending normalization only |
packages/twenty-front/src/modules/ui/layout/selectable-list/states/selectableItemIdsComponentState.ts |
5/5 | Line ending normalization only |
packages/twenty-front/scripts/inject-runtime-env.sh |
5/5 | Added clarifying comments to sed operation |
packages/twenty-docker/podman/install-systemd-user-service |
5/5 | Added blank lines for better formatting |
packages/twenty-docker/twenty/entrypoint.sh |
5/5 | No meaningful changes detected |
packages/twenty-server/scripts/render-worker.sh |
5/5 | Minor formatting changes only |
nx |
5/5 | Standard Nx wrapper script, appears unchanged |
Confidence score: 0/5
- This PR is not safe to merge and will cause immediate production failures due to critical infrastructure issues
- Score reflects multiple critical failures: completely emptied Yarn binaries breaking dependency management, deleted migration services breaking workspace functionality, and CSS syntax errors affecting UI rendering
- Pay immediate attention to all Yarn binary files, deleted migration services, and the CSS syntax error in field permissions component
Sequence Diagram
sequenceDiagram
participant User
participant SettingsRolePermissionsComponent as "Settings Role Permissions Component"
participant useObjectPermissionDerivedStates as "Permission Hooks"
participant RecoilState as "Recoil State"
participant PermissionActions as "Permission Actions"
User->>SettingsRolePermissionsComponent: "Interacts with 'All' header row"
SettingsRolePermissionsComponent->>RecoilState: "useRecoilState(settingsDraftRoleFamilyState)"
RecoilState-->>SettingsRolePermissionsComponent: "Returns current draft role state"
SettingsRolePermissionsComponent->>useObjectPermissionDerivedStates: "Check permission constraints"
useObjectPermissionDerivedStates-->>SettingsRolePermissionsComponent: "Returns cannotAllowFieldReadRestrict, cannotAllowFieldUpdateRestrict"
alt User clicks read permission checkbox
User->>SettingsRolePermissionsComponent: "handleReadAllChange()"
alt hasAnyRestrictionOnRead is true
SettingsRolePermissionsComponent->>PermissionActions: "removeReadOverrideOnAllFieldsOfObject(objectMetadataItem)"
else hasAnyRestrictionOnRead is false
SettingsRolePermissionsComponent->>PermissionActions: "restrictReadOnAllFieldsOfObject(objectMetadataItem)"
end
PermissionActions-->>RecoilState: "Updates field permissions state"
end
alt User clicks update permission checkbox
User->>SettingsRolePermissionsComponent: "handleUpdateAllChange()"
alt hasAnyRestrictionOnUpdate is true
SettingsRolePermissionsComponent->>PermissionActions: "removeUpdateOverrideOnAllFieldsOfObject(objectMetadataItem)"
else hasAnyRestrictionOnUpdate is false
SettingsRolePermissionsComponent->>PermissionActions: "restrictUpdateOnAllFieldsOfObject(objectMetadataItem)"
end
PermissionActions-->>RecoilState: "Updates field permissions state"
end
RecoilState-->>SettingsRolePermissionsComponent: "State updated"
SettingsRolePermissionsComponent-->>User: "UI reflects permission changes"
Additional Comments (15)
-
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/database-event-trigger/services/update-database-event-trigger-action-handler.service.ts
logic: Entire service deleted - verify that update functionality for database event triggers is handled elsewhere in the codebase to prevent broken migrations
-
packages/twenty-server/src/engine/core-modules/serverless/drivers/layers/engine/.yarn/releases/yarn-4.9.2.cjs
, line 1 (link)logic: Critical: This Yarn binary file is completely empty. This will break serverless layer package management since .yarnrc.yml references this specific executable.
-
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-builder-v2/builders/database-event-trigger/workspace-migration-v2-database-event-trigger-actions-builder.service.ts
logic: Entire service file deleted - verify this functionality is handled elsewhere or confirm it's intentionally removed
-
.yarn/releases/yarn-4.9.2.cjs
, line 1 (link)logic: Critical: The Yarn 4.9.2 binary file is completely empty. This will break the build system and package management across the entire project. This file should contain the actual Yarn binary code and must be restored immediately.
-
packages/twenty-server/scripts/ssl-generation/script.sh
, line 13 (link)style: Comment says 'Default is 825 days' but actual default value is 398 days
-
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-builder-v2/builders/serverless-function/workspace-migration-v2-serverless-function-actions-builder.service.ts
, line 1-152 (link)logic: Complete deletion of serverless function migration service - verify that equivalent functionality exists elsewhere to prevent breaking serverless function deployments
-
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/database-event-trigger/services/delete-database-event-trigger-action-handler.service.ts
logic: Entire service deleted - verify this functionality is handled elsewhere or if database event trigger deletion is no longer supported
-
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/database-event-trigger/services/create-database-event-trigger-action-handler.service.ts
logic: Critical issue: Entire service class deleted. This will break workspace migrations that need to create database event triggers. The deletion appears unintentional given the PR context.
-
packages/twenty-apps/hello-world/.yarn/releases/yarn-4.9.2.cjs
logic: This Yarn binary file has been completely emptied, which will break package management for the hello-world app. This appears unintentional and unrelated to field-level permissions.
-
packages/twenty-cli/src/constants/base-application-project/.yarn/releases/yarn-4.9.2.cjs
, line 1 (link)logic: Critical: Yarn executable has been completely emptied. This will break dependency management for CLI-generated projects.
-
packages/twenty-docker/scripts/install.sh
, line 89 (link)style: Missing space after
-i
flag - should besed -i ''
for consistency with line 86 -
packages/twenty-docker/scripts/install.sh
, line 115-116 (link)style: Missing space after
-i
flag - should besed -i ''
for consistency with lines 112-113 -
.yarn/patches/react-phone-number-input-npm-3.4.5-dc2895c306.patch
, line 10-12 (link)style: Missing space after 'if' and inconsistent indentation (tabs vs spaces). The line uses tabs while surrounding code uses spaces.
-
.yarn/patches/react-phone-number-input-npm-3.4.5-dc2895c306.patch
, line 10 (link)logic: This condition bypasses existing phone number validation logic. Consider whether this could cause issues with invalid phone numbers being accepted.
-
.yarn/patches/react-phone-number-input-npm-3.4.5-dc2895c306.patch
, line 25-27 (link)style: Same formatting issues and logic concerns as above - missing space after 'if' and inconsistent indentation.
22 files reviewed, 18 comments
}, | ||
); | ||
import { SelectableListComponentInstanceContext } from '@/ui/layout/selectable-list/states/contexts/SelectableListComponentInstanceContext'; | ||
import { createComponentState } from '@/ui/utilities/state/component-state/utils/createComponentState'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider updating to use createComponentStateV2 for consistency with the documented approach in the codebase
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/ui/layout/selectable-list/states/selectedItemIdComponentState.ts
Line: 2:2
Comment:
**style:** Consider updating to use createComponentStateV2 for consistency with the documented approach in the codebase
How can I resolve this? If you propose a fix, please make it concise.
...ions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTableAllHeaderRow.tsx
Outdated
Show resolved
Hide resolved
+ if(phoneDigits === prevPhoneDigits) { | ||
+ value = e164(phoneDigits, country, metadata) | ||
+ } else if (phoneDigits[0] === '+') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consistent with other occurrences - formatting and potential logic issues need to be addressed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .yarn/patches/react-phone-number-input-npm-3.4.5-dc2895c306.patch
Line: 40:42
Comment:
**style:** Consistent with other occurrences - formatting and potential logic issues need to be addressed.
How can I resolve this? If you propose a fix, please make it concise.
…ons/object-level-permissions/field-permissions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTableAllHeaderRow.tsx Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
No description provided.