diff --git a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.test.ts b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.test.ts new file mode 100644 index 00000000..be791c9f --- /dev/null +++ b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { buildFormSchema } from './EditSystemDefaultsForm'; + +const values = (ttl_days: number) => ({ + ttl_days, + engine: 'MergeTree', + common_header_type: 'common-header/timeseries', + common_header_version: '1.0.3', +}); + +describe('buildFormSchema', () => { + it.each([ + [0, true], + [365, true], + [1000, true], + [36500, true], + [36501, false], + [-1, false], + ])( + 'retention of %i days against a maximum of 36500 is valid: %s', + (days, ok) => { + expect(buildFormSchema(36500).safeParse(values(days)).success).toBe(ok); + }, + ); + + it('takes the bound it is given, not one of its own', () => { + expect(buildFormSchema(30).safeParse(values(31)).success).toBe(false); + expect(buildFormSchema(30).safeParse(values(30)).success).toBe(true); + }); +}); diff --git a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.tsx b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.tsx index a7047bdd..5ff86afa 100644 --- a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.tsx +++ b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/EditSystemDefaultsForm.tsx @@ -10,14 +10,16 @@ import isInteger from 'lodash/isInteger'; import { useCallback, useMemo, useState } from 'react'; import z from 'zod'; -const formSchema = z.object({ - ttl_days: z.number().min(0).max(365), - engine: z.string().min(1), - common_header_version: z.string().min(1), - common_header_type: z.string().min(1), -}); +/** The retention bound is the engine's, served with the defaults. */ +export const buildFormSchema = (maxTtlDays: number) => + z.object({ + ttl_days: z.number().min(0).max(maxTtlDays), + engine: z.string().min(1), + common_header_version: z.string().min(1), + common_header_type: z.string().min(1), + }); -type TFormValues = z.infer; +type TFormValues = z.infer>; const normalizeHeaderType = (value?: string) => (value ?? '').replace(/^common-header\//, ''); @@ -78,10 +80,12 @@ const hasDefaultsChanged = ({ }; export const EditSystemDefaultsForm = ({ initialValues, + maxTtlDays, onSuccess, onClose, }: { initialValues?: TFormValues; + maxTtlDays: number; onSuccess?: ( response: TUpdateSystemDefaultsResponse & { changed: boolean }, ) => void; @@ -116,6 +120,7 @@ export const EditSystemDefaultsForm = ({ }); const [form] = Form.useForm(); + const formSchema = useMemo(() => buildFormSchema(maxTtlDays), [maxTtlDays]); const formValidation = useAntdZodResolver(formSchema); const ttlDays = Form.useWatch('ttl_days', form); const engine = Form.useWatch('engine', form); diff --git a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/SystemDefaultSettings.test.tsx b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/SystemDefaultSettings.test.tsx index e738d113..4c752ae8 100644 --- a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/SystemDefaultSettings.test.tsx +++ b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/SystemDefaultSettings.test.tsx @@ -1,6 +1,7 @@ import { API_CONFIG_MOCKS } from '@/core/config/api/endpoints/generator'; import { buildTestWrapper } from '@/core/utils/test-utils/buildTestWrapper'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { delay, http, HttpResponse } from 'msw'; import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; import { SystemDefaultsSettings } from '.'; import { server } from './SystemDefaultSettings.mocks'; @@ -11,6 +12,27 @@ afterAll(() => server.close()); const { wrapper } = buildTestWrapper().withTheme().withReactQuery(); +const stringDefault = (effective: string) => ({ + effective, + stored: null, + origin: 'deployment' as const, + deployment_default: effective, +}); + +const defaultsBody = { + ttl_days: { + effective: 90, + stored: null, + origin: 'deployment' as const, + deployment_default: 90, + maximum: 36500, + }, + common_header_type: stringDefault('timeseries'), + common_header_version: stringDefault('1.0.3'), + engine: stringDefault('MergeTree'), + editable: true, +}; + describe('SystemDefaultsSettings', () => { it('shows the environment default, with the editor closed', async () => { render(, { wrapper }); @@ -36,6 +58,7 @@ describe('SystemDefaultsSettings', () => { stored: null, origin: 'deployment', deployment_default: 0, + maximum: 36500, }, common_header_type: { effective: 'common-header/timeseries', @@ -66,4 +89,47 @@ describe('SystemDefaultsSettings', () => { await screen.findByText('none (kept forever)', {}, { timeout: 15000 }), ).toBeInTheDocument(); }); + + it('says the defaults cannot be edited when they failed to load', async () => { + server.use(API_CONFIG_MOCKS.system.defaults.get.error()); + render(, { wrapper }); + + fireEvent.click( + await screen.findByRole('button', { name: 'System defaults actions' }), + ); + fireEvent.click(await screen.findByText('Edit Defaults')); + + expect( + await screen.findByText(/could not be loaded, so they cannot be edited/), + ).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + expect( + screen.queryByText(/so they cannot be edited/), + ).not.toBeInTheDocument(); + }); + + it('shows the same error notice as the edit form, and no zero values, when the defaults failed', async () => { + server.use(API_CONFIG_MOCKS.system.defaults.get.error()); + render(, { wrapper }); + + expect( + await screen.findByText(/The defaults could not be loaded:/), + ).toBeInTheDocument(); + expect(screen.queryByText('none (kept forever)')).not.toBeInTheDocument(); + expect(screen.queryByText(/@v/)).not.toBeInTheDocument(); + }); + + it('says it is loading, and shows no zero values, until the defaults arrive', async () => { + server.use( + http.get(API_CONFIG_MOCKS.system.defaults.mockedUrl, async () => { + await delay(300); + return HttpResponse.json(defaultsBody); + }), + ); + render(, { wrapper }); + + expect(await screen.findByText('Loading defaults...')).toBeInTheDocument(); + expect(screen.queryByText('none (kept forever)')).not.toBeInTheDocument(); + expect(await screen.findByText('90 days')).toBeInTheDocument(); + }); }); diff --git a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/ViewSystemDefaults.tsx b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/ViewSystemDefaults.tsx index 459417e3..cf06df40 100644 --- a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/ViewSystemDefaults.tsx +++ b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/ViewSystemDefaults.tsx @@ -1,4 +1,4 @@ -import { NotificationCard } from '@/core/components/NotificationCard'; +import { FormNotification } from '@/core/components/FormNotification'; import { RbacProtected } from '@/core/components/RbacProtected'; import { TSystemDefaultsResponse } from '@/core/hooks/useFetchDefaults/types'; @@ -9,27 +9,37 @@ const formatDays = (days: number) => export const ViewSystemDefaults = ({ defaults, error, + isLoading = false, }: { defaults?: TSystemDefaultsResponse; error: Error | null; + isLoading?: boolean; }) => { return ( <> - {error && } -
-
Default Retention
-
{formatDays(defaults?.ttl_days?.effective ?? 0)}
+ {error ? ( + + ) : isLoading || !defaults ? ( +

Loading defaults...

+ ) : ( +
+
Default Retention
+
{formatDays(defaults.ttl_days.effective)}
-
Default Engine
-
{defaults?.engine?.effective ?? ''}
-
Default Header & Version
-
- {defaults?.common_header_type?.effective ?? ''}@v - {defaults?.common_header_version?.effective ?? ''} -
-
+
Default Engine
+
{defaults.engine.effective}
+
Default Header & Version
+
+ {defaults.common_header_type.effective}@v + {defaults.common_header_version.effective} +
+
+ )}

Set by DFE_ENGINE_CLICKHOUSE_DEFAULT_TTL_DAYS. A change applies to every table that follows the default when the engine restarts. diff --git a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/index.tsx b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/index.tsx index 1b3504dd..53b7169c 100644 --- a/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/index.tsx +++ b/apps/dfe-core-ui/src/Platform/components/SystemManagement/SystemDefaultsSettings/index.tsx @@ -1,12 +1,14 @@ +import { FormNotification } from '@/core/components/FormNotification'; import { SectionCard } from '@/core/components/SectionCard'; import { useFetchDefaults } from '@/core/hooks/useFetchDefaults'; +import { Button } from 'antd'; import { useState } from 'react'; import { EditSystemDefaultsForm } from './EditSystemDefaultsForm'; import { SystemDefaultsActionMenu } from './SystemDefaultsActionMenu'; import { ViewSystemDefaults } from './ViewSystemDefaults'; export const SystemDefaultsSettings = () => { - const { data: defaults, error } = useFetchDefaults(); + const { data: defaults, error, isLoading } = useFetchDefaults(); const [isEditDefaultsOpen, setIsEditDefaultsOpen] = useState(false); const [hasDefaultsChanged, setHasDefaultsChanged] = useState(false); @@ -22,23 +24,37 @@ export const SystemDefaultsSettings = () => { /> } > - {isEditDefaultsOpen ? ( + {isEditDefaultsOpen && !defaults && error ? ( +

+ +
+ +
+
+ ) : isEditDefaultsOpen && defaults ? ( setIsEditDefaultsOpen(false)} onSuccess={({ changed }) => { setIsEditDefaultsOpen(false); setHasDefaultsChanged(changed); }} + maxTtlDays={defaults.ttl_days.maximum} initialValues={{ - ttl_days: defaults?.ttl_days?.effective ?? 0, - engine: defaults?.engine?.effective ?? '', - common_header_version: - defaults?.common_header_version?.effective ?? '', - common_header_type: `common-header/${defaults?.common_header_type?.effective?.replace('common-header/', '')}`, + ttl_days: defaults.ttl_days.effective, + engine: defaults.engine.effective, + common_header_version: defaults.common_header_version.effective, + common_header_type: `common-header/${defaults.common_header_type.effective.replace('common-header/', '')}`, }} /> ) : ( - + )} ); diff --git a/apps/dfe-core-ui/src/Platform/hooks/system/useUpdateDefaults/useUpdateDefaults.test.tsx b/apps/dfe-core-ui/src/Platform/hooks/system/useUpdateDefaults/useUpdateDefaults.test.tsx index 95345be6..a4e8830f 100644 --- a/apps/dfe-core-ui/src/Platform/hooks/system/useUpdateDefaults/useUpdateDefaults.test.tsx +++ b/apps/dfe-core-ui/src/Platform/hooks/system/useUpdateDefaults/useUpdateDefaults.test.tsx @@ -57,6 +57,7 @@ describe('.useUpdateSystemDefaults', () => { stored: null, origin: 'deployment', deployment_default: 90, + maximum: 36500, }, common_header_type: { effective: 'string', diff --git a/apps/dfe-core-ui/src/Schemas/components/CreateDerivedSchemaDrawer/CreateDerivedSchemaDrawer.mocks.ts b/apps/dfe-core-ui/src/Schemas/components/CreateDerivedSchemaDrawer/CreateDerivedSchemaDrawer.mocks.ts index 27c29deb..af3ca7a1 100644 --- a/apps/dfe-core-ui/src/Schemas/components/CreateDerivedSchemaDrawer/CreateDerivedSchemaDrawer.mocks.ts +++ b/apps/dfe-core-ui/src/Schemas/components/CreateDerivedSchemaDrawer/CreateDerivedSchemaDrawer.mocks.ts @@ -60,6 +60,7 @@ export const server = setupServer( API_CONFIG_MOCKS.schemas.default.get.success({ mockedResponse: SCHEMA_LIST_RESPONSE, }), + API_CONFIG_MOCKS.schemas.vocabulary.get.success(), API_CONFIG_MOCKS.schemas.schemaDetail.get.success({ mockedResponse: BASE_COLUMNS_RESPONSE, schema_path: BASE_SCHEMA_PATH, diff --git a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.test.ts b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.test.ts index 59b311bc..6758f4d1 100644 --- a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.test.ts +++ b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.test.ts @@ -6,6 +6,7 @@ import { selectEntry, withIndexUseCase, } from '@/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers'; +import { SCHEMA_VOCABULARY_FIXTURE } from '@/core/hooks/useFetchSchemaVocabulary/vocabulary.fixture'; import { describe, expect, it } from 'vitest'; const baseColumns: DerivedBaseColumn[] = [ @@ -17,14 +18,21 @@ const baseColumns: DerivedBaseColumn[] = [ describe('baseColumnFrom', () => { it('reads the base column index through the rename', () => { - expect(baseColumnFrom({ name: 'message', use_case: 'fulltext' })).toEqual({ + expect( + baseColumnFrom( + { name: 'message', use_case: 'fulltext' }, + SCHEMA_VOCABULARY_FIXTURE, + ), + ).toEqual({ name: 'message', baseIndex: 'word_search', }); }); it('gives a column with no index use case none', () => { - expect(baseColumnFrom({ name: 'log_offset' })).toEqual({ + expect( + baseColumnFrom({ name: 'log_offset' }, SCHEMA_VOCABULARY_FIXTURE), + ).toEqual({ name: 'log_offset', baseIndex: 'none', }); diff --git a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.ts b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.ts index 9df75909..79fdc0f1 100644 --- a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.ts +++ b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.helpers.ts @@ -1,3 +1,4 @@ +import type { TSchemaVocabularyResponse } from '@/core/hooks/useFetchSchemaVocabulary/types'; import { IndexUseCase, resolveIndexUseCase, @@ -20,12 +21,15 @@ export type DerivedBaseColumn = { baseIndex: IndexUseCase; }; -export const baseColumnFrom = (column: { - name?: string | null; - use_case?: string | null; -}): DerivedBaseColumn => ({ +export const baseColumnFrom = ( + column: { + name?: string | null; + use_case?: string | null; + }, + vocabulary: TSchemaVocabularyResponse, +): DerivedBaseColumn => ({ name: column.name ?? '', - baseIndex: resolveIndexUseCase(column.use_case), + baseIndex: resolveIndexUseCase(column.use_case, vocabulary), }); /** An omitted `index` keeps the base column's, so only a change is written. */ diff --git a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.mocks.ts b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.mocks.ts index 90273dd4..4e22e711 100644 --- a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.mocks.ts +++ b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/DerivedSchemaFieldPicker.mocks.ts @@ -49,6 +49,7 @@ export const BASE_COLUMNS_RESPONSE: TMetaSchemaDetailResponse = { }; export const server = setupServer( + API_CONFIG_MOCKS.schemas.vocabulary.get.success(), API_CONFIG_MOCKS.schemas.schemaDetail.get.success({ mockedResponse: BASE_COLUMNS_RESPONSE, schema_path: BASE_SCHEMA_PATH, diff --git a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/index.tsx b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/index.tsx index d9c59f16..1388bf12 100644 --- a/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/index.tsx +++ b/apps/dfe-core-ui/src/Schemas/components/DerivedSchemaFieldPicker/index.tsx @@ -1,7 +1,8 @@ import type { FormControlAria } from '@/core/components/Form/formControlAria'; import { Table } from '@/core/components/Table'; import { Tooltip } from '@/core/components/Tooltip'; -import { INDEX_USE_CASE_OPTIONS } from '@/Schemas/constants/indexUseCase'; +import { useFetchSchemaVocabulary } from '@/core/hooks/useFetchSchemaVocabulary'; +import { indexUseCaseOptions } from '@/Schemas/constants/indexUseCase'; import { useFetchInfiniteFilteredSchemaDetailColumns } from '@/Schemas/hooks/useFetchInfiniteSchemaDetailColumns'; import { TMetaSchemaDetailColumnItem } from '@/Schemas/hooks/useFetchInfiniteSchemaDetailColumns/types'; import { IconInfoCircle } from '@repo/dfe-icons'; @@ -64,11 +65,26 @@ export const DerivedSchemaFieldPicker = ({ fetch_all: true, }); + const { + data: vocabulary, + isLoading: isLoadingVocabulary, + error: vocabularyError, + } = useFetchSchemaVocabulary(); + const columnItems = useMemo(() => data?.items ?? [], [data]); + // A base column's index is read through the vocabulary, so none is read before it is in. const baseColumns: DerivedBaseColumn[] = useMemo( - () => columnItems.map(baseColumnFrom), - [columnItems], + () => + vocabulary + ? columnItems.map((column) => baseColumnFrom(column, vocabulary)) + : [], + [columnItems, vocabulary], + ); + + const indexOptions = useMemo( + () => (vocabulary ? indexUseCaseOptions(vocabulary) : []), + [vocabulary], ); const baseColumnByName = useMemo( @@ -118,7 +134,7 @@ export const DerivedSchemaFieldPicker = ({ className="w-full" size="small" aria-label={`Index use case for ${column.name}`} - options={INDEX_USE_CASE_OPTIONS} + options={indexOptions} popupMatchSelectWidth={false} value={chosenIndexFor(value, column)} disabled={disabled || !selected} @@ -166,7 +182,7 @@ export const DerivedSchemaFieldPicker = ({ }, ]; // `value` and `onChange` drive the two interactive cells; the rest are static. - }, [baseColumnByName, value, onChange, disabled]); + }, [baseColumnByName, indexOptions, value, onChange, disabled]); if (!basePath || !baseVersion) { return ( @@ -179,12 +195,16 @@ export const DerivedSchemaFieldPicker = ({ ); } - if (isError) { + if (isError || vocabularyError) { return ( ); } @@ -220,7 +240,7 @@ export const DerivedSchemaFieldPicker = ({ columns={tableColumns} dataSource={visibleItems} rowKey="name" - loading={isLoading || isFetchingNextPage} + loading={isLoading || isLoadingVocabulary || isFetchingNextPage} pagination={false} scroll={{ y: 360, x: 'max-content' }} rowSelection={{ diff --git a/apps/dfe-core-ui/src/Schemas/components/ListSchemaDetail/ViewSchemaDetails.tsx b/apps/dfe-core-ui/src/Schemas/components/ListSchemaDetail/ViewSchemaDetails.tsx index 0276fadd..9cec58ae 100644 --- a/apps/dfe-core-ui/src/Schemas/components/ListSchemaDetail/ViewSchemaDetails.tsx +++ b/apps/dfe-core-ui/src/Schemas/components/ListSchemaDetail/ViewSchemaDetails.tsx @@ -188,6 +188,12 @@ export const ViewSchemaDetails = ({ ) ?? false, }), }, + { + title: 'Cardinality', + dataIndex: 'cardinality', + key: 'cardinality', + render: (cardinality?: string | null) => cardinality || undefined, + }, { dataIndex: 'use_case', key: 'use_case', @@ -343,7 +349,14 @@ export const ViewSchemaDetails = ({ columns={tableColumns} - visibleColumns={['_field_type', 'name', 'type', 'expr', 'comment']} + visibleColumns={[ + '_field_type', + 'name', + 'type', + 'cardinality', + 'expr', + 'comment', + ]} lockedColumns={['_field_type', 'name']} showSearchableColumns={showSearchableColumns} title={() => ( diff --git a/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.test.ts b/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.test.ts index 746f081f..4ad8f311 100644 --- a/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.test.ts +++ b/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.test.ts @@ -1,52 +1,43 @@ +import { SCHEMA_VOCABULARY_FIXTURE } from '@/core/hooks/useFetchSchemaVocabulary/vocabulary.fixture'; import { - INDEX_USE_CASE_LABELS, - INDEX_USE_CASE_OPTIONS, + indexUseCaseOptions, resolveIndexUseCase, } from '@/Schemas/constants/indexUseCase'; import { describe, expect, it } from 'vitest'; +const vocabulary = SCHEMA_VOCABULARY_FIXTURE; + describe('resolveIndexUseCase', () => { - it('keeps a use case that is already current', () => { - expect(resolveIndexUseCase('dimension')).toBe('dimension'); - expect(resolveIndexUseCase('key_search')).toBe('key_search'); + it('keeps a use case the vocabulary has', () => { + expect(resolveIndexUseCase('dimension', vocabulary)).toBe('dimension'); + expect(resolveIndexUseCase('key_search', vocabulary)).toBe('key_search'); }); - it('maps the pre-rename vocabulary a base column may still carry', () => { - expect(resolveIndexUseCase('fulltext')).toBe('word_search'); - expect(resolveIndexUseCase('text_search')).toBe('substring_search'); - expect(resolveIndexUseCase('bloom')).toBe('exact_match'); + it('maps a retired name to the use case that replaced it', () => { + expect(resolveIndexUseCase('fulltext', vocabulary)).toBe('word_search'); + expect(resolveIndexUseCase('text_search', vocabulary)).toBe( + 'substring_search', + ); + expect(resolveIndexUseCase('bloom', vocabulary)).toBe('exact_match'); }); it('treats an empty, missing or unknown value as no index', () => { - expect(resolveIndexUseCase(undefined)).toBe('none'); - expect(resolveIndexUseCase(null)).toBe('none'); - expect(resolveIndexUseCase(' ')).toBe('none'); - expect(resolveIndexUseCase('tokenbf_v1')).toBe('none'); + expect(resolveIndexUseCase(undefined, vocabulary)).toBe('none'); + expect(resolveIndexUseCase(null, vocabulary)).toBe('none'); + expect(resolveIndexUseCase(' ', vocabulary)).toBe('none'); + expect(resolveIndexUseCase('tokenbf_v1', vocabulary)).toBe('none'); }); it('ignores case and surrounding space', () => { - expect(resolveIndexUseCase(' Fulltext ')).toBe('word_search'); + expect(resolveIndexUseCase(' Fulltext ', vocabulary)).toBe('word_search'); }); }); -describe('index use case labels', () => { - it('names the question, never the ClickHouse primitive', () => { - const labels = Object.values(INDEX_USE_CASE_LABELS).join(' ').toLowerCase(); - for (const primitive of [ - 'set(0)', - 'minmax', - 'bloom_filter', - 'tokenbf', - 'ngrambf', - 'tokenizer', - 'granularity', - ]) { - expect(labels).not.toContain(primitive); - } - }); - - it('offers every use case as an option', () => { - expect(INDEX_USE_CASE_OPTIONS.map((option) => option.value)).toEqual([ +describe('indexUseCaseOptions', () => { + it('offers the use cases that take no argument, then no index', () => { + expect( + indexUseCaseOptions(vocabulary).map((option) => option.value), + ).toEqual([ 'dimension', 'exact_match', 'range', @@ -55,9 +46,35 @@ describe('index use case labels', () => { 'key_search', 'none', ]); - expect( - INDEX_USE_CASE_OPTIONS.find((option) => option.value === 'word_search') - ?.label, - ).toBe('I search for whole words'); + }); + + it('labels each by name and gives the question it answers as the hint, never a ClickHouse primitive', () => { + const options = indexUseCaseOptions(vocabulary); + + expect(options.find((option) => option.value === 'word_search')).toEqual({ + label: 'Word search', + value: 'word_search', + title: 'I search for whole words', + }); + const text = options + .flatMap((option) => [option.label, option.title]) + .join(' ') + .toLowerCase(); + for (const primitive of ['set(0)', 'minmax', 'bloom_filter', 'tokenbf']) { + expect(text).not.toContain(primitive); + } + }); + + it('has no hint when a use case has no description', () => { + const bare = { + ...vocabulary, + use_cases: [{ ...vocabulary.use_cases[0], description: '' }], + }; + + expect(indexUseCaseOptions(bare)[0]).toEqual({ + label: 'Dimension', + value: 'dimension', + title: undefined, + }); }); }); diff --git a/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.ts b/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.ts index de6e3696..1d99a8b9 100644 --- a/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.ts +++ b/apps/dfe-core-ui/src/Schemas/constants/indexUseCase.ts @@ -1,65 +1,50 @@ +import { indexOptionsFor } from '@/core/hooks/useFetchSchemaVocabulary/options'; +import type { TSchemaVocabularyResponse } from '@/core/hooks/useFetchSchemaVocabulary/types'; + /** * Index use cases: the question the user wants to ask of a column. * - * The engine turns the use case into a ClickHouse index, so nothing here names - * an index type, a tokenizer or a granularity. + * The engine owns the list and serves it with the schema vocabulary. It turns a + * use case into a ClickHouse index, so nothing here names an index type, a + * tokenizer or a granularity. */ -export const INDEX_USE_CASES = [ - 'dimension', - 'exact_match', - 'range', - 'word_search', - 'substring_search', - 'key_search', - 'none', -] as const; - -export type IndexUseCase = (typeof INDEX_USE_CASES)[number]; -/** The question each use case answers, which is what the picker shows. */ -export const INDEX_USE_CASE_LABELS: Record = { - dimension: 'I filter and group by this', - exact_match: 'I look up specific values', - range: 'I query ranges, between, time windows', - word_search: 'I search for whole words', - substring_search: 'I search for fragments inside words', - key_search: 'I search the keys and values of a map column', - none: 'No index', -}; +/** A use case name from the engine's vocabulary, or {@link NO_INDEX}. */ +export type IndexUseCase = string; -export const INDEX_USE_CASE_OPTIONS: { - label: string; - value: IndexUseCase; -}[] = INDEX_USE_CASES.map((useCase) => ({ - label: INDEX_USE_CASE_LABELS[useCase], - value: useCase, -})); - -/** A base column written before the rename still carries the old name. */ -const RENAMED_INDEX_USE_CASES: Record = { - fulltext: 'word_search', - text_search: 'substring_search', - bloom: 'exact_match', -}; +/** Not a use case of the engine's: the choice to carry no index. */ +export const NO_INDEX = 'none'; -const isIndexUseCase = (value: string): value is IndexUseCase => - (INDEX_USE_CASES as readonly string[]).includes(value); +/** What the picker offers: the engine's use cases, then no index. */ +export const indexUseCaseOptions = ( + vocabulary: TSchemaVocabularyResponse, +): { label: string; value: IndexUseCase; title?: string }[] => [ + ...indexOptionsFor(vocabulary), + { label: 'No index', value: NO_INDEX }, +]; /** * A base column's index use case in the current vocabulary. * - * Returns `none` for a column with no index and for a value this build does not - * know, so an unreadable base never turns into an index nobody asked for. + * A name the engine retired maps to the one that replaced it. Returns `none` + * for a column with no index and for a value the vocabulary does not know, so an + * unreadable base never turns into an index nobody asked for. */ export const resolveIndexUseCase = ( value: string | null | undefined, + vocabulary: TSchemaVocabularyResponse, ): IndexUseCase => { const normalised = (value ?? '').trim().toLowerCase(); if (normalised === '') { - return 'none'; + return NO_INDEX; } - if (isIndexUseCase(normalised)) { - return normalised; + for (const useCase of vocabulary.use_cases) { + if (useCase.name === normalised) { + return useCase.name; + } + if (useCase.retired_names.includes(normalised)) { + return useCase.name; + } } - return RENAMED_INDEX_USE_CASES[normalised] ?? 'none'; + return NO_INDEX; }; diff --git a/apps/dfe-core-ui/src/Schemas/validationSchemas/CreateDerivedSchemaForm.schema.ts b/apps/dfe-core-ui/src/Schemas/validationSchemas/CreateDerivedSchemaForm.schema.ts index f0235db7..20230efb 100644 --- a/apps/dfe-core-ui/src/Schemas/validationSchemas/CreateDerivedSchemaForm.schema.ts +++ b/apps/dfe-core-ui/src/Schemas/validationSchemas/CreateDerivedSchemaForm.schema.ts @@ -3,12 +3,12 @@ import { schemaNameValidator, schemaVersionValidator, } from '@/core/validationSchemas/CreateSchemaForm/utils'; -import { INDEX_USE_CASES } from '@/Schemas/constants/indexUseCase'; import z from 'zod'; const selectEntrySchema = z.object({ name: z.string().min(1, { message: 'Column name is required' }), - index: z.enum(INDEX_USE_CASES).optional(), + // The engine owns the use case names and refuses one it does not know. + index: z.string().min(1).optional(), }); export const formSchema = z.object({ diff --git a/apps/dfe-core-ui/src/Sources/components/CreateUpdateSourceForm/SchemaConfigTabContent/MetaSchemaForm.mocks.ts b/apps/dfe-core-ui/src/Sources/components/CreateUpdateSourceForm/SchemaConfigTabContent/MetaSchemaForm.mocks.ts index 026357d6..53cd718e 100644 --- a/apps/dfe-core-ui/src/Sources/components/CreateUpdateSourceForm/SchemaConfigTabContent/MetaSchemaForm.mocks.ts +++ b/apps/dfe-core-ui/src/Sources/components/CreateUpdateSourceForm/SchemaConfigTabContent/MetaSchemaForm.mocks.ts @@ -31,6 +31,7 @@ export const server = setupServer( stored: null, origin: 'deployment', deployment_default: 90, + maximum: 36500, }, common_header_type: { effective: 'common-header/timeseries', diff --git a/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/ConfigurationDetailsTabContent/ConfigurationDetailsTabContent.mocks.ts b/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/ConfigurationDetailsTabContent/ConfigurationDetailsTabContent.mocks.ts index c11d3df9..a2cc532f 100644 --- a/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/ConfigurationDetailsTabContent/ConfigurationDetailsTabContent.mocks.ts +++ b/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/ConfigurationDetailsTabContent/ConfigurationDetailsTabContent.mocks.ts @@ -9,6 +9,7 @@ const handlers = [ stored: 90, origin: 'deployment', deployment_default: 90, + maximum: 36500, }, common_header_type: { effective: 'common-header/timeseries', diff --git a/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/SourceColumnsTabContent.test.tsx b/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/SourceColumnsTabContent.test.tsx new file mode 100644 index 00000000..6a7fc9bb --- /dev/null +++ b/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/SourceColumnsTabContent.test.tsx @@ -0,0 +1,27 @@ +import { buildTestWrapper } from '@/core/utils/test-utils/buildTestWrapper'; +import { + MOCK_SOURCE_NAME, + server, +} from '@/Sources/hooks/useFetchInfiniteSourceColumns/useFetchInfiniteSourceColumns.mocks'; +import { render, screen } from '@testing-library/react'; +import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { SourceColumnsTabContent } from '.'; + +beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); +afterEach(() => server.resetHandlers()); +afterAll(() => server.close()); + +const { wrapper } = buildTestWrapper().withTheme().withReactQuery(); + +describe('SourceColumnsTabContent', () => { + it('shows each column cardinality beside its attributes', async () => { + render(, { + wrapper, + }); + + expect((await screen.findAllByText('Cardinality')).length).toBeGreaterThan( + 0, + ); + expect((await screen.findAllByText('low')).length).toBeGreaterThan(0); + }); +}); diff --git a/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/index.tsx b/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/index.tsx index 8348cbad..ef318194 100644 --- a/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/index.tsx +++ b/apps/dfe-core-ui/src/Sources/components/ViewSourceTabs/SourceColumnsTabContent/index.tsx @@ -119,6 +119,11 @@ export const SourceColumnsTabContent = ({ dataIndex: 'attribute', key: 'attribute', }, + { + title: 'Cardinality', + dataIndex: 'cardinality', + key: 'cardinality', + }, { title: 'Index Type', dataIndex: 'use_case', @@ -147,7 +152,14 @@ export const SourceColumnsTabContent = ({

)} - visibleColumns={['name', 'type', 'attribute', 'use_case', 'comment']} + visibleColumns={[ + 'name', + 'type', + 'attribute', + 'cardinality', + 'use_case', + 'comment', + ]} lockedColumns={['_field_type', 'name']} dataSource={sourceColumns ?? []} columns={columns} diff --git a/apps/dfe-core-ui/src/Sources/hooks/useFetchInfiniteSourceColumns/useFetchInfiniteSourceColumns.mocks.tsx b/apps/dfe-core-ui/src/Sources/hooks/useFetchInfiniteSourceColumns/useFetchInfiniteSourceColumns.mocks.tsx index f9546ee8..4696b083 100644 --- a/apps/dfe-core-ui/src/Sources/hooks/useFetchInfiniteSourceColumns/useFetchInfiniteSourceColumns.mocks.tsx +++ b/apps/dfe-core-ui/src/Sources/hooks/useFetchInfiniteSourceColumns/useFetchInfiniteSourceColumns.mocks.tsx @@ -11,6 +11,7 @@ const SOURCE_COLUMN_ITEM: TSourceColumnItem = { type: 'string', use_case: 'string', attribute: ['string'], + cardinality: 'low', description: 'string', }; diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.test.ts b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.test.ts index d4248c2b..b0a9212c 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.test.ts +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.test.ts @@ -1,7 +1,10 @@ import { SCHEMA_FIELD_TYPES } from '@/core/components/CreateSchemaForm/fieldType.constants'; import { RowFormSchema } from '@/core/validationSchemas/CreateSchemaForm/AddSchemaTable.schema'; import { describe, expect, test } from 'vitest'; -import { listItemFromPartial } from './AddSchemaTable.helpers'; +import { + foldRetiredAttributes, + listItemFromPartial, +} from './AddSchemaTable.helpers'; import type { SchemaColumnRow } from './types'; describe('listItemFromPartial', () => { @@ -149,3 +152,94 @@ describe('listItemFromPartial', () => { }); }); }); + +describe('listItemFromPartial cardinality', () => { + const column = (extra: Partial): SchemaColumnRow => ({ + id: '1', + name: 'host', + type: 'string', + _field_type: SCHEMA_FIELD_TYPES.USER_DEFINED, + ...extra, + }); + + test('keeps a declared cardinality', () => { + expect( + listItemFromPartial(column({ cardinality: 'high' })).cardinality, + ).toBe('high'); + }); + + test('a cardinality the API does not know is dropped', () => { + const row = listItemFromPartial( + column({ cardinality: 'huge' } as unknown as Partial), + ); + + expect(row.cardinality).toBeUndefined(); + }); + + test('reads a Cardinality CSV-style key', () => { + const row = listItemFromPartial({ + id: '1', + Name: 'host', + Type: 'string', + Cardinality: 'low', + } as unknown as SchemaColumnRow); + + expect(row.cardinality).toBe('low'); + }); +}); + +describe('foldRetiredAttributes', () => { + const retired = { lowcardinality: 'low' }; + + test.each([ + [ + 'a declared cardinality is kept', + 'high' as const, + ['nullable'], + 'high', + ['nullable'], + ], + [ + 'the retired attribute reads as the cardinality it names', + undefined, + ['lowcardinality', 'not_null'], + 'low', + ['not_null'], + ], + [ + 'a declared cardinality wins and the attribute is left alone', + 'high' as const, + ['lowcardinality'], + 'high', + ['lowcardinality'], + ], + [ + 'a column with neither has no cardinality', + undefined, + ['nullable'], + undefined, + ['nullable'], + ], + ])( + '%s', + ( + _label, + cardinality, + attribute, + expectedCardinality, + expectedAttribute, + ) => { + expect(foldRetiredAttributes(cardinality, attribute, retired)).toEqual({ + cardinality: expectedCardinality, + attribute: expectedAttribute, + }); + }, + ); + + test('an attribute the engine has no meaning for stays', () => { + expect(foldRetiredAttributes(undefined, ['lowcardinality'], {})).toEqual({ + cardinality: undefined, + attribute: ['lowcardinality'], + }); + }); +}); diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.ts b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.ts index 8beadbc9..cda797c7 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.ts +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.helpers.ts @@ -37,6 +37,35 @@ const normalizeAttribute = (raw: unknown): string[] => { return []; }; +type Cardinality = z.infer['cardinality']; + +export const asCardinality = (raw: unknown): Cardinality => { + const parsed = rowSchema.shape.cardinality.safeParse(raw); + return parsed.success ? parsed.data : undefined; +}; + +/** + * A column that still carries a retired attribute for its cardinality, such as + * `lowcardinality`, reads as that cardinality and no longer holds the attribute. + * The engine refuses a column that declares both and disagrees with itself. + */ +export const foldRetiredAttributes = ( + cardinality: Cardinality, + attribute: string[], + retired: Record, +) => { + if (cardinality) { + return { cardinality, attribute }; + } + const folded = attribute.find((name) => name in retired); + return folded + ? { + cardinality: asCardinality(retired[folded]), + attribute: attribute.filter((name) => name !== folded), + } + : { cardinality, attribute }; +}; + export const listItemFromPartial = ( column: Partial, ): z.infer => { @@ -54,6 +83,10 @@ export const listItemFromPartial = ( rowLookup(column, 'comment', 'Comment') ?? column.comment ?? '', ); + const declaredCardinality = asCardinality( + rowLookup(column, 'cardinality', 'Cardinality') ?? column.cardinality, + ); + const ch_override = String( rowLookup(column, 'ch_override', 'chOverride') ?? column.ch_override ?? '', ); @@ -64,6 +97,7 @@ export const listItemFromPartial = ( name, type, ch_override: ch_override || undefined, + cardinality: declaredCardinality, attribute: normalizeAttribute(attributeRaw), use_case, expr, diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.test.tsx b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.test.tsx index 4503c484..32711ff1 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.test.tsx +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/AddSchemaTable.test.tsx @@ -1,13 +1,26 @@ import { SCHEMA_FIELD_TYPES } from '@/core/components/CreateSchemaForm/fieldType.constants'; import { UploadedSchemaRow } from '@/core/components/CreateSchemaForm/types'; import { Form as AntdForm, Form } from '@/core/components/Form'; +import { API_CONFIG_MOCKS } from '@/core/config/api/endpoints/generator'; +import { server } from '@/core/hooks/useFetchSchemaVocabulary/useFetchSchemaVocabulary.mocks'; import { buildTestWrapper } from '@/core/utils/test-utils/buildTestWrapper'; import { rowSchema } from '@/core/validationSchemas/CreateSchemaForm/AddSchemaTable.schema'; import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { delay, http, HttpResponse } from 'msw'; +import { SCHEMA_VOCABULARY_FIXTURE } from '@/core/hooks/useFetchSchemaVocabulary/vocabulary.fixture'; import type { FormInstance, FormRule } from 'antd'; import { useEffect } from 'react'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, + vi, +} from 'vitest'; import { AddSchemaTable, isBlankSchemaListRow, @@ -19,7 +32,11 @@ const bypassFormValidation = { validator: async () => Promise.resolve(), } as FormRule; -const { wrapper } = buildTestWrapper(); +beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); +afterEach(() => server.resetHandlers()); +afterAll(() => server.close()); + +const { wrapper } = buildTestWrapper().withReactQuery(); const validRow = (overrides: Partial = {}): RowSchema => ({ id: 'row-1', @@ -500,6 +517,124 @@ describe('AddSchemaTable', { timeout: 15_000 }, () => { }); }); + test('offers the primitives the engine serves', async () => { + const user = userEvent.setup(); + renderAddSchemaTable({ + initialValues: [validRow()], + visibleColumns: ['name', 'type', '__rowId'], + config: { defaultEditFields: ['type'] }, + }); + + await user.click((await screen.findAllByRole('combobox'))[0]); + + expect(await screen.findByText('Map')).toBeInTheDocument(); + expect(screen.getByText('Vector')).toBeInTheDocument(); + }); + + test.each([ + ['string', ['Word search'], ['Key search', 'Range']], + ['map', ['Key search'], ['Word search', 'Dimension']], + ])( + 'offers the use cases valid for a %s column, by label, never a retired name', + async (type, offered, absent) => { + const user = userEvent.setup(); + renderAddSchemaTable({ + initialValues: [validRow({ type })], + visibleColumns: ['name', 'type', 'use_case', '__rowId'], + config: { defaultEditFields: ['use_case'] }, + }); + + await user.click(await screen.findByRole('combobox')); + + for (const label of offered) { + expect(await screen.findByText(label)).toBeInTheDocument(); + } + for (const label of [...absent, 'Fulltext', 'Bloom']) { + expect(screen.queryByText(label)).not.toBeInTheDocument(); + } + }, + ); + + test('a use case carries the question it answers as its hint', async () => { + const user = userEvent.setup(); + renderAddSchemaTable({ + initialValues: [validRow()], + visibleColumns: ['name', 'type', 'use_case', '__rowId'], + config: { defaultEditFields: ['use_case'] }, + }); + + await user.click(await screen.findByRole('combobox')); + + expect(await screen.findByTitle('I search for whole words')).toBeVisible(); + }); + + test('a column with no cardinality shows the one the engine reads it as', async () => { + renderAddSchemaTable({ + initialValues: [validRow()], + visibleColumns: ['name', 'cardinality', '__rowId'], + }); + + expect(await screen.findByText('unknown')).toBeInTheDocument(); + }); + + test('a retired lowcardinality attribute shows as Cardinality Low', async () => { + const { onColumns } = renderAddSchemaTable({ + initialValues: [validRow({ attribute: ['lowcardinality'] })], + visibleColumns: ['name', 'cardinality', 'attribute', '__rowId'], + }); + + expect(await screen.findByText('low')).toBeInTheDocument(); + await waitFor(() => + expect(onColumns).toHaveBeenLastCalledWith([ + expect.objectContaining({ cardinality: 'low', attribute: [] }), + ]), + ); + }); + + test('a stored retired use case name shows as the one that replaced it', async () => { + const { onColumns } = renderAddSchemaTable({ + initialValues: [validRow({ use_case: 'fulltext' })], + visibleColumns: ['name', 'use_case', '__rowId'], + }); + + expect(await screen.findByText('word_search')).toBeInTheDocument(); + await waitFor(() => + expect(onColumns).toHaveBeenLastCalledWith([ + expect.objectContaining({ use_case: 'word_search' }), + ]), + ); + }); + + test('a select says it is loading, not that there is no data, while the vocabulary loads', async () => { + server.use( + http.get(API_CONFIG_MOCKS.schemas.vocabulary.mockedUrl, async () => { + await delay(300); + return HttpResponse.json(SCHEMA_VOCABULARY_FIXTURE); + }), + ); + const user = userEvent.setup(); + renderAddSchemaTable({ + initialValues: [validRow()], + visibleColumns: ['name', 'type', '__rowId'], + config: { defaultEditFields: ['type'] }, + }); + + await user.click((await screen.findAllByRole('combobox'))[0]); + + expect(await screen.findByText('Loading...')).toBeInTheDocument(); + expect(screen.queryByText('No data')).not.toBeInTheDocument(); + expect(await screen.findByText('Map')).toBeInTheDocument(); + }); + + test('says so when the vocabulary cannot be loaded', async () => { + server.use(API_CONFIG_MOCKS.schemas.vocabulary.get.error()); + renderAddSchemaTable(); + + expect( + await screen.findByText(/The column types could not be loaded/), + ).toBeInTheDocument(); + }); + test('forwards table props to the underlying table', () => { renderAddSchemaTable({ className: 'custom-schema-table' }); diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/LazyMountedSelect.tsx b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/LazyMountedSelect.tsx index 6dd36873..71ad6b75 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/LazyMountedSelect.tsx +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/LazyMountedSelect.tsx @@ -1,9 +1,10 @@ import { - ADVANCED_OPTIONS, + overrideSelectOptions, TransformFunctionParams, } from '@/core/components/CreateSchemaForm/AddSchemaTable/fieldOptions.constants'; import { Form } from '@/core/components/Form'; import { FormNotification } from '@/core/components/FormNotification'; +import { useFetchSchemaVocabulary } from '@/core/hooks/useFetchSchemaVocabulary'; import { Button, FormInstance, @@ -29,35 +30,49 @@ interface LazyMountedSelectProps extends SelectProps { form: FormInstance; } +const NO_OVERRIDES: never[] = []; + export const LazyMountedSelect = ({ typeDetails, form, onChange, ...props }: LazyMountedSelectProps) => { + const { data: vocabulary } = useFetchSchemaVocabulary(); + const overrides = vocabulary?.ch_overrides ?? NO_OVERRIDES; const [overrideType, setOverrideType] = useState( - getInitialOverrideType(props.value), + getInitialOverrideType(props.value, overrides), ); const [validationError, setValidationError] = useState(null); const [advancedOptions, setAdvancedOptions] = - useState(getInitialAdvancedOptions(props.value)); + useState( + getInitialAdvancedOptions(props.value, overrides), + ); // Only fetch the form values when the select is mounted const baseTypeValue = Form.useWatch(typeDetails.type_form_name, form); const options = useMemo(() => { if (!baseTypeValue) return []; - return ADVANCED_OPTIONS.filter( - (option) => option.primitive === baseTypeValue, - ); - }, [baseTypeValue]); + return overrideSelectOptions(overrides, baseTypeValue); + }, [baseTypeValue, overrides]); + + const parameters = + overrides.find((option) => option.value === overrideType)?.parameters ?? []; + const numberParameters = parameters.filter(({ kind }) => kind === 'integer'); + const textParameters = parameters.filter(({ kind }) => kind === 'text'); + + const setParameter = (name: string, value: number | string | undefined) => { + setValidationError(null); + setAdvancedOptions({ ...advancedOptions, [name]: value }); + }; const handleApplyOverride = () => { - const error = validateOverride(overrideType, advancedOptions); + const error = validateOverride(overrideType, advancedOptions, overrides); if (error) { setValidationError(error); return; } - onChange?.(transformOverride(overrideType, advancedOptions)); + onChange?.(transformOverride(overrideType, advancedOptions, overrides)); }; return ( @@ -74,80 +89,36 @@ export const LazyMountedSelect = ({ value={overrideType} /> - {(overrideType === 'Decimal' || - overrideType === 'Decimal64' || - overrideType === 'DateTime64') && ( + {numberParameters.length > 0 && (
- {overrideType !== 'Decimal64' && ( - - + {numberParameters.map(({ name, label }) => ( + + { - setValidationError(null); - setAdvancedOptions({ - ...advancedOptions, - precision: value || undefined, - }); - }} - placeholder="Input Precision" + value={advancedOptions[name] as number | undefined} + onChange={(value: number | null) => + setParameter(name, value || undefined) + } + placeholder={`Input ${label}`} /> - )} - {overrideType !== 'DateTime64' && ( - - - { - setValidationError(null); - setAdvancedOptions({ - ...advancedOptions, - scale: value || undefined, - }); - }} - /> - - )} + ))}
)} - {overrideType === 'FixedString' && ( - - - { - setValidationError(null); - setAdvancedOptions({ - ...advancedOptions, - length: value || undefined, - }); - }} - /> - - )} - {overrideType === 'Enum16' && ( - - + {textParameters.map(({ name, label }) => ( + + ) => { - setValidationError(null); - setAdvancedOptions({ - ...advancedOptions, - values: e.target.value, - }); - }} + placeholder={`Input ${label.split(' (')[0]}`} + value={advancedOptions[name] as string | undefined} + onChange={(e: React.ChangeEvent) => + setParameter(name, e.target.value) + } /> - )} + ))} {validationError && ( )} diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.test.tsx b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.test.tsx index 6efae319..1b5f172a 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.test.tsx +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.test.tsx @@ -1,3 +1,4 @@ +import { SCHEMA_VOCABULARY_FIXTURE } from '@/core/hooks/useFetchSchemaVocabulary/vocabulary.fixture'; import { describe, expect, test } from 'vitest'; import { getInitialAdvancedOptions, @@ -6,162 +7,207 @@ import { validateOverride, } from './TypeOverrideSelect.helpers'; +const options = SCHEMA_VOCABULARY_FIXTURE.ch_overrides; + describe('validateOverride', () => { test('returns null when override is null or empty', () => { - expect(validateOverride(null, {})).toBeNull(); - expect(validateOverride('', {})).toBeNull(); + expect(validateOverride(null, {}, options)).toBeNull(); + expect(validateOverride('', {}, options)).toBeNull(); }); test('returns null for overrides that need no params', () => { - expect(validateOverride('Int64', {})).toBeNull(); - expect(validateOverride('IPv4', {})).toBeNull(); + expect(validateOverride('Int64', {}, options)).toBeNull(); + expect(validateOverride('IPv4', {}, options)).toBeNull(); }); test('requires precision and scale for Decimal', () => { - expect(validateOverride('Decimal', {})).toBe( - 'Precision and scale are required', + expect(validateOverride('Decimal', {}, options)).toBe( + 'Precision and scale required', ); - expect(validateOverride('Decimal', { precision: 10 })).toBe( - 'Precision and scale are required', + expect(validateOverride('Decimal', { precision: 10 }, options)).toBe( + 'Scale required', ); - expect(validateOverride('Decimal', { scale: 2 })).toBe( - 'Precision and scale are required', + expect(validateOverride('Decimal', { scale: 2 }, options)).toBe( + 'Precision required', ); - expect(validateOverride('Decimal', { precision: 10, scale: 2 })).toBeNull(); + expect( + validateOverride('Decimal', { precision: 10, scale: 2 }, options), + ).toBeNull(); }); test('requires scale for Decimal64', () => { - expect(validateOverride('Decimal64', {})).toBe('Scale is required'); - expect(validateOverride('Decimal64', { scale: 4 })).toBeNull(); + expect(validateOverride('Decimal64', {}, options)).toBe('Scale required'); + expect(validateOverride('Decimal64', { scale: 4 }, options)).toBeNull(); }); test('requires precision for DateTime64', () => { - expect(validateOverride('DateTime64', {})).toBe('Precision is required'); - expect(validateOverride('DateTime64', { precision: 3 })).toBeNull(); + expect(validateOverride('DateTime64', {}, options)).toBe( + 'Precision required', + ); + expect( + validateOverride('DateTime64', { precision: 3 }, options), + ).toBeNull(); }); test('requires length for FixedString', () => { - expect(validateOverride('FixedString', {})).toBe('Length is required'); - expect(validateOverride('FixedString', { length: 16 })).toBeNull(); + expect(validateOverride('FixedString', {}, options)).toBe( + 'Length required', + ); + expect(validateOverride('FixedString', { length: 16 }, options)).toBeNull(); }); test('requires values for Enum16', () => { - expect(validateOverride('Enum16', {})).toBe('Values are required'); + expect(validateOverride('Enum16', {}, options)).toBe('Values required'); expect( - validateOverride('Enum16', { values: "'a' = 1, 'b' = 2" }), + validateOverride('Enum16', { values: "'a' = 1, 'b' = 2" }, options), ).toBeNull(); }); }); describe('transformOverride', () => { test('returns null when override type is null or empty', () => { - expect(transformOverride(null, {})).toBeNull(); - expect(transformOverride('', {})).toBeNull(); + expect(transformOverride(null, {}, options)).toBeNull(); + expect(transformOverride('', {}, options)).toBeNull(); }); test('returns the override type when no transformFunction exists', () => { - expect(transformOverride('Int64', {})).toBe('Int64'); + expect(transformOverride('Int64', {}, options)).toBe('Int64'); }); test('returns null for unknown override types', () => { - expect(transformOverride('NotARealType', {})).toBeNull(); + expect(transformOverride('NotARealType', {}, options)).toBeNull(); }); test('applies Decimal transform', () => { - expect(transformOverride('Decimal', { precision: 10, scale: 2 })).toBe( - 'Decimal(10,2)', - ); + expect( + transformOverride('Decimal', { precision: 10, scale: 2 }, options), + ).toBe('Decimal(10,2)'); }); test('applies Decimal64 transform', () => { - expect(transformOverride('Decimal64', { scale: 4 })).toBe('Decimal64(4)'); + expect(transformOverride('Decimal64', { scale: 4 }, options)).toBe( + 'Decimal64(4)', + ); }); test('applies FixedString transform', () => { - expect(transformOverride('FixedString', { length: 32 })).toBe( + expect(transformOverride('FixedString', { length: 32 }, options)).toBe( 'FixedString(32)', ); }); test('applies DateTime64 transform', () => { - expect(transformOverride('DateTime64', { precision: 3 })).toBe( + expect(transformOverride('DateTime64', { precision: 3 }, options)).toBe( "DateTime64(3,'UTC')", ); }); test('applies Enum16 transform', () => { - expect(transformOverride('Enum16', { values: "'ok' = 1, 'err' = 2" })).toBe( - "Enum16('ok' = 1, 'err' = 2)", - ); + expect( + transformOverride('Enum16', { values: "'ok' = 1, 'err' = 2" }, options), + ).toBe("Enum16('ok' = 1, 'err' = 2)"); }); }); describe('getInitialOverrideType', () => { test('returns null for null or empty values', () => { - expect(getInitialOverrideType(null)).toBeNull(); - expect(getInitialOverrideType('')).toBeNull(); + expect(getInitialOverrideType(null, options)).toBeNull(); + expect(getInitialOverrideType('', options)).toBeNull(); }); test('returns the bare type when there are no params', () => { - expect(getInitialOverrideType('Int64')).toBe('Int64'); + expect(getInitialOverrideType('Int64', options)).toBe('Int64'); }); test('strips parenthetical params', () => { - expect(getInitialOverrideType('Decimal(10, 2)')).toBe('Decimal'); - expect(getInitialOverrideType('Decimal64(4)')).toBe('Decimal64'); - expect(getInitialOverrideType("DateTime64(3, 'UTC')")).toBe('DateTime64'); - expect(getInitialOverrideType('FixedString(16)')).toBe('FixedString'); + expect(getInitialOverrideType('Decimal(10, 2)', options)).toBe('Decimal'); + expect(getInitialOverrideType('Decimal64(4)', options)).toBe('Decimal64'); + expect(getInitialOverrideType("DateTime64(3, 'UTC')", options)).toBe( + 'DateTime64', + ); + expect(getInitialOverrideType('FixedString(16)', options)).toBe( + 'FixedString', + ); }); test('trims whitespace around the type name', () => { - expect(getInitialOverrideType(' Decimal64 (4)')).toBe('Decimal64'); + expect(getInitialOverrideType(' Decimal64 (4)', options)).toBe( + 'Decimal64', + ); + }); +}); + +describe('an override with arguments and a bare one', () => { + test('a stored value that is an option whole keeps its type', () => { + const withWhole = [ + ...options, + { + value: 'Array(String)', + label: 'Array of String', + primitive: 'json', + parameters: [], + template: null, + hint: '', + }, + ]; + + expect(getInitialOverrideType('Array(String)', withWhole)).toBe( + 'Array(String)', + ); + expect(getInitialOverrideType('Decimal(10, 2)', withWhole)).toBe('Decimal'); }); }); describe('getInitialAdvancedOptions', () => { test('returns empty object for null or empty values', () => { - expect(getInitialAdvancedOptions(null)).toEqual({}); - expect(getInitialAdvancedOptions('')).toEqual({}); + expect(getInitialAdvancedOptions(null, options)).toEqual({}); + expect(getInitialAdvancedOptions('', options)).toEqual({}); }); test('returns empty object for overrides without params', () => { - expect(getInitialAdvancedOptions('Int64')).toEqual({}); + expect(getInitialAdvancedOptions('Int64', options)).toEqual({}); }); test('parses Decimal precision and scale', () => { - expect(getInitialAdvancedOptions('Decimal(18, 4)')).toEqual({ + expect(getInitialAdvancedOptions('Decimal(18, 4)', options)).toEqual({ precision: 18, scale: 4, }); }); test('parses Decimal64 scale', () => { - expect(getInitialAdvancedOptions('Decimal64(6)')).toEqual({ scale: 6 }); + expect(getInitialAdvancedOptions('Decimal64(6)', options)).toEqual({ + scale: 6, + }); }); test('parses DateTime64 precision from the first arg', () => { - expect(getInitialAdvancedOptions("DateTime64(3, 'UTC')")).toEqual({ + expect(getInitialAdvancedOptions("DateTime64(3, 'UTC')", options)).toEqual({ precision: 3, }); }); test('parses FixedString length', () => { - expect(getInitialAdvancedOptions('FixedString(64)')).toEqual({ + expect(getInitialAdvancedOptions('FixedString(64)', options)).toEqual({ length: 64, }); }); test('parses Enum16 values string', () => { - expect(getInitialAdvancedOptions("Enum16('a' = 1, 'b' = 2)")).toEqual({ + expect( + getInitialAdvancedOptions("Enum16('a' = 1, 'b' = 2)", options), + ).toEqual({ values: "'a' = 1, 'b' = 2", }); }); test('defaults missing numeric params to 0', () => { - expect(getInitialAdvancedOptions('Decimal()')).toEqual({ + expect(getInitialAdvancedOptions('Decimal()', options)).toEqual({ precision: 0, scale: 0, }); - expect(getInitialAdvancedOptions('FixedString()')).toEqual({ length: 0 }); + expect(getInitialAdvancedOptions('FixedString()', options)).toEqual({ + length: 0, + }); }); }); diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.ts b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.ts index 255e39e5..9982d971 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.ts +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/TypeOverrideSelect.helpers.ts @@ -1,99 +1,95 @@ -import { - ADVANCED_OPTIONS_MAP, - TransformFunctionParams, -} from '@/core/components/CreateSchemaForm/AddSchemaTable/fieldOptions.constants'; +import type { TransformFunctionParams } from '@/core/components/CreateSchemaForm/AddSchemaTable/fieldOptions.constants'; +import type { TSchemaVocabularyChOverride } from '@/core/hooks/useFetchSchemaVocabulary/types'; + +type OverrideOptions = TSchemaVocabularyChOverride[]; + +const findOption = ( + override: string | null, + options: OverrideOptions, +): TSchemaVocabularyChOverride | undefined => + options.find((option) => option.value === override); export const validateOverride = ( override: string | null, - options: TransformFunctionParams, + params: TransformFunctionParams, + options: OverrideOptions, ): string | null => { if (!override) return null; - switch (override) { - case 'Decimal': - if (options.precision === undefined || options.scale === undefined) { - return 'Precision and scale are required'; - } - return null; - case 'Decimal64': - if (options.scale === undefined) { - return 'Scale is required'; - } - return null; - case 'DateTime64': - if (options.precision === undefined) { - return 'Precision is required'; - } - return null; - case 'FixedString': - if (options.length === undefined) { - return 'Length is required'; - } - return null; - case 'Enum16': - if (options.values === undefined) { - return 'Values are required'; - } - return null; - default: - return null; - } + const missing = (findOption(override, options)?.parameters ?? []).filter( + ({ name }) => params[name] === undefined, + ); + if (missing.length === 0) return null; + + // A label such as `Values (comma separated)` names its input, so only the noun is used. + const [first, ...rest] = missing.map(({ label }) => label.split(' (')[0]); + const names = [first, ...rest.map((label) => label.toLowerCase())]; + return `${names.join(' and ')} required`; }; export const transformOverride = ( overrideType: string | null, - advancedOptions: TransformFunctionParams, + params: TransformFunctionParams, + options: OverrideOptions, ): string | null => { if (!overrideType) return null; - const option = ADVANCED_OPTIONS_MAP[overrideType]; + const option = findOption(overrideType, options); if (!option) return null; - return option.transformFunction?.(advancedOptions) ?? overrideType; + return ( + option.template?.replace(/\{(\w+)\}/g, (_, name: string) => + String(params[name] ?? ''), + ) ?? option.value + ); }; -export const getInitialOverrideType = (value: string | null) => { +/** + * The override type a stored value belongs to. A value that is an option whole, + * such as `Array(String)`, is that option. Any other has its arguments dropped, + * so `Decimal(10, 2)` is `Decimal`. + */ +export const getInitialOverrideType = ( + value: string | null, + options: OverrideOptions = [], +) => { if (!value) return null; + if (findOption(value, options)) return value; return value.split('(')[0].trim(); }; +const parseNum = (raw: string | undefined) => { + const parsed = parseInt(raw?.trim() ?? '0', 10); + return Number.isNaN(parsed) ? 0 : parsed; +}; + +/** + * The parameter values inside a stored override such as `Decimal(18, 4)`. + * + * Arguments are split on commas, and the last parameter takes whatever is left, + * so a text parameter may hold commas of its own. An integer reads as far as its + * first non-digit, which is how `3` comes out of `3, 'UTC'`. + */ export const getInitialAdvancedOptions = ( value: string | null, + options: OverrideOptions = [], ): TransformFunctionParams => { - const overrideType = getInitialOverrideType(value); - if (!overrideType) return {}; + const option = findOption(getInitialOverrideType(value, options), options); + if (!value || !option || option.parameters.length === 0) return {}; - const advancedOptions = value?.replace(')', '').split('(')[1]; - const parseNum = (raw: string | undefined) => { - const parsed = parseInt(raw?.trim() ?? '0', 10); - return Number.isNaN(parsed) ? 0 : parsed; - }; + const inner = value.slice( + value.indexOf('(') + 1, + value.lastIndexOf(')') === -1 ? undefined : value.lastIndexOf(')'), + ); + const pieces = inner.split(','); + const last = option.parameters.length - 1; - switch (overrideType) { - case 'Decimal': { - const [precision, scale] = (advancedOptions ?? '').split(','); - return { - precision: parseNum(precision), - scale: parseNum(scale), - }; - } - case 'Decimal64': - return { - scale: parseNum(advancedOptions), - }; - case 'DateTime64': - return { - precision: parseNum(advancedOptions?.split(',')[0]), - }; - case 'FixedString': - return { - length: parseNum(advancedOptions?.split(',')[0]), - }; - case 'Enum16': - return { - values: advancedOptions?.trim(), - }; - default: - return {}; - } + return Object.fromEntries( + option.parameters.map(({ name, kind }, index) => { + const raw = ( + index === last ? pieces.slice(index).join(',') : (pieces[index] ?? '') + ).trim(); + return [name, kind === 'integer' ? parseNum(raw) : raw]; + }), + ); }; diff --git a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/index.tsx b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/index.tsx index a8d025f9..0f3e6bd5 100644 --- a/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/index.tsx +++ b/apps/dfe-core-ui/src/core/components/CreateSchemaForm/AddSchemaTable/TypeOverrideSelect/index.tsx @@ -1,4 +1,5 @@ import { Popover } from '@/core/components/Popover'; +import { useFetchSchemaVocabulary } from '@/core/hooks/useFetchSchemaVocabulary'; import { IconDots, IconX } from '@repo/dfe-icons'; import { Button, FormInstance, SelectProps } from 'antd'; import { useState } from 'react'; @@ -19,11 +20,18 @@ export const TypeOverrideSelect = ({ ...props }: TypeOverrideSelectProps) => { const [open, setOpen] = useState(false); + const { data: vocabulary } = useFetchSchemaVocabulary(); + // A free-text argument can be long, so only the type name is shown for it. + const takesText = vocabulary?.ch_overrides.some( + (option) => + value?.startsWith(option.value) && + option.parameters.some(({ kind }) => kind === 'text'), + ); return (
{value && ( - {value.startsWith('Enum16') ? value.split('(')[0] : value} + {takesText ? value.split('(')[0] : value}
+ ); + }, }, - }, - { - title: 'Attributes', - dataIndex: 'attribute', - key: 'attribute', - width: 200, - render: (_: unknown, record: SchemaColumnListRow) => { - const { key: _rowKey, name: rowIndex, ...restField } = record; - return ( - - - - ); + { + title: 'Index Type', + dataIndex: 'use_case', + key: 'use_case', + render: (_: unknown, record: SchemaColumnListRow) => { + const { key: _rowKey, name: rowIndex, ...restField } = record; + return ( + + {() => ( + + + + )} + + ); + }, }, - }, - { - title: 'Expression (CTE)', - dataIndex: 'expr', - key: 'expr', - render: (_: unknown, record: SchemaColumnListRow) => { - const { key: _rowKey, name: rowIndex, ...restField } = record; - return ( - - - - ); + { + title: 'Attributes', + dataIndex: 'attribute', + key: 'attribute', + width: 200, + render: (_: unknown, record: SchemaColumnListRow) => { + const { key: _rowKey, name: rowIndex, ...restField } = record; + return ( + + + + ); + }, }, - }, - { - title: 'Comment', - dataIndex: 'comment', - key: 'comment', - render: (_: unknown, record: SchemaColumnListRow) => { - const { key: _rowKey, name: rowIndex, ...restField } = record; - return ( - - - - ); + { + title: 'Cardinality', + dataIndex: 'cardinality', + key: 'cardinality', + width: 180, + render: (_: unknown, record: SchemaColumnListRow) => { + const { key: _rowKey, name: rowIndex, ...restField } = record; + return ( + + + + ); + }, }, - }, - /** Ant Design Form only persists fields registered via Form.Item — `id` / `_field_type` must be stored for list merges and API mapping. */ - { - title: '', - key: '__rowId', - width: 0, - render: (_: unknown, record: SchemaColumnListRow) => { - const { key: _rowKey, name: rowIndex, ...restField } = record; - return ( - <> -