Granular console scopes#10458
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a granular console-role permission model behind a feature flag, updates backend and frontend permission resolution for CRUD scopes, and adjusts user-group and user-profile UI gating based on scoped feature permissions. ChangesGranular Console Permissions
User Permission Gating
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
identity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.java (1)
273-304: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGranular feature scopes are not resolved in the feature-scope branch.
Because Line 622-625 now classifies
*_create|*_update|*_deleteas feature permissions, roles with granular feature scopes enter this branch (Line 273). But Line 281-303 only resolvesview/edit, and the early return at Line 304 skips Line 339 consolidation entirely. Result: granular-scope roles can resolve to empty/incorrect permission sets.🐛 Proposed fix
@@ - if (!consoleFeaturePermissions.isEmpty()) { + List<APIResourceCollection> apiResourceCollections = getAPIResourceCollections(tenantDomain); + List<Permission> systemPermissions = new ArrayList<>(systemPermissionsMap.values()); + List<Permission> upgradedPermissions; + + if (!consoleFeaturePermissions.isEmpty()) { @@ - Map<String, Set<String>> consoleFeaturePermissionsMap = getConsoleFeaturePermissionsMap(tenantDomain); List<Permission> resolvedRolePermissions = new ArrayList<>(); consoleFeaturePermissions.forEach(permission -> { apiResourceCollections.forEach(apiResourceCollection -> { @@ if (apiResourceCollection.getViewFeatureScope() != null && apiResourceCollection.getViewFeatureScope().equals(permission.getName())) { @@ } + if (apiResourceCollection.getCreateFeatureScope() != null && + apiResourceCollection.getCreateFeatureScope().equals(permission.getName())) { + apiResourceCollection.getCreateScopes().forEach(createScope -> { + Permission newPermission = systemPermissionsMap.get(createScope); + if (newPermission != null) { + resolvedRolePermissions.add(newPermission); + } + }); + } + if (apiResourceCollection.getUpdateFeatureScope() != null && + apiResourceCollection.getUpdateFeatureScope().equals(permission.getName())) { + apiResourceCollection.getUpdateScopes().forEach(updateScope -> { + Permission newPermission = systemPermissionsMap.get(updateScope); + if (newPermission != null) { + resolvedRolePermissions.add(newPermission); + } + }); + } + if (apiResourceCollection.getDeleteFeatureScope() != null && + apiResourceCollection.getDeleteFeatureScope().equals(permission.getName())) { + apiResourceCollection.getDeleteScopes().forEach(deleteScope -> { + Permission newPermission = systemPermissionsMap.get(deleteScope); + if (newPermission != null) { + resolvedRolePermissions.add(newPermission); + } + }); + } }); }); - return resolvedRolePermissions; + upgradedPermissions = resolvedRolePermissions; } else { @@ - resolveWriteFeatureScopes(upgradedPermissions, apiResourceCollections, systemPermissions); + resolveWriteFeatureScopes(upgradedPermissions, apiResourceCollections, systemPermissions); return upgradedPermissions;Also applies to: 339-340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@identity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.java` around lines 273 - 304, The forEach loop in the consoleFeaturePermissions handling block (lines 281-303) only resolves view and edit feature scopes but does not handle granular feature scopes like create, update, and delete. Extend the nested conditions that check for editFeatureScope and viewFeatureScope to also check for granular feature scope types (create, update, delete). For each granular scope type found in the permission name, map it to the corresponding scope collection from apiResourceCollection (similar to how editFeatureScope maps to writeScopes and viewFeatureScope maps to readScopes) and add the resolved permissions to resolvedRolePermissions. This ensures that roles with granular feature scopes do not resolve to empty or incorrect permission sets.
🧹 Nitpick comments (2)
features/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsx (2)
163-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename metadata interface to follow repository naming contract.
CollectionLevelMetadatashould use the requiredInterfacesuffix.Suggested patch
-interface CollectionLevelMetadata { +interface CollectionLevelMetadataInterface { availableLevels: GranularPermissionLevel[]; scopeNamesByLevel: Record<GranularPermissionLevel, string[]>; marginalScopeNamesByLevel: Record<GranularPermissionLevel, string[]>; }As per coding guidelines, "Interface names must use
Interfacesuffix."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsx` around lines 163 - 167, The interface CollectionLevelMetadata does not follow the repository naming convention which requires all interfaces to use the Interface suffix. Rename the CollectionLevelMetadata interface to CollectionLevelMetadataInterface and update all references to this interface throughout the file to use the new name.Source: Coding guidelines
216-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit
useSelectorreturn types and aligndisabledFeaturesnullability.These selectors still rely on inferred return types, and
disabledFeaturesis typed as non-optional while its path is optional.As per coding guidelines, "Always include explicit type annotations on
useSelectorhook with the selector function parameter type and return type" and "Always use explicit type annotations for variables."Also applies to: 226-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsx` around lines 216 - 221, The useSelector hooks for featureConfig, enabledFeatureOverridesInConsoleRolePermissions, and disabledFeatures lack explicit return type annotations and rely on type inference. Additionally, disabledFeatures is typed as string[] (non-optional) but the selector path uses optional chaining which could return undefined. Add explicit return type annotations to each useSelector call matching their expected types, and update the disabledFeatures variable type to string[] | undefined (or appropriate nullable type) to align with the optional chaining in the selector path that accesses state?.config?.ui?.features?.consoleSettings?.disabledFeatures.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx`:
- Around line 106-110: The disabledFeatures variable is typed as string[] but
the useSelector hook can return undefined when the config path is not fully
defined, creating a type mismatch. Fix this by adding an explicit return type
annotation to the useSelector hook (specifying string[] | undefined as the
return type) and update the disabledFeatures variable declaration to reflect
this optional type (string[] | undefined). Apply the same fix to the
featureConfig variable and its useSelector call on lines 114-115, ensuring
explicit type annotations are included on both the selector function and
variable declarations as per coding guidelines.
In
`@features/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsx`:
- Around line 1188-1189: Replace all hardcoded English aria-label strings with
i18n translation calls using the t() function. For each aria-label attribute
that contains hardcoded strings (including "Select all tenant permissions" and
similar strings at the mentioned locations), wrap the string value with t() and
use the i18next namespace-based key format namespace:path.to.key to reference
the appropriate translation key. Ensure the t() function is imported from the
i18n module as per the project's i18n setup, and apply this change consistently
across all aria-label attributes in the
create-console-role-wizard-permissions-form component.
- Around line 857-867: The `computeAccordionSelectAllState` function always
returns `indeterminate: false`, which prevents partial selections from rendering
correctly. Calculate the proper indeterminate state by checking if the number of
selected permissions for the given type is greater than zero but less than the
total sourceCollections length. This will ensure the summary checkbox properly
displays the indeterminate state when some items are selected but not all.
In `@features/admin.users.v1/components/user-groups-edit.tsx`:
- Around line 92-93: The useRequiredScopes hook call on the
hasGroupsUpdatePermission variable is passing an potentially undefined value
when groupsFeatureConfig?.scopes?.update is absent, which violates the expected
string array parameter type. Guard against this undefined case by providing a
fallback empty array using the nullish coalescing operator, so that
useRequiredScopes always receives a valid string array regardless of whether the
update scope exists in the configuration.
In `@features/admin.users.v1/components/user-profile.tsx`:
- Around line 1016-1020: The password block guard at lines 1041-1042 is checking
featureConfig?.users?.scopes?.update which only verifies if the update scope is
configured in the feature config, not whether the current user has update
permissions. This allows delete-only users to see update actions if they pass
the earlier permission gate. Replace the featureConfig?.users?.scopes?.update
check with a direct check of hasUsersUpdatePermissions to ensure only users with
actual update permissions can access the password block, maintaining consistency
with the earlier permission gate that combines hasUsersDeletePermissions or
hasUsersUpdatePermissions.
In `@modules/i18n/src/translations/en-US/portals/console-settings.ts`:
- Line 64: The permissionsCount translation string in console-settings.ts uses
the fixed plural form "Permissions" which produces grammatically incorrect
output for singular values (e.g., "1 Permissions"). Update the permissionsCount
string to use count-safe wording that handles both singular and plural cases,
such as conditional formatting or a format that gracefully handles both "1
Permission" and "n Permissions" based on the count value.
---
Outside diff comments:
In
`@identity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.java`:
- Around line 273-304: The forEach loop in the consoleFeaturePermissions
handling block (lines 281-303) only resolves view and edit feature scopes but
does not handle granular feature scopes like create, update, and delete. Extend
the nested conditions that check for editFeatureScope and viewFeatureScope to
also check for granular feature scope types (create, update, delete). For each
granular scope type found in the permission name, map it to the corresponding
scope collection from apiResourceCollection (similar to how editFeatureScope
maps to writeScopes and viewFeatureScope maps to readScopes) and add the
resolved permissions to resolvedRolePermissions. This ensures that roles with
granular feature scopes do not resolve to empty or incorrect permission sets.
---
Nitpick comments:
In
`@features/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsx`:
- Around line 163-167: The interface CollectionLevelMetadata does not follow the
repository naming convention which requires all interfaces to use the Interface
suffix. Rename the CollectionLevelMetadata interface to
CollectionLevelMetadataInterface and update all references to this interface
throughout the file to use the new name.
- Around line 216-221: The useSelector hooks for featureConfig,
enabledFeatureOverridesInConsoleRolePermissions, and disabledFeatures lack
explicit return type annotations and rely on type inference. Additionally,
disabledFeatures is typed as string[] (non-optional) but the selector path uses
optional chaining which could return undefined. Add explicit return type
annotations to each useSelector call matching their expected types, and update
the disabledFeatures variable type to string[] | undefined (or appropriate
nullable type) to align with the optional chaining in the selector path that
accesses state?.config?.ui?.features?.consoleSettings?.disabledFeatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9b20c243-f5f2-4043-8638-de168175c780
📒 Files selected for processing (15)
features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsxfeatures/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.scssfeatures/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsxfeatures/admin.console-settings.v1/constants/console-roles-onboarding-constants.tsfeatures/admin.console-settings.v1/models/console-roles.tsfeatures/admin.console-settings.v1/models/permissions-ui.tsfeatures/admin.console-settings.v1/utils/get-eligibility-scope-names.tsfeatures/admin.console-settings.v1/utils/is-permission-level-actionable.tsfeatures/admin.users.v1/components/user-groups-edit.tsxfeatures/admin.users.v1/components/user-profile.tsxidentity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.javaidentity-apps-core/components/org.wso2.identity.apps.common/src/test/java/org/wso2/identity/apps/common/listener/ConsoleRoleListenerTest.javaidentity-apps-core/components/org.wso2.identity.apps.common/src/test/resources/testng.xmlmodules/i18n/src/models/namespaces/console-settings-ns.tsmodules/i18n/src/translations/en-US/portals/console-settings.ts
57ae4b6 to
87b8d32
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10458 +/- ##
==========================================
+ Coverage 72.83% 72.86% +0.02%
==========================================
Files 470 470
Lines 71191 71254 +63
Branches 240 240
==========================================
+ Hits 51855 51918 +63
Misses 19225 19225
Partials 111 111
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
identity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.java (1)
282-293: 🚀 Performance & Scalability | 🔵 TrivialRedundant
getAPIResourceCollectionsfetch on the read path.
apiResourceCollectionsis fetched at Line 282, but the new-console-role branch then callsgetConsoleFeaturePermissionsMap(tenantDomain)(Line 293), which internally callsgetAPIResourceCollections(tenantDomain)again (Line 456). SimilarlygetSystemRolePermissionsfetches collections both via the map and at Line 260. This resolution runs per role — andpostGetPermissionListOfRoles/populateSystemConsoleRolesPermissionsinvoke it in a loop — so the duplicate remote lookups multiply. Consider fetching the collections once per invocation and threading them throughgetConsoleFeaturePermissionsMap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@identity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.java` around lines 282 - 293, `ConsoleRoleListener` is fetching API resource collections twice on the same read path, once before the role branch and again inside `getConsoleFeaturePermissionsMap`, which also affects `getSystemRolePermissions` and multiplies remote lookups in the role-processing loops. Refactor `getConsoleFeaturePermissionsMap` to accept the already-fetched `apiResourceCollections` (or otherwise reuse the single fetch from `postGetPermissionListOfRoles`/`populateSystemConsoleRolesPermissions`) and thread that data through the helper methods so each invocation resolves collections only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@identity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.java`:
- Around line 282-293: `ConsoleRoleListener` is fetching API resource
collections twice on the same read path, once before the role branch and again
inside `getConsoleFeaturePermissionsMap`, which also affects
`getSystemRolePermissions` and multiplies remote lookups in the role-processing
loops. Refactor `getConsoleFeaturePermissionsMap` to accept the already-fetched
`apiResourceCollections` (or otherwise reuse the single fetch from
`postGetPermissionListOfRoles`/`populateSystemConsoleRolesPermissions`) and
thread that data through the helper methods so each invocation resolves
collections only once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: d804dc83-1760-4f4b-b2ac-2747ac859721
📒 Files selected for processing (11)
apps/console/src/public/deployment.config.jsonfeatures/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsxfeatures/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsxfeatures/admin.console-settings.v1/constants/console-roles-onboarding-constants.tsfeatures/admin.console-settings.v1/utils/get-eligibility-scope-names.tsfeatures/admin.users.v1/components/user-groups-edit.tsxfeatures/admin.users.v1/components/user-profile.tsxidentity-apps-core/components/org.wso2.identity.apps.common/src/main/java/org/wso2/identity/apps/common/listner/ConsoleRoleListener.javaidentity-apps-core/components/org.wso2.identity.apps.common/src/test/java/org/wso2/identity/apps/common/listener/ConsoleRoleListenerTest.javaidentity-apps-core/components/org.wso2.identity.apps.common/src/test/resources/testng.xmlidentity-apps-core/pom.xml
✅ Files skipped from review due to trivial changes (1)
- identity-apps-core/components/org.wso2.identity.apps.common/src/test/resources/testng.xml
🚧 Files skipped from review as they are similar to previous changes (7)
- features/admin.console-settings.v1/utils/get-eligibility-scope-names.ts
- features/admin.users.v1/components/user-groups-edit.tsx
- features/admin.console-settings.v1/constants/console-roles-onboarding-constants.ts
- features/admin.users.v1/components/user-profile.tsx
- features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx
- identity-apps-core/components/org.wso2.identity.apps.common/src/test/java/org/wso2/identity/apps/common/listener/ConsoleRoleListenerTest.java
- features/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsx
60e542a to
3bdae54
Compare
6db1a6f to
77691f3
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx (4)
130-134: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
| nullto variable type annotations.The
useMemohooks returnnullwhen the underlying API collections are falsy. As per coding guidelines, explicit variable types must accurately reflect the assigned values. Update the type annotations to include| null.💻 Suggested patch
- const filteredTenantAPIResourceCollections: APIResourceCollectionResponseInterface = useMemo(() => { + const filteredTenantAPIResourceCollections: APIResourceCollectionResponseInterface | null = useMemo(() => { if (!tenantAPIResourceCollections) { return null; }- const filteredOrganizationAPIResourceCollections: APIResourceCollectionResponseInterface = useMemo(() => { + const filteredOrganizationAPIResourceCollections: APIResourceCollectionResponseInterface | null = useMemo(() => { if (!organizationAPIResourceCollections) { return null; }Also applies to: 154-158
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx` around lines 130 - 134, Update the explicit type annotations for filteredTenantAPIResourceCollections and the other filtered collection variable in their useMemo hooks to include | null, matching the null returned when the underlying API collections are falsy.Source: Coding guidelines
185-188: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSafeguard against missing
scopesarrays.If an API resource category is returned without the
scopesarray, calling.map()will cause a runtime error. Use optional chaining and a fallback array to ensure safe extraction.🛡️ Suggested patch
return categories.flatMap( (resource: APIResourceCollectionPermissionCategoryInterface) => - resource.scopes.map((scope: APIResourceCollectionPermissionScopeInterface) => scope.name) + (resource?.scopes ?? []).map((scope: APIResourceCollectionPermissionScopeInterface) => scope.name) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx` around lines 185 - 188, Update the category scope extraction in the flatMap callback to safely handle missing resource.scopes by using optional chaining with an empty-array fallback before calling map, while preserving the existing scope.name results for categories that provide scopes.
245-285: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse optional chaining for
collection.apiResourcesaccesses.If
collection.apiResourcesis missing or undefined in the API response, accessing its properties directly (e.g.,collection.apiResources.read) will throw aTypeError. Use optional chaining (collection?.apiResources?.<level>) throughout this function to ensure safe extraction.🛡️ Suggested patch
- const hasRead: boolean = allScopesSelected(collection.apiResources.read, selectedFeatures); - const hasCreate: boolean = isPermissionLevelActionable(collection, "create") - && allScopesSelected(collection.apiResources.create, selectedFeatures); - const hasUpdate: boolean = isPermissionLevelActionable(collection, "update") - && allScopesSelected(collection.apiResources.update, selectedFeatures); - const hasDelete: boolean = isPermissionLevelActionable(collection, "delete") - && allScopesSelected(collection.apiResources.delete, selectedFeatures); + const hasRead: boolean = allScopesSelected(collection?.apiResources?.read, selectedFeatures); + const hasCreate: boolean = isPermissionLevelActionable(collection, "create") + && allScopesSelected(collection?.apiResources?.create, selectedFeatures); + const hasUpdate: boolean = isPermissionLevelActionable(collection, "update") + && allScopesSelected(collection?.apiResources?.update, selectedFeatures); + const hasDelete: boolean = isPermissionLevelActionable(collection, "delete") + && allScopesSelected(collection?.apiResources?.delete, selectedFeatures); if (!hasRead && !hasCreate && !hasUpdate && !hasDelete) { return null; } // Collect scopes for every checked level and deduplicate. const activeCategories: APIResourceCollectionPermissionCategoryInterface[] = [ - ...(hasRead ? (collection.apiResources.read ?? []) : []), - ...(hasCreate ? (collection.apiResources.create ?? []) : []), - ...(hasUpdate ? (collection.apiResources.update ?? []) : []), - ...(hasDelete ? (collection.apiResources.delete ?? []) : []) + ...(hasRead ? (collection?.apiResources?.read ?? []) : []), + ...(hasCreate ? (collection?.apiResources?.create ?? []) : []), + ...(hasUpdate ? (collection?.apiResources?.update ?? []) : []), + ...(hasDelete ? (collection?.apiResources?.delete ?? []) : []) ]; return { create: hasCreate, delete: hasDelete, permissions: transformResourceCollectionToPermissions(activeCategories), read: hasRead, update: hasUpdate, write: false }; } // Legacy mode. - const hasReadPermission: boolean = allScopesSelected(collection.apiResources.read, selectedFeatures); - const hasWritePermission: boolean = allScopesSelected(collection.apiResources.write, selectedFeatures); + const hasReadPermission: boolean = allScopesSelected(collection?.apiResources?.read, selectedFeatures); + const hasWritePermission: boolean = allScopesSelected(collection?.apiResources?.write, selectedFeatures); if (!hasReadPermission && !hasWritePermission) { return null; } const legacyCategories: APIResourceCollectionPermissionCategoryInterface[] = - (hasWritePermission ? collection.apiResources.write : undefined) ?? collection.apiResources.read; + (hasWritePermission ? collection?.apiResources?.write : undefined) ?? collection?.apiResources?.read;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx` around lines 245 - 285, Update the permission extraction logic in the current function to use optional chaining for every collection.apiResources access, including read, create, update, delete, and write in both actionable and legacy branches. Preserve the existing fallback behavior for missing category arrays while preventing errors when collection.apiResources is undefined.
106-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix selector typing for
disabledFeaturesandfeatureConfig.
disabledFeaturesis declared asstring[], but the selector can returnundefined. Add an explicit selector return type and align the variable type to properly handleundefinedduring evaluation. Apply the explicit return type annotation to thefeatureConfigselector hook as well.💻 Suggested patch
- const disabledFeatures: string[] = useSelector((state: AppState) => - state?.config?.ui?.features?.consoleSettings?.disabledFeatures); + const disabledFeatures: string[] | undefined = useSelector((state: AppState): string[] | undefined => + state?.config?.ui?.features?.consoleSettings?.disabledFeatures); - const featureConfig: FeatureConfigInterface = useSelector((state: AppState) => state.config.ui.features); + const featureConfig: FeatureConfigInterface = useSelector((state: AppState): FeatureConfigInterface => state.config.ui.features); /** * Switches the permissions evaluation between the legacy and the granular mode. */ - const useGranularConsolePermissions: boolean = !disabledFeatures?.includes( + const useGranularConsolePermissions: boolean = !(disabledFeatures ?? []).includes( ConsoleRolesOnboardingConstants.GRANULAR_CONSOLE_PERMISSIONS_FEATURE_KEY);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx` around lines 106 - 115, Update the disabledFeatures selector and variable in the console-role permissions component to explicitly support an undefined selector result, while preserving the existing optional evaluation. Add an explicit return type to the featureConfig selector callback as well, using FeatureConfigInterface, and keep its variable type aligned with that return type.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@features/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsx`:
- Around line 130-134: Update the explicit type annotations for
filteredTenantAPIResourceCollections and the other filtered collection variable
in their useMemo hooks to include | null, matching the null returned when the
underlying API collections are falsy.
- Around line 185-188: Update the category scope extraction in the flatMap
callback to safely handle missing resource.scopes by using optional chaining
with an empty-array fallback before calling map, while preserving the existing
scope.name results for categories that provide scopes.
- Around line 245-285: Update the permission extraction logic in the current
function to use optional chaining for every collection.apiResources access,
including read, create, update, delete, and write in both actionable and legacy
branches. Preserve the existing fallback behavior for missing category arrays
while preventing errors when collection.apiResources is undefined.
- Around line 106-115: Update the disabledFeatures selector and variable in the
console-role permissions component to explicitly support an undefined selector
result, while preserving the existing optional evaluation. Add an explicit
return type to the featureConfig selector callback as well, using
FeatureConfigInterface, and keep its variable type aligned with that return
type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 100c27d7-c09c-4344-babe-d08d269de331
📒 Files selected for processing (6)
apps/console/src/public/deployment.config.jsonfeatures/admin.console-settings.v1/components/console-roles/console-roles-edit/console-role-permissions.tsxfeatures/admin.console-settings.v1/components/console-roles/create-console-role-wizard/create-console-role-wizard-permissions-form.tsxfeatures/admin.console-settings.v1/utils/get-eligibility-scope-names.tsfeatures/admin.users.v1/components/user-groups-edit.tsxfeatures/admin.users.v1/components/user-profile.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/console/src/public/deployment.config.json
- features/admin.users.v1/components/user-groups-edit.tsx
- features/admin.console-settings.v1/utils/get-eligibility-scope-names.ts
- features/admin.users.v1/components/user-profile.tsx
Purpose
Adds the Console UI for the granular console permission model (Read / Create / Update / Delete per API resource collection), replacing the legacy View / Edit toggle when
consoleSettings.useGranularConsolePermissionsis enabled. Legacy mode is preserved as default — the flag is indisabledFeaturesuntil customers opt in.What changed
ConsoleRoleListenerbackend logic ported to master to expand the persistedconsole:*_view/_edit/_create/_update/_deletefeature scopes back into their full management-scope set on read.useGranularConsolePermissionskey underconsoleSettings.disabledFeaturestoggles between legacy and granular evaluation throughout the UI.UserProfile/UserGroupsEditto honour the granularusers.scopes.deleteandgroups.scopes.updatechecks.getEligibilityScopeNamesandisPermissionLevelActionableto keep checkbox state consistent when multiple levels resolve to the same management scope, and to disable cells whose action carries no real capability.Related Issues
Test plan
useGranularConsolePermissionsindisabledFeatures(default): legacy View / Edit toggle still works for tenant + organization roles.disabledFeaturesand backend[console.console_settings] use_granular_console_permissions = true: the granular grid renders; Read / Create / Update / Delete behave independently.applicationAuthenticationScript).users.scopes.deleteandusers.scopes.updateindependently.