-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: Role linking to ACL fails with TypeError #3095
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: alpha
Are you sure you want to change the base?
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
Both analyses identify real issues, but they cause different errors: My race condition fix (commits 2a8d09a, 99ba2fa):
Missing prefix issue (coderabbitai's analysis):
The reported error message specifically mentions "reading 'set'" which matches the race condition, not the prefix issue. However, the prefix bug exists and could cause problems in other scenarios. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@lstarkgv could you please try out this PR and see whether is solves the issue? |
New Pull Request Checklist
Issue Description
Adding a role to an ACL record throws
TypeError: Cannot read properties of undefined (reading 'set')in version 8.0+, working in 7.5.Approach
Race condition:
entryTypesstate initialized asundefined, set toMapin asynccomponentDidMount. Fast user interactions trigger.set()calls before initialization completes.Changes:
PermissionsDialog.react.js: InitializeentryTypes: new Map()instead ofundefinedProtectedFieldsDialog.react.js: Same fix for protected fields dialogNote: This PR specifically fixes the race condition causing the reported "Cannot read properties of undefined (reading 'set')" error. A separate potential issue exists where validateEntry returns role entries without the "role:" prefix when roles are not found, which would cause a different error ("TypeError: next.role.getName is not a function"). That is a distinct bug not addressed in this PR.
TODOs before merging
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.