From 31ed96d8f383e6d890a253fd3017b1ecd7a0724d Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Thu, 7 Sep 2023 08:18:18 +0200 Subject: [PATCH] 1-1320: allow you to update example with no pattern + update state better (#4623) While having a pattern when you have no example doesn't make a lot of sense, it's a problem that you can't delete the example after deleting the pattern: you previously had to remove the example before the pattern. This PR fixes that by always allowing you to update the example, even if there is no pattern. Our server doesn't currently accept submitting an example with no pattern, but we could allow that if we want to (and probably just discard it on the back-end). This PR also updates the validation of the example and the regex. There were more unhandled edge cases previously where the validation would disappear or be wrong. This should be fixed now. The new logic is that, whenever you update the either the pattern or the example, we check: - if you have an error in your pattern, no pattern, or no example, then delete the example error if it exists - have a well-formed pattern and an example then check if the example matches the pattern and add/delete an error accordingly This does have some consequences: editing the pattern can render your example invalid. You'll also get immediate feedback instead of when you switch focus. I think this is often a bad pattern (giving the user too much negative feedback), but in terms of working with regexes, I think it might be a good thing. We also give immediate feedback today, so I don't think this is a regression. Any thoughts are welcome. --- .../ProjectForm/FeatureFlagNamingTooltip.tsx | 2 +- .../Project/ProjectForm/ProjectForm.tsx | 73 +++++++++++++++---- .../validate-feature-naming.test.ts | 50 +++++++++++++ 3 files changed, 111 insertions(+), 14 deletions(-) create mode 100644 frontend/src/component/project/Project/ProjectForm/validate-feature-naming.test.ts diff --git a/frontend/src/component/project/Project/ProjectForm/FeatureFlagNamingTooltip.tsx b/frontend/src/component/project/Project/ProjectForm/FeatureFlagNamingTooltip.tsx index 26de63ea0c..639a7617da 100644 --- a/frontend/src/component/project/Project/ProjectForm/FeatureFlagNamingTooltip.tsx +++ b/frontend/src/component/project/Project/ProjectForm/FeatureFlagNamingTooltip.tsx @@ -14,7 +14,7 @@ export const FeatureFlagNamingTooltip: FC = () => {

Enforce a naming convention for feature flags

-

{`eg. ^[A - Za - z0 - 9]{2}[.][a-z]{4,12}$ matches 'a1.project'`}

+

{`eg. ^[A-Za-z0-9]{2}[.][a-z]{4,12}$ matches 'a1.project'`}

Brackets:

diff --git a/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx b/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx index db12adf53b..dab56a1bdc 100644 --- a/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx +++ b/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx @@ -106,6 +106,29 @@ const StyledFlagNamingContainer = styled('div')(({ theme }) => ({ '& > *': { width: '100%' }, })); +export const validateFeatureNamingExample = ({ + pattern, + example, + featureNamingPatternError, +}: { + pattern: string; + example: string; + featureNamingPatternError: string | undefined; +}): { state: 'valid' } | { state: 'invalid'; reason: string } => { + if (featureNamingPatternError || !example || !pattern) { + return { state: 'valid' }; + } else if (example && pattern) { + const regex = new RegExp(pattern); + const matches = regex.test(example); + if (!matches) { + return { state: 'invalid', reason: 'Example does not match regex' }; + } else { + return { state: 'valid' }; + } + } + return { state: 'valid' }; +}; + const ProjectForm: React.FC = ({ children, handleSubmit, @@ -135,28 +158,51 @@ const ProjectForm: React.FC = ({ }) => { const { uiConfig } = useUiConfig(); const shouldShowFlagNaming = uiConfig.flags.featureNamingPattern; + + const updateNamingExampleError = ({ + example, + pattern, + }: { + example: string; + pattern: string; + }) => { + const validationResult = validateFeatureNamingExample({ + pattern, + example, + featureNamingPatternError: errors.featureNamingPattern, + }); + + switch (validationResult.state) { + case 'invalid': + errors.namingExample = validationResult.reason; + break; + case 'valid': + delete errors.namingExample; + break; + } + }; + const onSetFeatureNamingPattern = (regex: string) => { try { new RegExp(regex); setFeatureNamingPattern && setFeatureNamingPattern(regex); - clearErrors(); + delete errors.featureNamingPattern; } catch (e) { errors.featureNamingPattern = 'Invalid regular expression'; setFeatureNamingPattern && setFeatureNamingPattern(regex); } + updateNamingExampleError({ + pattern: regex, + example: featureNamingExample || '', + }); }; const onSetFeatureNamingExample = (example: string) => { - if (featureNamingPattern) { - const regex = new RegExp(featureNamingPattern); - const matches = regex.test(example); - if (!matches) { - errors.namingExample = 'Example does not match regex'; - } else { - delete errors.namingExample; - } - setFeatureNamingExample && setFeatureNamingExample(example); - } + setFeatureNamingExample && setFeatureNamingExample(example); + updateNamingExampleError({ + pattern: featureNamingPattern || '', + example, + }); }; const onSetFeatureNamingDescription = (description: string) => { @@ -190,7 +236,9 @@ const ProjectForm: React.FC = ({ onChange={e => setProjectName(e.target.value)} error={Boolean(errors.name)} errorText={errors.name} - onFocus={() => clearErrors()} + onFocus={() => { + delete errors.name; + }} data-testid={PROJECT_NAME_INPUT} required /> @@ -333,7 +381,6 @@ const ProjectForm: React.FC = ({ value={featureNamingPattern || ''} error={Boolean(errors.featureNamingPattern)} errorText={errors.featureNamingPattern} - onFocus={() => clearErrors()} onChange={e => onSetFeatureNamingPattern( e.target.value diff --git a/frontend/src/component/project/Project/ProjectForm/validate-feature-naming.test.ts b/frontend/src/component/project/Project/ProjectForm/validate-feature-naming.test.ts new file mode 100644 index 0000000000..f54233fd9e --- /dev/null +++ b/frontend/src/component/project/Project/ProjectForm/validate-feature-naming.test.ts @@ -0,0 +1,50 @@ +import { validateFeatureNamingExample } from './ProjectForm'; + +describe('validateFeatureNaming', () => { + test.each(['+', 'valid regex$'])( + `if the featureNamingPatternError prop is present, it's always valid: %s`, + pattern => { + const result = validateFeatureNamingExample({ + pattern, + example: 'aohutnasoehutns', + featureNamingPatternError: 'error', + }); + + expect(result.state).toBe('valid'); + } + ); + test(`if the pattern is empty, the example is always valid`, () => { + const result = validateFeatureNamingExample({ + pattern: '', + example: 'aohutnasoehutns', + featureNamingPatternError: undefined, + }); + + expect(result.state).toBe('valid'); + }); + test(`if the example is empty, the it's always valid`, () => { + const result = validateFeatureNamingExample({ + pattern: '^dx-[a-z]{1,5}$', + example: '', + featureNamingPatternError: undefined, + }); + + expect(result.state).toBe('valid'); + }); + + test.each([ + ['valid', 'dx-logs'], + ['invalid', 'axe-battles'], + ])( + `if example is %s, the state should be be the same`, + (state, example) => { + const result = validateFeatureNamingExample({ + pattern: '^dx-[a-z]{1,5}$', + example, + featureNamingPatternError: undefined, + }); + + expect(result.state).toBe(state); + } + ); +});