diff --git a/go.work.sum b/go.work.sum index b1e83f8a3a..81952df676 100644 --- a/go.work.sum +++ b/go.work.sum @@ -1665,7 +1665,6 @@ github.com/prometheus/common v0.30.0/go.mod h1:vu+V0TpY+O6vW9J44gczi3Ap/oXXR10b+ github.com/prometheus/common v0.37.0/go.mod h1:phzohg0JFMnBEFGxTDbfu3QyL5GI8gTQJFhYO5B3mfA= github.com/prometheus/common v0.48.0/go.mod h1:0/KsvlIEfPQCQ5I2iNSAWKPZziNCvRs5EC6ILDTlAPc= github.com/prometheus/common v0.55.0/go.mod h1:2SECS4xJG1kd8XF9IcM1gMX6510RAEL65zxzNImwdc8= -github.com/prometheus/common v0.60.1/go.mod h1:h0LYf1R1deLSKtD4Vdg8gy4RuOvENW2J/h19V5NADQw= github.com/prometheus/procfs v0.0.0-20180125133057-cb4147076ac7/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/prometheus/procfs v0.0.0-20190522114515-bc1a522cf7b1/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= diff --git a/ui/.gitignore b/ui/.gitignore index 7918d58908..707144fc62 100644 --- a/ui/.gitignore +++ b/ui/.gitignore @@ -27,4 +27,5 @@ dist-ssr /playwright/.cache/ .packagehash /screenshots/ -.env \ No newline at end of file +.env +coverage diff --git a/ui/src/app/console/Console.tsx b/ui/src/app/console/Console.tsx index d37aee76e4..7ecc4560c4 100644 --- a/ui/src/app/console/Console.tsx +++ b/ui/src/app/console/Console.tsx @@ -12,7 +12,7 @@ import { useListAuthProvidersQuery } from '~/app/auth/authApi'; import { useListFlagsQuery } from '~/app/flags/flagsApi'; import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice'; import { selectCurrentRef } from '~/app/refs/refsSlice'; -import { ContextEditor } from '~/components/console/ContextEditor'; +import { JsonEditor } from '~/components/json/JsonEditor'; import EmptyState from '~/components/EmptyState'; import { Button } from '~/components/Button'; import Combobox from '~/components/Combobox'; @@ -285,8 +285,9 @@ export default function Console() { Request Context
- { formik.setFieldValue('context', v); }} diff --git a/ui/src/app/flags/flagsApi.ts b/ui/src/app/flags/flagsApi.ts index 891d08d1de..ae9e775d83 100644 --- a/ui/src/app/flags/flagsApi.ts +++ b/ui/src/app/flags/flagsApi.ts @@ -91,9 +91,9 @@ export const flagsApi = createApi({ { namespaceKey: string; flagKey: string; values: IFlagBase } >({ query({ namespaceKey, flagKey, values }) { - // create new object 'values' to remap defaultVariant to defaultVariantId const update = { defaultVariantId: values.defaultVariant?.id || null, + metadata: values.metadata || [], ...values }; delete update.defaultVariant; diff --git a/ui/src/components/console/lint.ts b/ui/src/components/console/lint.ts deleted file mode 100644 index fc72b22e06..0000000000 --- a/ui/src/components/console/lint.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { Diagnostic } from '@codemirror/lint'; -import { EditorView } from '@codemirror/view'; -import { Text } from '@codemirror/state'; -import { SearchCursor } from '@codemirror/search'; - -function getJsonErrorPosition(error: SyntaxError, doc: Text): number { - let m; - if ((m = error.message.match(/at position (\d+)/))) - return Math.min(+m[1], doc.length); - if ((m = error.message.match(/at line (\d+) column (\d+)/))) - return Math.min(doc.line(+m[1]).from + +m[2] - 1, doc.length); - return 0; -} - -export const parseLinter = - () => - (view: EditorView): Diagnostic[] => { - const doc = view.state.doc; - try { - const data = JSON.parse(doc.toString()); - if (Array.isArray(data) || typeof data !== 'object') { - return [ - { - from: 0, - message: 'Request Context: unexpected array', - severity: 'error', - to: doc.length - } - ]; - } - for (const [key, value] of Object.entries(data)) { - if (typeof value !== 'string') { - const keyCursor = new SearchCursor(doc, `"${key}"`); - const keyPosition = keyCursor.next().value; - const valueCursor = new SearchCursor( - doc, - JSON.stringify(value), - keyPosition.to - ); - const pos = valueCursor.next().value; - return [ - { - from: pos.from, - message: 'Request Context: expected string value', - severity: 'error', - to: pos.to - } - ]; - } - } - } catch (e) { - if (!(e instanceof SyntaxError)) throw e; - const pos = getJsonErrorPosition(e, doc); - return [ - { - from: pos, - message: 'JSON: unexpected keyword', - severity: 'error', - to: pos - } - ]; - } - return []; - }; diff --git a/ui/src/components/flags/forms/FlagForm.tsx b/ui/src/components/flags/forms/FlagForm.tsx index 75792cd48a..ccd27cbc2c 100644 --- a/ui/src/components/flags/forms/FlagForm.tsx +++ b/ui/src/components/flags/forms/FlagForm.tsx @@ -14,11 +14,13 @@ import { Button } from '~/components/Button'; import Input from '~/components/forms/Input'; import Toggle from '~/components/forms/Toggle'; import Loading from '~/components/Loading'; +import { MetadataForm } from '~/components/flags/forms/MetadataForm'; import { useError } from '~/data/hooks/error'; import { useSuccess } from '~/data/hooks/success'; import { keyValidation, requiredValidation } from '~/data/validations'; import { FlagType, IFlag, IFlagBase } from '~/types/Flag'; import { cls, copyTextToClipboard, stringAsKey } from '~/utils/helpers'; +import MetadataFormErrorBoundary from './MetadataFormErrorBoundary'; const flagTypes = [ { @@ -37,7 +39,8 @@ const flagTypes = [ const flagValidationSchema = Yup.object({ key: keyValidation, - name: requiredValidation + name: requiredValidation, + metadata: Yup.object() }); export default function FlagForm(props: { flag?: IFlag }) { @@ -61,14 +64,14 @@ export default function FlagForm(props: { flag?: IFlag }) { if (isNew) { return createFlag({ namespaceKey: namespace.key, - values: values + values }).unwrap(); } return updateFlag({ namespaceKey: namespace.key, flagKey: flag?.key, - values: values + values }).unwrap(); }; @@ -78,10 +81,12 @@ export default function FlagForm(props: { flag?: IFlag }) { description: flag?.description || '', type: flag?.type || FlagType.VARIANT, enabled: flag?.enabled || false, - defaultVariant: flag?.defaultVariant + defaultVariant: flag?.defaultVariant, + metadata: flag?.metadata || {} }; const [keyCopied, setKeyCopied] = useState(false); + const [hasMetadataErrors, setHasMetadataErrors] = useState(false); return (
+
+
+ + + Optional + +
+
+ + formik.setFieldValue('metadata', metadata) + } + disabled={readOnly} + > + + formik.setFieldValue('metadata', metadata) + } + disabled={readOnly} + onErrorChange={setHasMetadataErrors} + /> + +
+
+
+ ); + })} + + + + ); +} diff --git a/ui/src/components/flags/forms/MetadataFormErrorBoundary.tsx b/ui/src/components/flags/forms/MetadataFormErrorBoundary.tsx new file mode 100644 index 0000000000..ea7de4dd8d --- /dev/null +++ b/ui/src/components/flags/forms/MetadataFormErrorBoundary.tsx @@ -0,0 +1,50 @@ +import React from 'react'; +import { JsonEditor } from '~/components/json/JsonEditor'; + +interface MetadataFormErrorBoundaryProps { + children: React.ReactNode; + metadata?: Record; + onChange: (metadata: Record) => void; + disabled?: boolean; +} + +class MetadataFormErrorBoundary extends React.Component< + MetadataFormErrorBoundaryProps, + { hasError: boolean } +> { + constructor(props: MetadataFormErrorBoundaryProps) { + super(props); + this.state = { hasError: false }; + } + + static getDerivedStateFromError(_: Error) { + return { hasError: true }; + } + + render() { + if (this.state.hasError) { + return ( + { + try { + const parsed = JSON.parse(value); + this.props.onChange(parsed); + } catch (e) { + console.warn('Invalid JSON:', e); + } + }} + disabled={this.props.disabled} + strict={false} + height="20vh" + data-testid="metadata-fallback" + /> + ); + } + + return this.props.children; + } +} + +export default MetadataFormErrorBoundary; diff --git a/ui/src/components/console/ContextEditor.module.css b/ui/src/components/json/JsonEditor.module.css similarity index 75% rename from ui/src/components/console/ContextEditor.module.css rename to ui/src/components/json/JsonEditor.module.css index fcfbbd6d83..28821704c7 100644 --- a/ui/src/components/console/ContextEditor.module.css +++ b/ui/src/components/json/JsonEditor.module.css @@ -1,4 +1,4 @@ -.ContextEditor { +.JsonEditor { width: 100%; min-height: 50vh; height: 100%; diff --git a/ui/src/components/console/ContextEditor.tsx b/ui/src/components/json/JsonEditor.tsx similarity index 54% rename from ui/src/components/console/ContextEditor.tsx rename to ui/src/components/json/JsonEditor.tsx index a05da89f80..1668eeaae2 100644 --- a/ui/src/components/console/ContextEditor.tsx +++ b/ui/src/components/json/JsonEditor.tsx @@ -1,19 +1,31 @@ import { json } from '@codemirror/lang-json'; -import { linter, lintGutter } from '@codemirror/lint'; +import { linter } from '@codemirror/lint'; import { tokyoNight } from '@uiw/codemirror-theme-tokyo-night'; import CodeMirror from '@uiw/react-codemirror'; import React from 'react'; import { parseLinter } from './lint'; -type ContextEditorProps = { +type JsonEditorProps = { id: string; + value: string; setValue(value: string): void; + disabled?: boolean; + strict?: boolean; + height?: string; + 'data-testid'?: string; }; -export const ContextEditor: React.FC = ( - props: ContextEditorProps +export const JsonEditor: React.FC = ( + props: JsonEditorProps ) => { - const { setValue } = props; + const { + value = '{}', + setValue, + disabled = false, + strict = true, + height = '50vh', + 'data-testid': dataTestId + } = props; const onChange = React.useCallback( (val: any, _: any) => { setValue(val); @@ -22,10 +34,11 @@ export const ContextEditor: React.FC = ( ); return ( = ( highlightActiveLine: true }} theme={tokyoNight} + data-testid={dataTestId} /> ); }; diff --git a/ui/src/components/json/lint.ts b/ui/src/components/json/lint.ts new file mode 100644 index 0000000000..c717caa35c --- /dev/null +++ b/ui/src/components/json/lint.ts @@ -0,0 +1,80 @@ +import { Diagnostic } from '@codemirror/lint'; +import { EditorView } from '@codemirror/view'; +import { Text } from '@codemirror/state'; +import { SearchCursor } from '@codemirror/search'; + +function getJsonErrorPosition(error: SyntaxError, doc: Text): number { + let m; + if ((m = error.message.match(/at position (\d+)/))) + return Math.min(+m[1], doc.length); + if ((m = error.message.match(/at line (\d+) column (\d+)/))) + return Math.min(doc.line(+m[1]).from + +m[2] - 1, doc.length); + return 0; +} + +export const parseLinter = + (strict: boolean = true) => + (view: EditorView): Diagnostic[] => { + const doc = view.state.doc; + try { + const data = JSON.parse(doc.toString()); + if (strict && Array.isArray(data)) { + return [ + { + from: 0, + message: 'JSON: unexpected array', + severity: 'error', + to: doc.length + } + ]; + } + if (!Array.isArray(data) && typeof data !== 'object') { + return [ + { + from: 0, + message: 'JSON: unexpected type', + severity: 'error', + to: doc.length + } + ]; + } + if (!Array.isArray(data)) { + for (const [key, value] of Object.entries(data)) { + if (strict && typeof value !== 'string') { + const keyCursor = new SearchCursor(doc, `"${key}"`); + const keyMatch = keyCursor.next(); + if (!keyMatch?.value) continue; + + const valueCursor = new SearchCursor( + doc, + JSON.stringify(value), + keyMatch.value.to + ); + const valueMatch = valueCursor.next(); + if (!valueMatch?.value) continue; + + return [ + { + from: valueMatch.value.from, + message: 'JSON: expected string value', + severity: 'error', + to: valueMatch.value.to + } + ]; + } + } + } + } catch (e) { + if (!(e instanceof SyntaxError)) throw e; + const pos = getJsonErrorPosition(e, doc); + return [ + { + from: pos, + message: 'JSON: unexpected keyword', + severity: 'error', + to: pos + } + ]; + } + return []; + }; diff --git a/ui/src/types/Flag.ts b/ui/src/types/Flag.ts index a14d671ca3..492c835aa0 100644 --- a/ui/src/types/Flag.ts +++ b/ui/src/types/Flag.ts @@ -17,6 +17,15 @@ export function flagTypeToLabel(flagType: FlagType): string { return 'Unknown'; } } + +export interface IFlagMetadata { + key: string; + value: any; + type: 'primitive' | 'object' | 'array'; + subtype?: 'string' | 'number' | 'boolean'; + isNew?: boolean; +} + export interface IFlagBase { key: string; type: FlagType; @@ -24,6 +33,7 @@ export interface IFlagBase { enabled: boolean; description: string; defaultVariant?: IVariant; + metadata?: Record; } export interface IFlag extends IFlagBase { diff --git a/ui/tests/flags.spec.ts b/ui/tests/flags.spec.ts index 6090a2f84b..f22180cdff 100644 --- a/ui/tests/flags.spec.ts +++ b/ui/tests/flags.spec.ts @@ -121,7 +121,6 @@ test.describe('Flags', () => { // verify flag was copied await page.getByRole('link', { name: 'test-flag' }).click(); - await expect(page.getByText('Test Flag')).toBeVisible(); // verify variants were copied await expect( @@ -131,6 +130,45 @@ test.describe('Flags', () => { page.getByRole('cell', { name: 'firefox', exact: true }) ).toBeVisible(); }); + + test('can create flag with metadata', async ({ page }) => { + await page.getByRole('button', { name: 'New Flag' }).click(); + await page.getByLabel('Name').fill('Metadata Flag'); + await page.getByRole('button', { name: 'Add Metadata' }).click(); + await page.getByTestId('metadata-key-0').fill('foo'); + await page.getByTestId('metadata-value-0').fill('bar'); + await page.getByRole('button', { name: 'Add Metadata' }).click(); + await page.getByTestId('metadata-key-1').fill('baz'); + await page.getByTestId('metadata-type-1').click(); + await page.getByLabel('Object').getByText('Object').click(); + await page + .getByTestId('metadata-value-1') + .getByRole('textbox') + .fill('{"foo":"bar","baz":{"boz":"1"}}'); + await page.getByRole('button', { name: 'Create' }).click(); + await expect(page.getByText('Successfully created flag')).toBeVisible(); + await expect( + page.getByTestId('metadata-value-0').getByRole('textbox') + ).toHaveText('{"baz":{"boz":"1"},"foo":"bar"}'); + await expect(page.getByTestId('metadata-value-1')).toBeDisabled(); + }); + + test('can delete flag metadata', async ({ page }) => { + await page.getByRole('link', { name: 'metadata-flag' }).click(); + await page.getByLabel('Remove metadata entry').first().click(); + await page.getByRole('button', { name: 'Update' }).click(); + await expect(page.getByText('Successfully updated flag')).toBeVisible(); + await expect(page.getByTestId('metadata-value-0')).toBeDisabled(); + }); + + test('can not update flag with duplicate metadata keys', async ({ page }) => { + await page.getByRole('link', { name: 'metadata-flag' }).click(); + await page.getByRole('button', { name: 'Add Metadata' }).click(); + await page.getByTestId('metadata-key-1').fill('foo'); + await page.getByTestId('metadata-value-1').fill('bar'); + await expect(page.getByText('Key must be unique')).toBeVisible(); + await expect(page.getByRole('button', { name: 'Update' })).toBeDisabled(); + }); }); test.describe('Flags - Read Only', () => { @@ -187,4 +225,11 @@ test.describe('Flags - Read Only', () => { page.getByRole('menuitem', { name: 'Copy to Namespace' }) ).toBeDisabled(); }); + + test('can not edit metadata', async ({ page }) => { + await page.getByRole('link', { name: 'metadata-flag' }).click(); + await expect( + page.getByRole('button', { name: 'Add Metadata' }) + ).toBeDisabled(); + }); });