diff --git a/App/Features/Lists/ListColumn.swift b/App/Features/Lists/ListColumn.swift new file mode 100644 index 0000000..9ae5a59 --- /dev/null +++ b/App/Features/Lists/ListColumn.swift @@ -0,0 +1,27 @@ +// ListColumn +// +// One rendered column of a list's row table: the row-data **key** it reads and +// the **label** it shows. +// +// These are separate on the server (`propertyKey` / `propertyName`) and were +// collapsed into a single `name` throughout the lists UI. That was harmless only +// because the client could not create a schema at all — the server rejected the +// DSL string it sent — so every schema in the wild had key == label. Fixing the +// wire shape (GitHub #85) makes the distinction load-bearing: a column labelled +// "Publication Year" over a key of `year` renders nothing if the label is used +// as the subscript. +// +// Per Decision 0003 this type consumes only `InterlinedDomain`. + +import Foundation + +struct ListColumn: Identifiable, Hashable, Sendable { + + /// The `ListRow.fields` key this column reads. The identity. + let key: String + + /// The header text. + let label: String + + var id: String { key } +} diff --git a/App/Features/Lists/ListDetailViewModel.swift b/App/Features/Lists/ListDetailViewModel.swift index 185a0c4..56df428 100644 --- a/App/Features/Lists/ListDetailViewModel.swift +++ b/App/Features/Lists/ListDetailViewModel.swift @@ -96,6 +96,11 @@ final class ListDetailViewModel { /// is not exposed by the API today (see /// `/API-backend-prompts-to-build.md` — no documented public-list /// clone endpoint), so this is a deliberate degradation. + /// + /// The schema is not copied, and now says so rather than passing a value + /// that was always `nil`: `GET /api/users/[username]/lists/[id]` returns a + /// light projection with no columns at all (GitHub #85), so there was never + /// a schema here to carry across. func saveToMyLists(suggestedName: String) async { guard let detail else { return } saveState = .saving @@ -103,7 +108,7 @@ final class ListDetailViewModel { let created = try await lists.create( title: suggestedName, description: detail.description, - schema: detail.schemaDescription, + schema: nil, parentId: nil, isPublic: false ) diff --git a/App/Features/Lists/ListRowsView.swift b/App/Features/Lists/ListRowsView.swift index 2900c19..cae1be6 100644 --- a/App/Features/Lists/ListRowsView.swift +++ b/App/Features/Lists/ListRowsView.swift @@ -156,9 +156,10 @@ struct ListRowsView: View { let columns = effectiveColumns(viewModel) VStack(spacing: 0) { Table(viewModel.rows, selection: $selection) { - TableColumnForEach(columns, id: \.self) { column in - TableColumn(column) { (row: ListRow) in - Text(row.fields[column]?.displayText ?? "") + TableColumnForEach(columns) { column in + // Header from `label`, cell lookup by `key` — see `ListColumn`. + TableColumn(column.label) { (row: ListRow) in + Text(row.fields[column.key]?.displayText ?? "") .lineLimit(2) } } @@ -192,14 +193,15 @@ struct ListRowsView: View { /// Ordered column set for the table: the schema-derived columns when /// present, else the sorted union of keys across loaded rows so a /// schemaless list still renders a sensible grid. - private func effectiveColumns(_ viewModel: ListRowsViewModel) -> [String] { + private func effectiveColumns(_ viewModel: ListRowsViewModel) -> [ListColumn] { if !viewModel.columns.isEmpty { return viewModel.columns } var seen = Set() - var ordered: [String] = [] + var ordered: [ListColumn] = [] for row in viewModel.rows { for key in row.fields.keys.sorted() where !seen.contains(key) { seen.insert(key) - ordered.append(key) + // No schema means no separate label; the key is the header. + ordered.append(ListColumn(key: key, label: key)) } } return ordered @@ -332,15 +334,17 @@ struct ListRowsView: View { } @ViewBuilder - private func rowCard(row: ListRow, columns: [String]) -> some View { - let keys = columns.isEmpty ? row.fields.keys.sorted() : columns + private func rowCard(row: ListRow, columns: [ListColumn]) -> some View { + let keys = columns.isEmpty + ? row.fields.keys.sorted().map { ListColumn(key: $0, label: $0) } + : columns VStack(alignment: .leading, spacing: 4) { - ForEach(keys, id: \.self) { key in + ForEach(keys) { column in HStack(alignment: .firstTextBaseline, spacing: 6) { - Text(key) + Text(column.label) .font(.caption.weight(.semibold)) .foregroundStyle(.secondary) - Text(row.fields[key]?.displayText ?? "") + Text(row.fields[column.key]?.displayText ?? "") .font(.ilBody()) .lineLimit(2) Spacer() @@ -351,11 +355,13 @@ struct ListRowsView: View { .background(ILColor.surface2, in: RoundedRectangle(cornerRadius: ILMetric.radiusMd)) } - private func rowAccessibilityLabel(row: ListRow, columns: [String]) -> String { - let keys = columns.isEmpty ? row.fields.keys.sorted() : columns - let pairs = keys.compactMap { key -> String? in - guard let value = row.fields[key]?.displayText, !value.isEmpty else { return nil } - return "\(key): \(value)" + private func rowAccessibilityLabel(row: ListRow, columns: [ListColumn]) -> String { + let keys = columns.isEmpty + ? row.fields.keys.sorted().map { ListColumn(key: $0, label: $0) } + : columns + let pairs = keys.compactMap { column -> String? in + guard let value = row.fields[column.key]?.displayText, !value.isEmpty else { return nil } + return "\(column.label): \(value)" } return pairs.isEmpty ? "Row" : pairs.joined(separator: ", ") } diff --git a/App/Features/Lists/ListRowsViewModel.swift b/App/Features/Lists/ListRowsViewModel.swift index 9494623..a06bcee 100644 --- a/App/Features/Lists/ListRowsViewModel.swift +++ b/App/Features/Lists/ListRowsViewModel.swift @@ -68,7 +68,7 @@ final class ListRowsViewModel { var entityFields: [SchemaEntityField] { schema.fields.map { field in SchemaEntityField( - name: field.name, + name: field.label, typeToken: field.type.dslToken, options: field.type.carriesOptions ? (field.enumValues ?? []) : [], nullable: field.nullable @@ -101,8 +101,20 @@ final class ListRowsViewModel { /// non-empty; falls back to the union of observed row keys when /// the list has no schema yet (so the table still shows something /// useful for schema-less lists). - var columns: [String] { - if !schema.fields.isEmpty { return schema.fields.map(\.name) } + /// + /// Each column carries its **key** and its **label** separately. This used + /// to be a `[String]` of `field.name` used both as the header text and as + /// the `row.fields[…]` subscript — correct only while the two were the same + /// token, which they are in the client's DSL and are not on the server + /// (`propertyKey` vs `propertyName`). With the schema wire shape fixed + /// (GitHub #85), a column labelled "Publication Year" over a key of `year` + /// would have rendered every cell empty. + var columns: [ListColumn] { + if !schema.fields.isEmpty { + return schema.orderedFields + .filter { $0.isVisible != false } + .map { ListColumn(key: $0.key, label: $0.label) } + } var seen: Set = [] var ordered: [String] = [] for row in rows { @@ -110,7 +122,8 @@ final class ListRowsViewModel { ordered.append(key) } } - return ordered.sorted() + // With no schema the key is all there is, so it doubles as the label. + return ordered.sorted().map { ListColumn(key: $0, label: $0) } } /// Currently selected row, if any. diff --git a/App/Features/Lists/NewListViewModel.swift b/App/Features/Lists/NewListViewModel.swift index 887659a..6fb6b0b 100644 --- a/App/Features/Lists/NewListViewModel.swift +++ b/App/Features/Lists/NewListViewModel.swift @@ -94,11 +94,30 @@ final class NewListViewModel { let trimmedTitle = title.trimmingCharacters(in: .whitespacesAndNewlines) let trimmedDescription = descriptionText.trimmingCharacters(in: .whitespacesAndNewlines) let trimmedSchema = schemaDSL.trimmingCharacters(in: .whitespacesAndNewlines) + + // The DSL the user typed is parsed here, client-side, and the *columns* + // go on the wire. It used to be sent as a string, which the server + // rejects outright — so creating a list with columns from macOS never + // worked (GitHub #85). A malformed DSL is now caught before the network + // call and reported against the field the user typed it into, rather + // than coming back as an opaque 400. + let parsedSchema: ListSchema? + if trimmedSchema.isEmpty { + parsedSchema = nil + } else { + do { + parsedSchema = try SchemaDSL.parse(trimmedSchema) + } catch { + self.error = error + return + } + } + do { let created = try await lists.create( title: trimmedTitle, description: trimmedDescription.isEmpty ? nil : trimmedDescription, - schema: trimmedSchema.isEmpty ? nil : trimmedSchema, + schema: parsedSchema, parentId: parentID, isPublic: visibility == .public ) diff --git a/App/Features/Lists/RowInspectorView.swift b/App/Features/Lists/RowInspectorView.swift index 7e7a76d..63264e5 100644 --- a/App/Features/Lists/RowInspectorView.swift +++ b/App/Features/Lists/RowInspectorView.swift @@ -41,6 +41,9 @@ struct RowInspectorView: View { ForEach(row.fields.keys.sorted(), id: \.self) { key in cellEditor( key: key, + label: key, + helpText: nil, + placeholder: nil, type: .text, options: [], current: row.fields[key] ?? .null, @@ -49,12 +52,19 @@ struct RowInspectorView: View { ) } } else { - ForEach(viewModel.schema.fields) { field in + // `key` reads and writes the cell; `label` is what the user + // sees. They are the same token for a DSL-authored schema + // and differ for one authored on the web (GitHub #85), so + // both are passed rather than one standing in for the other. + ForEach(viewModel.schema.orderedFields) { field in cellEditor( - key: field.name, + key: field.key, + label: field.label, + helpText: field.helpText, + placeholder: field.placeholder, type: field.type, options: field.enumValues ?? [], - current: row.fields[field.name] ?? .null, + current: row.fields[field.key] ?? .null, row: row, viewModel: viewModel ) @@ -68,6 +78,9 @@ struct RowInspectorView: View { @ViewBuilder private func cellEditor( key: String, + label: String, + helpText: String?, + placeholder: String?, type: SchemaFieldType, options: [String], current: ListCellValue, @@ -75,12 +88,14 @@ struct RowInspectorView: View { viewModel: ListRowsViewModel ) -> some View { VStack(alignment: .leading, spacing: 4) { - Text(key) + Text(label) .font(.caption.weight(.semibold)) .foregroundStyle(.secondary) switch type { case .text, .url, .email, .date, .number: - TextField(label(for: type), text: Binding( + // The column's own placeholder when it has one, else the + // type-derived hint the editor has always shown. + TextField(placeholder ?? self.label(for: type), text: Binding( get: { editingValues[key] ?? current.displayText }, set: { editingValues[key] = $0 } )) @@ -102,7 +117,7 @@ struct RowInspectorView: View { commitChange(row: row, key: key, type: type, viewModel: viewModel) } )) - .accessibilityLabel(key) + .accessibilityLabel(label) case .select: selectEditor( key: key, @@ -119,6 +134,13 @@ struct RowInspectorView: View { viewModel: viewModel ) } + // The server has always stored per-column help text; nothing ever + // read it (GitHub #85, and the blocked half of #50). + if let helpText, !helpText.isEmpty { + Text(helpText) + .font(.caption) + .foregroundStyle(.secondary) + } } } diff --git a/App/Features/Lists/SchemaEditorViewModel.swift b/App/Features/Lists/SchemaEditorViewModel.swift index b3d1c63..f3462c5 100644 --- a/App/Features/Lists/SchemaEditorViewModel.swift +++ b/App/Features/Lists/SchemaEditorViewModel.swift @@ -66,6 +66,10 @@ final class SchemaEditorViewModel { /// Surfaced error from the most recent failed save. private(set) var error: Error? + /// The schema the server refused as destructive, held so the user can + /// confirm it. Non-nil is what the view binds a confirmation dialog to. + private(set) var pendingDestructiveSave: ListSchema? + /// Set to `true` after a successful save; the view dismisses. private(set) var didFinish: Bool = false @@ -222,7 +226,41 @@ final class SchemaEditorViewModel { ) }) do { - let saved = try await lists.updateSchema(of: listId, schema: schema) + let saved = try await lists.updateSchema( + of: listId, + schema: schema, + // Never force on the first attempt. Dropping a column that still + // holds data is a question for the user, not a default — the + // server asks it, and `confirmDestructiveSave()` is how the + // answer gets back. + force: false + ) + eventBus.post(.schemaChanged(listId: listId, schema: saved)) + didFinish = true + } catch let listsError as ListsError { + if case .schemaChangeWouldLoseData = listsError { + pendingDestructiveSave = schema + } + self.error = listsError + } catch { + self.error = error + } + } + + /// Re-submits the schema the server refused, confirming the data loss. + /// + /// Only reachable after `save()` has surfaced + /// `ListsError.schemaChangeWouldLoseData`, so there is no path that forces a + /// destructive change without the server having asked first. + func confirmDestructiveSave() async { + guard let schema = pendingDestructiveSave, !isSaving else { return } + isSaving = true + error = nil + pendingDestructiveSave = nil + defer { isSaving = false } + + do { + let saved = try await lists.updateSchema(of: listId, schema: schema, force: true) eventBus.post(.schemaChanged(listId: listId, schema: saved)) didFinish = true } catch { diff --git a/AppTests/ListRowsViewModelTests.swift b/AppTests/ListRowsViewModelTests.swift index faaae61..bbb6c3a 100644 --- a/AppTests/ListRowsViewModelTests.swift +++ b/AppTests/ListRowsViewModelTests.swift @@ -23,7 +23,7 @@ final class ListRowsViewModelTests: XCTestCase { XCTAssertEqual(viewModel.rows.map(\.id), ["R1"]) XCTAssertEqual(viewModel.schema.fields.map(\.name), ["Title"]) - XCTAssertEqual(viewModel.columns, ["Title"]) + XCTAssertEqual(viewModel.columns.map(\.key), ["Title"]) } // MARK: - entityFields (schema-entity view, work-consolidation.md §1b) @@ -94,7 +94,7 @@ final class ListRowsViewModelTests: XCTestCase { XCTAssertEqual(viewModel.rows.count, 1) XCTAssertTrue(viewModel.schema.fields.isEmpty) - XCTAssertEqual(viewModel.columns, ["A"]) + XCTAssertEqual(viewModel.columns.map(\.key), ["A"]) } // MARK: - addRow optimistic insert @@ -294,3 +294,78 @@ final class ListRowsViewModelTests: XCTestCase { XCTAssertEqual(viewModel.schema.fields.map(\.name), ["Z"]) } } + +// MARK: - Column key vs. label (GitHub #85) +// +// The lists UI used one `name` as both the header text and the `row.fields` +// subscript. That was correct only while the two were the same token — true for +// a schema typed as the client's DSL, false for one authored on the web, where +// the server keeps `propertyKey` and `propertyName` apart. Now that macOS can +// read a server-authored schema at all, a column labelled "Publication Year" +// over a key of `year` must still find its cell. + +extension ListRowsViewModelTests { + + // Happy path + + func test_givenColumnsWhoseLabelDiffersFromKey_whenRendering_thenCellsStillResolve() async { + let stub = StubListsService() + let schema = ListSchema(fields: [ + SchemaField(key: "title", label: "Title", type: .text), + SchemaField(key: "year", label: "Publication Year", type: .number) + ]) + await stub.enqueueSchema(success: schema) + let row = ListsFixtures.row( + id: "R1", + listId: "L1", + fields: ["title": .string("Dune"), "year": .int(1965)] + ) + await stub.enqueueRows(success: RowsPage(rows: [row], hasMore: false, nextOffset: nil)) + let viewModel = ListRowsViewModel(lists: stub, eventBus: ListsEventBus(), listId: "L1") + + await viewModel.initialLoad() + + // The header shows the label; the subscript uses the key. + XCTAssertEqual(viewModel.columns.map(\.label), ["Title", "Publication Year"]) + XCTAssertEqual(viewModel.columns.map(\.key), ["title", "year"]) + let loaded = try? XCTUnwrap(viewModel.rows.first) + XCTAssertEqual(loaded?.fields[viewModel.columns[1].key], .int(1965)) + // The failure this guards against: looking the cell up by its label. + XCTAssertNil(loaded?.fields["Publication Year"]) + } + + // Boundary — ordering and visibility come from the server + + func test_givenOutOfOrderAndHiddenColumns_whenRendering_thenOrderIsHonouredAndHiddenAreDropped() async { + let stub = StubListsService() + let schema = ListSchema(fields: [ + SchemaField(key: "c", label: "Third", type: .text, displayOrder: 2), + SchemaField(key: "a", label: "First", type: .text, displayOrder: 0), + SchemaField(key: "secret", label: "Hidden", type: .text, isVisible: false, displayOrder: 1) + ]) + await stub.enqueueSchema(success: schema) + await stub.enqueueRows(success: .empty) + let viewModel = ListRowsViewModel(lists: stub, eventBus: ListsEventBus(), listId: "L1") + + await viewModel.initialLoad() + + // `displayOrder` is the authority, not array order — and a hidden column + // keeps its data but does not take a column in the table. + XCTAssertEqual(viewModel.columns.map(\.key), ["a", "c"]) + } + + // Invalid — a schema with no columns falls back to the observed row keys + + func test_givenNoSchema_whenRowsHaveKeys_thenColumnsUseTheKeysAsTheirOwnLabels() async { + let stub = StubListsService() + await stub.enqueueSchema(success: .empty) + let row = ListsFixtures.row(id: "R1", listId: "L1", fields: ["b": .string("x"), "a": .string("y")]) + await stub.enqueueRows(success: RowsPage(rows: [row], hasMore: false, nextOffset: nil)) + let viewModel = ListRowsViewModel(lists: stub, eventBus: ListsEventBus(), listId: "L1") + + await viewModel.initialLoad() + + XCTAssertEqual(viewModel.columns.map(\.key), ["a", "b"]) + XCTAssertEqual(viewModel.columns.map(\.label), ["a", "b"], "with no schema the key is all there is") + } +} diff --git a/AppTests/NewListViewModelTests.swift b/AppTests/NewListViewModelTests.swift index 7136fa1..03f1b9c 100644 --- a/AppTests/NewListViewModelTests.swift +++ b/AppTests/NewListViewModelTests.swift @@ -83,3 +83,70 @@ final class NewListViewModelTests: XCTestCase { } } } + +// MARK: - The schema goes on the wire as columns, not as a DSL string (GitHub #85) +// +// `POST /api/lists` rejects a string schema outright — +// `400 {"error":"Invalid schema: DSL must be an object"}` — so a list created +// from macOS with columns never worked. The DSL survives as an *authoring* +// convenience: what the user types is parsed here, client-side, and the parsed +// columns are what travel. + +extension NewListViewModelTests { + + // Happy path + + func test_givenTypedDSL_whenSubmitting_thenParsedColumnsAreSentNotTheString() async { + let stub = StubListsService() + await stub.enqueueCreate(success: ListsFixtures.ownedList(id: "L1", title: "Films")) + let viewModel = NewListViewModel(lists: stub, eventBus: ListsEventBus()) + viewModel.title = "Films" + viewModel.schemaDSL = "Title:text, Year:number" + + await viewModel.submit() + + XCTAssertTrue(viewModel.didFinish) + let recorded = await stub.recorded + guard case .create(_, _, let schema, _, _)? = recorded.first?.kind else { + return XCTFail("expected create, got \(String(describing: recorded.first))") + } + XCTAssertEqual(schema?.fields.map(\.key), ["Title", "Year"]) + XCTAssertEqual(schema?.fields.map(\.type), [.text, .number]) + } + + // Invalid input — rejected before the service is called + + func test_givenMalformedDSL_whenSubmitting_thenNoCreateCallIsMade() async { + // The parse failure is now caught client-side and reported against the + // field the user typed into, instead of arriving as an opaque 400. + let stub = StubListsService() + let viewModel = NewListViewModel(lists: stub, eventBus: ListsEventBus()) + viewModel.title = "Films" + viewModel.schemaDSL = "Bogus without a colon" + + await viewModel.submit() + + XCTAssertFalse(viewModel.didFinish) + XCTAssertNotNil(viewModel.error) + let recorded = await stub.recorded + XCTAssertTrue(recorded.isEmpty, "a malformed schema never reaches the network") + } + + // Boundary — no schema typed at all + + func test_givenNoDSL_whenSubmitting_thenSchemaIsNilRatherThanEmpty() async { + let stub = StubListsService() + await stub.enqueueCreate(success: ListsFixtures.ownedList(id: "L1", title: "Bare")) + let viewModel = NewListViewModel(lists: stub, eventBus: ListsEventBus()) + viewModel.title = "Bare" + viewModel.schemaDSL = " " + + await viewModel.submit() + + let recorded = await stub.recorded + guard case .create(_, _, let schema, _, _)? = recorded.first?.kind else { + return XCTFail("expected create") + } + XCTAssertNil(schema, "whitespace is not a schema") + } +} diff --git a/AppTests/SchemaEditorViewModelTests.swift b/AppTests/SchemaEditorViewModelTests.swift index 81d4f7e..857c7de 100644 --- a/AppTests/SchemaEditorViewModelTests.swift +++ b/AppTests/SchemaEditorViewModelTests.swift @@ -104,9 +104,10 @@ final class SchemaEditorViewModelTests: XCTestCase { XCTAssertTrue(viewModel.didFinish) XCTAssertNil(viewModel.error) let recorded = await stub.recorded - if case .updateSchema(let listId, let count) = recorded.first?.kind { + if case .updateSchema(let listId, let count, let force) = recorded.first?.kind { XCTAssertEqual(listId, "L1") XCTAssertEqual(count, 1) + XCTAssertFalse(force, "a routine save never confirms a destructive change up front") } else { XCTFail("expected updateSchema, got \(String(describing: recorded.first))") } @@ -322,3 +323,108 @@ final class SchemaEditorViewModelTests: XCTestCase { ) } } + +// MARK: - The destructive-change confirmation (GitHub #85) +// +// The server refuses a schema rebuild that would drop a column still holding row +// data, answering `400` with the affected column keys. That is a question, not a +// malfunction — so the editor has to ask it rather than showing a bare "Bad +// Request", and must never pre-answer it by forcing on the first attempt. + +extension SchemaEditorViewModelTests { + + // Upstream failure → the confirmation is offered + + func test_givenServerRefusesADestructiveChange_whenSaving_thenTheConfirmationIsOffered() async { + let stub = StubListsService() + await stub.enqueueUpdateSchema( + failure: ListsError.schemaChangeWouldLoseData( + serverMessage: "Removing these columns would delete existing data." + ) + ) + let viewModel = SchemaEditorViewModel( + lists: stub, + eventBus: ListsEventBus(), + listId: "L1", + role: .owner, + initialSchema: ListSchema(fields: [SchemaField(name: "Title", type: .text)]) + ) + + await viewModel.save() + + XCTAssertFalse(viewModel.didFinish, "nothing was saved") + XCTAssertNotNil(viewModel.pendingDestructiveSave, "the editor holds the schema to re-submit") + XCTAssertEqual( + (viewModel.error as? ListsError)?.localizedDescription, + "Removing these columns would delete existing data.", + "the server's own sentence is shown, not a client-written paraphrase" + ) + } + + // Happy path → confirming re-sends with force + + func test_givenTheUserConfirms_whenReSaving_thenTheCallCarriesForce() async { + let stub = StubListsService() + await stub.enqueueUpdateSchema( + failure: ListsError.schemaChangeWouldLoseData(serverMessage: "would delete data") + ) + let saved = ListSchema(fields: [SchemaField(name: "Title", type: .text)]) + await stub.enqueueUpdateSchema(success: saved) + let viewModel = SchemaEditorViewModel( + lists: stub, + eventBus: ListsEventBus(), + listId: "L1", + role: .owner, + initialSchema: saved + ) + + await viewModel.save() + await viewModel.confirmDestructiveSave() + + XCTAssertTrue(viewModel.didFinish) + XCTAssertNil(viewModel.pendingDestructiveSave, "the confirmation is consumed, not sticky") + let recorded = await stub.recorded + let forces = recorded.compactMap { record -> Bool? in + if case .updateSchema(_, _, let force) = record.kind { return force } + return nil + } + XCTAssertEqual(forces, [false, true], "asked first, then confirmed — never forced up front") + } + + // Invalid — confirming without having been asked is a no-op + + func test_givenNoPendingConfirmation_whenConfirming_thenNoCallIsMade() async { + let stub = StubListsService() + let viewModel = SchemaEditorViewModel( + lists: stub, + eventBus: ListsEventBus(), + listId: "L1", + role: .owner, + initialSchema: ListSchema(fields: [SchemaField(name: "Title", type: .text)]) + ) + + await viewModel.confirmDestructiveSave() + + let recorded = await stub.recorded + XCTAssertTrue(recorded.isEmpty, "there is no path to a forced write the server did not ask for") + } + + // Boundary — an ordinary bad request is not mistaken for the confirmation + + func test_givenAnOrdinaryFailure_whenSaving_thenNoConfirmationIsOffered() async { + let stub = StubListsService() + await stub.enqueueUpdateSchema(failure: TestError.upstream("boom")) + let viewModel = SchemaEditorViewModel( + lists: stub, + eventBus: ListsEventBus(), + listId: "L1", + role: .owner, + initialSchema: ListSchema(fields: [SchemaField(name: "Title", type: .text)]) + ) + + await viewModel.save() + + XCTAssertNotNil(viewModel.error) + XCTAssertNil(viewModel.pendingDestructiveSave, "a 500 is not a question to answer") + } +} diff --git a/AppTests/Support/StubListsService.swift b/AppTests/Support/StubListsService.swift index 969acf6..cc9bea6 100644 --- a/AppTests/Support/StubListsService.swift +++ b/AppTests/Support/StubListsService.swift @@ -18,11 +18,14 @@ struct RecordedListsCall: Sendable, Equatable { case publicRows(username: String, slug: String, limit: Int, offset: Int) case myLists(limit: Int, offset: Int) case detail(listId: String) - case create(title: String, description: String?, schema: String?, parentId: String?, isPublic: Bool) + // `schema` is the parsed columns, not a DSL string — the string form + // is rejected by the server (GitHub #85), and recording it lets a test + // assert the DSL was parsed before the call rather than after. + case create(title: String, description: String?, schema: ListSchema?, parentId: String?, isPublic: Bool) case update(listId: String, title: String?, description: String?, isPublic: Bool?, parentId: String?) case delete(listId: String) case schema(listId: String) - case updateSchema(listId: String, fieldsCount: Int) + case updateSchema(listId: String, fieldsCount: Int, force: Bool) case refresh(listId: String) case rows(listId: String, limit: Int, offset: Int) case row(listId: String, rowId: String) @@ -187,7 +190,7 @@ actor StubListsService: ListsServicing { return try take(&detailOutcomes, label: "detail") } - func create(title: String, description: String?, schema: String?, parentId: String?, isPublic: Bool) async throws -> OwnedList { + func create(title: String, description: String?, schema: ListSchema?, parentId: String?, isPublic: Bool) async throws -> OwnedList { recorded.append(.init(kind: .create(title: title, description: description, schema: schema, parentId: parentId, isPublic: isPublic))) return try take(&createOutcomes, label: "create") } @@ -209,8 +212,8 @@ actor StubListsService: ListsServicing { return try take(&schemaOutcomes, label: "schema") } - func updateSchema(of listId: String, schema: ListSchema) async throws -> ListSchema { - recorded.append(.init(kind: .updateSchema(listId: listId, fieldsCount: schema.fields.count))) + func updateSchema(of listId: String, schema: ListSchema, force: Bool) async throws -> ListSchema { + recorded.append(.init(kind: .updateSchema(listId: listId, fieldsCount: schema.fields.count, force: force))) lastUpdatedSchema = schema return try take(&updateSchemaOutcomes, label: "updateSchema") } diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListMappers.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListMappers.swift index 4ef5cb7..afe6870 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListMappers.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListMappers.swift @@ -106,3 +106,28 @@ extension RowsPage { ) } } + + +// MARK: - Wire projection helper + +/// Recursive projection from the domain's loose `ListCellValue` back to the +/// kit's `ListJSONValue` — the inverse of `ListCellValue.init(from:)` above. +/// +/// Used when writing a row and when serialising a column's `defaultValue`. It +/// lived `fileprivate` in `ListsService.swift`, which meant the schema mappers +/// could not reuse it; the two directions belong side by side. +extension ListJSONValue { + init(from value: ListCellValue) { + switch value { + case .null: self = .null + case .bool(let v): self = .bool(v) + case .int(let v): self = .int(v) + case .double(let v): self = .double(v) + case .string(let v): self = .string(v) + case .array(let items): + self = .array(items.map { ListJSONValue(from: $0) }) + case .object(let dict): + self = .object(dict.mapValues { ListJSONValue(from: $0) }) + } + } +} diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchema.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchema.swift index ac2e74c..57f6a93 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchema.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchema.swift @@ -2,44 +2,174 @@ import Foundation /// A single column in a list's schema (PLAN.md §1 "Structured lists", §6 M3). /// -/// Mirrors the `"Name:type"` pair in the schema DSL (`"Title:text"`, -/// `"Year:number"`). `nullable` and `enumValues` are reserved for future -/// per-type metadata as the upstream API documents it (see -/// `/API-backend-prompts-to-build.md` item 2.2); both are `nil` for the -/// starter set. +/// The server keeps the column's **identity** and its **display name** apart — +/// `propertyKey` vs `propertyName` on the wire — and row data is keyed by the +/// identity: +/// +/// ```json +/// "rowData": { "title": "Dune", "year": 1965 } +/// ``` +/// +/// This type had a single `name` doing both jobs, which was harmless only +/// because the client could not create a schema at all (GitHub #85: the server +/// rejects the DSL string the client sent). The moment that was fixed, a column +/// whose label differed from its key would have rendered every cell empty. So +/// `key` and `label` are separate here, and `key` is the identity. +/// +/// The remaining fields are the per-column metadata the server has always +/// stored and the client never read — captured live 2026-09-15 and recorded in +/// `docs/spikes/list-schema-wire-shapes.md`. They unblock the validation and +/// help-text work in GitHub #50, which `work-consolidation.md` `P2-G` had listed +/// as API-unconfirmed. public struct SchemaField: Sendable, Equatable, Hashable, Identifiable { - /// Column name as written in the DSL. Case- and whitespace-sensitive. - public let name: String + /// The row-data key. `ListRow.fields` is keyed by this, **not** by `label`. + public let key: String + + /// The display name shown in a column header and a row-form label. + public let label: String /// Column type. public let type: SchemaFieldType - /// Whether the cell may be null. `nil` means "the DSL did not state it" — - /// distinct from `false`, which means "the DSL declared this column NOT - /// NULL". The starter parser does not consume this field; it is reserved - /// for future DSL extensions per prompts file 2.2. - public let nullable: Bool? + /// Whether a row must supply a value. `nil` means the source did not say. + public let isRequired: Bool? + + /// Whether the column is shown. A hidden column keeps its data — this is + /// not deletion. + public let isVisible: Bool? + + /// Zero-based column order as the server reports it. `nil` when the source + /// was the client-side DSL, where array order is the only ordering. + public let displayOrder: Int? + + /// Hint text shown under the field in a row form. + public let helpText: String? + + /// Placeholder text for an empty field. + public let placeholder: String? + + /// The column's default for a new row, as its own type would express it. + public let defaultValue: ListCellValue? + + /// Per-type validation rules the server enforces. + public let validation: SchemaFieldValidation? - /// For `enum(...)` columns, the closed set of allowed values. `nil` for - /// non-enum columns. Reserved for the M3 schema-editor enum picker; the - /// starter parser does not consume this field. + /// For `select` columns, the closed set of allowed values. public let enumValues: [String]? - /// Identity is the column name (schemas forbid duplicates). - public var id: String { name } + /// Legacy spelling of `label`, kept because the DSL — where a column's key + /// and its label are the same token — is still how lists are authored by + /// hand in the New List sheet. + /// + /// - Warning: never use this to index `ListRow.fields`. Use ``key``. + public var name: String { label } + /// Whether the cell may be null. + /// + /// Inverted view of ``isRequired`` and kept for the callers that read it. + /// `nil` when the source stated neither. + public var nullable: Bool? { + guard let isRequired else { return nil } + return !isRequired + } + + /// Identity is the column key (schemas forbid duplicate keys). + public var id: String { key } + + /// Hashed on identity alone. + /// + /// Written out rather than synthesised because `defaultValue` is a + /// `ListCellValue`, which is deliberately `Equatable` but not `Hashable` — + /// it can carry arbitrary nested JSON. Hashing the key is both cheaper and + /// the correct notion of identity for a column. + public func hash(into hasher: inout Hasher) { + hasher.combine(key) + } + + /// Full initialiser — used when projecting a server payload, which supplies + /// a distinct key and label. public init( - name: String, + key: String, + label: String, type: SchemaFieldType, - nullable: Bool? = nil, + isRequired: Bool? = nil, + isVisible: Bool? = nil, + displayOrder: Int? = nil, + helpText: String? = nil, + placeholder: String? = nil, + defaultValue: ListCellValue? = nil, + validation: SchemaFieldValidation? = nil, enumValues: [String]? = nil ) { - self.name = name + self.key = key + self.label = label self.type = type - self.nullable = nullable + self.isRequired = isRequired + self.isVisible = isVisible + self.displayOrder = displayOrder + self.helpText = helpText + self.placeholder = placeholder + self.defaultValue = defaultValue + self.validation = validation self.enumValues = enumValues } + + /// DSL initialiser — the hand-authored case, where the one token the user + /// typed is both the key and the label. + public init( + name: String, + type: SchemaFieldType, + nullable: Bool? = nil, + enumValues: [String]? = nil + ) { + self.init( + key: name, + label: name, + type: type, + isRequired: nullable.map { !$0 }, + enumValues: enumValues + ) + } +} + +/// The validation rules the server stores against a column. +/// +/// Every rule is optional and they combine per type: `text` carries +/// `minLength`/`maxLength`/`pattern`, `number` carries `min`/`max`, `select` +/// carries its options (surfaced on `SchemaField.enumValues`). +/// +/// `pattern` is deliberately **not** evaluated client-side. It is a server-side +/// regular expression, and pre-validating it against a different engine would +/// let the client reject a value the server would accept — a worse failure than +/// a round-trip. +public struct SchemaFieldValidation: Sendable, Equatable, Hashable { + public let min: Double? + public let max: Double? + public let minLength: Int? + public let maxLength: Int? + public let pattern: String? + + public init( + min: Double? = nil, + max: Double? = nil, + minLength: Int? = nil, + maxLength: Int? = nil, + pattern: String? = nil + ) { + self.min = min + self.max = max + self.minLength = minLength + self.maxLength = maxLength + self.pattern = pattern + } + + /// `true` when no rule is set. A column with an empty rule set should send + /// no `validation` object at all rather than an empty one, which the server + /// reads as "clear the rules". + public var isEmpty: Bool { + min == nil && max == nil && minLength == nil && maxLength == nil && pattern == nil + } } /// The parsed schema for a list. @@ -61,8 +191,26 @@ public struct ListSchema: Sendable, Equatable, Hashable { /// for round-tripping `""` (rejected by the parser) and for tests. public static let empty = ListSchema(fields: []) + /// Lookup by column **key** — the identity `ListRow.fields` is keyed by. + public func field(key: String) -> SchemaField? { + fields.first { $0.key == key } + } + /// Lookup helper used by the row-cell typed accessor (M3 schema editor). + /// + /// Matches on `key` first and falls back to `label`, because callers that + /// predate the key/label split pass whichever one they had — and for a + /// DSL-authored schema the two are the same token anyway. public func field(named name: String) -> SchemaField? { - fields.first { $0.name == name } + fields.first { $0.key == name } ?? fields.first { $0.label == name } + } + + /// The columns in the order the UI should render them: `displayOrder` when + /// the server supplied it, declaration order otherwise. + public var orderedFields: [SchemaField] { + guard fields.contains(where: { $0.displayOrder != nil }) else { return fields } + return fields.enumerated() + .sorted { ($0.element.displayOrder ?? $0.offset) < ($1.element.displayOrder ?? $1.offset) } + .map(\.element) } } diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchemaMappers.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchemaMappers.swift new file mode 100644 index 0000000..ef5300e --- /dev/null +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/ListSchemaMappers.swift @@ -0,0 +1,129 @@ +import Foundation +import InterlinedKit + +// MARK: - Schema DSL object ⇄ domain +// +// The one place the wire's schema object and the domain's `ListSchema` meet +// (GitHub #85). Everything about the two-spellings problem is contained here: +// the server sends a column as `{key, type, label, …}` on the schema routes and +// as `{propertyKey, propertyType, propertyName, validationRules, …}` on the +// property projection, and `ListPropertyDTO.asSchemaField` funnels the second +// into the first so this file only ever sees one shape. + +extension SchemaFieldValidation { + + /// Projects the wire's validation object, dropping `options` — those live + /// on `SchemaField.enumValues` because they are a type-level fact about a + /// `select` column, not a constraint the user edits alongside min/max. + init?(dto: ListFieldValidationDTO?) { + guard let dto else { return nil } + let projected = SchemaFieldValidation( + min: dto.min, + max: dto.max, + minLength: dto.minLength, + maxLength: dto.maxLength, + pattern: dto.pattern + ) + // An object carrying nothing but `options` is not a validation rule set; + // returning it would make every select column look constrained. + if projected.isEmpty { return nil } + self = projected + } + + /// The wire form. `options` is supplied by the caller, which is the only + /// place that knows whether the column is a `select`. + func asDTO(options: [String]?) -> ListFieldValidationDTO? { + let dto = ListFieldValidationDTO( + min: min, + max: max, + minLength: minLength, + maxLength: maxLength, + pattern: pattern, + options: options + ) + // Never encode an empty rules object: the server reads it as "clear the + // rules", so sending one on an untouched column would quietly erase + // rules that were authored on the web. + return dto.isEmpty ? nil : dto + } +} + +extension SchemaField { + + /// Projects one wire column. + /// + /// An unrecognised `type` token is **not** a decode failure — it maps to + /// `.text`, the type whose editor can display any value without lying about + /// it. Failing the whole schema because the server added a column type the + /// client does not know yet would take out the entire list. + init(dto: ListSchemaFieldDTO) { + self.init( + key: dto.key, + label: dto.label ?? dto.key, + type: SchemaFieldType(rawValue: dto.type) ?? .text, + isRequired: dto.required, + isVisible: dto.visible, + displayOrder: dto.displayOrder, + helpText: dto.helpText, + placeholder: dto.placeholder, + defaultValue: dto.defaultValue.map { ListCellValue(from: $0) }, + validation: SchemaFieldValidation(dto: dto.validation), + enumValues: dto.resolvedOptions + ) + } + + /// The wire form of this column. + var asDTO: ListSchemaFieldDTO { + // An empty string is not the same as an absent hint: sending `""` sets + // the column's help text to an empty string, where omitting the key + // leaves whatever is stored alone. + let trimmedHelp = helpText.flatMap { $0.isEmpty ? nil : $0 } + let trimmedPlaceholder = placeholder.flatMap { $0.isEmpty ? nil : $0 } + // Spelled out rather than `.map(ListJSONValue.init(from:))`: that + // reference is ambiguous against `Decodable.init(from:)`. + let wireDefault: ListJSONValue? = defaultValue.map { ListJSONValue(from: $0) } + + return ListSchemaFieldDTO( + key: key, + type: type.rawValue, + label: label, + displayOrder: displayOrder, + required: isRequired, + visible: isVisible, + helpText: trimmedHelp, + placeholder: trimmedPlaceholder, + defaultValue: wireDefault, + validation: validation?.asDTO(options: enumValues), + options: enumValues + ) + } +} + +extension ListSchema { + + /// Projects the schema object returned by `GET /api/lists/[id]/schema`. + init(dto: ListSchemaDSLDTO) { + self.init(fields: dto.fields.map(SchemaField.init(dto:))) + } + + /// The wire form, for a create or a schema rebuild. + /// + /// - Parameter name: the schema's own `name`. The server accepts it and + /// uses it as the list description's sibling; passing the list title is + /// what the web does. + func asDTO(name: String?, description: String? = nil) -> ListSchemaDSLDTO { + ListSchemaDSLDTO( + name: name, + description: description, + fields: orderedFields.map(\.asDTO) + ) + } + + /// A one-line rendering of the columns, for the places that show a list's + /// shape without opening the editor (the detail header, the Markdown + /// export). Uses the client DSL spelling because that is what those surfaces + /// already display. + var dslDescription: String { + SchemaDSL.serialize(self) + } +} diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedList.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedList.swift index ae42790..10fcee8 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedList.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedList.swift @@ -28,10 +28,23 @@ public struct OwnedList: Sendable, Equatable, Hashable, Identifiable { /// Visibility. The owned routes return `isPublic` on every row. public let visibility: Visibility - /// The raw schema DSL string from the API (e.g. `"Title:text, Year:number"`). - /// `nil` for lists with no schema yet (the API may omit the field). + /// A one-line rendering of the columns, in the client's DSL spelling, for + /// surfaces that show a list's shape without opening the editor. + /// + /// This used to be read straight off `ListDTO.schema` — a wire field the + /// server has **never** sent, so it was always `nil` and every surface that + /// displayed it showed nothing (GitHub #85). It is now derived from + /// ``schema`` when the route returned the list's columns. public let schemaDescription: String? + /// The list's columns, when the route returned them. + /// + /// `nil` means *"this route does not carry columns"* — the lightweight + /// collection rows do not — which is **not** the same as a list with no + /// columns (`ListSchema.empty`). Collapsing the two would make every list in + /// a collection page look column-less. + public let schema: ListSchema? + /// Parent list id for nested lists (PLAN.md §1 "Nested lists"). public let parentID: String? @@ -49,6 +62,7 @@ public struct OwnedList: Sendable, Equatable, Hashable, Identifiable { description: String? = nil, visibility: Visibility = .private, schemaDescription: String? = nil, + schema: ListSchema? = nil, parentID: String? = nil, gitHubSource: GitHubListSource? = nil, createdAt: Date? = nil, @@ -58,7 +72,10 @@ public struct OwnedList: Sendable, Equatable, Hashable, Identifiable { self.title = title self.description = description self.visibility = visibility - self.schemaDescription = schemaDescription + // Derive the display string from the columns when the caller supplied + // them and no explicit string — so the two can never disagree. + self.schemaDescription = schemaDescription ?? schema?.dslDescription + self.schema = schema self.parentID = parentID self.gitHubSource = gitHubSource self.createdAt = createdAt diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedListMappers.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedListMappers.swift index a0a12e6..abe0023 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedListMappers.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/OwnedListMappers.swift @@ -13,12 +13,20 @@ extension OwnedList { /// the API omits the flag — the authenticated routes return private /// lists by default, so the safe-default is `.private`. public init(from dto: ListDTO) { + // `dto.properties` is the field that actually carries the columns; + // `dto.schema` is a DSL string the server has never sent (GitHub #85). + // Absent properties leave `schema` nil — "this route does not return + // columns" — rather than collapsing to an empty schema. + let columns = dto.schemaFields.map { fields in + ListSchema(fields: fields.map(SchemaField.init(dto:))) + } self.init( id: dto.id, title: dto.title, description: dto.description, visibility: Visibility(publiclyVisible: dto.isPublic ?? false), - schemaDescription: dto.schema, + schemaDescription: nil, + schema: columns, parentID: dto.parentId, // The kit's `ListDTO` does not yet carry GitHub-source fields // (prompts file item 2.3); leave the field `nil` and let the diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/ListsService.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/ListsService.swift index f3489e7..47e326c 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/ListsService.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/ListsService.swift @@ -28,6 +28,15 @@ public enum ListsError: Error, Sendable, Equatable { /// the route treats a missing `userId` as "subscribe *me* to this list", /// which is a different action entirely (work-consolidation.md G23). case invalidWatcher + + /// A schema rebuild would drop a column that still holds row data, and the + /// server refused it pending confirmation. Re-submit with `force: true` to + /// accept the data loss. + /// + /// Carries the server's own sentence rather than a client-written one: the + /// server names the situation accurately and the column list it also sends + /// is not reachable from here (see `updateSchema(of:schema:force:)`). + case schemaChangeWouldLoseData(serverMessage: String?) } extension ListsError: LocalizedError, CustomStringConvertible { @@ -41,6 +50,9 @@ extension ListsError: LocalizedError, CustomStringConvertible { return "Schema \"\(raw)\" could not be parsed: \(reason.description)" case .invalidWatcher: return "Choose a person to share this list with." + case .schemaChangeWouldLoseData(let serverMessage): + return serverMessage + ?? "This change would delete columns that still hold data. Confirm to continue." } } } @@ -108,7 +120,7 @@ public protocol ListsServicing: Sendable { func create( title: String, description: String?, - schema: String?, + schema: ListSchema?, parentId: String?, isPublic: Bool ) async throws -> OwnedList @@ -135,7 +147,12 @@ public protocol ListsServicing: Sendable { /// Writes a typed schema to a list. Serializes the schema to the DSL /// form before posting. - func updateSchema(of listId: String, schema: ListSchema) async throws -> ListSchema + /// Rebuilds a list's columns. + /// + /// - Parameter force: confirms a destructive change. Without it the server + /// refuses to drop a column that still holds row data, and the call throws + /// `ListsError.schemaChangeWouldLoseData`. + func updateSchema(of listId: String, schema: ListSchema, force: Bool) async throws -> ListSchema // MARK: - M3 refresh (GitHub-backed) @@ -298,8 +315,8 @@ public final class ListsService: ListsServicing { username: String, slug: String ) async throws -> ListDetail { - let dto = try await api.send(Lists.publicList(username: username, id: slug)) - return ListDetail(from: dto) + let response = try await api.send(Lists.publicList(username: username, id: slug)) + return ListDetail(from: response.list) } public func publicRows( @@ -365,14 +382,24 @@ public final class ListsService: ListsServicing { } public func detail(listId: String) async throws -> OwnedList { - let dto = try await api.send(Lists.get(id: listId)) - return OwnedList(from: dto) + let response = try await api.send(Lists.get(id: listId)) + return OwnedList(from: response.data) } + /// Creates a list, optionally with its columns. + /// + /// `schema` used to be the client's DSL **string**, which the server rejects + /// outright (`400 "Invalid schema: DSL must be an object"`) — so a list with + /// columns could never be created from macOS (GitHub #85). It is now the + /// parsed `ListSchema`, serialised to the object the server wants. + /// + /// Callers that hold a DSL string parse it first: `SchemaDSL.parse` is still + /// how the New List sheet turns what the user typed into columns. The DSL + /// remains an authoring convenience; it is no longer a wire format. public func create( title: String, description: String?, - schema: String?, + schema: ListSchema?, parentId: String?, isPublic: Bool ) async throws -> OwnedList { @@ -380,12 +407,17 @@ public final class ListsService: ListsServicing { let request = CreateListRequest( title: title, description: description, - schema: schema, + // An empty schema is not a schema: sending `{fields: []}` would ask + // the server to create a column-less list explicitly, where omitting + // the key lets it apply its own default. + schema: (schema?.fields.isEmpty == false) + ? schema?.asDTO(name: title, description: description) + : nil, parentId: parentId, isPublic: isPublic ) - let dto = try await api.send(Lists.create(request)) - return OwnedList(from: dto) + let response = try await api.send(Lists.create(request)) + return OwnedList(from: response.data) } public func update( @@ -401,8 +433,8 @@ public final class ListsService: ListsServicing { isPublic: isPublic, parentId: parentId ) - let dto = try await api.send(Lists.update(id: listId, request)) - return OwnedList(from: dto) + let response = try await api.send(Lists.update(id: listId, request)) + return OwnedList(from: response.data) } public func delete(listId: String) async throws { @@ -411,23 +443,62 @@ public final class ListsService: ListsServicing { // MARK: - M3 schema + /// Reads a list's columns. + /// + /// The response is the schema **object**; there is no DSL string to parse + /// and therefore no `malformedSchema` failure mode on this path any more — + /// an unrecognised column type degrades to `.text` rather than failing the + /// whole schema (see `SchemaField.init(dto:)`). public func schema(of listId: String) async throws -> ListSchema { - let dto = try await api.send(Lists.schema(id: listId)) - return try parseSchema(dto.schema) + let response = try await api.send(Lists.schema(id: listId)) + return ListSchema(dto: response.data) } - public func updateSchema(of listId: String, schema: ListSchema) async throws -> ListSchema { - let dsl = SchemaDSL.serialize(schema) - let request = UpdateListSchemaRequest(schema: dsl) - let dto = try await api.send(Lists.updateSchema(id: listId, request)) - return try parseSchema(dto.schema) + /// Rebuilds a list's columns. + /// + /// - Parameter force: confirms a destructive change. The server refuses to + /// drop a column that still holds row data unless this is set, answering + /// `400` with the affected column keys — surfaced as + /// `ListsError.schemaChangeWouldLoseData` so the UI can name them and ask, + /// rather than showing a bare "Bad Request" for what is really a question. + public func updateSchema( + of listId: String, + schema: ListSchema, + force: Bool = false + ) async throws -> ListSchema { + let request = UpdateListSchemaRequest(schema: schema.asDTO(name: nil)) + do { + let response = try await api.send(Lists.updateSchema(id: listId, request, force: force)) + // The write answers the list plus its stored columns, so the result + // is read back from `properties` rather than re-fetching. + if let fields = response.data.schemaFields { + return ListSchema(fields: fields.map(SchemaField.init(dto:))) + } + return schema + } catch let error as APIError { + // The server refuses a destructive rebuild with `400` plus a + // `propertiesWithData` array naming the columns that still hold + // data. Re-badge it so the UI can offer the confirmation instead of + // showing a bare "Bad Request" for what is really a question. + // + // The column list is **not** available here: `APIError.badRequest` + // carries only the decoded `{error}` string, so the rest of the body + // is discarded before this point. `ListSchemaConflictDTO` models the + // full shape and this becomes a one-line change once the kit keeps + // the body — filed as its own issue rather than worked around with a + // second request that could race the first. + if case .badRequest(let message) = error, !force { + throw ListsError.schemaChangeWouldLoseData(serverMessage: message) + } + throw error + } } // MARK: - M3 refresh public func refresh(listId: String) async throws -> OwnedList { - let dto = try await api.send(Lists.refresh(id: listId)) - return OwnedList(from: dto) + let response = try await api.send(Lists.refresh(id: listId)) + return OwnedList(from: response.data) } // MARK: - M3 row CRUD @@ -662,25 +733,7 @@ public final class ListsService: ListsServicing { } } -// MARK: - Wire projection helper - -/// Recursive projection from the domain's loose `ListCellValue` back to the -/// kit's `ListJSONValue` — used when writing rows. The two enums are -/// structurally identical (M1 chose to project the wire union into a domain -/// equivalent so view code never sees `ListJSONValue`); this is the inverse -/// of the `init(from value:)` already in `ListMappers.swift`. -extension ListJSONValue { - fileprivate init(from value: ListCellValue) { - switch value { - case .null: self = .null - case .bool(let v): self = .bool(v) - case .int(let v): self = .int(v) - case .double(let v): self = .double(v) - case .string(let v): self = .string(v) - case .array(let items): - self = .array(items.map(ListJSONValue.init(from:))) - case .object(let dict): - self = .object(dict.mapValues(ListJSONValue.init(from:))) - } - } -} +// The `ListCellValue` → `ListJSONValue` projection this file uses when writing +// rows moved to `ListMappers.swift`, next to its inverse. It was `fileprivate` +// here, which stopped the schema mappers reusing it for a column's +// `defaultValue` (GitHub #85). diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/ListsServiceTests.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/ListsServiceTests.swift index 51f2eec..d2d63c4 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/ListsServiceTests.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/ListsServiceTests.swift @@ -84,15 +84,15 @@ final class ListsServiceTests: XCTestCase { // MARK: - publicList (detail) - func test_givenListExists_whenLoadingPublicList_thenMapsDetailAndSchema() async throws { - // Given + func test_givenListExists_whenLoadingPublicList_thenUnwrapsTheListEnvelope() async throws { + // Given the real envelope: `{ "list": {…}, "ancestors": [] }`, not a + // bare list object. This decoded a bare `ListDTO`, so the public list + // page could not load at all (GitHub #85). let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject( + await api.enqueue(json: Fixtures.publicListEnvelope( id: "books", title: "Books", - description: "Things I have read", - isPublic: true, - schema: "Title:text, Year:number" + description: "Things I have read" )) let service = ListsService(api: api) @@ -102,16 +102,28 @@ final class ListsServiceTests: XCTestCase { // Then XCTAssertEqual(detail.id, "books") XCTAssertEqual(detail.title, "Books") - XCTAssertEqual(detail.schemaDescription, "Title:text, Year:number") - XCTAssertEqual(detail.visibility, .public) + XCTAssertEqual(detail.description, "Things I have read") let recorded = await api.recorded XCTAssertEqual(recorded.first?.path, "/api/users/ada/lists/books") } + func test_givenPublicListEnvelope_whenLoading_thenSchemaDescriptionIsNil() async throws { + // The public projection carries **no columns** — confirmed against the + // live route. The old test asserted a schema string here, which only + // passed because the fixture invented one. + let api = StubAPIClient() + await api.enqueue(json: Fixtures.publicListEnvelope(id: "books")) + let service = ListsService(api: api) + + let detail = try await service.publicList(username: "ada", slug: "books") + + XCTAssertNil(detail.schemaDescription, "the public route returns no schema to show") + } + func test_givenListMissingSchema_whenLoadingPublicList_thenSchemaDescriptionIsNil() async throws { // Given — boundary: list with no schema defined yet. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject(id: "raw", schema: nil)) + await api.enqueue(json: Fixtures.publicListEnvelope(id: "raw")) let service = ListsService(api: api) // When diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/OwnedListsServiceTests.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/OwnedListsServiceTests.swift index ded98aa..ff6a51d 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/OwnedListsServiceTests.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/OwnedListsServiceTests.swift @@ -274,16 +274,19 @@ final class OwnedListsServiceTests: XCTestCase { // MARK: - detail - func test_givenOwnedListExists_whenLoadingDetail_thenMapsAllFields() async throws { - // Given + func test_givenOwnedListExists_whenLoadingDetail_thenUnwrapsTheDataEnvelope() async throws { + // Given the real `{ "data": { …, "properties": [...] } }` envelope. + // `Lists.get` decoded a bare `ListDTO`, so `detail(listId:)` could never + // decode a live response (GitHub #75) — and the test that said otherwise + // was asserting against a hand-written bare object. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject( + await api.enqueue(json: Fixtures.listEnvelope( id: "books", title: "Books", description: "Read pile", isPublic: false, - schema: "Title:text, Year:number", - parentId: "parent-list" + parentId: "parent-list", + properties: Fixtures.listPropertiesJSON )) let service = ListsService(api: api) @@ -295,8 +298,11 @@ final class OwnedListsServiceTests: XCTestCase { XCTAssertEqual(list.title, "Books") XCTAssertEqual(list.description, "Read pile") XCTAssertEqual(list.visibility, .private) - XCTAssertEqual(list.schemaDescription, "Title:text, Year:number") XCTAssertEqual(list.parentID, "parent-list") + // The columns come from `properties` — the field the server actually + // sends — and keep key and label apart. + XCTAssertEqual(list.schema?.fields.map(\.key), ["title", "year"]) + XCTAssertEqual(list.schema?.fields.map(\.label), ["Title", "Publication Year"]) let recorded = await api.recorded XCTAssertEqual(recorded.first?.path, "/api/lists/books") } @@ -304,7 +310,7 @@ final class OwnedListsServiceTests: XCTestCase { func test_givenIsPublicMissing_whenLoadingDetail_thenDefaultsToPrivate() async throws { // Given — boundary: API omits `isPublic`. Authenticated path defaults to private. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject(id: "books", isPublic: nil)) + await api.enqueue(json: Fixtures.listEnvelope(id: "books", isPublic: nil)) let service = ListsService(api: api) // When @@ -331,17 +337,22 @@ final class OwnedListsServiceTests: XCTestCase { // MARK: - create - func test_givenTitleAndSchema_whenCreating_thenPOSTsListAndMapsResponse() async throws { - // Given + func test_givenTitleAndSchema_whenCreating_thenSendsTheSchemaObjectAndUnwrapsTheEnvelope() async throws { + // Given — the create answers `{ message, data }` with the stored columns. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject(id: "new-list", title: "Films")) + await api.enqueue(json: Fixtures.listEnvelope( + id: "new-list", + title: "Films", + properties: Fixtures.listPropertiesJSON, + message: "List created successfully" + )) let service = ListsService(api: api) // When let list = try await service.create( title: "Films", description: nil, - schema: "Title:text, Year:number", + schema: try SchemaDSL.parse("Title:text, Year:number"), parentId: nil, isPublic: false ) @@ -352,6 +363,44 @@ final class OwnedListsServiceTests: XCTestCase { let recorded = await api.recorded XCTAssertEqual(recorded.first?.method, "POST") XCTAssertEqual(recorded.first?.path, "/api/lists") + + // And the wire body carries the schema as an **object**. A string here + // is a flat 400 from the server — "Invalid schema: DSL must be an + // object" — which is why creating a list with columns never worked + // (GitHub #85). Asserting the shape is the whole point of this case. + let body = try XCTUnwrap(recorded.first?.body) + let json = try XCTUnwrap( + try JSONSerialization.jsonObject(with: body) as? [String: Any] + ) + let schema = try XCTUnwrap(json["schema"] as? [String: Any], + "schema must be an object, not a string") + let fields = try XCTUnwrap(schema["fields"] as? [[String: Any]]) + XCTAssertEqual(fields.map { $0["key"] as? String }, ["Title", "Year"]) + XCTAssertEqual(fields.map { $0["type"] as? String }, ["text", "number"]) + } + + func test_givenNoSchema_whenCreating_thenTheSchemaKeyIsOmittedEntirely() async throws { + // Boundary. An empty schema is not the same as no schema: sending + // `{"fields": []}` asks for an explicitly column-less list, where + // omitting the key lets the server apply its own default. + let api = StubAPIClient() + await api.enqueue(json: Fixtures.listEnvelope(id: "bare", title: "Bare")) + let service = ListsService(api: api) + + _ = try await service.create( + title: "Bare", + description: nil, + schema: ListSchema.empty, + parentId: nil, + isPublic: false + ) + + let recorded = await api.recorded + let body = try XCTUnwrap(recorded.first?.body) + let json = try XCTUnwrap( + try JSONSerialization.jsonObject(with: body) as? [String: Any] + ) + XCTAssertNil(json["schema"], "an empty schema sends no schema key at all") } func test_givenEmptyTitle_whenCreating_thenAPIRejection() async throws { @@ -401,7 +450,7 @@ final class OwnedListsServiceTests: XCTestCase { func test_givenChanges_whenUpdating_thenPUTsAndReturnsUpdatedList() async throws { // Given let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject(id: "books", title: "Books v2")) + await api.enqueue(json: Fixtures.listEnvelope(id: "books", title: "Books v2", message: "List updated")) let service = ListsService(api: api) // When @@ -423,7 +472,7 @@ final class OwnedListsServiceTests: XCTestCase { func test_givenAllFieldsNil_whenUpdating_thenStillIssuesPut() async throws { // Given — boundary: a no-op update body still hits the endpoint. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject(id: "books")) + await api.enqueue(json: Fixtures.listEnvelope(id: "books")) let service = ListsService(api: api) // When @@ -495,38 +544,73 @@ final class OwnedListsServiceTests: XCTestCase { // MARK: - schema (read) - func test_givenValidDSL_whenLoadingSchema_thenParsesIntoFields() async throws { - // Given + func test_givenCapturedSchemaPayload_whenLoading_thenMapsEveryColumnFacet() async throws { + // Given the **captured** `GET /api/lists/[id]/schema` payload. The + // previous fixture was `{"schema": "Title:text, Year:number"}` — a shape + // the server has never sent; the test passed and the feature did not + // work (GitHub #85). let api = StubAPIClient() - await api.enqueue(json: Fixtures.listSchemaEnvelope("Title:text, Year:number")) + await api.enqueue(json: Fixtures.listSchemaEnvelope) let service = ListsService(api: api) // When let schema = try await service.schema(of: "books") - // Then - XCTAssertEqual(schema.fields.map(\.name), ["Title", "Year"]) - XCTAssertEqual(schema.fields.map(\.type), [.text, .number]) + // Then — key and label are separate, and the metadata the server has + // always stored finally arrives. + XCTAssertEqual(schema.orderedFields.map(\.key), ["title", "year", "status"]) + XCTAssertEqual(schema.orderedFields.map(\.label), ["Title", "Publication Year", "Status"]) + XCTAssertEqual(schema.orderedFields.map(\.type), [.text, .number, .select]) + + let title = try XCTUnwrap(schema.field(key: "title")) + XCTAssertEqual(title.isRequired, true) + XCTAssertEqual(title.helpText, "What is it called?") + XCTAssertEqual(title.placeholder, "e.g. Dune") + XCTAssertEqual(title.validation?.minLength, 2) + XCTAssertEqual(title.validation?.maxLength, 80) + XCTAssertEqual(title.validation?.pattern, "^[A-Za-z].*$") + + let year = try XCTUnwrap(schema.field(key: "year")) + XCTAssertEqual(year.validation?.min, 1000) + XCTAssertEqual(year.validation?.max, 2100) + + // The select column's options arrive under both spellings live; either + // alone must be enough. + let status = try XCTUnwrap(schema.field(key: "status")) + XCTAssertEqual(status.enumValues, ["todo", "doing", "done"]) + XCTAssertEqual(status.defaultValue, .string("todo")) + let recorded = await api.recorded XCTAssertEqual(recorded.first?.path, "/api/lists/books/schema") } - func test_givenMalformedDSL_whenLoadingSchema_thenThrowsMalformedSchema() async throws { - // Given — invalid-input case at the domain boundary. + func test_givenUnknownColumnType_whenLoadingSchema_thenDegradesToTextRatherThanFailing() async throws { + // Invalid input from upstream. A column type the client has never heard + // of must not take out the whole schema — and with it the row table — + // so it maps to `.text`, the editor that can display anything. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listSchemaEnvelope("Bogus without colon")) + await api.enqueue(json: Fixtures.listSchemaEnvelope(fields: [ + (key: "title", type: "text", label: "Title"), + (key: "colour", type: "colour-picker-2027", label: "Colour") + ])) let service = ListsService(api: api) - // When / Then - do { - _ = try await service.schema(of: "books") - XCTFail("Expected ListsError.malformedSchema") - } catch let error as ListsError { - guard case .malformedSchema(let raw, _) = error else { - return XCTFail("Expected .malformedSchema, got \(error)") - } - XCTAssertEqual(raw, "Bogus without colon") - } + let schema = try await service.schema(of: "books") + + XCTAssertEqual(schema.fields.map(\.key), ["title", "colour"]) + XCTAssertEqual(schema.field(key: "colour")?.type, .text) + } + + func test_givenEmptyFieldList_whenLoadingSchema_thenSchemaIsEmptyNotAFailure() async throws { + // Boundary: a list with no columns yet. The live route answers + // `{"data":{"name":"New list","fields":[]}}` for exactly this. + let api = StubAPIClient() + await api.enqueue(json: Fixtures.listSchemaEnvelope(fields: [])) + let service = ListsService(api: api) + + let schema = try await service.schema(of: "books") + + XCTAssertEqual(schema, .empty) } func test_givenAPIFailure_whenLoadingSchema_thenThrows() async throws { @@ -546,10 +630,14 @@ final class OwnedListsServiceTests: XCTestCase { // MARK: - schema (write) - func test_givenSchema_whenUpdatingSchema_thenSerializesAndReparsesResult() async throws { - // Given + func test_givenSchema_whenUpdatingSchema_thenSendsAnObjectAndReadsBackTheStoredColumns() async throws { + // Given — the write answers `{ message, data: { …, properties[] } }`. let api = StubAPIClient() - await api.enqueue(json: Fixtures.listSchemaEnvelope("Title:text, Year:number")) + await api.enqueue(json: Fixtures.listEnvelope( + id: "books", + properties: Fixtures.listPropertiesJSON, + message: "Schema updated successfully" + )) let service = ListsService(api: api) let schema = ListSchema(fields: [ SchemaField(name: "Title", type: .text), @@ -557,30 +645,84 @@ final class OwnedListsServiceTests: XCTestCase { ]) // When - let reparsed = try await service.updateSchema(of: "books", schema: schema) + let saved = try await service.updateSchema(of: "books", schema: schema, force: false) + + // Then — the result comes from `properties`, so key and label are the + // server's, not the ones we sent. + XCTAssertEqual(saved.fields.map(\.key), ["title", "year"]) + XCTAssertEqual(saved.fields.map(\.label), ["Title", "Publication Year"]) - // Then - XCTAssertEqual(reparsed, schema) let recorded = await api.recorded XCTAssertEqual(recorded.first?.method, "PUT") XCTAssertEqual(recorded.first?.path, "/api/lists/books/schema") + XCTAssertNil(recorded.first?.query["force"], "a first save never forces") + + // The body carries a schema **object**, not the DSL string that the + // server rejects. + let body = try XCTUnwrap(recorded.first?.body) + let json = try XCTUnwrap(try JSONSerialization.jsonObject(with: body) as? [String: Any]) + XCTAssertNotNil(json["schema"] as? [String: Any], "schema must be an object") } - func test_givenEmptySchema_whenUpdatingSchema_thenServerDSLIsEmpty() async throws { - // Given — boundary: serializing an empty schema yields `""`; the - // request is still issued, and the server's reply drives the parse. - // The parser rejects `""`, so we model the server returning a - // single-field schema instead — this confirms the response is what - // dictates the returned value. + func test_givenDestructiveRejection_whenUpdatingSchema_thenSurfacesTheConfirmationError() async throws { + // Upstream failure. The server refuses to drop a column that still holds + // row data with a 400 — which is a question, not a malfunction — so it + // must not reach the UI as a bare "Bad Request". let api = StubAPIClient() - await api.enqueue(json: Fixtures.listSchemaEnvelope("Title:text")) + await api.enqueue(failure: .badRequest( + serverMessage: "Removing these columns would delete existing data." + )) let service = ListsService(api: api) - // When - let result = try await service.updateSchema(of: "books", schema: ListSchema.empty) + do { + _ = try await service.updateSchema( + of: "books", + schema: ListSchema(fields: [SchemaField(name: "Title", type: .text)]), + force: false + ) + XCTFail("Expected ListsError.schemaChangeWouldLoseData") + } catch let error as ListsError { + guard case .schemaChangeWouldLoseData(let message) = error else { + return XCTFail("Expected .schemaChangeWouldLoseData, got \(error)") + } + XCTAssertEqual(message, "Removing these columns would delete existing data.") + } + } - // Then - XCTAssertEqual(result.fields.map(\.name), ["Title"]) + func test_givenForceRequested_whenUpdatingSchema_thenTheQueryCarriesIt() async throws { + // And with the user's confirmation the same call goes out with `force`, + // so a 400 after that is a real failure rather than the same question + // asked twice. + let api = StubAPIClient() + await api.enqueue(json: Fixtures.listEnvelope( + id: "books", + properties: Fixtures.listPropertiesJSON + )) + let service = ListsService(api: api) + + _ = try await service.updateSchema( + of: "books", + schema: ListSchema(fields: [SchemaField(name: "Title", type: .text)]), + force: true + ) + + let recorded = await api.recorded + XCTAssertEqual(recorded.first?.query["force"], "true") + } + + func test_givenEmptySchema_whenUpdatingSchema_thenStillIssuesThePut() async throws { + // Boundary: clearing every column is a legitimate request. It is also + // the maximally destructive one, so it must still go through the + // unforced path first. + let api = StubAPIClient() + await api.enqueue(json: Fixtures.listEnvelope(id: "books", properties: "[]")) + let service = ListsService(api: api) + + let result = try await service.updateSchema(of: "books", schema: .empty, force: false) + + XCTAssertEqual(result, .empty) + let recorded = await api.recorded + XCTAssertEqual(recorded.first?.method, "PUT") } // MARK: - refresh @@ -588,7 +730,7 @@ final class OwnedListsServiceTests: XCTestCase { func test_givenGitHubBackedList_whenRefreshing_thenReturnsFreshList() async throws { // Given let api = StubAPIClient() - await api.enqueue(json: Fixtures.listObject(id: "gh-list")) + await api.enqueue(json: Fixtures.listEnvelope(id: "gh-list")) let service = ListsService(api: api) // When diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/Fixtures.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/Fixtures.swift index 246f1e5..e498ff6 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/Fixtures.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/Fixtures.swift @@ -94,29 +94,211 @@ enum Fixtures { // MARK: - Lists fixtures - /// A single `ListDTO` object body (the inner JSON of one list). + /// A single list object, as the live API actually sends one. + /// + /// Captured 2026-09-15 from `GET /api/lists` and `POST /api/lists` on the + /// `.env` test account. The previous version of this fixture carried a + /// `"schema": "Title:text, Year:number"` key that **the server has never + /// sent** — a fabricated field that kept `OwnedList.schemaDescription`'s + /// tests green while the real thing was always `nil` (GitHub #85). + /// + /// - Parameter properties: the column projection, present on the + /// single-list and write routes and absent from the collection rows. + /// `nil` reproduces a collection row. static func listObject( id: String, title: String = "Books", description: String? = "Things I have read", isPublic: Bool? = true, - schema: String? = "Title:text, Year:number", - parentId: String? = nil + parentId: String? = nil, + properties: String? = nil ) -> String { let descJSON = description.map { "\"\($0)\"" } ?? "null" - let schemaJSON = schema.map { "\"\($0)\"" } ?? "null" let parentJSON = parentId.map { "\"\($0)\"" } ?? "null" let isPublicJSON = isPublic.map { $0 ? "true" : "false" } ?? "null" + let propertiesJSON = properties.map { ",\n \"properties\": \($0)" } ?? "" return """ { "id": "\(id)", + "userId": "usr_1", + "messageId": null, + "parentId": \(parentJSON), + "folderId": null, "title": "\(title)", "description": \(descJSON), "isPublic": \(isPublicJSON), - "schema": \(schemaJSON), - "parentId": \(parentJSON), + "metadata": null, + "source": "local", + "githubRepo": null, + "githubRepoPrivate": null, "createdAt": "\(createdAtISO)", - "updatedAt": "\(createdAtISO)" + "updatedAt": "\(createdAtISO)", + "deletedAt": null\(propertiesJSON) + } + """ + } + + /// The `properties` array exactly as `POST /api/lists` returned it for a + /// two-column schema. Captured, not written by hand. + /// + /// Note `propertyKey` differs from `propertyName` on the second column — + /// that is deliberate, and is what makes the key/label split testable. + static let listPropertiesJSON = """ + [ + { + "id": "prp_1", + "listId": "L1", + "propertyKey": "title", + "propertyName": "Title", + "propertyType": "text", + "displayOrder": 0, + "isRequired": true, + "defaultValue": null, + "validationRules": { "minLength": 2, "maxLength": 80 }, + "helpText": "What is it called?", + "placeholder": "e.g. Dune", + "isVisible": true, + "visibilityCondition": null, + "createdAt": "\(createdAtISO)", + "updatedAt": "\(createdAtISO)" + }, + { + "id": "prp_2", + "listId": "L1", + "propertyKey": "year", + "propertyName": "Publication Year", + "propertyType": "number", + "displayOrder": 1, + "isRequired": false, + "defaultValue": null, + "validationRules": { "min": 1000, "max": 2100 }, + "helpText": null, + "placeholder": null, + "isVisible": true, + "visibilityCondition": null, + "createdAt": "\(createdAtISO)", + "updatedAt": "\(createdAtISO)" + } + ] + """ + + /// `GET /api/lists/[id]` → `{ "data": { … } }`, and with a `message` the + /// same envelope the create / update / schema writes answer. + static func listEnvelope( + id: String, + title: String = "Books", + description: String? = "Things I have read", + isPublic: Bool? = true, + parentId: String? = nil, + properties: String? = nil, + message: String? = nil + ) -> String { + let object = listObject( + id: id, + title: title, + description: description, + isPublic: isPublic, + parentId: parentId, + properties: properties + ) + let messageJSON = message.map { "\"message\": \"\($0)\",\n " } ?? "" + return """ + { \(messageJSON)"data": \(object) } + """ + } + + /// `GET /api/lists/[id]/schema` → `{ "data": { name, description?, fields[] } }`. + /// + /// The field list is the captured payload from the 2026-09-15 probe, right + /// down to the `select` column carrying its options under **both** + /// `validation.options` and `options`. + static let listSchemaEnvelope = """ + { + "data": { + "name": "probe", + "description": "recon probe", + "fields": [ + { + "key": "title", + "type": "text", + "label": "Title", + "displayOrder": 0, + "required": true, + "helpText": "What is it called?", + "placeholder": "e.g. Dune", + "visible": true, + "validation": { "pattern": "^[A-Za-z].*$", "maxLength": 80, "minLength": 2 } + }, + { + "key": "year", + "type": "number", + "label": "Publication Year", + "displayOrder": 1, + "required": false, + "visible": true, + "validation": { "max": 2100, "min": 1000 } + }, + { + "key": "status", + "type": "select", + "label": "Status", + "displayOrder": 2, + "required": false, + "visible": true, + "defaultValue": "todo", + "validation": { "options": ["todo", "doing", "done"] }, + "options": ["todo", "doing", "done"] + } + ] + } + } + """ + + /// A schema envelope built from `(key, type, label)` triples, for the cases + /// that care about mapping rather than about the full captured payload. + /// + /// Passing a `label` that differs from the `key` is the point: it is the + /// combination the client used to get wrong. + static func listSchemaEnvelope(fields: [(key: String, type: String, label: String)]) -> String { + let fieldObjects = fields.enumerated().map { index, field in + """ + { + "key": "\(field.key)", + "type": "\(field.type)", + "label": "\(field.label)", + "displayOrder": \(index), + "required": false, + "visible": true + } + """ + }.joined(separator: ",") + return """ + { "data": { "name": "fixture", "fields": [\(fieldObjects)] } } + """ + } + + /// `GET /api/users/[username]/lists/[id]` → `{ "list": {…}, "ancestors": [] }`. + /// + /// A much lighter projection than the authenticated route: no `isPublic`, + /// no timestamps, and no columns. Captured 2026-09-15. + static func publicListEnvelope( + id: String, + title: String = "Books", + description: String? = "Things I have read", + parentId: String? = nil + ) -> String { + let descJSON = description.map { "\"\($0)\"" } ?? "null" + let parentJSON = parentId.map { "\"\($0)\"" } ?? "null" + return """ + { + "list": { + "id": "\(id)", + "title": "\(title)", + "description": \(descJSON), + "parentId": \(parentJSON), + "children": [] + }, + "ancestors": [] } """ } diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift index a0d042f..791ccb5 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift @@ -26,6 +26,13 @@ actor StubAPIClient: APIClientProtocol { let method: String let path: String let query: [String: String] + /// The encoded JSON body, when the request carried one. + /// + /// Recorded so a test can assert the **shape** that goes on the wire, + /// not only that a call was made. Without it, `create(…)` sending its + /// schema as a string rather than an object was untestable at this + /// layer — and the server rejects the string outright (GitHub #85). + let body: Data? } private var outcomes: [Outcome] = [] @@ -87,8 +94,22 @@ actor StubAPIClient: APIClientProtocol { for item in request.query where item.value != nil { query[item.name] = item.value } + // Encoded with the client's own encoder so key naming and date strategy + // match what really goes out; a body that fails to encode is recorded as + // `nil` rather than failing the recording. + var bodyData: Data? + if case .json(let payload)? = request.body { + bodyData = try? JSONCoders.makeEncoder().encode(payload) + } else if case .raw(let data, _)? = request.body { + bodyData = data + } recorded.append( - RecordedRequest(method: request.method.rawValue, path: request.path, query: query) + RecordedRequest( + method: request.method.rawValue, + path: request.path, + query: query, + body: bodyData + ) ) } } diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListDTO.swift b/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListDTO.swift index 3e4c8b1..a1d91f6 100644 --- a/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListDTO.swift +++ b/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListDTO.swift @@ -71,9 +71,20 @@ public struct ListDTO: Codable, Sendable, Equatable, Identifiable { public let title: String public let description: String? public let isPublic: Bool? - /// The schema DSL string (e.g. `"Title:text, Year:number"`). Present on - /// detail and create responses. + /// Was documented as "the schema DSL string, present on detail and create + /// responses". It is present on **no** captured payload — the server has + /// never sent a `schema` key on a list object, so this decoded to `nil` + /// every time and `OwnedList.schemaDescription` was always empty (GitHub #85). + /// + /// Kept decodable rather than deleted so an older cached payload still + /// round-trips, but `properties` is the field that actually carries the + /// columns. Nothing should read this. public let schema: String? + + /// The list's columns, as `GET /api/lists/[id]`, `POST /api/lists` and + /// `PUT /api/lists/[id]/schema` return them. Absent on the lightweight + /// collection rows, which is why it is optional. + public let properties: [ListPropertyDTO]? /// Parent list id for nested lists. public let parentId: String? public let createdAt: Date? @@ -133,7 +144,8 @@ public struct ListDTO: Codable, Sendable, Equatable, Identifiable { githubRepoPrivate: Bool? = nil, role: String? = nil, user: ListUserDTO? = nil, - parent: ListParentDTO? = nil + parent: ListParentDTO? = nil, + properties: [ListPropertyDTO]? = nil ) { self.id = id self.title = title @@ -151,6 +163,18 @@ public struct ListDTO: Codable, Sendable, Equatable, Identifiable { self.role = role self.user = user self.parent = parent + self.properties = properties + } + + /// The list's columns expressed in the DSL spelling, ordered by + /// `displayOrder`. `nil` when the route did not return them — which is + /// different from "the list has no columns", and callers must not collapse + /// the two. + public var schemaFields: [ListSchemaFieldDTO]? { + guard let properties else { return nil } + return properties + .sorted { ($0.displayOrder ?? 0) < ($1.displayOrder ?? 0) } + .map(\.asSchemaField) } } @@ -191,16 +215,10 @@ public struct ListParentDTO: Codable, Sendable, Equatable, Identifiable { } // MARK: - List schema - -/// Response of `GET /api/lists/[id]/schema` and `PUT /api/lists/[id]/schema`: -/// `{ "schema": "" }`. -public struct ListSchemaDTO: Codable, Sendable, Equatable { - public let schema: String - - public init(schema: String) { - self.schema = schema - } -} +// +// The schema types moved to `ListSchemaDSLDTO.swift` when the wire shape was +// corrected (GitHub #85). The old `ListSchemaDTO { schema: String }` modelled a +// DSL string the API never accepted or returned on these routes. // MARK: - List rows @@ -547,14 +565,20 @@ public struct ListRowWriteResponse: Codable, Sendable, Equatable { public struct CreateListRequest: Codable, Sendable, Equatable { public let title: String public let description: String? - public let schema: String? + /// The list's columns, as the **List Schema DSL object**. + /// + /// This was a `String` carrying the client's own `"Title:text"` DSL, which + /// the server rejects outright: + /// `400 {"error":"Invalid schema: DSL must be an object"}`. Creating a + /// schema-bearing list from macOS therefore never worked (GitHub #85). + public let schema: ListSchemaDSLDTO? public let parentId: String? public let isPublic: Bool? public init( title: String, description: String? = nil, - schema: String? = nil, + schema: ListSchemaDSLDTO? = nil, parentId: String? = nil, isPublic: Bool? = nil ) { @@ -586,14 +610,8 @@ public struct UpdateListRequest: Codable, Sendable, Equatable { } } -/// `PUT /api/lists/[id]/schema` body: `{ "schema": "" }`. -public struct UpdateListSchemaRequest: Codable, Sendable, Equatable { - public let schema: String - - public init(schema: String) { - self.schema = schema - } -} +// `UpdateListSchemaRequest` now lives in `ListSchemaDSLDTO.swift` and carries an +// object, not a string. See GitHub #85 for the captured payloads. /// `POST /api/lists/[id]/data` body: `{ "data": { ... } }`. /// diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListSchemaDSLDTO.swift b/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListSchemaDSLDTO.swift new file mode 100644 index 0000000..f7060d1 --- /dev/null +++ b/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/ListSchemaDSLDTO.swift @@ -0,0 +1,351 @@ +import Foundation + +// MARK: - The List Schema DSL object +// +// The client used to model a list's schema as a **string** (`"Title:text, +// Year:number"`). The API models it as an **object**, and always has — +// `POST /api/lists` with a string schema answers a flat +// `400 {"error":"Invalid schema: DSL must be an object"}`. So creating a +// schema-bearing list from macOS was impossible, reading a schema could not +// decode, and saving one was rejected (GitHub #85, and #75 for the sibling +// envelope defect on `GET /api/lists/{id}`). +// +// Everything below is modelled against **captured** payloads, not against the +// OpenAPI examples — the spec's `PUT .../schema` 200 example shows a bare +// `{properties: […]}` where the live route answers `{message, data}`. Where the +// two disagree, the capture wins. Probe transcripts: `docs/spikes/list-schema-wire-shapes.md`. + +/// The schema of a list, as `GET /api/lists/[id]/schema` returns it under +/// `data` and as `POST /api/lists` / `PUT /api/lists/[id]/schema` accept it +/// under `schema`. +/// +/// Read and write use the **same** shape, which is why one type serves both. +public struct ListSchemaDSLDTO: Codable, Sendable, Equatable { + + /// The schema's own name. On a read this mirrors the list title; on a write + /// the server accepts it and does not appear to use it to rename the list. + public let name: String? + + /// Optional prose describing the schema. Round-trips; a write sets the + /// list's `description`. + public let description: String? + + /// The ordered columns. The server also returns `displayOrder` per field, + /// so array order and `displayOrder` agree on a read — but `displayOrder` + /// is the authority, because a future partial update could disagree. + public let fields: [ListSchemaFieldDTO] + + public init(name: String? = nil, description: String? = nil, fields: [ListSchemaFieldDTO]) { + self.name = name + self.description = description + self.fields = fields + } +} + +/// One column in the schema DSL object. +/// +/// - Important: `key` and `label` are **different things**, and conflating them +/// is the trap this shape sets. Row data is keyed by `key` +/// (`"rowData":{"title":"Dune","year":1965}`), while `label` is only what the +/// header renders. A client that used one value for both would render every +/// cell empty for any column whose display name differs from its key. +public struct ListSchemaFieldDTO: Codable, Sendable, Equatable { + + /// The row-data key. This is what `ListRowDTO.rowData` is keyed by. + public let key: String + + /// The column type token: `text`, `number`, `boolean`, `date`, `url`, + /// `email`, `select`, `markdown`. + public let type: String + + /// The display name shown in the column header. + public let label: String? + + /// Zero-based column order. Returned on every read. + public let displayOrder: Int? + + /// Whether a row must supply this column. + public let required: Bool? + + /// Whether the column is shown. Distinct from deletion — a hidden column + /// keeps its data. + public let visible: Bool? + + /// Hint text shown under the field in the row form. + public let helpText: String? + + /// Placeholder text for an empty field. + public let placeholder: String? + + /// The column's default for a new row. Type-erased because it follows the + /// column's own type: `false` for a boolean, `"todo"` for a select. + public let defaultValue: ListJSONValue? + + /// Per-type validation rules. See `ListFieldValidationDTO`. + public let validation: ListFieldValidationDTO? + + /// The option set for a `select` column. + /// + /// The server returns the options **twice** on a read — once here and once + /// as `validation.options` — and accepts either on a write. Both are + /// modelled rather than picking one, because dropping the redundant spelling + /// would silently lose the options if the server ever stops sending the + /// other. `ListSchemaFieldDTO.resolvedOptions` is the single reader. + public let options: [String]? + + public init( + key: String, + type: String, + label: String? = nil, + displayOrder: Int? = nil, + required: Bool? = nil, + visible: Bool? = nil, + helpText: String? = nil, + placeholder: String? = nil, + defaultValue: ListJSONValue? = nil, + validation: ListFieldValidationDTO? = nil, + options: [String]? = nil + ) { + self.key = key + self.type = type + self.label = label + self.displayOrder = displayOrder + self.required = required + self.visible = visible + self.helpText = helpText + self.placeholder = placeholder + self.defaultValue = defaultValue + self.validation = validation + self.options = options + } + + /// The option set, from whichever of the two spellings the payload carried. + public var resolvedOptions: [String]? { + options ?? validation?.options + } +} + +/// The validation rules the server stores per column, as +/// `ListSchemaFieldDTO.validation` on a read/write and as +/// `ListPropertyDTO.validationRules` on the property projection. +/// +/// Every rule is optional and they are not mutually exclusive: a `text` column +/// can carry `minLength`/`maxLength`/`pattern`, a `number` column `min`/`max`, +/// and a `select` column `options`. Captured live 2026-09-15. +public struct ListFieldValidationDTO: Codable, Sendable, Equatable { + /// Minimum numeric value (`number` columns). + public let min: Double? + /// Maximum numeric value (`number` columns). + public let max: Double? + /// Minimum character count (`text`-family columns). + public let minLength: Int? + /// Maximum character count (`text`-family columns). + public let maxLength: Int? + /// A regular expression the value must match. Server-side grammar; treat as + /// opaque and surface the server's rejection rather than pre-validating + /// against a different regex engine. + public let pattern: String? + /// The allowed values for a `select` column. + public let options: [String]? + + public init( + min: Double? = nil, + max: Double? = nil, + minLength: Int? = nil, + maxLength: Int? = nil, + pattern: String? = nil, + options: [String]? = nil + ) { + self.min = min + self.max = max + self.minLength = minLength + self.maxLength = maxLength + self.pattern = pattern + self.options = options + } + + /// `true` when no rule is set — used to avoid encoding an empty object on a + /// write, which the server treats as "clear the rules". + public var isEmpty: Bool { + min == nil && max == nil && minLength == nil + && maxLength == nil && pattern == nil && (options?.isEmpty ?? true) + } +} + +// MARK: - The property projection + +/// A stored column as the server returns it alongside a list — under +/// `data.properties` on `GET /api/lists/[id]`, `POST /api/lists` and +/// `PUT /api/lists/[id]/schema`. +/// +/// This is the **same** information as `ListSchemaFieldDTO` under different +/// key names (`propertyKey`/`propertyName` rather than `key`/`label`, +/// `validationRules` rather than `validation`). Both are modelled because both +/// are what the server sends; `ListPropertyDTO.asSchemaField` is the one place +/// that reconciles them, so no caller has to know there are two spellings. +public struct ListPropertyDTO: Codable, Sendable, Equatable, Identifiable { + public let id: String + public let listId: String? + public let propertyKey: String + public let propertyName: String? + public let propertyType: String + public let displayOrder: Int? + public let isRequired: Bool? + public let defaultValue: ListJSONValue? + public let validationRules: ListFieldValidationDTO? + public let helpText: String? + public let placeholder: String? + public let isVisible: Bool? + /// A rule making this column's visibility depend on another column's value. + /// Never non-null on any captured payload, so it is kept type-erased rather + /// than guessed at — modelling an unseen shape is how the G21 and G25 + /// silent-decode defects happened. + public let visibilityCondition: ListJSONValue? + public let createdAt: Date? + public let updatedAt: Date? + + public init( + id: String, + listId: String? = nil, + propertyKey: String, + propertyName: String? = nil, + propertyType: String, + displayOrder: Int? = nil, + isRequired: Bool? = nil, + defaultValue: ListJSONValue? = nil, + validationRules: ListFieldValidationDTO? = nil, + helpText: String? = nil, + placeholder: String? = nil, + isVisible: Bool? = nil, + visibilityCondition: ListJSONValue? = nil, + createdAt: Date? = nil, + updatedAt: Date? = nil + ) { + self.id = id + self.listId = listId + self.propertyKey = propertyKey + self.propertyName = propertyName + self.propertyType = propertyType + self.displayOrder = displayOrder + self.isRequired = isRequired + self.defaultValue = defaultValue + self.validationRules = validationRules + self.helpText = helpText + self.placeholder = placeholder + self.isVisible = isVisible + self.visibilityCondition = visibilityCondition + self.createdAt = createdAt + self.updatedAt = updatedAt + } + + /// The same column expressed in the DSL spelling, so a caller that has a + /// `properties` array and a caller that has a `fields` array can share one + /// mapping path. + public var asSchemaField: ListSchemaFieldDTO { + ListSchemaFieldDTO( + key: propertyKey, + type: propertyType, + label: propertyName, + displayOrder: displayOrder, + required: isRequired, + visible: isVisible, + helpText: helpText, + placeholder: placeholder, + defaultValue: defaultValue, + validation: validationRules, + options: validationRules?.options + ) + } +} + +// MARK: - Envelopes + +/// `GET /api/lists/[id]/schema` → `{ "data": { …schema DSL… } }`. +public struct ListSchemaResponse: Codable, Sendable, Equatable { + public let data: ListSchemaDSLDTO + + public init(data: ListSchemaDSLDTO) { + self.data = data + } +} + +/// The `{ message?, data }` envelope every single-list write answers: +/// `POST /api/lists` (201), `PUT /api/lists/[id]`, `PUT /api/lists/[id]/schema` +/// and `POST /api/lists/[id]/refresh`. `GET /api/lists/[id]` uses the same +/// shape with no `message`, which is why `message` is optional. +public struct ListResponse: Codable, Sendable, Equatable { + public let message: String? + public let data: ListDTO + /// Present only for GitHub-backed lists, per the spec's `POST /api/lists` + /// 201 description. Never observed populated on the test account, which has + /// no accessible repositories (GitHub #51). + public let refreshStatus: String? + + public init(message: String? = nil, data: ListDTO, refreshStatus: String? = nil) { + self.message = message + self.data = data + self.refreshStatus = refreshStatus + } +} + +// MARK: - Write bodies + +/// `PUT /api/lists/[id]/schema` body — the destructive whole-schema rebuild. +/// +/// The route also accepts a `properties` array for a non-destructive per-column +/// update. Only the rebuild form is modelled here because it is the one the +/// schema editor performs; the per-column form is worth adding when a caller +/// needs it, and is noted rather than half-built. +/// +/// - Important: the rebuild has a **destructive-change guard**. Dropping a +/// column that still holds row data is rejected with `400` and a +/// `propertiesWithData` array naming the columns; the caller must re-submit +/// with `force: true` to confirm the data loss. See `ListSchemaConflictDTO`. +public struct UpdateListSchemaRequest: Codable, Sendable, Equatable { + public let schema: ListSchemaDSLDTO + public let parentId: String? + public let isPublic: Bool? + + public init(schema: ListSchemaDSLDTO, parentId: String? = nil, isPublic: Bool? = nil) { + self.schema = schema + self.parentId = parentId + self.isPublic = isPublic + } +} + +/// The `400` body returned when a schema rebuild would drop a column that still +/// holds row data. +/// +/// Modelled so the UI can name the columns and offer the confirmation, rather +/// than showing the user a bare "Bad Request" for what is really a question. +public struct ListSchemaConflictDTO: Codable, Sendable, Equatable { + public let error: String? + public let code: String? + /// The column keys that still hold data. + public let propertiesWithData: [String]? + + public init(error: String? = nil, code: String? = nil, propertiesWithData: [String]? = nil) { + self.error = error + self.code = code + self.propertiesWithData = propertiesWithData + } +} + +// MARK: - Public browse + +/// `GET /api/users/[username]/lists/[id]` → `{ "list": {…}, "ancestors": [] }`. +/// +/// The `ancestors` array is the breadcrumb trail up the parent chain, which the +/// client has never had and which is the natural source for a "Lists ▸ Parent ▸ +/// This" header on a public list page. It is modelled here so the information +/// stops being discarded at the wire; rendering it is a UI change for another +/// day. +public struct PublicListResponse: Codable, Sendable, Equatable { + public let list: ListDTO + public let ancestors: [ListParentDTO]? + + public init(list: ListDTO, ancestors: [ListParentDTO]? = nil) { + self.list = list + self.ancestors = ancestors + } +} diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/ListsEndpoint.swift b/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/ListsEndpoint.swift index 2fd2751..7f4c1cc 100644 --- a/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/ListsEndpoint.swift +++ b/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/ListsEndpoint.swift @@ -38,17 +38,32 @@ public enum Lists { } /// `POST /api/lists` - public static func create(_ body: CreateListRequest) -> Request { + /// + /// VERIFIED live 2026-09-15: answers `201 { message, data, refreshStatus? }`, + /// not a bare `ListDTO`, and `data.properties` carries the created columns. + /// The request's `schema` is an **object**; a string is rejected outright + /// (`400 "Invalid schema: DSL must be an object"`), so creating a + /// schema-bearing list from macOS never worked (GitHub #85). + public static func create(_ body: CreateListRequest) -> Request { Request(method: .post, path: "/api/lists", body: .json(body), auth: .bearer) } /// `GET /api/lists/[id]` - public static func get(id: String) -> Request { + /// + /// VERIFIED live 2026-09-15: answers `{ "data": { …list…, "properties": […] } }`. + /// This decoded a **bare** `ListDTO`, so `ListsService.detail(listId:)` could + /// never decode a real response (GitHub #75). No shipped path called it, and + /// its test passed against a fabricated bare fixture — the same combination + /// that produced the G21 link-metadata and G25 org-member defects. + public static func get(id: String) -> Request { Request(method: .get, path: "/api/lists/\(id)", auth: .bearer) } /// `PUT /api/lists/[id]` - public static func update(id: String, _ body: UpdateListRequest) -> Request { + /// + /// Answers the same `{ message, data }` envelope as create. Was decoding a + /// bare `ListDTO` (GitHub #85). + public static func update(id: String, _ body: UpdateListRequest) -> Request { Request(method: .put, path: "/api/lists/\(id)", body: .json(body), auth: .bearer) } @@ -60,19 +75,53 @@ public enum Lists { // MARK: - Schema /// `GET /api/lists/[id]/schema` - public static func schema(id: String) -> Request { + /// + /// VERIFIED live 2026-09-15: answers + /// `{ "data": { name, description?, fields: [ { key, type, label, displayOrder, + /// required, visible, helpText?, placeholder?, defaultValue?, validation?, + /// options? } ] } }` — an object, under `data`. + /// + /// This decoded `{ schema: String }`, so the route failed on every real + /// response. Unlike `get(id:)` it is **not** dead code: `ListRowsViewModel` + /// and `SchemaEditorView` both call it, so the row table and the schema + /// editor could not load a schema at all (GitHub #85). + public static func schema(id: String) -> Request { Request(method: .get, path: "/api/lists/\(id)/schema", auth: .bearer) } - /// `PUT /api/lists/[id]/schema` - public static func updateSchema(id: String, _ body: UpdateListSchemaRequest) -> Request { - Request(method: .put, path: "/api/lists/\(id)/schema", body: .json(body), auth: .bearer) + /// `PUT /api/lists/[id]/schema` — rebuild the schema from a DSL object. + /// + /// VERIFIED live 2026-09-15: the body is `{ schema: { name, description?, + /// fields[] } }` and the response is `{ message, data: { …list…, properties[] } }`. + /// Note the response **contradicts the OpenAPI 200 example**, which shows a + /// bare `{ properties: […] }`; the capture wins. + /// + /// `force` confirms a destructive change. Without it, dropping a column that + /// still holds row data is rejected with `400` and a `propertiesWithData` + /// array — see `ListSchemaConflictDTO`. + public static func updateSchema( + id: String, + _ body: UpdateListSchemaRequest, + force: Bool = false + ) -> Request { + Request( + method: .put, + path: "/api/lists/\(id)/schema", + query: force ? [.bool("force", true)] : [], + body: .json(body), + auth: .bearer + ) } // MARK: - Refresh (GitHub-backed) /// `POST /api/lists/[id]/refresh` - public static func refresh(id: String) -> Request { + /// + /// Shares the `{ message, data }` envelope with the other single-list + /// writes. Not exercised live — the test account has no accessible GitHub + /// repositories (GitHub #51) — so this follows the family rather than a + /// capture, and is flagged as such rather than claimed as verified. + public static func refresh(id: String) -> Request { Request(method: .post, path: "/api/lists/\(id)/refresh", auth: .bearer) } @@ -318,7 +367,17 @@ public enum Lists { } /// `GET /api/users/[username]/lists/[id]` — public, no auth. - public static func publicList(username: String, id: String) -> Request { + /// + /// VERIFIED live 2026-09-15 against a list made public for the probe: + /// answers `{ "list": { id, title, description, parentId, children[] }, "ancestors": [] }` + /// — a **named envelope with a breadcrumb trail**, not a bare `ListDTO`. So + /// the public list detail view could not decode either (GitHub #85). + /// + /// Note the projection is far lighter than the authenticated one: no + /// `isPublic`, no `createdAt`, and **no columns**. `ListDetail.schemaDescription` + /// has therefore always been `nil` on this path — there is no schema to show + /// for someone else's public list without a second call. + public static func publicList(username: String, id: String) -> Request { Request(method: .get, path: "/api/users/\(username)/lists/\(id)", auth: .none) } diff --git a/Packages/InterlinedKit/Tests/InterlinedKitTests/ListSchemaWireShapeTests.swift b/Packages/InterlinedKit/Tests/InterlinedKitTests/ListSchemaWireShapeTests.swift new file mode 100644 index 0000000..9586934 --- /dev/null +++ b/Packages/InterlinedKit/Tests/InterlinedKitTests/ListSchemaWireShapeTests.swift @@ -0,0 +1,254 @@ +// ListSchemaWireShapeTests +// +// Decode tests for the list and list-schema wire shapes, against payloads +// **captured live** on 2026-09-15 rather than written by hand (GitHub #75, #85). +// +// Why that distinction is the whole point of this file: three separate silent +// decode defects in this client — G21 link metadata, G25 org members, and the +// two fixed here — all shipped with green tests, because each test invented the +// payload it then asserted against. A fabricated fixture tests that the decoder +// matches the fixture, which is a tautology. Every JSON literal below is a +// verbatim response body; the transcripts are in +// `docs/spikes/list-schema-wire-shapes.md`. + +import XCTest +@testable import InterlinedKit + +final class ListSchemaWireShapeTests: XCTestCase { + + private func decode(_ type: T.Type, _ json: String) throws -> T { + try JSONCoders.makeDecoder().decode(type, from: Data(json.utf8)) + } + + // MARK: - GET /api/lists/{id} — the #75 envelope + + /// Captured from `GET /api/lists/851954cb-…`. + private let singleListJSON = """ + { + "data": { + "id": "851954cb-f9ee-4a1f-b864-d57ddeb639d6", + "userId": "15e3d575-98bc-40e5-9aba-0d9cc9e30799", + "messageId": null, + "parentId": null, + "folderId": null, + "title": "probe-object-schema", + "description": "recon probe", + "isPublic": false, + "metadata": null, + "source": "local", + "githubRepo": null, + "githubRepoPrivate": null, + "createdAt": "2026-09-15T10:55:39.152Z", + "updatedAt": "2026-09-15T10:55:39.152Z", + "deletedAt": null, + "properties": [ + { + "id": "3096cb0f-1f45-4935-a613-e1d5c2075e13", + "listId": "851954cb-f9ee-4a1f-b864-d57ddeb639d6", + "propertyKey": "title", + "propertyName": "Title", + "propertyType": "text", + "displayOrder": 0, + "isRequired": true, + "defaultValue": null, + "validationRules": null, + "helpText": null, + "placeholder": null, + "isVisible": true, + "visibilityCondition": null, + "createdAt": "2026-09-15T10:55:39.282Z", + "updatedAt": "2026-09-15T10:55:39.282Z" + }, + { + "id": "495aa482-5a6c-4e59-8205-37e4c64f4f42", + "listId": "851954cb-f9ee-4a1f-b864-d57ddeb639d6", + "propertyKey": "status", + "propertyName": "Status", + "propertyType": "select", + "displayOrder": 1, + "isRequired": false, + "defaultValue": null, + "validationRules": { "options": ["todo", "doing", "done"] }, + "helpText": null, + "placeholder": null, + "isVisible": true, + "visibilityCondition": null, + "createdAt": "2026-09-15T10:55:39.282Z", + "updatedAt": "2026-09-15T10:55:39.282Z" + } + ] + } + } + """ + + // Happy path + + func test_givenTheCapturedSingleListBody_whenDecoding_thenTheEnvelopeAndItsColumnsArrive() throws { + let response = try decode(ListResponse.self, singleListJSON) + + XCTAssertNil(response.message, "the read carries no message; only the writes do") + XCTAssertEqual(response.data.title, "probe-object-schema") + XCTAssertEqual(response.data.properties?.count, 2) + XCTAssertEqual(response.data.schemaFields?.map(\.key), ["title", "status"]) + XCTAssertEqual(response.data.schemaFields?.map(\.label), ["Title", "Status"]) + } + + func test_givenTheCapturedBody_whenDecodedAsABareList_thenItFails() throws { + // The regression this pins. `Lists.get` declared `Request`, and + // a bare decode of the real body cannot work — which is why + // `ListsService.detail(listId:)` failed on every live call while its + // test, written against a bare fixture, stayed green (GitHub #75). + XCTAssertThrowsError(try decode(ListDTO.self, singleListJSON)) + } + + // MARK: - GET /api/lists/{id}/schema + + /// Captured from `GET /api/lists/851954cb-…/schema` after setting help text, + /// placeholders and validation on the columns. + private let schemaJSON = """ + { + "data": { + "name": "probe-object-schema", + "description": "recon probe", + "fields": [ + { + "key": "title", + "type": "text", + "label": "Title", + "displayOrder": 0, + "required": true, + "helpText": "What is it called?", + "placeholder": "e.g. Dune", + "visible": true, + "validation": { "pattern": "^[A-Za-z].*$", "maxLength": 80, "minLength": 2 } + }, + { + "key": "year", + "type": "number", + "label": "Year", + "displayOrder": 1, + "required": false, + "helpText": "Publication year", + "visible": true, + "validation": { "max": 2100, "min": 1000 } + }, + { + "key": "status", + "type": "select", + "label": "Status", + "displayOrder": 5, + "required": false, + "visible": true, + "defaultValue": "todo", + "validation": { "options": ["todo", "doing", "done"] }, + "options": ["todo", "doing", "done"] + } + ] + } + } + """ + + // Happy path + + func test_givenTheCapturedSchemaBody_whenDecoding_thenEveryColumnFacetArrives() throws { + let response = try decode(ListSchemaResponse.self, schemaJSON) + let fields = response.data.fields + + XCTAssertEqual(response.data.name, "probe-object-schema") + XCTAssertEqual(response.data.description, "recon probe") + XCTAssertEqual(fields.map(\.key), ["title", "year", "status"]) + + XCTAssertEqual(fields[0].helpText, "What is it called?") + XCTAssertEqual(fields[0].placeholder, "e.g. Dune") + XCTAssertEqual(fields[0].validation?.minLength, 2) + XCTAssertEqual(fields[0].validation?.maxLength, 80) + XCTAssertEqual(fields[0].validation?.pattern, "^[A-Za-z].*$") + XCTAssertEqual(fields[1].validation?.min, 1000) + XCTAssertEqual(fields[1].validation?.max, 2100) + XCTAssertEqual(fields[2].defaultValue, .string("todo")) + } + + func test_givenASelectColumn_whenOnlyOneOptionSpellingIsPresent_thenItStillResolves() throws { + // Boundary. The live payload sends a select column's options **twice**, + // under `options` and under `validation.options`. Reading only one would + // work today and lose the options the day the server stops sending it, + // so `resolvedOptions` accepts either — asserted in both directions. + let onlyNested = """ + { "key": "s", "type": "select", "validation": { "options": ["a", "b"] } } + """ + let onlyFlat = """ + { "key": "s", "type": "select", "options": ["a", "b"] } + """ + XCTAssertEqual(try decode(ListSchemaFieldDTO.self, onlyNested).resolvedOptions, ["a", "b"]) + XCTAssertEqual(try decode(ListSchemaFieldDTO.self, onlyFlat).resolvedOptions, ["a", "b"]) + } + + // Boundary + + func test_givenAColumnlessList_whenDecodingItsSchema_thenFieldsIsEmptyNotMissing() throws { + // Captured from a list created with no columns — the ordinary first + // state of a list, and one the decoder must not treat as a failure. + let json = """ + { "data": { "name": "New list", "fields": [] } } + """ + XCTAssertTrue(try decode(ListSchemaResponse.self, json).data.fields.isEmpty) + } + + func test_givenTheSchemaBody_whenDecodedAsTheOldStringShape_thenItFails() throws { + // The other half of the regression: `ListSchemaDTO` was `{schema: String}`, + // which cannot read this body at all. `ListsService.schema(of:)` is + // called by the row table and the schema editor, so neither could load + // a schema (GitHub #85). + struct OldShape: Decodable { let schema: String } + XCTAssertThrowsError(try decode(OldShape.self, schemaJSON)) + } + + // MARK: - Round trip + + func test_givenAColumn_whenEncodedAndDecoded_thenEveryFacetSurvives() throws { + // A macOS-side schema edit must not quietly drop rules authored on the + // web. Encoding and re-decoding is the cheapest guard against a facet + // that decodes but never encodes. + let field = ListSchemaFieldDTO( + key: "year", + type: "number", + label: "Publication Year", + displayOrder: 3, + required: true, + visible: false, + helpText: "When was it published?", + placeholder: "1965", + defaultValue: .int(1965), + validation: ListFieldValidationDTO(min: 1000, max: 2100) + ) + let data = try JSONCoders.makeEncoder().encode(field) + let round = try JSONCoders.makeDecoder().decode(ListSchemaFieldDTO.self, from: data) + XCTAssertEqual(round, field) + } + + // MARK: - GET /api/users/{username}/lists/{id} + + func test_givenTheCapturedPublicListBody_whenDecoding_thenTheNamedEnvelopeAndAncestorsArrive() throws { + // Captured after making the probe list public. A third envelope + // convention on the same resource — `{list, ancestors}` — where the + // client decoded a bare `ListDTO`. + let json = """ + { + "list": { + "id": "851954cb-f9ee-4a1f-b864-d57ddeb639d6", + "title": "probe-object-schema", + "description": "recon probe", + "parentId": null, + "children": [] + }, + "ancestors": [] + } + """ + let response = try decode(PublicListResponse.self, json) + XCTAssertEqual(response.list.title, "probe-object-schema") + XCTAssertEqual(response.ancestors?.count, 0) + // And the projection really is this thin — no columns to show. + XCTAssertNil(response.list.properties) + XCTAssertNil(response.list.isPublic) + } +} diff --git a/Packages/InterlinedKit/Tests/InterlinedKitTests/ListsEndpointTests.swift b/Packages/InterlinedKit/Tests/InterlinedKitTests/ListsEndpointTests.swift index 36af400..cd07f43 100644 --- a/Packages/InterlinedKit/Tests/InterlinedKitTests/ListsEndpointTests.swift +++ b/Packages/InterlinedKit/Tests/InterlinedKitTests/ListsEndpointTests.swift @@ -39,7 +39,20 @@ final class ListsEndpointTests: XCTestCase { XCTAssertEqual(Lists.delete(id: "7").method, .delete) XCTAssertEqual(Lists.schema(id: "7").path, "/api/lists/7/schema") - XCTAssertEqual(Lists.updateSchema(id: "7", UpdateListSchemaRequest(schema: "A:text")).method, .put) + let schemaBody = UpdateListSchemaRequest( + schema: ListSchemaDSLDTO( + name: "A", + fields: [ListSchemaFieldDTO(key: "a", type: "text", label: "A")] + ) + ) + XCTAssertEqual(Lists.updateSchema(id: "7", schemaBody).method, .put) + // `force` is opt-in and absent by default, so a routine save can never + // silently confirm a destructive change (GitHub #85). + XCTAssertTrue(Lists.updateSchema(id: "7", schemaBody).query.isEmpty) + XCTAssertEqual( + Lists.updateSchema(id: "7", schemaBody, force: true).query.first?.value, + "true" + ) XCTAssertEqual(Lists.refresh(id: "7").method, .post) XCTAssertEqual(Lists.refresh(id: "7").path, "/api/lists/7/refresh") diff --git a/docs/spikes/list-schema-wire-shapes.md b/docs/spikes/list-schema-wire-shapes.md new file mode 100644 index 0000000..124403c --- /dev/null +++ b/docs/spikes/list-schema-wire-shapes.md @@ -0,0 +1,153 @@ +# Spike — the list and list-schema wire shapes + +**Date:** 2026-09-15 +**Account:** the `.env` contract-test account (`messenger@interlinedlist.com`) +**Method:** read-only `GET` probes, plus one create/update/delete cycle on a throwaway list +**Issues:** [#75](https://github.com/CompositeCode/interlinedlist-macos-native/issues/75), [#85](https://github.com/CompositeCode/interlinedlist-macos-native/issues/85); unblocks `P2-G` and item 1 of [#50](https://github.com/CompositeCode/interlinedlist-macos-native/issues/50) + +## Why this exists + +Three silent decode defects have shipped in this client — G21 link metadata, G25 +org members, and the two fixed here — and every one of them had a **green test +written against a fabricated fixture**. A test that invents the payload it then +asserts against proves only that the decoder matches the invention. + +So this file is the transcript. The tests in +`Packages/InterlinedKit/Tests/InterlinedKitTests/ListSchemaWireShapeTests.swift` +use these bodies verbatim. + +## Headline finding + +**The client modelled a list's schema as a DSL string. The API models it as an +object, and always has.** + +``` +POST /api/lists {"title":"probe-string-schema","schema":"Title:text, Year:number"} +→ 400 {"error":"Invalid schema: DSL must be an object","code":"bad_request"} +``` + +That is what `CreateListRequest.schema: String?` was sending. Creating a list +with columns from macOS was a hard failure, not a degradation. + +## Envelope map + +| Route | Live shape | Client decoded | Consequence | +|---|---|---|---| +| `GET /api/lists` | `{lists[], pagination}` | ✅ correct | — | +| `GET /api/lists/{id}` | `{data:{…, properties[]}}` | bare `ListDTO` | `detail(listId:)` never decoded (#75) | +| `POST /api/lists` | `201 {message, data, refreshStatus?}` | bare `ListDTO` | create never decoded | +| `PUT /api/lists/{id}` | `{message, data}` | bare `ListDTO` | update never decoded | +| `GET /api/lists/{id}/schema` | `{data:{name, description?, fields[]}}` | `{schema:String}` | **schema editor + row table dead** | +| `PUT /api/lists/{id}/schema` | `{message, data:{…, properties[]}}` | `{schema:String}` | schema save was a 400 | +| `GET /api/users/{u}/lists/{id}` | `{list:{…}, ancestors[]}` | bare `ListDTO` | public list page never decoded | +| `GET /api/users/{u}/lists` | `{lists[], pagination}` | ✅ correct | — | +| `GET /api/lists/{id}/data` | `{rows[], pagination}` | ✅ correct | — | + +Four distinct envelope conventions on one resource family: bare-keyed +collections, `{data}`, `{message, data}`, and `{list, ancestors}`. `OPTIONS` +proves the verb and says nothing about any of this. + +⚠️ The `PUT …/schema` response **contradicts the OpenAPI 200 example**, which +shows a bare `{properties:[…]}`. The capture wins. + +## The schema DSL object + +`GET /api/lists/{id}/schema`, after setting help text, placeholders and +validation: + +```json +{"data":{ + "name":"probe-object-schema", + "description":"recon probe", + "fields":[ + {"key":"title","type":"text","label":"Title","displayOrder":0,"required":true, + "helpText":"What is it called?","placeholder":"e.g. Dune","visible":true, + "validation":{"pattern":"^[A-Za-z].*$","maxLength":80,"minLength":2}}, + {"key":"year","type":"number","label":"Year","displayOrder":1,"required":false, + "helpText":"Publication year","visible":true, + "validation":{"max":2100,"min":1000}}, + {"key":"email","type":"email","label":"Contact","displayOrder":2,"required":false,"visible":true}, + {"key":"url","type":"url","label":"Link","displayOrder":3,"required":false,"visible":true}, + {"key":"due","type":"date","label":"Due","displayOrder":4,"required":false,"visible":true}, + {"key":"status","type":"select","label":"Status","displayOrder":5,"required":false, + "visible":true,"defaultValue":"todo", + "validation":{"options":["todo","doing","done"]}, + "options":["todo","doing","done"]}]}} +``` + +Three things to notice. + +**1. `key` and `label` are different things, and row data is keyed by `key`.** + +```json +"rowData":{"due":"2026-01-02","url":"https://example.com","read":true, + "year":1965,"email":"a@b.com","title":"Dune","status":"done"} +``` + +`SchemaField` had one `name` serving as both. That was harmless only while the +client could not create a schema at all; the moment it could, a column labelled +"Publication Year" over a key of `year` would have rendered every cell empty. + +**2. A `select` column carries its options twice** — under `options` and under +`validation.options`. Both are modelled and either alone resolves; reading only +one would work today and lose the options the day the server stops sending it. + +**3. The validation vocabulary is first-class**, not DSL syntax: + +``` +validationRules: { min, max, minLength, maxLength, pattern, options } +helpText · placeholder · isRequired · isVisible · visibilityCondition +defaultValue · displayOrder +``` + +`work-consolidation.md` `P2-G` recorded this encoding as API-unconfirmed, which +is what blocked item 1 of #50. It is confirmed, and there is no encoding to +reverse-engineer — the fields simply exist. + +`visibilityCondition` is conditional column visibility. It was `null` on every +captured payload, so it is decoded type-erased rather than guessed at; modelling +an unseen shape is precisely how G21 and G25 happened. + +## The property projection + +The same columns come back under a **second spelling** wherever a list object is +returned (`data.properties`): + +```json +{"id":"3096cb0f-…","listId":"851954cb-…", + "propertyKey":"title","propertyName":"Title","propertyType":"text", + "displayOrder":0,"isRequired":true,"defaultValue":null, + "validationRules":{"pattern":"^[A-Za-z].*$","maxLength":80,"minLength":2}, + "helpText":"What is it called?","placeholder":"e.g. Dune", + "isVisible":true,"visibilityCondition":null, + "createdAt":"…","updatedAt":"…"} +``` + +`ListPropertyDTO.asSchemaField` reconciles the two so no caller above the kit has +to know there are two. + +## The destructive-change guard + +`PUT /api/lists/{id}/schema` refuses to drop a column that still holds row data: +`400` plus a `propertiesWithData` array naming them, and `?force=true` confirms. +The client knew nothing about this, so it presented as an unexplained failure. + +⚠️ **The column list is not reachable from `APIError` today.** `APIError.badRequest` +carries only the decoded `{error}` string; the rest of the body is discarded at +`APIClient.swift:234`. `ListSchemaConflictDTO` models the full shape, and +surfacing the names becomes a one-line change once the kit keeps the body. Filed +separately. + +## `ListDTO.schema` was always nil + +`ListDTO` declared `schema: String?`, documented as *"present on detail and +create responses"*. **No captured payload carries a `schema` key on a list +object.** So `OwnedList.schemaDescription` — rendered in the list detail header +and in the Markdown export — has always been empty. It is now derived from the +real columns. + +## What was written during this spike + +One throwaway list (`probe-object-schema`) was created, given a schema twice, had +one row added, was made public for the public-route probe, and was deleted +afterwards. Nothing else on the account was touched.