-
Notifications
You must be signed in to change notification settings - Fork 367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upcoming: [M3-7455] - Add child account access column and disable button when account has child accounts #10012
upcoming: [M3-7455] - Add child account access column and disable button when account has child accounts #10012
Conversation
|
||
import CloseAccountDialog from './CloseAccountDialog'; | ||
|
||
const CloseAccountSetting = () => { | ||
const [dialogOpen, setDialogOpen] = React.useState<boolean>(false); | ||
|
||
const { data: childAccounts } = useChildAccounts({}); | ||
const flags = useFlags(); | ||
const closeAccountDisabled = flags.parentChildAccountAccess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the current account has child accounts, disable the 'Close Account' button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to simplify this:
const closeAccountDisabled = flags.parentChildAccountAccess && Boolean(childAccounts?.data?.length);
return ( | ||
<> | ||
<Accordion defaultExpanded={true} heading="Close Account"> | ||
<Grid container direction="column"> | ||
<Grid> | ||
<Button buttonType="outlined" onClick={() => setDialogOpen(true)}> | ||
{closeAccountDisabled && ( | ||
<Notice spacingBottom={20} variant="info"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a good call. Exact copy TBD - I know you reached out to Magda; we can be sure to get a response from her or Jan on that, whether it's finalized in this ticket or a follow up for copy changes.
…d-access-column-to-users-table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Just some minor things here. If there's any outstanding feedback you don't get to, I can pick this up.
✅ Unit tests are passing 🎉
✅ Ensured there is an additional 'Child Account Access' column with accurate labels ('Enabled' and 'Disabled')
✅ Ensured the Close Account button is disabled since the current account has a child account, and ensure there is a notice explaining why it is disabled.
✅ Ensured the Close Account button is enabled if the current account does not have any child accounts.
Ensure the parent/child feature flag works as expected
- See the comment in UserRow.tsx for a crash fix.
return ( | ||
<> | ||
<Accordion defaultExpanded={true} heading="Close Account"> | ||
<Grid container direction="column"> | ||
<Grid> | ||
<Button buttonType="outlined" onClick={() => setDialogOpen(true)}> | ||
{closeAccountDisabled && ( | ||
<Notice spacingBottom={20} variant="info"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a good call. Exact copy TBD - I know you reached out to Magda; we can be sure to get a response from her or Jan on that, whether it's finalized in this ticket or a follow up for copy changes.
packages/manager/src/features/Account/CloseAccountSetting.test.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Account/CloseAccountSetting.test.tsx
Outdated
Show resolved
Hide resolved
|
||
import CloseAccountDialog from './CloseAccountDialog'; | ||
|
||
const CloseAccountSetting = () => { | ||
const [dialogOpen, setDialogOpen] = React.useState<boolean>(false); | ||
|
||
const { data: childAccounts } = useChildAccounts({}); | ||
const flags = useFlags(); | ||
const closeAccountDisabled = flags.parentChildAccountAccess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to simplify this:
const closeAccountDisabled = flags.parentChildAccountAccess && Boolean(childAccounts?.data?.length);
@@ -36,6 +40,11 @@ export const UserRow = ({ onDelete, user }: Props) => { | |||
<TableCell>{user.email}</TableCell> | |||
</Hidden> | |||
<TableCell>{user.restricted ? 'Limited' : 'Full'}</TableCell> | |||
{flags.parentChildAccountAccess && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should provide a better check here as opposed to just relying on the feature flag, since that will disappear in the future and this entire column will be conditionally rendered.
Coverage Report: ✅ |
Description 📝
Add child account access column and disable button when account has child accounts.
Changes 🔄
List any change relevant to the reviewer.
Preview 📷
How to test 🧪
Prerequisites
Verification steps
As an Author I have considered 🤔
Check all that apply