Skip to content
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: variant with number payload #4654

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Conversation

andreas-unleash
Copy link
Contributor

Adds number as possible payload type for variant.
Adds a flag to enable the feature
Updates all relevant models and schemas
Adds the option to the UI

Closes: # 1-1357

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
@vercel
Copy link

vercel bot commented Sep 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 11, 2023 11:53am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Sep 11, 2023 11:53am

@@ -19,6 +19,7 @@ import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashCon
import { WeightType } from 'constants/variantTypes';
import { IFeatureVariantEdit } from '../EnvironmentVariantsModal';
import { Delete } from '@mui/icons-material';
import useUiConfig from '../../../../../../../hooks/api/getters/useUiConfig/useUiConfig';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dots

@@ -195,6 +197,16 @@ export const VariantForm = ({

const [errors, setErrors] = useState<IVariantFormErrors>({});

const shouldAddTypeNumberToOptions = Boolean(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call it after flag: const variantTypeNumber

uiConfig.flags.variantTypeNumber
);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming it's because the flag hasn't loaded yet, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

payloadOptions is defined before the component - we need to load the flag and determine whether to add the option to the list

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Copy link
Contributor

@kwasniew kwasniew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also check with the strategy variants

@kwasniew
Copy link
Contributor

If you'd like to write automated tests for variants I added some in StrategyVariants.test.tsx

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
@andreas-unleash andreas-unleash merged commit 1cd0edb into main Sep 11, 2023
16 checks passed
@andreas-unleash andreas-unleash deleted the feat/variant_type_number branch September 11, 2023 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants