diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyCreate/FeatureStrategyCreate.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyCreate/FeatureStrategyCreate.tsx index 811e259c3a..39072d93b8 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyCreate/FeatureStrategyCreate.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyCreate/FeatureStrategyCreate.tsx @@ -16,7 +16,6 @@ import { createStrategyPayload, featureStrategyDocsLinkLabel, } from '../FeatureStrategyEdit/FeatureStrategyEdit'; -import { useStrategies } from 'hooks/api/getters/useStrategies/useStrategies'; import { CREATE_FEATURE_STRATEGY } from 'component/providers/AccessProvider/permissions'; import { ISegment } from 'interfaces/segment'; import { useSegmentsApi } from 'hooks/api/actions/useSegmentsApi/useSegmentsApi'; @@ -24,6 +23,7 @@ import { formatStrategyName } from 'utils/strategyNames'; import { useFeatureImmutable } from 'hooks/api/getters/useFeature/useFeatureImmutable'; import { useFormErrors } from 'hooks/useFormErrors'; import { createFeatureStrategy } from 'utils/createFeatureStrategy'; +import { useStrategy } from 'hooks/api/getters/useStrategy/useStrategy'; export const FeatureStrategyCreate = () => { const projectId = useRequiredPathParam('projectId'); @@ -32,7 +32,7 @@ export const FeatureStrategyCreate = () => { const strategyName = useRequiredQueryParam('strategyName'); const [strategy, setStrategy] = useState>({}); const [segments, setSegments] = useState([]); - const { strategies } = useStrategies(); + const { strategyDefinition } = useStrategy(strategyName); const errors = useFormErrors(); const { addStrategyToFeature, loading } = useFeatureStrategyApi(); @@ -47,10 +47,6 @@ export const FeatureStrategyCreate = () => { featureId ); - const strategyDefinition = strategies.find(strategy => { - return strategy.name === strategyName; - }); - useEffect(() => { if (strategyDefinition) { setStrategy(createFeatureStrategy(featureId, strategyDefinition)); diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.test.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.test.tsx index 6104e7a15d..b35a1fc096 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.test.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.test.tsx @@ -1,20 +1,53 @@ import { formatUpdateStrategyApiCode } from 'component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit'; +import { IFeatureStrategy, IStrategy } from 'interfaces/strategy'; test('formatUpdateStrategyApiCode', () => { + const strategy: IFeatureStrategy = { + id: 'a', + name: 'b', + parameters: { + c: 1, + b: 2, + a: 3, + }, + constraints: [], + }; + + const strategyDefinition: IStrategy = { + name: 'c', + displayName: 'd', + description: 'e', + editable: false, + deprecated: false, + parameters: [ + { name: 'a', description: '', type: '', required: false }, + { name: 'b', description: '', type: '', required: false }, + { name: 'c', description: '', type: '', required: false }, + ], + }; + expect( formatUpdateStrategyApiCode( 'projectId', 'featureId', 'environmentId', - { id: 'strategyId' }, + strategy, + strategyDefinition, 'unleashUrl' ) ).toMatchInlineSnapshot(` - "curl --location --request PUT 'unleashUrl/api/admin/projects/projectId/features/featureId/environments/environmentId/strategies/strategyId' \\\\ + "curl --location --request PUT 'unleashUrl/api/admin/projects/projectId/features/featureId/environments/environmentId/strategies/a' \\\\ --header 'Authorization: INSERT_API_KEY' \\\\ --header 'Content-Type: application/json' \\\\ --data-raw '{ - \\"id\\": \\"strategyId\\" + \\"id\\": \\"a\\", + \\"name\\": \\"b\\", + \\"parameters\\": { + \\"a\\": 3, + \\"b\\": 2, + \\"c\\": 1 + }, + \\"constraints\\": [] }'" `); }); diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx index 338db01bf3..6a452c78a1 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx @@ -8,7 +8,11 @@ import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFe import { formatUnknownError } from 'utils/formatUnknownError'; import { useNavigate } from 'react-router-dom'; import useToast from 'hooks/useToast'; -import { IFeatureStrategy, IFeatureStrategyPayload } from 'interfaces/strategy'; +import { + IFeatureStrategy, + IFeatureStrategyPayload, + IStrategy, +} from 'interfaces/strategy'; import { UPDATE_FEATURE_STRATEGY } from 'component/providers/AccessProvider/permissions'; import { ISegment } from 'interfaces/segment'; import { useSegmentsApi } from 'hooks/api/actions/useSegmentsApi/useSegmentsApi'; @@ -16,6 +20,8 @@ import { useSegments } from 'hooks/api/getters/useSegments/useSegments'; import { formatStrategyName } from 'utils/strategyNames'; import { useFeatureImmutable } from 'hooks/api/getters/useFeature/useFeatureImmutable'; import { useFormErrors } from 'hooks/useFormErrors'; +import { useStrategy } from 'hooks/api/getters/useStrategy/useStrategy'; +import { sortStrategyParameters } from 'utils/sortStrategyParameters'; export const FeatureStrategyEdit = () => { const projectId = useRequiredPathParam('projectId'); @@ -27,6 +33,7 @@ export const FeatureStrategyEdit = () => { const [segments, setSegments] = useState([]); const { updateStrategyOnFeature, loading } = useFeatureStrategyApi(); const { setStrategySegments } = useSegmentsApi(); + const { strategyDefinition } = useStrategy(strategy.name); const { setToastData, setToastApiError } = useToast(); const errors = useFormErrors(); const { uiConfig } = useUiConfig(); @@ -85,8 +92,7 @@ export const FeatureStrategyEdit = () => { } }; - // Wait until the strategy has loaded before showing the form. - if (!strategy.id) { + if (!strategy.id || !strategyDefinition) { return null; } @@ -103,6 +109,7 @@ export const FeatureStrategyEdit = () => { featureId, environmentId, strategy, + strategyDefinition, unleashUrl ) } @@ -156,14 +163,25 @@ export const formatUpdateStrategyApiCode = ( featureId: string, environmentId: string, strategy: Partial, + strategyDefinition: IStrategy, unleashUrl?: string ): string => { if (!unleashUrl) { return ''; } + // Sort the strategy parameters payload so that they match + // the order of the input fields in the form, for usability. + const sortedStrategy = { + ...strategy, + parameters: sortStrategyParameters( + strategy.parameters ?? {}, + strategyDefinition + ), + }; + const url = `${unleashUrl}/api/admin/projects/${projectId}/features/${featureId}/environments/${environmentId}/strategies/${strategy.id}`; - const payload = JSON.stringify(strategy, undefined, 2); + const payload = JSON.stringify(sortedStrategy, undefined, 2); return `curl --location --request PUT '${url}' \\ --header 'Authorization: INSERT_API_KEY' \\ diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx index a96b024a7d..0b9eb7b6c2 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx @@ -25,8 +25,8 @@ import PermissionButton from 'component/common/PermissionButton/PermissionButton import { FeatureStrategySegment } from 'component/feature/FeatureStrategy/FeatureStrategySegment/FeatureStrategySegment'; import { ISegment } from 'interfaces/segment'; import { IFormErrors } from 'hooks/useFormErrors'; -import { useStrategies } from 'hooks/api/getters/useStrategies/useStrategies'; import { validateParameterValue } from 'utils/validateParameterValue'; +import { useStrategy } from 'hooks/api/getters/useStrategy/useStrategy'; interface IFeatureStrategyFormProps { feature: IFeatureToggle; @@ -60,13 +60,9 @@ export const FeatureStrategyForm = ({ const hasValidConstraints = useConstraintsValidation(strategy.constraints); const enableProdGuard = useFeatureStrategyProdGuard(feature, environmentId); const { hasAccess } = useContext(AccessContext); - const { strategies } = useStrategies(); + const { strategyDefinition } = useStrategy(strategy?.name); const navigate = useNavigate(); - const strategyDefinition = strategies.find(definition => { - return definition.name === strategy.name; - }); - const { uiConfig, error: uiConfigError, diff --git a/frontend/src/component/strategies/EditStrategy/EditStrategy.tsx b/frontend/src/component/strategies/EditStrategy/EditStrategy.tsx index e5c9e30bc5..12b5d26cff 100644 --- a/frontend/src/component/strategies/EditStrategy/EditStrategy.tsx +++ b/frontend/src/component/strategies/EditStrategy/EditStrategy.tsx @@ -1,3 +1,4 @@ +import React from 'react'; import { useNavigate } from 'react-router-dom'; import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; import useToast from 'hooks/useToast'; @@ -8,7 +9,7 @@ import { UPDATE_STRATEGY } from 'component/providers/AccessProvider/permissions' import useStrategiesApi from 'hooks/api/actions/useStrategiesApi/useStrategiesApi'; import { useStrategies } from 'hooks/api/getters/useStrategies/useStrategies'; import { formatUnknownError } from 'utils/formatUnknownError'; -import useStrategy from 'hooks/api/getters/useStrategy/useStrategy'; +import { useStrategy } from 'hooks/api/getters/useStrategy/useStrategy'; import { UpdateButton } from 'component/common/UpdateButton/UpdateButton'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { GO_BACK } from 'constants/navigate'; @@ -18,7 +19,7 @@ export const EditStrategy = () => { const { uiConfig } = useUiConfig(); const navigate = useNavigate(); const name = useRequiredPathParam('name'); - const { strategy } = useStrategy(name); + const { strategyDefinition } = useStrategy(name); const { strategyName, strategyDesc, @@ -32,9 +33,9 @@ export const EditStrategy = () => { setErrors, errors, } = useStrategyForm( - strategy?.name, - strategy?.description, - strategy?.parameters + strategyDefinition?.name, + strategyDefinition?.description, + strategyDefinition?.parameters ); const { updateStrategy, loading } = useStrategiesApi(); const { refetchStrategies } = useStrategies(); diff --git a/frontend/src/hooks/api/getters/useStrategy/defaultStrategy.ts b/frontend/src/hooks/api/getters/useStrategy/defaultStrategy.ts deleted file mode 100644 index a89ba584af..0000000000 --- a/frontend/src/hooks/api/getters/useStrategy/defaultStrategy.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { IStrategy } from 'interfaces/strategy'; - -export const defaultStrategy: IStrategy = { - name: '', - description: '', - displayName: '', - editable: false, - deprecated: false, - parameters: [], -}; diff --git a/frontend/src/hooks/api/getters/useStrategy/useStrategy.ts b/frontend/src/hooks/api/getters/useStrategy/useStrategy.ts index 243b50a857..5097323f63 100644 --- a/frontend/src/hooks/api/getters/useStrategy/useStrategy.ts +++ b/frontend/src/hooks/api/getters/useStrategy/useStrategy.ts @@ -1,30 +1,40 @@ -import useSWR, { mutate, SWRConfiguration } from 'swr'; +import useSWR from 'swr'; +import { useCallback } from 'react'; import { formatApiPath } from 'utils/formatPath'; +import { IStrategy } from 'interfaces/strategy'; import handleErrorResponses from '../httpErrorResponseHandler'; -import { defaultStrategy } from './defaultStrategy'; -const useStrategy = (strategyName: string, options: SWRConfiguration = {}) => { - const STRATEGY_CACHE_KEY = `api/admin/strategies/${strategyName}`; - const path = formatApiPath(STRATEGY_CACHE_KEY); +interface IUseStrategyOutput { + strategyDefinition?: IStrategy; + refetchStrategy: () => void; + loading: boolean; + error?: Error; +} - const fetcher = () => { - return fetch(path) - .then(handleErrorResponses(`${strategyName} strategy`)) - .then(res => res.json()); - }; +export const useStrategy = ( + strategyName: string | undefined +): IUseStrategyOutput => { + const { data, error, mutate } = useSWR( + strategyName + ? formatApiPath(`api/admin/strategies/${strategyName}`) + : null, // Don't fetch until we have a strategyName. + fetcher + ); - const { data, error } = useSWR(STRATEGY_CACHE_KEY, fetcher, options); - - const refetchStrategy = () => { - mutate(STRATEGY_CACHE_KEY); - }; + const refetchStrategy = useCallback(() => { + mutate().catch(console.warn); + }, [mutate]); return { - strategy: data || defaultStrategy, - error, - loading: !error && !data, + strategyDefinition: data, refetchStrategy, + loading: !error && !data, + error, }; }; -export default useStrategy; +const fetcher = (path: string): Promise => { + return fetch(path) + .then(handleErrorResponses('Strategy')) + .then(res => res.json()); +}; diff --git a/frontend/src/utils/sortStrategyParameters.test.ts b/frontend/src/utils/sortStrategyParameters.test.ts new file mode 100644 index 0000000000..323b9f2e62 --- /dev/null +++ b/frontend/src/utils/sortStrategyParameters.test.ts @@ -0,0 +1,29 @@ +import { sortStrategyParameters } from 'utils/sortStrategyParameters'; + +test('sortStrategyParameters', () => { + expect( + sortStrategyParameters( + { + c: 1, + b: 2, + a: 3, + }, + { + name: '', + displayName: '', + description: '', + editable: false, + deprecated: false, + parameters: [ + { name: 'a', description: '', type: '', required: false }, + { name: 'b', description: '', type: '', required: false }, + { name: 'c', description: '', type: '', required: false }, + ], + } + ) + ).toEqual({ + a: 3, + b: 2, + c: 1, + }); +}); diff --git a/frontend/src/utils/sortStrategyParameters.ts b/frontend/src/utils/sortStrategyParameters.ts new file mode 100644 index 0000000000..702032a44f --- /dev/null +++ b/frontend/src/utils/sortStrategyParameters.ts @@ -0,0 +1,24 @@ +import { + IFeatureStrategyParameters, + IStrategy, + IFeatureStrategy, +} from 'interfaces/strategy'; + +// Sort the keys in a parameters payload object by the +// order of the parameters in the strategy definition. +export const sortStrategyParameters = ( + parameters: IFeatureStrategyParameters, + strategyDefinition: IStrategy +): Partial => { + const sortedParameterNames = strategyDefinition.parameters.map( + parameter => parameter.name + ); + + return Object.fromEntries( + Object.entries(parameters).sort( + (a, b) => + sortedParameterNames.indexOf(a[0]) - + sortedParameterNames.indexOf(b[0]) + ) + ); +};