-
Notifications
You must be signed in to change notification settings - Fork 429
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
feat: Clone identities (FE) #3725
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
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 project
frontend/common/types/responses.ts
Outdated
@@ -287,6 +287,7 @@ export type IdentityFeatureState = { | |||
enabled: boolean | |||
feature_state_value: FlagsmithValue | |||
segment: null | |||
overridden_by: string |
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.
I'm guessing this is string|null
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.
Corrected
return openConfirm({ | ||
body: ( | ||
<div> | ||
{'Are you sure you want to clone all the feature states from '} |
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.
Does cloning clone overrides and traits?
If so I think that might read better
"Cloning an identity will copy any traits and identity overrides. Are you sure?"
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 don't clone the traits.
Maybe could be: "Cloning {identityA} will copy any identity overrides in {identityB}. Are you sure?"
toast('Clonation Completed!') | ||
}) | ||
}, | ||
title: 'Clone the values', |
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.
Clone identity might be better here ? Not clear what "The values" means
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.
Changed
}} | ||
className='ms-2 me-2' | ||
> | ||
{'Clone features states'} |
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.
Clone identity overrides I think?
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.
This text was updated.
</Button> | ||
} | ||
> | ||
{`Clone the features states from ${leftId?.label} to ${rightId?.label}`} |
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.
Same here
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.
this text was updated.
@@ -120,6 +127,38 @@ const CompareIdentities: FC<CompareIdentitiesType> = ({ | |||
) | |||
} | |||
|
|||
const clonedentityValues = ( |
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.
cloneIdentityValues
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.
Good catch, was corrected
Thanks for submitting a PR! Please check the boxes below:
pre-commit
to check lintingdocs/
if required so people know about the feature!Changes
Please describe.