Add object level form to role creation #12826
Conversation
| @@ -57,7 +56,7 @@ export const ReadOnly: Story = { | |||
|
|
|||
| export const PendingRole: Story = { | |||
| args: { | |||
| roleId: PENDING_ROLE_ID, | |||
| roleId: 'newRoleId', | |||
There was a problem hiding this comment.
why don't we use a const anymore?
| @@ -57,7 +56,7 @@ export const ReadOnly: Story = { | |||
|
|
|||
| export const PendingRole: Story = { | |||
| args: { | |||
| roleId: PENDING_ROLE_ID, | |||
| roleId: 'newRoleId', | |||
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements object-level permissions during role creation and adds loading state feedback for save operations in the settings interface.
- Exposed object-level permissions section during role creation in
SettingsRolePermissions.tsxby removing conditional rendering, allowing full permission configuration upfront - Added
isSavingprop for loading state feedback during save operations, improving UX by indicating when operations are in progress - Restructured role creation flow in
SettingsRole.tsxto follow a logical sequence: create role → settings → object permissions → workspace members - Replaced hardcoded
PENDING_ROLE_IDwith dynamic UUID generation across stories and components - Modified
RoleServiceto accept custom IDs during role creation, enabling better state management between frontend and backend
9 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
| const persistedRole = useRecoilValue( | ||
| settingsPersistedRoleFamilyState(roleId ?? ''), | ||
| ); |
There was a problem hiding this comment.
style: Using empty string as fallback for undefined roleId could cause issues with Recoil selectors. Consider using undefined or handling this case differently.
| if (!isDefined(roleId)) { | ||
| return <Navigate to={getSettingsPath(SettingsPath.Roles)} />; | ||
| } |
There was a problem hiding this comment.
logic: This navigation check should happen before the Recoil value access to prevent unnecessary state lookups.
| setSettingsPersistedRole(undefined); | ||
| setSettingsDraftRole(newRole); |
There was a problem hiding this comment.
logic: Setting persisted role to undefined before draft role. Consider if this order is intentional for state management
|
|
||
| export const SettingsRoleCreate = () => { | ||
| const newRoleId = v4(); |
There was a problem hiding this comment.
logic: UUID generation in component body will create new ID on every render. Move to useMemo or useState to prevent unnecessary role ID changes.
| const newRoleId = v4(); | |
| import { useMemo } from 'react'; | |
| export const SettingsRoleCreate = () => { | |
| const newRoleId = useMemo(() => v4(), []); |
charlesBochet
deleted the
c--permission-add-object-level-input-to-role-creation
branch
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:47536 This environment will automatically shut down when the PR is closed or after 5 hours. |
## Context - Add object-level form to role creation - Add isSaving props for save button isLoading state <img width="594" alt="Screenshot 2025-06-24 at 15 03 59" src="https://github.com/user-attachments/assets/77d9d399-4e1a-4e35-be45-c19100ef06c1" /> --------- Co-authored-by: Charles Bochet <[email protected]>
Context