feat: [UIE-9358] - IAM Parent/Child: Child Account - Default Entity Access#12993
Conversation
aaleksee-akamai
added
the
IAM Delegation
| location.pathname.includes('/iam/roles/defaults')) ?? | ||
| false, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
@mpolotsk-akamai just FYI - this is the same approached we had discussed during pair programming
You will probably need this hook for your own PR (with some additional path logic)
There was a problem hiding this comment.
that /iam/roles/defaults path includes both tabs, but something else might be needed here
abailly-akamai
left a comment
abailly-akamai
left a comment
There was a problem hiding this comment.
PR looks great and testing with mock data worked well enough (tho with entity seeds it seems like we don't get all the data in the entities table but i think it just has to do with the delegation handler which you fixed to use relevant IDs, only for two entities)
Validated UI, data flows and logic ✅
| currentRoute: '/iam/users/$username/entities', | ||
| currentRoute: isDefaultDelegationRolesForChildAccount | ||
| ? '/iam/roles/defaults/entity-access' | ||
| : `/iam/users/$username/entities`, |
There was a problem hiding this comment.
No change needed, but eventually we may just allow location.pathname here.. It was done this way because location.pathname isn't type strict as the other router utils, but it does make sense to have it set as such
| assignedUserRolesLoading, | ||
| delegateDefaultRolesLoading, | ||
| ] | ||
| ); |
There was a problem hiding this comment.
Could we try to simplify?
const loading = assignedUserRolesLoading || assignedUserRolesLoadingseems like it would work the same - same with the error above
There was a problem hiding this comment.
we still need to check isDefaultDelegationRolesForChildAccount, or you're saying that useMemo doesn't play a huge role there?
There was a problem hiding this comment.
Both: useMemo doesn't play a role here since this value is always changing, and depending what mode you are on one will be a boolean and one will be undefined. I don't think we need the isDefaultDelegationRolesForChildAccount at all from what I can read
There was a problem hiding this comment.
turns out that it won't be undefined: loading'd be false and error'd be null. So I think we need to check for the mode
Cloud Manager UI test results🔺 2 failing tests on test run #5 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/placementGroups/create-linode-with-placement-groups.spec.ts,cypress/e2e/core/linodes/clone-linode.spec.ts" |
||||||||||||||||||||
mpolotsk-akamai
left a comment
mpolotsk-akamai
left a comment
There was a problem hiding this comment.
LGTM, Default Entity Access table looks good; role updates and removals work as expected. ✅
…ccess (linode#12993) * feat: [UIE-9358] - IAM Parent/Child: Child Account - Default Entity Access * move logic to the hook * fix assigned default roles * test and filter out assigned roles * remove useMemo
4 participants
Description 📝
This PR adds the Assigned entity table for Default Access
Changes 🔄
List any change(s) relevant to the reviewer.
Scope 🚢
Upon production release, changes in this PR will be visible to:
Target release date 🗓️
11/04
Preview 📷
How to test 🧪
Prerequisites
(How to setup test environment)
Verification steps
(How to verify changes)
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
As an Author, before moving this PR from Draft to Open, I confirmed ✅