Skip to content

Commit

Permalink
feat: variant name change on create (#5742)
Browse files Browse the repository at this point in the history
This PR refactores the StrategyVariants component to be passed in from
the outside to the new form component. This allows us to pass in the
StrategyVariants with an "editable" property in the create form which we
use to determine the editable state of the name input field. If the
editable field is not passed in we keep the old behavior.

Notable changes:
* StrategyVariants is now passed in from the outside, allowing us to
define different props at call time
* Added tests for the new behavior, and for keeping the old behavior
(such as in edit strategy)
* Added tracking
  • Loading branch information
FredrikOseberg authored Jan 2, 2024
1 parent f53204c commit 049c5b9
Show file tree
Hide file tree
Showing 11 changed files with 342 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { useSegments } from 'hooks/api/getters/useSegments/useSegments';
import { useUiFlag } from 'hooks/useUiFlag';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import { NewFeatureStrategyForm } from 'component/feature/FeatureStrategy/FeatureStrategyForm/NewFeatureStrategyForm';
import { StrategyVariants } from 'component/feature/StrategyTypes/StrategyVariants';

interface IEditChangeProps {
change: IChangeRequestAddStrategy | IChangeRequestUpdateStrategy;
Expand Down Expand Up @@ -172,6 +173,14 @@ export const EditChange = ({
)}
tab={tab}
setTab={setTab}
StrategyVariants={
<StrategyVariants
strategy={strategy}
setStrategy={setStrategy}
environment={environment}
projectId={projectId}
/>
}
/>
}
elseShow={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { useSegments } from 'hooks/api/getters/useSegments/useSegments';
import { useUiFlag } from 'hooks/useUiFlag';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import { NewFeatureStrategyForm } from 'component/feature/FeatureStrategy/FeatureStrategyForm/NewFeatureStrategyForm';
import { StrategyVariants } from 'component/feature/StrategyTypes/StrategyVariants';

interface IEditChangeProps {
change: IChangeRequestAddStrategy | IChangeRequestUpdateStrategy;
Expand Down Expand Up @@ -172,6 +173,14 @@ export const NewEditChange = ({
)}
tab={tab}
setTab={setTab}
StrategyVariants={
<StrategyVariants
strategy={strategy}
setStrategy={setStrategy}
environment={environment}
projectId={projectId}
/>
}
/>
}
elseShow={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ export const ConstraintList = forwardRef<
<StyledContainer id={constraintAccordionListId}>
{constraints.map((constraint, index) => (
<Fragment key={objectId(constraint)}>
{console.log(state.get(constraint))}
<ConditionallyRender
condition={index > 0}
show={<StrategySeparator text='AND' />}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import { useNavigate } from 'react-router-dom';
import {
Alert,
Expand Down Expand Up @@ -40,13 +40,10 @@ import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequ
import { useHasProjectEnvironmentAccess } from 'hooks/useHasAccess';
import { FeatureStrategyTitle } from './FeatureStrategyTitle/FeatureStrategyTitle';
import { FeatureStrategyEnabledDisabled } from './FeatureStrategyEnabledDisabled/FeatureStrategyEnabledDisabled';
import { StrategyVariants } from 'component/feature/StrategyTypes/StrategyVariants';
import { usePlausibleTracker } from 'hooks/usePlausibleTracker';
import { formatStrategyName } from 'utils/strategyNames';
import { Badge } from 'component/common/Badge/Badge';
import EnvironmentIcon from 'component/common/EnvironmentIcon/EnvironmentIcon';
import { useProjectEnvironments } from 'hooks/api/getters/useProjectEnvironments/useProjectEnvironments';
import { useFeature } from 'hooks/api/getters/useFeature/useFeature';

interface IFeatureStrategyFormProps {
feature: IFeatureToggle;
Expand All @@ -66,6 +63,7 @@ interface IFeatureStrategyFormProps {
errors: IFormErrors;
tab: number;
setTab: React.Dispatch<React.SetStateAction<number>>;
StrategyVariants: JSX.Element;
}

const StyledDividerContent = styled(Box)(({ theme }) => ({
Expand Down Expand Up @@ -185,6 +183,7 @@ export const NewFeatureStrategyForm = ({
isChangeRequest,
tab,
setTab,
StrategyVariants,
}: IFeatureStrategyFormProps) => {
const { trackEvent } = usePlausibleTracker();
const [showProdGuard, setShowProdGuard] = useState(false);
Expand All @@ -197,6 +196,14 @@ export const NewFeatureStrategyForm = ({
);
const { strategyDefinition } = useStrategy(strategy?.name);

useEffect(() => {
trackEvent('new-strategy-form', {
props: {
eventType: 'seen',
},
});
});

const foundEnvironment = feature.environments.find(
(environment) => environment.name === environmentId,
);
Expand Down Expand Up @@ -271,6 +278,12 @@ export const NewFeatureStrategyForm = ({
return;
}

trackEvent('new-strategy-form', {
props: {
eventType: 'submitted',
},
});

if (enableProdGuard && !isChangeRequest) {
setShowProdGuard(true);
} else {
Expand Down Expand Up @@ -440,14 +453,7 @@ export const NewFeatureStrategyForm = ({
strategy.parameters != null &&
'stickiness' in strategy.parameters
}
show={
<StrategyVariants
strategy={strategy}
setStrategy={setStrategy}
environment={environmentId}
projectId={projectId}
/>
}
show={StrategyVariants}
/>
}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,92 +9,17 @@ import {
UPDATE_FEATURE_STRATEGY,
} from 'component/providers/AccessProvider/permissions';
import { NewFeatureStrategyCreate } from './NewFeatureStrategyCreate';
import { testServerRoute, testServerSetup } from 'utils/testServer';

const server = testServerSetup();
import {
setupProjectEndpoint,
setupSegmentsEndpoint,
setupStrategyEndpoint,
setupFeaturesEndpoint,
setupUiConfigEndpoint,
setupContextEndpoint,
} from './featureStrategyFormTestSetup';

const featureName = 'my-new-feature';

const setupProjectEndpoint = () => {
testServerRoute(server, '/api/admin/projects/default', {
environments: [
{
name: 'development',
enabled: true,
type: 'development',
},
],
});
};

const setupSegmentsEndpoint = () => {
testServerRoute(server, '/api/admin/segments', {
segments: [
{
name: 'test',
constraints: [],
},
],
});
};

const setupStrategyEndpoint = () => {
testServerRoute(server, '/api/admin/strategies/flexibleRollout', {
displayName: 'Gradual rollout',
name: 'flexibleRollout',
parameters: [
{
name: 'rollout',
},
{
name: 'stickiness',
},
{
name: 'groupId',
},
],
});
};

const setupFeaturesEndpoint = () => {
testServerRoute(
server,
`/api/admin/projects/default/features/${featureName}`,
{
environments: [
{
name: 'development',
type: 'development',
},
],
name: featureName,
project: 'default',
},
);
};

const setupUiConfigEndpoint = () => {
testServerRoute(server, '/api/admin/ui-config', {
versionInfo: {
current: {
enterprise: '5.7.0-main',
},
},
environment: 'enterprise',
flags: {
newStrategyConfiguration: true,
},
});
};

const setupContextEndpoint = () => {
testServerRoute(server, '/api/admin/context', [
{
name: 'appName',
},
]);
};

const setupComponent = () => {
return {
wrapper: render(
Expand Down Expand Up @@ -139,7 +64,7 @@ beforeEach(() => {
setupProjectEndpoint();
setupSegmentsEndpoint();
setupStrategyEndpoint();
setupFeaturesEndpoint();
setupFeaturesEndpoint(featureName);
setupUiConfigEndpoint();
setupContextEndpoint();
});
Expand Down Expand Up @@ -270,4 +195,39 @@ describe('NewFeatureStrategyCreate', () => {

expect(screen.getByText(expectedVariantName)).toBeInTheDocument();
});

test('should change variant name after changing tab', async () => {
const { expectedVariantName } = setupComponent();

await waitFor(() => {
expect(screen.getByText('Gradual rollout')).toBeInTheDocument();
});

const variantsEl = screen.getByText('Variants');
fireEvent.click(variantsEl);

await waitFor(() => {
const addVariantEl = screen.getByText('Add variant');
fireEvent.click(addVariantEl);
});

await waitFor(() => {
const targetingEl = screen.getByText('Targeting');
fireEvent.click(targetingEl);
});

await waitFor(() => {
const addConstraintEl = screen.getByText('Add constraint');
expect(addConstraintEl).toBeInTheDocument();
});

fireEvent.click(variantsEl);

const inputElement = screen.getAllByRole('textbox')[0];
fireEvent.change(inputElement, {
target: { value: expectedVariantName },
});

expect(screen.getByText(expectedVariantName)).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import useQueryParams from 'hooks/useQueryParams';
import { useSegments } from 'hooks/api/getters/useSegments/useSegments';
import { useDefaultStrategy } from '../../../project/Project/ProjectSettings/ProjectDefaultStrategySettings/ProjectEnvironment/ProjectEnvironmentDefaultStrategy/EditDefaultStrategy';
import { NewFeatureStrategyForm } from 'component/feature/FeatureStrategy/FeatureStrategyForm/NewFeatureStrategyForm';
import { StrategyVariants } from 'component/feature/StrategyTypes/StrategyVariants';

export const NewFeatureStrategyCreate = () => {
const [tab, setTab] = useState(0);
Expand Down Expand Up @@ -209,6 +210,15 @@ export const NewFeatureStrategyCreate = () => {
isChangeRequest={isChangeRequestConfigured(environmentId)}
tab={tab}
setTab={setTab}
StrategyVariants={
<StrategyVariants
strategy={strategy}
setStrategy={setStrategy}
environment={environmentId}
projectId={projectId}
editable
/>
}
/>
{staleDataNotification}
</FormTemplate>
Expand Down
Loading

0 comments on commit 049c5b9

Please sign in to comment.