Skip to content
This repository has been archived by the owner on Oct 27, 2022. It is now read-only.

Commit

Permalink
refactor: clean up strategy parameter types (#944)
Browse files Browse the repository at this point in the history
* refactor: fix splash page button background color

* refactor: regenerate OpenAPI client

* refactor: clean up strategy parameter types

* refactor: remove index signature from IConstraint

* refactor: fix never-seen status in features list
  • Loading branch information
olav authored May 4, 2022
1 parent 31d32d2 commit ec3a76a
Show file tree
Hide file tree
Showing 42 changed files with 1,152 additions and 238 deletions.
4 changes: 2 additions & 2 deletions cypress/integration/feature/feature.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('feature', () => {
expect(req.body.name).to.equal('flexibleRollout');
expect(req.body.parameters.groupId).to.equal(featureToggleName);
expect(req.body.parameters.stickiness).to.equal('default');
expect(req.body.parameters.rollout).to.equal(100);
expect(req.body.parameters.rollout).to.equal('100');

if (ENTERPRISE) {
expect(req.body.constraints.length).to.equal(1);
Expand Down Expand Up @@ -159,7 +159,7 @@ describe('feature', () => {
req => {
expect(req.body.parameters.groupId).to.equal('new-group-id');
expect(req.body.parameters.stickiness).to.equal('sessionId');
expect(req.body.parameters.rollout).to.equal(100);
expect(req.body.parameters.rollout).to.equal('100');

if (ENTERPRISE) {
expect(req.body.constraints.length).to.equal(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Input from 'component/common/Input/Input';
import StringTruncator from 'component/common/StringTruncator/StringTruncator';
import React, { useState } from 'react';
import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader';
import { parseParameterStrings } from 'utils/parseParameter';

interface IFreeTextInputProps {
values: string[];
Expand Down Expand Up @@ -74,17 +75,9 @@ export const FreeTextInput = ({
return;
}
setError('');

if (inputValues.includes(',')) {
const newValues = inputValues
.split(',')
.filter(values => values)
.map(value => value.trim());
setValues(uniqueValues([...values, ...newValues]));
} else {
setValues(uniqueValues([...values, inputValues.trim()]));
}

setValues(
uniqueValues([...values, ...parseParameterStrings(inputValues)])
);
setInputValues('');
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFe
import { formatUnknownError } from 'utils/formatUnknownError';
import { useHistory } from 'react-router-dom';
import useToast from 'hooks/useToast';
import { IFeatureStrategy, IStrategyPayload } from 'interfaces/strategy';
import { IFeatureStrategy, IFeatureStrategyPayload } 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';
Expand Down Expand Up @@ -122,7 +122,7 @@ export const FeatureStrategyEdit = () => {

export const createStrategyPayload = (
strategy: Partial<IFeatureStrategy>
): IStrategyPayload => {
): IFeatureStrategyPayload => {
return {
name: strategy.name,
constraints: strategy.constraints ?? [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const FeatureStrategyType = ({
return definition.name === strategy.name;
});

const updateParameter = (field: string, value: unknown) => {
const updateParameter = (field: string, value: string) => {
setStrategy(
produce(draft => {
draft.parameters = draft.parameters ?? {};
Expand All @@ -48,15 +48,15 @@ export const FeatureStrategyType = ({
return (
<FlexibleStrategy
context={context}
parameters={strategy.parameters ?? []}
parameters={strategy.parameters ?? {}}
updateParameter={updateParameter}
editable={hasAccess}
/>
);
case 'userWithId':
return (
<UserWithIdStrategy
parameters={strategy.parameters ?? []}
parameters={strategy.parameters ?? {}}
updateParameter={updateParameter}
editable={hasAccess}
/>
Expand All @@ -65,7 +65,7 @@ export const FeatureStrategyType = ({
return (
<GeneralStrategy
strategyDefinition={strategyDefinition}
parameters={strategy.parameters ?? []}
parameters={strategy.parameters ?? {}}
updateParameter={updateParameter}
editable={hasAccess}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,9 @@ export const FeatureToggleListItem = memo<IFeatureToggleListItemProps>(
className={classnames(styles.listItem, className)}
>
<span className={styles.listItemMetric}>
<ConditionallyRender
condition={Boolean(lastSeenAt)}
show={() => (
<FeatureStatus
lastSeenAt={lastSeenAt as Date}
tooltipPlacement="left"
/>
)}
<FeatureStatus
lastSeenAt={lastSeenAt}
tooltipPlacement="left"
/>
</span>
<span
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { Fragment } from 'react';
import { IConstraint, IFeatureStrategy, IParameter } from 'interfaces/strategy';
import {
IFeatureStrategy,
IFeatureStrategyParameters,
IConstraint,
} from 'interfaces/strategy';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import PercentageCircle from 'component/common/PercentageCircle/PercentageCircle';
import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator';
Expand All @@ -10,9 +14,14 @@ import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
import { FeatureOverviewSegment } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment';
import { ConstraintAccordionList } from 'component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
import { useStyles } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewExecution/FeatureOverviewExecution.styles';
import {
parseParameterString,
parseParameterNumber,
parseParameterStrings,
} from 'utils/parseParameter';

interface IFeatureOverviewExecutionProps {
parameters: IParameter;
parameters: IFeatureStrategyParameters;
constraints?: IConstraint[];
strategy: IFeatureStrategy;
percentageFill?: string;
Expand Down Expand Up @@ -52,15 +61,16 @@ const FeatureOverviewExecution = ({
is included.
</p>

<PercentageCircle percentage={parameters[key]} />
<PercentageCircle
percentage={parseParameterNumber(
parameters[key]
)}
/>
</Fragment>
);
case 'userIds':
case 'UserIds':
const users = parameters[key]
.split(',')
.filter((userId: string) => userId);

const users = parseParameterStrings(parameters[key]);
return (
<FeatureOverviewExecutionChips
key={key}
Expand All @@ -70,10 +80,7 @@ const FeatureOverviewExecution = ({
);
case 'hostNames':
case 'HostNames':
const hosts = parameters[key]
.split(',')
.filter((hosts: string) => hosts);

const hosts = parseParameterStrings(parameters[key]);
return (
<FeatureOverviewExecutionChips
key={key}
Expand All @@ -82,10 +89,7 @@ const FeatureOverviewExecution = ({
/>
);
case 'IPs':
const IPs = parameters[key]
.split(',')
.filter((hosts: string) => hosts);

const IPs = parseParameterStrings(parameters[key]);
return (
<FeatureOverviewExecutionChips
key={key}
Expand All @@ -108,10 +112,9 @@ const FeatureOverviewExecution = ({
const notLastItem = index !== definition?.parameters?.length - 1;
switch (param?.type) {
case 'list':
const values = strategy?.parameters[param.name]
.split(',')
.filter((val: string) => val);

const values = parseParameterStrings(
strategy?.parameters[param.name]
);
return (
<Fragment key={param?.name}>
<FeatureOverviewExecutionChips
Expand All @@ -134,9 +137,10 @@ const FeatureOverviewExecution = ({
: ''}{' '}
is included.
</p>

<PercentageCircle
percentage={strategy.parameters[param.name]}
percentage={parseParameterNumber(
strategy.parameters[param.name]
)}
/>
<ConditionallyRender
condition={notLastItem}
Expand All @@ -156,7 +160,10 @@ const FeatureOverviewExecution = ({
{strategy.parameters[param.name]}
</p>
<ConditionallyRender
condition={strategy.parameters[param.name]}
condition={
typeof strategy.parameters[param.name] !==
'undefined'
}
show={
<ConditionallyRender
condition={notLastItem}
Expand All @@ -167,10 +174,15 @@ const FeatureOverviewExecution = ({
</Fragment>
);
case 'string':
const value = strategy.parameters[param.name];
const value = parseParameterString(
strategy.parameters[param.name]
);
return (
<ConditionallyRender
condition={value !== undefined}
condition={
typeof strategy.parameters[param.name] !==
'undefined'
}
key={param.name}
show={
<>
Expand Down Expand Up @@ -198,7 +210,9 @@ const FeatureOverviewExecution = ({
/>
);
case 'number':
const number = strategy.parameters[param.name];
const number = parseParameterNumber(
strategy.parameters[param.name]
);
return (
<ConditionallyRender
condition={number !== undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function getColor(unit?: string): string {
}

interface IFeatureStatusProps {
lastSeenAt?: string | Date;
lastSeenAt?: string | Date | null;
tooltipPlacement?: TooltipProps['placement'];
}

Expand Down Expand Up @@ -75,7 +75,7 @@ const FeatureStatus = ({
return (
<ConditionallyRender
condition={Boolean(lastSeenAt)}
show={
show={() => (
<TimeAgo
date={lastSeenAt!}
title=""
Expand All @@ -98,7 +98,7 @@ const FeatureStatus = ({
);
}}
/>
}
)}
elseShow={
<Wrapper
toolTip="No usage reported from connected applications"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Typography } from '@mui/material';
import { IParameter } from 'interfaces/strategy';
import { IFeatureStrategyParameters } from 'interfaces/strategy';
import RolloutSlider from '../RolloutSlider/RolloutSlider';
import Select from 'component/common/select';
import React from 'react';
Expand All @@ -9,6 +9,10 @@ import {
FLEXIBLE_STRATEGY_STICKINESS_ID,
} from 'utils/testIds';
import { HelpIcon } from 'component/common/HelpIcon/HelpIcon';
import {
parseParameterNumber,
parseParameterString,
} from 'utils/parseParameter';

const builtInStickinessOptions = [
{ key: 'default', label: 'default' },
Expand All @@ -18,8 +22,8 @@ const builtInStickinessOptions = [
];

interface IFlexibleStrategyProps {
parameters: IParameter;
updateParameter: (field: string, value: any) => void;
parameters: IFeatureStrategyParameters;
updateParameter: (field: string, value: string) => void;
context: any;
editable: boolean;
}
Expand Down Expand Up @@ -54,15 +58,15 @@ const FlexibleStrategy = ({
const stickinessOptions = resolveStickiness();

const rollout =
parameters.rollout !== undefined ? parameters.rollout : '100';
const stickiness = parameters.stickiness;
const groupId = parameters.groupId;
parameters.rollout !== undefined
? parseParameterNumber(parameters.rollout)
: 100;

return (
<div>
<RolloutSlider
name="Rollout"
value={parseInt(rollout)}
value={rollout}
disabled={!editable}
onChange={updateRollout}
/>
Expand All @@ -86,7 +90,7 @@ const FlexibleStrategy = ({
name="stickiness"
label="Stickiness"
options={stickinessOptions}
value={stickiness}
value={parseParameterString(parameters.stickiness)}
disabled={!editable}
data-testid={FLEXIBLE_STRATEGY_STICKINESS_ID}
onChange={e => onUpdate('stickiness')(e.target.value)}
Expand All @@ -109,7 +113,7 @@ const FlexibleStrategy = ({
<Input
label="groupId"
id="groupId-input"
value={groupId || ''}
value={parseParameterString(parameters.groupId)}
disabled={!editable}
onChange={e => onUpdate('groupId')(e.target.value)}
data-testid={FLEXIBLE_STRATEGY_GROUP_ID}
Expand Down
Loading

0 comments on commit ec3a76a

Please sign in to comment.