diff --git a/SW.Bitween.Web/ClientApp/e2e/helpers.ts b/SW.Bitween.Web/ClientApp/e2e/helpers.ts index 82af48a9..6b52ae10 100644 --- a/SW.Bitween.Web/ClientApp/e2e/helpers.ts +++ b/SW.Bitween.Web/ClientApp/e2e/helpers.ts @@ -1,4 +1,4 @@ -import { expect, type Page } from "@playwright/test"; +import { type Page } from "@playwright/test"; /** Checkbox labels carry their description in the accessible name, so anchor at the start. */ export const startsWith = (text: string) => diff --git a/SW.Bitween.Web/ClientApp/e2e/mapper-cases.spec.ts b/SW.Bitween.Web/ClientApp/e2e/mapper-cases.spec.ts index bd61f321..ea800f98 100644 --- a/SW.Bitween.Web/ClientApp/e2e/mapper-cases.spec.ts +++ b/SW.Bitween.Web/ClientApp/e2e/mapper-cases.spec.ts @@ -6,12 +6,12 @@ import { addFixedRule, addList, addListField, + addListValue, addPathRule, buildFromSample, createSubscription, expectPreview, openDetail, - openMapper, openWithSample, preview, saveAndReload, @@ -372,7 +372,7 @@ test("every filter comparison keeps the entries it should", async ({ page }) => await openWithSample(page, { line: [{ qty: 1 }, { qty: 2 }, { qty: 3 }] }); for (const { field, operator } of OPERATOR_CASES) { - await addList(page, field, "line"); + const list = await addList(page, field, "line"); await page.getByRole("button", { name: `Settings for the list ${field}` }).click(); await page.getByRole("checkbox", { name: "Only some entries" }).last().check(); @@ -380,12 +380,11 @@ test("every filter comparison keeps the entries it should", async ({ page }) => await page.getByRole("combobox", { name: "Filter comparison" }).last().selectOption(operator); await page.getByRole("textbox", { name: "Filter value" }).last().fill("2"); + await page.getByRole("button", { name: `Settings for the list ${field}` }).click(); + // A list of plain values, so what survived the filter reads straight off the // preview rather than through a wrapper object. - await page.getByRole("checkbox", { name: /A list of plain values/ }).last().check(); - await setSourcePath(page, "qty"); - - await page.getByRole("button", { name: `Settings for the list ${field}` }).click(); + await addListValue(list, field, "qty"); } for (const { expect: shape } of OPERATOR_CASES) @@ -464,9 +463,10 @@ test("the whole output can be a list, of records or of plain values", async ({ p await expect(preview(page)).toHaveText(/^\[[\s\S]*\]$/); // ── And the same thing as plain values ───────────────────────────────────── - await page.getByRole("button", { name: "Settings for the list at the root" }).click(); - await page.getByRole("checkbox", { name: /A list of plain values/ }).check(); - await setSourcePath(page, "sku"); + // A list holds one or the other, and says so by what it will let you add: the + // record's field has to go before the value can be put in its place. + await root.getByRole("button", { name: "Remove the rule for code" }).click(); + await addListValue(root, "the root list", "sku"); await expect(preview(page)).toHaveText(/^\[\s*"A1",\s*"B7"\s*\]$/, { timeout: 15000 }); }); @@ -999,10 +999,6 @@ test("a checkbox in a settings panel can be ticked by its text", async ({ page } // The entry with qty 0 is gone, which is the whole point of the checkbox. await expectPreview(page, '"qty": 2'); await expect(preview(page)).not.toContainText('"qty": 0'); - - // And the same for the other checkbox in the panel. - await page.getByText("A list of plain values, not records").click(); - await expect(page.getByText("each walked entry is")).toBeVisible(); }); test("a checkbox in a rule's detail can be ticked by its text", async ({ page }) => { diff --git a/SW.Bitween.Web/ClientApp/e2e/mapperHelpers.ts b/SW.Bitween.Web/ClientApp/e2e/mapperHelpers.ts index 912fb9f7..fe0386e6 100644 --- a/SW.Bitween.Web/ClientApp/e2e/mapperHelpers.ts +++ b/SW.Bitween.Web/ClientApp/e2e/mapperHelpers.ts @@ -155,6 +155,23 @@ export async function addListField( await setSourcePath(list, path, from); } +/** + * Makes a list hold plain values, and points its one value at a path. + * + * The counterpart of `addListField`. What a list holds is decided by what is put into + * it, so this is a click that adds a row rather than a setting that changes a mode — + * and it is only offered while the list is still empty. + */ +export async function addListValue( + list: Locator, + addTo: string, + path: string, + from: "entry" | "document" = "entry", +) { + await list.getByRole("button", { name: `Add a value to ${addTo}` }).click(); + await setSourcePath(list, path, from); +} + /** The mapped document, which the server produces. */ export const preview = (page: Page): Locator => page.locator("pre").first(); diff --git a/SW.Bitween.Web/ClientApp/e2e/native-mapper.spec.ts b/SW.Bitween.Web/ClientApp/e2e/native-mapper.spec.ts index d7285358..07137d34 100644 --- a/SW.Bitween.Web/ClientApp/e2e/native-mapper.spec.ts +++ b/SW.Bitween.Web/ClientApp/e2e/native-mapper.spec.ts @@ -2,6 +2,8 @@ import { test, expect } from "@playwright/test"; import { pickOption, signInAsAdmin } from "./helpers"; import { SAMPLE, + addList, + addListValue, addPathRule, buildFromSample, createSubscription, @@ -298,6 +300,63 @@ test("builds the whole output from a sample of it, and matches the source fields await expect(preview).toContainText('"sku": "B7"'); }); +test("a list of plain values built from a sample is wired up and says so", async ({ + page, +}) => { + // The shape that sent this round: both sides hold `[1,2,3]`, and the scaffolder + // wires each entry to the entry itself — the right answer, which used to be shown + // as an empty box behind a checkbox and read as nothing configured at all. + const subscriptionId = await createSubscription(page); + await openMapper(page, subscriptionId); + + await page + .getByRole("textbox", { name: "Sample source document" }) + .fill(JSON.stringify({ city: "errr", test: [1, 2, 3] })); + await buildFromSample(page, { city: "", test: [1, 2, 3] }); + await page.getByRole("button", { name: "Build from a sample of the output" }).click(); + await page.keyboard.press("Escape"); + + const list = page.getByRole("group", { name: "Rules for the list test" }); + + // A row in the tree, not a setting behind a chevron — and it reads as an answer + // rather than as a box waiting to be filled in. + const value = list.getByRole("combobox", { name: "Source field" }); + await expect(value).toHaveAttribute("placeholder", "the entry itself"); + await expect(value).toHaveValue(""); + await expect(list.getByText("each entry")).toBeVisible(); + + // Nothing is left unassigned, which is what the count above the tree has to agree + // with: an empty path here is the answer, not a blank. + await expect(page.getByText("2 rules · 2 assigned")).toBeVisible(); + + // And it runs: the source values come straight through. + await expect(page.locator("pre").first()).toHaveText(/"test":\s*\[\s*1,\s*2,\s*3\s*\]/, { + timeout: 15000, + }); +}); + +test("a list's value takes a type and a transform like any other rule", async ({ page }) => { + const subscriptionId = await createSubscription(page); + await openMapper(page, subscriptionId); + await page + .getByRole("textbox", { name: "Sample source document" }) + .fill(JSON.stringify({ price: [10, 20] })); + + const list = await addList(page, "totals", "price"); + await addListValue(list, "totals", ""); + + // The row carries the whole rule, which is the point of it being a row: the value + // each entry produces can be multiplied and typed exactly like a named field. + await list.getByRole("button", { name: "Details for each entry" }).click(); + await list.getByRole("combobox", { name: "Transform" }).selectOption("multiply"); + await list.getByRole("textbox", { name: /Multiply.*By/ }).fill("2"); + await list.getByRole("combobox", { name: "Value type" }).selectOption("number"); + + await expect(page.locator("pre").first()).toHaveText(/"totals":\s*\[\s*20,\s*40\s*\]/, { + timeout: 15000, + }); +}); + test("a list inside a list offers the entry's own lists, not the document's", async ({ page }) => { const subscriptionId = await createSubscription(page); await openMapper(page, subscriptionId); @@ -499,11 +558,10 @@ test("a list of values with a slot per rule, walking nothing", async ({ page }) // Nothing to walk, so the list is exactly what is written into it. This is what // the old mapper called a primitive array. await page.getByRole("combobox", { name: "Source list" }).selectOption("none"); - await page.getByRole("button", { name: "Settings for the list codes" }).click(); - await page.getByRole("checkbox", { name: /A list of plain values/ }).check(); - // Each entry mirrors the list, so each is one value rather than a record. - await page.getByRole("button", { name: "Add an entry to codes" }).click(); + // What the list holds is decided by what is put in it, not by a setting: the first + // slot says these are plain values, and every entry after it follows. + await page.getByRole("button", { name: "Add a value to codes" }).click(); await page.getByRole("button", { name: "Add an entry to codes" }).click(); const first = page.getByRole("group", { name: "Entry 1" }); diff --git a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/EntryRow.tsx b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/EntryRow.tsx index 06eb9af1..9afaf5f9 100644 --- a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/EntryRow.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/EntryRow.tsx @@ -1,8 +1,15 @@ -import { CornerDownRight, Trash2 } from "lucide-react"; +import { useState } from "react"; +import { ChevronDown, ChevronRight, CornerDownRight, Trash2 } from "lucide-react"; import type { OutputEntryNode } from "../../lib/nativeMapper/outputTree"; import { useRules, useRulesDispatch } from "../../lib/nativeMapper/RulesEditorContext"; -import { SOURCE_KINDS, freshSource } from "../../lib/nativeMapper/types"; +import { + SOURCE_KINDS, + TYPE_BADGES, + freshSource, + type EditorFieldRule, +} from "../../lib/nativeMapper/types"; import { SegmentedControl } from "../ui/SegmentedControl"; +import { RuleDetail } from "./RuleDetail"; import { ValueCell, type SourcePaths } from "./ValueCell"; /** @@ -12,6 +19,11 @@ import { ValueCell, type SourcePaths } from "./ValueCell"; * from somewhere different. Its rules read whatever the list reads, because there * is no entry of its own to read — which is what lets a constant entry still pull a * value out of the document, the partner or a values set. + * + * An entry holding one value is a whole rule on this line, so it gets what any other + * rule gets: a transform, a substitution table and a type, behind the chevron. It is + * a slot in the output like any other, and "it is written in by hand" is no reason + * for it to be the one value in the mapping that cannot be rounded or reformatted. */ export function EntryRow({ node, @@ -24,9 +36,18 @@ export function EntryRow({ const { entry, position } = node; const dispatch = useRulesDispatch(); const { ruleErrors } = useRules(); + const [open, setOpen] = useState(false); const error = ruleErrors[node.errorKey]; - const isValue = entry.item !== undefined; + const item = entry.item; + const extras = (item?.transform ? 1 : 0) + (item?.lookup ? 1 : 0); + + const updateItem = (changes: Partial>) => + dispatch({ + type: "UPDATE_FIXED_ENTRY", + id: entry.id, + changes: { item: { ...item!, ...changes } }, + }); return (
- {isValue && entry.item && ( + {item && ( <> ← @@ -53,27 +74,35 @@ export function EntryRow({ size="sm" label={`Where entry ${position} comes from`} options={SOURCE_KINDS} - value={entry.item.from.kind === "rootPath" ? "path" : entry.item.from.kind} - onChange={(kind) => - dispatch({ - type: "UPDATE_FIXED_ENTRY", - id: entry.id, - changes: { item: { ...entry.item!, from: freshSource(kind) } }, - }) - } + value={item.from.kind === "rootPath" ? "path" : item.from.kind} + onChange={(kind) => updateItem({ from: freshSource(kind) })} /> - dispatch({ - type: "UPDATE_FIXED_ENTRY", - id: entry.id, - changes: { item: { ...entry.item!, from } }, - }) - } + valueType={item.type} + onChange={(from) => updateItem({ from })} /> + )} @@ -88,6 +117,12 @@ export function EntryRow({
+ {open && item && ( +
+ +
+ )} + {/* A colour is not a message. The reason is rendered here as a list's is, so it reaches a reader who cannot tell the two borders apart. */} {error && ( diff --git a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/ListRow.tsx b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/ListRow.tsx index 72914645..0afc06ab 100644 --- a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/ListRow.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/ListRow.tsx @@ -5,15 +5,10 @@ import type { OutputListNode } from "../../lib/nativeMapper/outputTree"; import { useRules, useRulesDispatch } from "../../lib/nativeMapper/RulesEditorContext"; import { FILTER_OPERATORS, - SOURCE_KINDS, - emptyFieldRule, - freshSource, type FilterOperatorName, } from "../../lib/nativeMapper/types"; import { Checkbox } from "../ui/forms"; import { RowInput, RowSelect } from "./rowControls"; -import { SegmentedControl } from "../ui/SegmentedControl"; -import { ValueCell, type SourcePaths } from "./ValueCell"; /** * The three states of a list's source, encoded for a select. @@ -34,22 +29,20 @@ const decodeOver = (value: string) => * A list in the output: one entry per entry of a list on the way in. * * The header is one line like a field's, and what the list walks sits on it because - * that is the thing you check when reading a mapping. The filter and the - * plain-values switch are behind the chevron. + * that is the thing you check when reading a mapping. The filter is behind the + * chevron. What each entry holds is not a setting at all — it is the rows in the + * list, the same as everywhere else in the tree. */ export function ListRow({ node, /** The source node this list's own list is named against. */ scope, - itemPaths, prefix, collapsed, onToggleCollapsed, }: { node: OutputListNode; scope: DocumentNode | null; - /** The paths one entry of this list may read, for a list of plain values. */ - itemPaths: SourcePaths; prefix: string[]; collapsed: boolean; onToggleCollapsed: () => void; @@ -65,7 +58,7 @@ export function ListRow({ // the list's own rules away, like an object's does; this one shows the list's // settings. Kept shut, a list that uses neither costs one line like anything else. const [detail, setDetail] = useState(false); - const settings = (list.where ? 1 : 0) + (list.item ? 1 : 0); + const settings = list.where ? 1 : 0; const update = (changes: Partial>) => dispatch({ type: "UPDATE_LIST", id: list.id, changes }); @@ -172,7 +165,7 @@ export function ListRow({ onClick={() => setDetail((d) => !d)} aria-expanded={detail} aria-label={`Settings for the list ${node.name || "at the root"}`} - title="Filter the entries, or make it a list of plain values" + title="Skip some of the entries" className={`flex flex-shrink-0 items-center gap-0.5 rounded px-1 py-0.5 hover:bg-ink-100 ${ settings > 0 ? "text-crimson-600" : "text-ink-400 hover:text-ink-700" }`} @@ -236,40 +229,6 @@ export function ListRow({ /> )} - - update({ item: e.target.checked ? emptyFieldRule() : undefined })} - /> - - {/* Configures the entry a walked source produces, so it has nothing to - say for a list that walks nothing — those entries carry their own. */} - {list.item && list.over !== undefined && ( -
- each walked entry is - - update({ item: { ...list.item!, from: freshSource(kind) } }) - } - /> - update({ item: { ...list.item!, from } })} - /> -
- )} )} diff --git a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputPanel.tsx b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputPanel.tsx index be28c063..45a4ebe1 100644 --- a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputPanel.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputPanel.tsx @@ -3,7 +3,7 @@ import { Search } from "lucide-react"; import type { DocumentNode } from "../../lib/nativeMapper/documentTree"; import { filterTree, outputTreeOf } from "../../lib/nativeMapper/outputTree"; import { useRules, useRulesDispatch } from "../../lib/nativeMapper/RulesEditorContext"; -import { everyFieldRule, isAssigned } from "../../lib/nativeMapper/rulesReducer"; +import { everyFieldRule, isAssigned, isItemAssigned } from "../../lib/nativeMapper/rulesReducer"; import { TextInput } from "../ui/forms"; import { AddRuleButtons, OutputTreeView } from "./OutputTreeView"; @@ -22,7 +22,9 @@ export function OutputPanel({ sourceRoot }: { sourceRoot: DocumentNode | null }) const shown = useMemo(() => filterTree(tree, searchTarget), [tree, searchTarget]); const fields = everyFieldRule(rules); - const assigned = fields.filter((f) => isAssigned(f.rule)).length; + const assigned = fields.filter((f) => + f.isItem ? isItemAssigned(f.rule) : isAssigned(f.rule), + ).length; const empty = rules.fields.length === 0 && rules.lists.length === 0 && !rules.root; return ( diff --git a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputRow.tsx b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputRow.tsx index 9007fd7f..8c9368f9 100644 --- a/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputRow.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/nativeMapper/OutputRow.tsx @@ -1,32 +1,23 @@ import { useState } from "react"; import { ChevronDown, ChevronRight, Trash2 } from "lucide-react"; import { useRules, useRulesDispatch } from "../../lib/nativeMapper/RulesEditorContext"; -import { isAssigned } from "../../lib/nativeMapper/rulesReducer"; -import type { OutputFieldNode } from "../../lib/nativeMapper/outputTree"; -import { SOURCE_KINDS, freshSource, type ValueTypeName } from "../../lib/nativeMapper/types"; +import { isAssigned, isItemAssigned } from "../../lib/nativeMapper/rulesReducer"; +import type { OutputRowNode } from "../../lib/nativeMapper/outputTree"; +import { SOURCE_KINDS, TYPE_BADGES, freshSource } from "../../lib/nativeMapper/types"; import { SegmentedControl } from "../ui/SegmentedControl"; import { RuleDetail } from "./RuleDetail"; import { RowInput } from "./rowControls"; import { ValueCell, type SourcePaths } from "./ValueCell"; /** - * Short enough for the row; "boo" is not a word anyone wants to read. + * One rule, on one line. * - * Keyed by the type union rather than by `string`, so adding a value type is a - * compile error here instead of a badge that silently renders as nothing. - */ -const TYPE_BADGES: Record = { - string: "txt", - number: "num", - boolean: "y/n", -}; - -/** - * One output field, on one line. - * - * The name shown is the last segment only — the indentation carries the rest, the - * way the source side already did. Typing dots into it still nests, because the - * rule stores segments; the tree redraws itself around the new path. + * Two kinds of rule are drawn this way. A field has a name, shown as its last + * segment only — the indentation carries the rest, the way the source side already + * did — and typing dots into it still nests, because the rule stores segments. The + * single value each entry of a list of plain values produces has no name to show, + * because `["A1","B7"]` has nowhere to put one; everything else about the row is + * the same, which is the point of it being a row at all. * * Everything that is not the value lives behind the chevron. A row that carries a * transform or a table says so with a dot rather than by taking three lines to @@ -41,7 +32,7 @@ export function OutputRow({ prefix, paths, }: { - node: OutputFieldNode; + node: OutputRowNode; prefix: string[]; paths: SourcePaths; }) { @@ -50,15 +41,33 @@ export function OutputRow({ const { selectedId, ruleErrors, hoveredPath } = useRules(); const [open, setOpen] = useState(false); + const isItem = node.kind === "item"; const error = ruleErrors[errorKey]; const selected = selectedId === rule.id; // Only a path read in this scope has a source row to draw a line to; one read // from the top of the document inside a list has no single row to point at. const sourcePath = rule.from.kind === "path" ? (rule.from.path ?? "") : ""; const extras = (rule.transform ? 1 : 0) + (rule.lookup ? 1 : 0); + // A field's name, or the words standing in for the name a list's value has not + // got. Used in every label on the row, so both kinds read the same way. + const describe = node.kind === "item" ? "each entry" : node.name || "this field"; + const assigned = isItem ? isItemAssigned(rule) : isAssigned(rule); const update = (changes: Partial>) => - dispatch({ type: "UPDATE_FIELD", id: rule.id, changes }); + node.kind === "item" + ? dispatch({ + type: "UPDATE_LIST", + id: node.listId, + changes: { item: { ...rule, ...changes } }, + }) + : dispatch({ type: "UPDATE_FIELD", id: rule.id, changes }); + + // Removing a list's value does not remove a row from the list — it puts the list + // back to having decided nothing, so it can be built out of fields instead. + const remove = () => + node.kind === "item" + ? dispatch({ type: "UPDATE_LIST", id: node.listId, changes: { item: undefined } }) + : dispatch({ type: "REMOVE_FIELD", id: rule.id }); return (
- - update({ - target: [...prefix, ...(e.target.value === "" ? [] : e.target.value.split("."))], - }) - } - /> + {node.kind === "item" ? ( + + each entry + + ) : ( + + update({ + target: [...prefix, ...(e.target.value === "" ? [] : e.target.value.split("."))], + }) + } + /> + )} ← @@ -145,6 +163,7 @@ export function OutputRow({ source={rule.from} paths={paths} valueType={rule.type} + emptyPathLabel={isItem ? "the entry itself" : undefined} onChange={(from) => update({ from })} /> @@ -152,7 +171,7 @@ export function OutputRow({ type="button" onClick={() => setOpen((o) => !o)} aria-expanded={open} - aria-label={`Details for ${node.name || "this field"}`} + aria-label={`Details for ${describe}`} title={ extras > 0 ? "Has a transform or a table" @@ -171,9 +190,11 @@ export function OutputRow({
@@ -225,6 +229,23 @@ function TreeNode({ ); } +/** + * Whether a list is still open to being made a list of plain values. + * + * Only while it holds nothing at all. A list's shape follows what is put in it, so + * one that already has fields, nested lists or written entries has answered the + * question — and offering to change it then would mean deciding what happens to + * everything already there. + */ +function stillUndecided(list: EditorListRule): boolean { + return ( + list.item === undefined && + list.fields.length === 0 && + list.lists.length === 0 && + list.fixed.length === 0 + ); +} + function BranchHeader({ name, count, @@ -276,6 +297,7 @@ export function AddRuleButtons({ canAddList, inside, onAddFixedEntry, + onAddValue, showPerEntryRules = true, }: { listId: RuleId | null; @@ -284,6 +306,14 @@ export function AddRuleButtons({ inside?: string; /** Offered on a list, where an entry can be written into it. */ onAddFixedEntry?: () => void; + /** + * Offered on a list that has not yet been made of anything. + * + * What a list holds is decided by what is put in it, so this sits beside Field and + * List rather than being a setting behind a chevron — and once one of the three has + * been used the other kind is no longer offered. + */ + onAddValue?: () => void; /** False for a list that walks nothing: there are no per-entry rules to add. */ showPerEntryRules?: boolean; }) { @@ -292,6 +322,17 @@ export function AddRuleButtons({ return (
+ {onAddValue && ( + + )} {onAddFixedEntry && (