From a62e2c61284f780f784f5acfb04334470d0180c2 Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Sun, 7 Jun 2026 18:45:08 +0300 Subject: [PATCH] =?UTF-8?q?Disable=20dataset=20Save=20until=20the=20form?= =?UTF-8?q?=20is=20valid=20(=C2=A705)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/app/components/DatasetsModal.tsx | 16 ++++++- src/app/stores/DatasetStore.test.ts | 46 ++++++++++++++++++- src/app/stores/DatasetStore.ts | 69 ++++++++++++++++++++-------- 3 files changed, 108 insertions(+), 23 deletions(-) diff --git a/src/app/components/DatasetsModal.tsx b/src/app/components/DatasetsModal.tsx index 0c85820..d1c1158 100644 --- a/src/app/components/DatasetsModal.tsx +++ b/src/app/components/DatasetsModal.tsx @@ -19,7 +19,12 @@ import { detectFormat, detectFormatFromUrl, type DataFormat } from '@core/format import { closeModal, openModal, resnapshot } from '../modals/ModalCoordinator'; import { confirm } from '../stores/ConfirmStore'; import { notify } from '../stores/NotificationStore'; -import { selectSelectedDataset, useDatasetStore, byModifiedDesc } from '../stores/DatasetStore'; +import { + selectCanSave, + selectSelectedDataset, + useDatasetStore, + byModifiedDesc, +} from '../stores/DatasetStore'; import { useSnippetStore } from '../stores/SnippetStore'; import { SegmentedControl, type SegmentedOption } from './SegmentedControl'; import { Icon } from './Icon'; @@ -338,6 +343,8 @@ function DatasetFormView({ editing }: { editing: boolean }) { const updateForm = useDatasetStore((s) => s.updateForm); const cancelForm = useDatasetStore((s) => s.cancelForm); const save = useDatasetStore((s) => s.save); + // Save stays disabled until a name and valid data/URL are present (spec §05). + const canSave = useDatasetStore(selectCanSave); // Live format/source hint from the current input (spec §05 → Auto-detection). const detected = @@ -433,7 +440,12 @@ function DatasetFormView({ editing }: { editing: boolean }) { - diff --git a/src/app/stores/DatasetStore.test.ts b/src/app/stores/DatasetStore.test.ts index 3642da9..773c9e9 100644 --- a/src/app/stores/DatasetStore.test.ts +++ b/src/app/stores/DatasetStore.test.ts @@ -1,7 +1,12 @@ import { beforeEach, describe, expect, test } from 'vitest'; import { createDataset } from '@core/dataset'; import { createSnippet } from '@core/snippet'; -import { selectSelectedDataset, useDatasetStore, type DatasetForm } from './DatasetStore'; +import { + selectCanSave, + selectSelectedDataset, + useDatasetStore, + type DatasetForm, +} from './DatasetStore'; import { useSnippetStore } from './SnippetStore'; const store = () => useDatasetStore.getState(); @@ -82,6 +87,45 @@ describe('save — validation', () => { }); }); +describe('selectCanSave — drives the Save button disabled state (§05)', () => { + const canSaveWith = (patch: Partial) => { + store().startCreate(); + store().updateForm(patch); + return selectCanSave(useDatasetStore.getState()); + }; + + test('false until a name and valid data are present', () => { + store().startCreate(); + expect(selectCanSave(useDatasetStore.getState())).toBe(false); // empty form + expect(canSaveWith({ name: 'Sales' })).toBe(false); // no data + expect(canSaveWith({ name: ' ', input: '[{"a":1}]' })).toBe(false); // blank name + expect(canSaveWith({ name: 'Sales', input: 'not data' })).toBe(false); // unrecognized + }); + + test('true for valid inline data and valid URLs', () => { + expect(canSaveWith({ name: 'Sales', input: '[{"a":1,"b":2}]' })).toBe(true); + expect(canSaveWith({ name: 'Regions', input: 'city,pop\nA,10' })).toBe(true); + expect(canSaveWith({ name: 'Remote', source: 'url', input: 'https://x/y.csv' })).toBe(true); + }); + + test('false for a non-http URL', () => { + expect(canSaveWith({ name: 'Bad', source: 'url', input: 'ftp://x/y.csv' })).toBe(false); + }); + + test('false on a duplicate name, true once the editing dataset keeps its own name', () => { + store().add( + createDataset({ name: 'Sales', data: [{ a: 1 }], format: 'json', source: 'inline', now: T }), + ); + const id = store().datasets[0].id; + // Creating another "Sales" is blocked. + expect(canSaveWith({ name: 'Sales', input: '[{"a":1}]' })).toBe(false); + // Editing the existing one may keep its own name (excludeId). + store().select(id); + store().startEdit(); + expect(selectCanSave(useDatasetStore.getState())).toBe(true); + }); +}); + describe('save — edit', () => { test('updating inline data re-profiles and advances modified', () => { store().add( diff --git a/src/app/stores/DatasetStore.ts b/src/app/stores/DatasetStore.ts index 855e800..13b2974 100644 --- a/src/app/stores/DatasetStore.ts +++ b/src/app/stores/DatasetStore.ts @@ -91,40 +91,59 @@ export function byModifiedDesc(a: Dataset, b: Dataset): number { return b.modified.localeCompare(a.modified); } +/** + * Cheap form validation: returns an error message, or `null` when the form is + * saveable. Deliberately does **not** `JSON.parse` the input — it only checks + * name/input presence, name uniqueness, and that a format is detectable — so it + * is safe to run on every render to drive the Save button's disabled state + * (spec §05). Error copy follows the council resolution (docs/architecture/10 + * §error copy → GOV.UK error-message): action-oriented, specific, says how to fix. + */ +function validateForm( + form: DatasetForm, + datasets: Dataset[], + excludeId: number | undefined, +): string | null { + const name = form.name.trim(); + if (name === '') return 'Enter a dataset name.'; + if (isNameTaken(name, datasets, excludeId)) { + return `A dataset named "${name}" already exists. Choose a different name.`; + } + + const input = form.input.trim(); + if (input === '') { + return form.source === 'url' ? 'Enter a URL.' : 'Paste JSON, CSV, or TSV data to save.'; + } + + if (form.source === 'url') { + if (!/^https?:\/\//i.test(input)) return 'Enter a URL starting with http:// or https://.'; + return null; + } + + // Inline: the data must auto-detect to a known format (spec §05 → Auto-detection). + if (!detectFormat(form.input).format) return 'Enter valid JSON, CSV, or TSV data.'; + return null; +} + /** Validate a form into a saveable shape, or return an error message. */ function resolveForm( form: DatasetForm, datasets: Dataset[], excludeId: number | undefined, ): { name: string; data: unknown; format: DataFormat; source: DataSource } | { error: string } { - // Error copy follows the council resolution (docs/architecture/10 §error copy → - // GOV.UK error-message): action-oriented, specific, and says how to fix it. - const name = form.name.trim(); - if (name === '') return { error: 'Enter a dataset name.' }; - if (isNameTaken(name, datasets, excludeId)) { - return { error: `A dataset named "${name}" already exists. Choose a different name.` }; - } + const error = validateForm(form, datasets, excludeId); + if (error) return { error }; + const name = form.name.trim(); const input = form.input.trim(); - if (input === '') { - return { - error: form.source === 'url' ? 'Enter a URL.' : 'Paste JSON, CSV, or TSV data to save.', - }; - } if (form.source === 'url') { - if (!/^https?:\/\//i.test(input)) { - return { error: 'Enter a URL starting with http:// or https://.' }; - } // Format is inferred from the extension; default to JSON when unknown (§05). return { name, data: input, format: detectFormatFromUrl(input) ?? 'json', source: 'url' }; } - // Inline: the data must auto-detect to a known format (spec §05 → Auto-detection). - const { format } = detectFormat(form.input); - if (!format) { - return { error: 'Enter valid JSON, CSV, or TSV data.' }; - } + // validateForm guaranteed a detectable format above. + const format = detectFormat(form.input).format as DataFormat; // JSON/TopoJSON are stored parsed; CSV/TSV keep their raw text (data model §09B). const data = format === 'json' || format === 'topojson' ? (JSON.parse(form.input) as unknown) : form.input; @@ -276,3 +295,13 @@ export const useDatasetStore = create((set, get) => ({ /** Selector: the selected dataset record, or null. Derive — never store. */ export const selectSelectedDataset = (s: DatasetState): Dataset | null => s.datasets.find((d) => d.id === s.selectedId) ?? null; + +/** + * Selector: whether the current form is saveable, powering the Save button's + * disabled state (spec §05 → Save is disabled until a name and valid data/URL + * are present). Mirrors the `save()` action's `excludeId` derivation so an edit + * can keep its own name. + */ +export const selectCanSave = (s: DatasetState): boolean => + validateForm(s.form, s.datasets, s.view === 'edit' ? (s.selectedId ?? undefined) : undefined) === + null;