Resolve: 'editEngineUserPermissions modal updates permission to read_only '

[JoshRome26]

Closes #565

[github-actions]

@CodiumAI-Agent /describe

[QodoAI-Agent]

Title

Resolve: 'editEngineUserPermissions modal updates permission to read_only '

---

User description

Closes #565

---

PR Type

Bug fix

---

Description

• Fixed issue where user permissions were incorrectly updated to read_only.

• Added validation for user role settings to ensure valid roles.

• Updated default role initialization to avoid unintended changes.

• Enhanced logic for permission mapping with validation checks.

---

Changes walkthrough 📝

	Relevant files
Bug fix	MembersAddOverlay.tsx Fix and validate user role handling logic                                packages/client/src/components/settings/MembersAddOverlay.tsx Added Setting_Role_Values array to define valid roles. Introduced validSetting function to validate user roles. Updated default selectedRole initialization to null. Enhanced permission mapping logic with validation checks. +10/-4

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

@CodiumAI-Agent /review

[QodoAI-Agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 565 - Partially compliant Compliant requirements: When an Author edits the model limit fields, only the model limits should be updated. The user permissions should not be erroneously updated to "read_only". Non-compliant requirements: [] Requires further human verification: []

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The validSetting function is introduced to validate roles, but its behavior should be thoroughly tested to ensure it correctly handles edge cases and unexpected inputs. const validSetting = (value: unknown) => { return Setting_Role_Values.includes(value as SETTINGS_ROLE); Logic Validation The conditional logic for permission assignment in the request payload should be validated to ensure it handles invalid roles gracefully. permission: validSetting(selectedRole) ? permissionMapper[selectedRole] : selectedRole,

[github-actions]

@CodiumAI-Agent /improve

[QodoAI-Agent]

PR Code Suggestions ✨

Latest suggestions up to 916876a

Category	Suggestion	Impact
Possible issue	Handle null or undefined values in validation Ensure that the validSetting function handles cases where value is null or undefined to prevent potential runtime errors when selectedRole is null or undefined. packages/client/src/components/settings/MembersAddOverlay.tsx [79-81] const validSetting = (value: unknown) => { - return Setting_Role_Values.includes(value as SETTINGS_ROLE); + return value != null && Setting_Role_Values.includes(value as SETTINGS_ROLE); }; Suggestion importance[1-10]: 9 __ Why: The suggestion ensures that the validSetting function handles null or undefined values, which is crucial for preventing runtime errors when selectedRole is null or undefined. This directly improves the robustness of the validation logic.	High
	Add fallback for invalid roles Add a default fallback value for selectedRole when it is null to ensure permissionMapper does not receive an invalid key. packages/client/src/components/settings/MembersAddOverlay.tsx [269-271] permission: validSetting(selectedRole) ? permissionMapper[selectedRole] - : selectedRole, + : permissionMapper['Read-Only'], Suggestion importance[1-10]: 8 __ Why: Adding a fallback value for selectedRole ensures that permissionMapper does not receive an invalid key, which could lead to unexpected behavior. This improves the reliability of the code, especially when selectedRole is null or invalid.	Medium
General	Initialize state with a valid default Initialize selectedRole with a valid default value like 'Read-Only' to avoid null-related issues and ensure consistent behavior. packages/client/src/components/settings/MembersAddOverlay.tsx [160] -const [selectedRole, setSelectedRole] = useState<SETTINGS_ROLE>(null); +const [selectedRole, setSelectedRole] = useState<SETTINGS_ROLE>('Read-Only'); Suggestion importance[1-10]: 7 __ Why: Initializing selectedRole with a valid default value like 'Read-Only' ensures consistent behavior and avoids potential null-related issues. While this is a good improvement, it is slightly less critical than handling null values dynamically.	Medium

---

Previous suggestions

Suggestions up to commit 916876a

Category	Suggestion	Impact
General	Add type check for input validation Ensure that the validSetting function is robust against unexpected input types by adding a type check for value before casting it to SETTINGS_ROLE. packages/client/src/components/settings/MembersAddOverlay.tsx [80] -return Setting_Role_Values.includes(value as SETTINGS_ROLE); +return typeof value === 'string' && Setting_Role_Values.includes(value as SETTINGS_ROLE); Suggestion importance[1-10]: 9 __ Why: Adding a type check ensures robustness by preventing runtime errors when value is not a string, which is critical for maintaining the integrity of the validSetting function.	High
	Set default value for state Initialize selectedRole with a default valid value instead of null to prevent potential issues when it is accessed before being set. packages/client/src/components/settings/MembersAddOverlay.tsx [160] -const [selectedRole, setSelectedRole] = useState<SETTINGS_ROLE>(null); +const [selectedRole, setSelectedRole] = useState<SETTINGS_ROLE>('Read-Only'); Suggestion importance[1-10]: 7 __ Why: Initializing selectedRole with a default valid value enhances code safety and prevents potential issues when the state is accessed before being explicitly set.	Medium
Possible issue	Add fallback for invalid roles Handle the case where selectedRole is null or invalid in the permission assignment to avoid potential runtime errors. packages/client/src/components/settings/MembersAddOverlay.tsx [269-271] -permission: validSetting(selectedRole) +permission: selectedRole && validSetting(selectedRole) ? permissionMapper[selectedRole] - : selectedRole, + : 'UNKNOWN', Suggestion importance[1-10]: 8 __ Why: The fallback for invalid roles improves the reliability of the code by handling cases where selectedRole is null or invalid, thereby avoiding potential runtime errors.	Medium

[johbaxter]

Code found its way in on another PR