|
| 1 | +/** |
| 2 | + * ObjectUI |
| 3 | + * Copyright (c) 2024-present ObjectStack Inc. |
| 4 | + * |
| 5 | + * This source code is licensed under the MIT license found in the |
| 6 | + * LICENSE file in the root directory of this source tree. |
| 7 | + */ |
| 8 | + |
| 9 | +/** |
| 10 | + * A CROSS-column drop loses the landing POSITION too — objectui#8826, question 3. |
| 11 | + * |
| 12 | + * ── What this file is ───────────────────────────────────────────────────── |
| 13 | + * A CHARACTERIZATION pin, not a gate. objectui#8826 measured that a SAME-column |
| 14 | + * reorder is persisted nowhere; its third question — is the landing position |
| 15 | + * within the TARGET column also lost on a CROSS-column move? — was filed |
| 16 | + * explicitly unmeasured, because the fix surface depends on the answer. This |
| 17 | + * file is that measurement, frozen. It asserts today's behaviour so that a |
| 18 | + * later change to it is announced rather than silent; it does NOT claim the |
| 19 | + * behaviour is correct, and it rules nothing about questions 1 and 2 (whether |
| 20 | + * in-column order should be persisted at all, and whether the optimistic update |
| 21 | + * should be removed if not). Those are the maintainer's. |
| 22 | + * |
| 23 | + * ── The answer, and where the index dies ────────────────────────────────── |
| 24 | + * `KanbanImpl.handleDragEnd`'s cross-column branch DOES compute a landing index |
| 25 | + * (`overIndex`) and DOES splice the card in at it, then hands that index to |
| 26 | + * `onCardMove` as its fourth argument. `ObjectKanban.handleCardMove` receives it |
| 27 | + * as `_newIndex` and discards it outright (`void _newIndex`), and |
| 28 | + * `persistCardMove` writes a body of exactly `{ [groupBy]: toColumnId }`. So the |
| 29 | + * index reaches a parameter and stops there: no sink, no PATCH key, no storage. |
| 30 | + * Column MEMBERSHIP flows back through the `columns` prop; ORDER never does — |
| 31 | + * the board's order is whatever `bucketCardsIntoColumns` produces, which is the |
| 32 | + * order of the underlying record array. |
| 33 | + * |
| 34 | + * ── ⚠️ The two data ownerships answer DIFFERENTLY, and that is the finding ── |
| 35 | + * The card anticipated one answer; there are two, because the optimistic write |
| 36 | + * that re-renders the board is itself gated on ownership. |
| 37 | + * |
| 38 | + * - INTERNAL data (the board fetches): `persistCardMove`'s optimistic |
| 39 | + * `setFetchedData` re-renders `ObjectKanban`, `KanbanRenderer` re-buckets, |
| 40 | + * the `columns` identity changes and `KanbanImpl`'s mirror re-syncs from it. |
| 41 | + * The dropped card therefore leaves the slot the user dropped it in |
| 42 | + * IMMEDIATELY — inside the same `act()` — and lands wherever record order |
| 43 | + * puts it. This is WORSE than the same-column signature the card describes: |
| 44 | + * not a silent revert on some later reflow, but a visible snap on the spot. |
| 45 | + * - EXTERNAL data (a parent owns `data`, e.g. ListView): the optimistic write |
| 46 | + * is deliberately skipped, so nothing re-renders and the mirror is never |
| 47 | + * reset. The drop position SURVIVES on screen — until the parent's next data |
| 48 | + * reflow, which carries membership and not order. That is the card's |
| 49 | + * silent-revert signature exactly. |
| 50 | + * |
| 51 | + * ⇒ The position is lost on BOTH paths. They differ only in WHEN the user sees |
| 52 | + * it happen. |
| 53 | + * |
| 54 | + * ── The instrument ──────────────────────────────────────────────────────── |
| 55 | + * The card's recorder: the `columns` prop the board receives, captured on every |
| 56 | + * render, via a module mock of `../KanbanImpl` that records and then renders the |
| 57 | + * real board. (`columnsReachDomSameCommit-8534.test.tsx` has a recorder too, but |
| 58 | + * it snapshots the DOM per commit at the `KanbanImpl` seam — a different |
| 59 | + * instrument, and it never drives a cross-column drop.) The board is driven |
| 60 | + * through `DndContext`'s real `onDragEnd`, captured by the second module mock: |
| 61 | + * dnd-kit's pointer sensors need layout and pointer capture that happy-dom does |
| 62 | + * not provide, so a synthesized drop on the production handler is the closest |
| 63 | + * honest reproduction. Everything downstream of that call is the real path. |
| 64 | + * |
| 65 | + * Both controls the measurement needs are here as their own tests: |
| 66 | + * - LIT positive control — a cross-column move DOES produce a new `columns` |
| 67 | + * snapshot, with the membership change in it. The instrument is connected, |
| 68 | + * so "the drop order never appears" is a reading and not a dead wire. |
| 69 | + * - DEAD negative control — a drag that ends on nothing produces NO new |
| 70 | + * snapshot and no write. The instrument does not fire spuriously. |
| 71 | + * |
| 72 | + * ── The fixture is built so the two orders cannot coincide ──────────────── |
| 73 | + * The moved card is LAST in the record array and is dropped FIRST in the target |
| 74 | + * column. A fixture where the dropped card also happens to sort first would |
| 75 | + * read green whether the index survived or not. |
| 76 | + */ |
| 77 | + |
| 78 | +import React from 'react'; |
| 79 | +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; |
| 80 | +import { render, screen, within, act, cleanup, waitFor } from '@testing-library/react'; |
| 81 | +import '@testing-library/jest-dom'; |
| 82 | +import { registerAllFields } from '@object-ui/fields'; |
| 83 | +import { toast } from '@object-ui/components'; |
| 84 | +import type { DataSource, ObjectKanbanSchema } from '@object-ui/types'; |
| 85 | +import { ObjectKanban } from '../ObjectKanban'; |
| 86 | + |
| 87 | +// Pay the board's lazy chunk at import time rather than racing it against a |
| 88 | +// `findBy` budget (AGENTS.md §测试纪律); specifier resolves to the same module |
| 89 | +// as `./KanbanImpl` in `../index`, so the `React.lazy` factory hits the cache. |
| 90 | +import '../KanbanImpl'; |
| 91 | + |
| 92 | +// `vi.hoisted` so the mock factories — hoisted above every import — can reach |
| 93 | +// these boxes. A plain `const` would still be in its TDZ when the mocked |
| 94 | +// modules are first requested. |
| 95 | +const dnd = vi.hoisted(() => ({ |
| 96 | + onDragEnd: undefined as undefined | ((event: unknown) => void), |
| 97 | +})); |
| 98 | +const recorder = vi.hoisted(() => ({ snapshots: [] as string[] })); |
| 99 | + |
| 100 | +// Capture the board's real `onDragEnd` while still rendering the real provider, |
| 101 | +// so `@dnd-kit/sortable`'s hooks keep reading the same context instance. |
| 102 | +vi.mock('@dnd-kit/core', async (importOriginal) => { |
| 103 | + const actual = await importOriginal<typeof import('@dnd-kit/core')>(); |
| 104 | + const ReactMod = await import('react'); |
| 105 | + const CapturingDndContext = (props: Record<string, unknown>) => { |
| 106 | + dnd.onDragEnd = props.onDragEnd as (event: unknown) => void; |
| 107 | + return ReactMod.createElement(actual.DndContext, props as never); |
| 108 | + }; |
| 109 | + return { ...actual, DndContext: CapturingDndContext }; |
| 110 | +}); |
| 111 | + |
| 112 | +// THE INSTRUMENT: record the `columns` prop on every render of the board, then |
| 113 | +// render the real board with it. Recording in the component body (not an |
| 114 | +// effect) is deliberate — it is the prop AS DELIVERED that is under test. |
| 115 | +vi.mock('../KanbanImpl', async (importOriginal) => { |
| 116 | + const actual = await importOriginal<typeof import('../KanbanImpl')>(); |
| 117 | + const ReactMod = await import('react'); |
| 118 | + const RecordingKanbanBoard = (props: Record<string, unknown>) => { |
| 119 | + const cols = (props.columns as Array<Record<string, unknown>> | undefined) ?? []; |
| 120 | + recorder.snapshots.push( |
| 121 | + cols |
| 122 | + .map((c) => { |
| 123 | + const cards = (c.cards as Array<Record<string, unknown>> | undefined) ?? []; |
| 124 | + return `${String(c.id)}:${cards.map((k) => String(k.title)).join(',')}`; |
| 125 | + }) |
| 126 | + .join(' | '), |
| 127 | + ); |
| 128 | + return ReactMod.createElement(actual.default as never, props as never); |
| 129 | + }; |
| 130 | + return { ...actual, default: RecordingKanbanBoard }; |
| 131 | +}); |
| 132 | + |
| 133 | +registerAllFields(); |
| 134 | + |
| 135 | +const objectDef = { |
| 136 | + name: 'task', |
| 137 | + fields: { |
| 138 | + title: { type: 'text', label: 'Title' }, |
| 139 | + status: { |
| 140 | + type: 'picklist', |
| 141 | + label: 'Status', |
| 142 | + options: [ |
| 143 | + { value: 'backlog', label: 'Backlog' }, |
| 144 | + { value: 'in_progress', label: 'In Progress' }, |
| 145 | + ], |
| 146 | + }, |
| 147 | + }, |
| 148 | +}; |
| 149 | + |
| 150 | +/** |
| 151 | + * Record order is the load-bearing part of this fixture. `Alpha` is LAST in the |
| 152 | + * array and alone in `backlog`; the two cards already in `in_progress` come |
| 153 | + * FIRST. Dropping `Alpha` onto `Xray` asks for slot 0 of the target column, |
| 154 | + * while record order puts it in slot 2 — so drop order and record order are |
| 155 | + * distinguishable, which is the whole point. |
| 156 | + */ |
| 157 | +const RECORDS = () => [ |
| 158 | + { id: 'x', title: 'Xray', status: 'in_progress' }, |
| 159 | + { id: 'y', title: 'Yankee', status: 'in_progress' }, |
| 160 | + { id: 'a', title: 'Alpha', status: 'backlog' }, |
| 161 | +]; |
| 162 | + |
| 163 | +/** What the recorder shows at mount, before any drag. */ |
| 164 | +const AT_MOUNT = 'backlog:Alpha | in_progress:Xray,Yankee'; |
| 165 | +/** What the drop asks for: `Alpha` in slot 0 of the target column. */ |
| 166 | +const DROP_ORDER = 'backlog: | in_progress:Alpha,Xray,Yankee'; |
| 167 | +/** What the prop actually carries back: record order, with `Alpha` last. */ |
| 168 | +const RECORD_ORDER = 'backlog: | in_progress:Xray,Yankee,Alpha'; |
| 169 | + |
| 170 | +const schema = { |
| 171 | + type: 'object-kanban', |
| 172 | + objectName: 'task', |
| 173 | + groupBy: 'status', |
| 174 | + cardTitle: 'title', |
| 175 | + columns: [ |
| 176 | + { id: 'backlog', title: 'Backlog' }, |
| 177 | + { id: 'in_progress', title: 'In Progress' }, |
| 178 | + ], |
| 179 | +} satisfies ObjectKanbanSchema; |
| 180 | + |
| 181 | +function makeDataSource(update: DataSource['update']): DataSource { |
| 182 | + return { |
| 183 | + getObjectSchema: vi.fn(async () => objectDef), |
| 184 | + find: vi.fn(async () => ({ value: RECORDS() })), |
| 185 | + update, |
| 186 | + } as unknown as DataSource; |
| 187 | +} |
| 188 | + |
| 189 | +/** The cards currently rendered inside a column, by the column's visible title. */ |
| 190 | +function cardsIn(columnTitle: string): string[] { |
| 191 | + const list = screen.getByRole('list', { name: `${columnTitle} cards` }); |
| 192 | + return within(list) |
| 193 | + .queryAllByRole('listitem') |
| 194 | + .map((el) => el.getAttribute('aria-label') ?? ''); |
| 195 | +} |
| 196 | + |
| 197 | +/** An accepted write — this file measures the SUCCESS path, not a refusal. */ |
| 198 | +const acceptingUpdate = () => vi.fn(async () => ({ id: 'a', status: 'in_progress' })); |
| 199 | + |
| 200 | +/** |
| 201 | + * Mount on one of the two data ownerships and wait until it has settled: the |
| 202 | + * object-schema fetch and (internal path) the record fetch both land as async |
| 203 | + * state updates, and a stray one arriving AFTER the drag would re-render the |
| 204 | + * board and add a snapshot that belongs to neither reading. |
| 205 | + */ |
| 206 | +async function mountBoard(mode: 'external' | 'internal', dataSource: DataSource) { |
| 207 | + render( |
| 208 | + <ObjectKanban |
| 209 | + schema={schema} |
| 210 | + dataSource={dataSource} |
| 211 | + {...(mode === 'external' ? { data: RECORDS() } : {})} |
| 212 | + />, |
| 213 | + ); |
| 214 | + expect(await screen.findByText('Alpha')).toBeInTheDocument(); |
| 215 | + await act(async () => { |
| 216 | + await Promise.resolve(); |
| 217 | + await Promise.resolve(); |
| 218 | + }); |
| 219 | + await waitFor(() => expect(cardsIn('Backlog')).toEqual(['Alpha'])); |
| 220 | + expect(recorder.snapshots[recorder.snapshots.length - 1]).toBe(AT_MOUNT); |
| 221 | + return recorder.snapshots.length; |
| 222 | +} |
| 223 | + |
| 224 | +/** Drop `Alpha` onto `Xray` — slot 0 of "In Progress" — exactly as the board would. */ |
| 225 | +async function dropAlphaOntoXray() { |
| 226 | + expect(dnd.onDragEnd).toBeTypeOf('function'); |
| 227 | + await act(async () => { |
| 228 | + dnd.onDragEnd!({ active: { id: 'a' }, over: { id: 'x' } }); |
| 229 | + }); |
| 230 | +} |
| 231 | + |
| 232 | +beforeEach(() => { |
| 233 | + dnd.onDragEnd = undefined; |
| 234 | + recorder.snapshots.length = 0; |
| 235 | + vi.spyOn(toast, 'error').mockImplementation(() => 'toast-id' as never); |
| 236 | +}); |
| 237 | + |
| 238 | +afterEach(() => { |
| 239 | + cleanup(); |
| 240 | + vi.restoreAllMocks(); |
| 241 | +}); |
| 242 | + |
| 243 | +describe('ObjectKanban — a cross-column drop loses the landing position too (#8826 Q3)', () => { |
| 244 | + it('LIT POSITIVE CONTROL: the drop does produce a new `columns` snapshot, carrying the membership change', async () => { |
| 245 | + // The card's own control. Without it, "the drop order never appears" would |
| 246 | + // be indistinguishable from a recorder that was never wired up. |
| 247 | + const before = await mountBoard('internal', makeDataSource(acceptingUpdate() as never)); |
| 248 | + |
| 249 | + await dropAlphaOntoXray(); |
| 250 | + |
| 251 | + const fresh = recorder.snapshots.slice(before); |
| 252 | + expect(fresh.length).toBeGreaterThan(0); |
| 253 | + const latest = fresh[fresh.length - 1]; |
| 254 | + // Membership DID flow back: `Alpha` left `backlog` and is in `in_progress`. |
| 255 | + expect(latest).not.toBe(AT_MOUNT); |
| 256 | + expect(latest).toMatch(/^backlog: \| in_progress:/); |
| 257 | + expect(latest).toContain('Alpha'); |
| 258 | + }); |
| 259 | + |
| 260 | + it('DEAD NEGATIVE CONTROL: a drag that ends on nothing records no snapshot and writes nothing', async () => { |
| 261 | + const update = acceptingUpdate(); |
| 262 | + const before = await mountBoard('internal', makeDataSource(update as never)); |
| 263 | + |
| 264 | + await act(async () => { |
| 265 | + dnd.onDragEnd!({ active: { id: 'a' }, over: null }); |
| 266 | + }); |
| 267 | + |
| 268 | + expect(recorder.snapshots.slice(before)).toEqual([]); |
| 269 | + expect(update).not.toHaveBeenCalled(); |
| 270 | + }); |
| 271 | + |
| 272 | + it('THE MEASUREMENT (internal data): the drop order is never recorded — the prop comes back in RECORD order', async () => { |
| 273 | + const before = await mountBoard('internal', makeDataSource(acceptingUpdate() as never)); |
| 274 | + |
| 275 | + await dropAlphaOntoXray(); |
| 276 | + |
| 277 | + // The whole reading: across every snapshot this board has ever received, |
| 278 | + // the order the user dropped in is not among them. |
| 279 | + expect(recorder.snapshots).not.toContain(DROP_ORDER); |
| 280 | + expect(recorder.snapshots.slice(before)).toEqual([RECORD_ORDER]); |
| 281 | + }); |
| 282 | + |
| 283 | + it('THE MEASUREMENT (internal data): the card visibly leaves the dropped slot in the same act', async () => { |
| 284 | + await mountBoard('internal', makeDataSource(acceptingUpdate() as never)); |
| 285 | + |
| 286 | + await dropAlphaOntoXray(); |
| 287 | + |
| 288 | + // Dropped at slot 0; on screen at slot 2. The optimistic write re-renders |
| 289 | + // the board, which re-buckets and re-syncs the mirror from the prop. |
| 290 | + expect(cardsIn('In Progress')).toEqual(['Xray', 'Yankee', 'Alpha']); |
| 291 | + expect(cardsIn('Backlog')).toEqual([]); |
| 292 | + }); |
| 293 | + |
| 294 | + it('THE ASYMMETRY (external data): no snapshot at all, so the dropped slot survives on screen', async () => { |
| 295 | + const before = await mountBoard('external', makeDataSource(acceptingUpdate() as never)); |
| 296 | + |
| 297 | + await dropAlphaOntoXray(); |
| 298 | + |
| 299 | + // The optimistic write is skipped on this ownership, so nothing re-renders |
| 300 | + // and `KanbanImpl`'s mirror is never reset — the local move stands. |
| 301 | + expect(recorder.snapshots.slice(before)).toEqual([]); |
| 302 | + expect(cardsIn('In Progress')).toEqual(['Alpha', 'Xray', 'Yankee']); |
| 303 | + // ...and it is still not persisted anywhere: the next reflow from the |
| 304 | + // parent will carry membership and not order. |
| 305 | + }); |
| 306 | + |
| 307 | + it('THE WIRE: the landing index reaches no sink — the write body carries the column and nothing else', async () => { |
| 308 | + for (const mode of ['internal', 'external'] as const) { |
| 309 | + const update = acceptingUpdate(); |
| 310 | + await mountBoard(mode, makeDataSource(update as never)); |
| 311 | + |
| 312 | + await dropAlphaOntoXray(); |
| 313 | + |
| 314 | + expect(update).toHaveBeenCalledTimes(1); |
| 315 | + // Exact body, not a subset match: an added positional key must fail here. |
| 316 | + expect(update).toHaveBeenCalledWith('task', 'a', { status: 'in_progress' }); |
| 317 | + cleanup(); |
| 318 | + recorder.snapshots.length = 0; |
| 319 | + dnd.onDragEnd = undefined; |
| 320 | + } |
| 321 | + }); |
| 322 | +}); |
0 commit comments