feat: [UIE-8139] - IAM RBAC: add new assigned entities table component part 1 #11588

aaleksee-akamai · 2025-01-30T13:43:24Z

Description 📝

IAM RBAC - new assigned entities table component.

Changes 🔄

List any change(s) relevant to the reviewer.

Table for the assigned entities
Filtering roles

do not include (will be introduced in the next PR):

Any action in the action menu
Redirecting to specific entity page by clicking on the entity name

Target release date 🗓️

2/11/25 (dev)

Preview 📷

Include a screenshot or screen recording of the change.

🔒 Use the Mask Sensitive Data setting for security.

💡 Use <video src="" /> tag when including recordings in table.

Before	After

How to test 🧪

Prerequisites

(How to setup test environment)

Ensure the Identity and Access Beta flag is enabled in dev tools
Ensure the MSW is enabled in dev tools
Click on the any username in the users table and go to the tab Assigned Entities or click on the user's menu View Assigned Entities

Verification steps

(How to verify changes)

Confirm table renders.
Confirm table renders data according to the applied filters

Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

All unit tests are passing
TypeScript compilation succeeded without errors
Code passes all linting rules

aaleksee-akamai · 2025-01-30T13:56:22Z

packages/manager/src/features/IAM/Shared/AssignedRolesTable/AssignedRolesTable.tsx


 export const StyledTypography = styled(Typography, {
  label: 'StyledTypography',
 })(({ theme }) => ({
-  color:


I believe we don't need it, as @jaalah-akamai mentioned earlier

github-actions · 2025-01-30T14:23:53Z

Coverage Report: ✅
Base Coverage: 78.94%
Current Coverage: 78.94%

aaleksee-akamai · 2025-01-30T14:30:26Z

packages/manager/src/features/IAM/Shared/utilities.ts

+  roles: EntitiesRole[] | RoleMap[];
+}
+
+export const getFilteredRoles = (


We have the same functionality in two components (AssignedRolesTable and AssignedEntitiesTable), with the only difference being the searchableFields. So, I’ve combined the logic for both components and passed searchableFields as a parameter

Could we make getSearchableFields an optional field in the options?

Yes, we could make getSearchableFields optional, but for that, we need a default implementation of searchableFields. Right now, this function is only used in two components (that I mentioned above), so making it optional would add flexibility, but it’s worth considering whether it’s necessary at this stage. If we expect to expand its usage further, introducing a default getSearchableFields function could make sense. Otherwise, keeping it explicitly required for now might be a simpler and more controlled approach

Okay. At least we should make it part of the options object, no need to have multiple arguments to this function.

sure, it makes sense, thanks @hkhalil-akamai

aaleksee-akamai · 2025-01-30T14:33:49Z

packages/manager/src/features/IAM/Shared/utilities.ts

+  value?: string;
+}
+
+export const mapEntityTypes = (


Same situation here — the only difference is the ending

hkhalil-akamai · 2025-01-30T21:04:46Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+        <DebouncedSearchTextField
+          clearable
+          debounceTime={250}
+          hideLabel
+          label="Filter"
+          onSearch={setQuery}
+          placeholder="Search"
+          sx={{ marginRight: 2, width: 410 }}
+          value={query}
+        />


Since the search doesn't involve any network requests, do we need to debounce it?

yep, remove it, thanks

hkhalil-akamai · 2025-01-31T16:19:46Z

packages/manager/src/features/IAM/Shared/utilities.ts

+  roles: EntitiesRole[] | RoleMap[];
+}
+
+export const getFilteredRoles = (


Okay. At least we should make it part of the options object, no need to have multiple arguments to this function.

hkhalil-akamai · 2025-01-31T17:16:36Z

Does this project have an API spec yet? I noticed that the return type of API getAccountResources is incorrect as the mocked response is an array of type IamAccountResource[].

Also, the accountResourcesFactory is not written according to convention. It should be defined as a single object of type IamAccountResource, not an array. The .buildList() factory method can be used to produce an array for the mock endpoint.

jaalah-akamai

PR looks really good, nice job

The failing test is just due to the name change View User Roles > View Assigned Roles

jaalah-akamai · 2025-01-31T17:11:31Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+    },
+  ];
+
+  const memoizedTableItems = React.useMemo(() => {


I would remove this useMemo all together. There's nothing in here that would greatly benefit from it. We wouldn't even need to worry about memoizing getFilteredRoles since that's a pure function and memoization is more for equality checks.

@cpathipa suggested using memo for the same logic in mine previous PR. So, there is a question - do we need to memoize this to avoid recalculating filtered roles on every render or not? Or maybe I got it wrong, and it needs to be done in a different way?

@jaalah-akamai , @cpathipa , just circling back on this — would appreciate your thoughts. Thanks!

I originally suggested using useMemo in the previous PR to avoid recalculating on every render. I think we are using map with a map there. The idea was to optimize performance, but if it's not providing significant benefits, I’m open to revisiting that decision.

jaalah-akamai · 2025-01-31T17:17:31Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+      if (resource) {
+        resourceRole.roles.forEach((r: RoleType) => {
+          result.push({
+            id: r + '-' + resourceRole.resource_id,


nit: lets use template strings

jaalah-akamai · 2025-01-31T17:19:44Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+
+      if (resource) {
+        resourceRole.roles.forEach((r: RoleType) => {
+          result.push({


nit: using a map here would a little more straightforward

cpathipa · 2025-01-31T20:05:56Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+    data: assignedRoles,
+    error: assignedRolesError,
+    isLoading: assignedRolesLoading,
+  } = useAccountUserPermissions(username ?? '');


Looks like the current implementation of useAccountUserPermissions does not handle cases where username is undefined or empty, which can lead to unnecessary API calls or API failures.

We might need to update useAccountUserPermissions hook accordingly
example:

export const useAccountUserPermissions = (username?: string) => { return useQuery<IamUserPermissions, APIError[]>( iamQueries.user(username ?? '')._ctx.permissions, { enabled: Boolean(username) } ); };

thanks for noticing that @cpathipa , I've had a look at the code and it seems like a lot of cases are missing that check

Sure! In CM, we generally use the enabled flag in most queries. We can always improve and update the missing ones as we revisit them.

aaleksee-akamai · 2025-02-04T16:15:24Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+
+      if (resource) {
+        result.push(
+          ...resourceRole.roles.map((r: RoleType) => ({


@jaalah-akamai , please check if this is what you mean with using map

@aaleksee-akamai I was more saying that by using a map you don't need to have the results variable or push anything to the array since the map returns an array of the values already. Spreading the results of the map and pushing that into the array is redundant.

const addResourceNamesToRoles = ( assignedRoles: IamUserPermissions, resources: IamAccountResource ): EntitiesRole[] => { const resourcesRoles = assignedRoles.resource_access; const resourcesArray: IamAccountResource[] = Object.values(resources); return resourcesRoles.flatMap((resourceRole: ResourceAccess) => { const resourceByType = resourcesArray.find( (r: IamAccountResource) => r.resource_type === resourceRole.resource_type ); if (resourceByType) { const resource = resourceByType.resources.find( (res: Resource) => res.id === resourceRole.resource_id ); if (resource) { return resourceRole.roles.map((r: RoleType) => ({ id: r + '-' + resourceRole.resource_id, resource_id: resourceRole.resource_id, resource_name: resource.name, resource_type: resourceRole.resource_type, role_name: r, })); } } return []; }); };

aaleksee-akamai · 2025-02-04T16:18:07Z

packages/manager/src/queries/iam/iam.ts

+export const useAccountUserPermissions = (username?: string) => {
+  return useQuery<IamUserPermissions, APIError[]>({
+    ...iamQueries.user(username ?? '')._ctx.permissions,
+    enabled: Boolean(username),


@cpathipa , please check if it's a correct implementation

cpathipa · 2025-02-05T15:55:19Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+    data: assignedRoles,
+    error: assignedRolesError,
+    isLoading: assignedRolesLoading,
+  } = useAccountUserPermissions(username ?? '');


Sure! In CM, we generally use the enabled flag in most queries. We can always improve and update the missing ones as we revisit them.

cpathipa · 2025-02-05T16:32:14Z

packages/manager/src/features/IAM/Shared/AssignedEntitiesTable/AssignedEntitiesTable.tsx

+    },
+  ];
+
+  const memoizedTableItems = React.useMemo(() => {


I originally suggested using useMemo in the previous PR to avoid recalculating on every render. I think we are using map with a map there. The idea was to optimize performance, but if it's not providing significant benefits, I’m open to revisiting that decision.

cpathipa · 2025-02-05T17:12:39Z

packages/manager/src/queries/iam/iam.ts

+export const useAccountUserPermissions = (username?: string) => {
+  return useQuery<IamUserPermissions, APIError[]>({
+    ...iamQueries.user(username ?? '')._ctx.permissions,
+    enabled: Boolean(username),


linode-gh-bot · 2025-02-07T11:41:34Z

Cloud Manager UI test results

🔺 1 failing test on test run #4 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`1 Failing`	`499 Passing`	`2 Skipped`	`119m 33s`

Details

Failing Tests
	Spec	Test
❌	`create-volume.spec.ts`	volume create flow » creates a volume from an existing Linode

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/volumes/create-volume.spec.ts"

aaleksee-akamai requested a review from a team as a code owner January 30, 2025 13:43

aaleksee-akamai requested review from hkhalil-akamai and pmakode-akamai and removed request for a team January 30, 2025 13:43

aaleksee-akamai self-assigned this Jan 30, 2025

aaleksee-akamai added the IAM (Identity & Access Management) label Jan 30, 2025

aaleksee-akamai force-pushed the UIE-8139-iam-rbac-assigned-entities-table-part-1 branch from 1ca7141 to f2f22e7 Compare January 30, 2025 13:45

aaleksee-akamai requested a review from corya-akamai January 30, 2025 13:51

aaleksee-akamai commented Jan 30, 2025

View reviewed changes

cpathipa requested review from cpathipa and removed request for pmakode-akamai January 30, 2025 14:48

jaalah-akamai self-requested a review January 31, 2025 15:29

hkhalil-akamai reviewed Jan 31, 2025

View reviewed changes

jaalah-akamai reviewed Jan 31, 2025

View reviewed changes

cpathipa reviewed Jan 31, 2025

View reviewed changes

aaleksee-akamai force-pushed the UIE-8139-iam-rbac-assigned-entities-table-part-1 branch from f2f22e7 to bda1fdf Compare February 4, 2025 16:12

aaleksee-akamai commented Feb 4, 2025

View reviewed changes

cpathipa approved these changes Feb 5, 2025

View reviewed changes

cpathipa requested review from jaalah-akamai and hkhalil-akamai February 5, 2025 17:19

feat: [UIE-8139] - add new assigned entities table component part 1

0d1b863

aaleksee-akamai force-pushed the UIE-8139-iam-rbac-assigned-entities-table-part-1 branch from bda1fdf to 0d1b863 Compare February 7, 2025 10:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [UIE-8139] - IAM RBAC: add new assigned entities table component part 1 #11588

feat: [UIE-8139] - IAM RBAC: add new assigned entities table component part 1 #11588

aaleksee-akamai commented Jan 30, 2025 •

edited by jaalah-akamai

Loading

aaleksee-akamai Jan 30, 2025

github-actions bot commented Jan 30, 2025 •

edited

Loading

aaleksee-akamai Jan 30, 2025

hkhalil-akamai Jan 30, 2025

aaleksee-akamai Jan 31, 2025

hkhalil-akamai Jan 31, 2025

aaleksee-akamai Feb 4, 2025

aaleksee-akamai Jan 30, 2025

hkhalil-akamai Jan 30, 2025

aaleksee-akamai Feb 4, 2025

hkhalil-akamai Jan 31, 2025

hkhalil-akamai commented Jan 31, 2025 •

edited

Loading

jaalah-akamai left a comment

jaalah-akamai Jan 31, 2025

aaleksee-akamai Feb 3, 2025

aaleksee-akamai Feb 4, 2025

cpathipa Feb 5, 2025

jaalah-akamai Jan 31, 2025

aaleksee-akamai Feb 4, 2025

jaalah-akamai Jan 31, 2025

cpathipa Jan 31, 2025

aaleksee-akamai Feb 4, 2025

cpathipa Feb 5, 2025

aaleksee-akamai Feb 4, 2025

jaalah-akamai Feb 7, 2025

aaleksee-akamai Feb 4, 2025

cpathipa Feb 5, 2025

cpathipa Feb 5, 2025

cpathipa Feb 5, 2025

cpathipa Feb 5, 2025

linode-gh-bot commented Feb 7, 2025

feat: [UIE-8139] - IAM RBAC: add new assigned entities table component part 1 #11588

Are you sure you want to change the base?

feat: [UIE-8139] - IAM RBAC: add new assigned entities table component part 1 #11588

Conversation

aaleksee-akamai commented Jan 30, 2025 • edited by jaalah-akamai Loading

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author, to speed up the review process, I considered 🤔

As an Author, before moving this PR from Draft to Open, I confirmed ✅

Choose a reason for hiding this comment

github-actions bot commented Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hkhalil-akamai commented Jan 31, 2025 • edited Loading

jaalah-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linode-gh-bot commented Feb 7, 2025

Cloud Manager UI test results

🔺 1 failing test on test run #4 ↗︎

Details

Troubleshooting

aaleksee-akamai commented Jan 30, 2025 •

edited by jaalah-akamai

Loading

github-actions bot commented Jan 30, 2025 •

edited

Loading

hkhalil-akamai commented Jan 31, 2025 •

edited

Loading