|
| 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 lookup column whose `options` are an EMPTY array must still be FILTERABLE |
| 11 | + * (objectui#5031). |
| 12 | + * |
| 13 | + * objectui#4874 (PR #5030) answered the VALUE-DOMAIN question for this shape: |
| 14 | + * `options: []` means "the static domain is not in place", so a value on such a |
| 15 | + * column is never cleared against it, and the criterion was written down once, |
| 16 | + * as `hasStaticOptionDomain(field)` (= `options` is a NON-EMPTY array). But |
| 17 | + * `renderValueInput`'s remote-picker branch was still asking the same question |
| 18 | + * in its own words — `!field?.options` — and `[]` is truthy, so it answered the |
| 19 | + * opposite way: the value-domain side treated the column as remote (keep the |
| 20 | + * value) while the control side treated it as static (draw the option-driven |
| 21 | + * Select over zero options). |
| 22 | + * |
| 23 | + * The user-visible result was a column that could not be filtered at all: a |
| 24 | + * scalar operator drew a Select with no candidates and no search box, and |
| 25 | + * `in`/`notIn` drew an empty checkbox list — while the SAME column with the |
| 26 | + * `options` key simply absent got full remote search. The two branches now read |
| 27 | + * one criterion, so a `referenceTo` column with `options: []` reaches |
| 28 | + * {@link LookupValuePicker}. |
| 29 | + * |
| 30 | + * DIRECTIONS, predicted before running (reverse verification restores |
| 31 | + * `!field?.options` in the branch condition and re-runs this file): |
| 32 | + * |
| 33 | + * - "routes an unloaded lookup column to the remote picker" — RED without the |
| 34 | + * fix. Every case there asserts a picker artifact (the search box, the |
| 35 | + * candidate list, the by-hand id input, the multi-value id input); on the |
| 36 | + * old condition the column draws a Radix Select or a checkbox list instead |
| 37 | + * and none of those artifacts exists; |
| 38 | + * - "a column that owns its option domain keeps the static Select" — GREEN in |
| 39 | + * both directions by design. It is the blast-radius guard: a fix that read |
| 40 | + * the criterion inverted, or that sent every lookup to the remote picker, |
| 41 | + * turns all of it red; |
| 42 | + * - "the branch's other conditions are untouched" — GREEN in both directions |
| 43 | + * too. `hasStaticOptionDomain` replaces ONE conjunct; a column with no |
| 44 | + * `referenceTo` and a non-lookup type must route exactly as before, or the |
| 45 | + * change widened the branch instead of correcting it. |
| 46 | + */ |
| 47 | +import { describe, it, expect, vi } from 'vitest'; |
| 48 | +import React from 'react'; |
| 49 | +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; |
| 50 | +import '@testing-library/jest-dom'; |
| 51 | +import { SchemaRendererContext } from '@object-ui/react'; |
| 52 | +import { FilterBuilder } from '../custom/filter-builder'; |
| 53 | + |
| 54 | +/** |
| 55 | + * The columns this card distinguishes. Every one of them is the SAME lookup |
| 56 | + * apart from its `options` key, because that key is the whole subject: |
| 57 | + * |
| 58 | + * - `account` — `referenceTo`, `options` ABSENT. The shape that already |
| 59 | + * reached the remote picker; here as the reference behaviour the others |
| 60 | + * are measured against; |
| 61 | + * - `account_pending` — `referenceTo`, `options: []`. The card's column: the |
| 62 | + * shape `deriveFilterFields` produces before an object's picklist values |
| 63 | + * arrive, and the one that used to fall through to an empty Select; |
| 64 | + * - `account_loaded` — `referenceTo` AND a NON-EMPTY `options`. A lookup whose |
| 65 | + * domain really is local and complete; it must keep the static Select, or |
| 66 | + * the fix has taken the static case with it; |
| 67 | + * - `owner_pending` — `type: 'owner'` with `options: []` and no `referenceTo`, |
| 68 | + * reached through the branch's `type === "owner"` conjunct; |
| 69 | + * - `stage_pending` — a `select` (NOT lookup-like) with `options: []`. It has |
| 70 | + * no remote search to fall back to, so it must be unaffected; |
| 71 | + * - `orphan_pending` — a lookup with `options: []` and NO `referenceTo`, of a |
| 72 | + * type that is neither `user` nor `owner`. There is nothing to search |
| 73 | + * against, so the branch's third conjunct must keep rejecting it. |
| 74 | + */ |
| 75 | +const FIELDS = [ |
| 76 | + { value: 'title', label: 'Title', type: 'text' }, |
| 77 | + { value: 'account', label: 'Account', type: 'lookup', referenceTo: 'accounts' }, |
| 78 | + { |
| 79 | + value: 'account_pending', |
| 80 | + label: 'Account (pending)', |
| 81 | + type: 'lookup', |
| 82 | + referenceTo: 'accounts', |
| 83 | + options: [], |
| 84 | + }, |
| 85 | + { |
| 86 | + value: 'account_loaded', |
| 87 | + label: 'Account (loaded)', |
| 88 | + type: 'lookup', |
| 89 | + referenceTo: 'accounts', |
| 90 | + options: [ |
| 91 | + { value: 'acc_001', label: 'Acme' }, |
| 92 | + { value: 'acc_002', label: 'Globex' }, |
| 93 | + ], |
| 94 | + }, |
| 95 | + { value: 'owner_pending', label: 'Owner', type: 'owner', options: [] }, |
| 96 | + { value: 'stage_pending', label: 'Stage', type: 'select', options: [] }, |
| 97 | + { value: 'orphan_pending', label: 'Orphan', type: 'lookup', options: [] }, |
| 98 | +]; |
| 99 | + |
| 100 | +/** The records the remote search returns — the candidates the card says are missing. */ |
| 101 | +const ACCOUNTS = [ |
| 102 | + { id: 'acc_007', name: 'Initech' }, |
| 103 | + { id: 'acc_008', name: 'Umbrella' }, |
| 104 | +]; |
| 105 | + |
| 106 | +function renderRow( |
| 107 | + condition: Record<string, unknown>, |
| 108 | + opts: { dataSource?: unknown } = {}, |
| 109 | +) { |
| 110 | + const onChange = vi.fn(); |
| 111 | + // Hoisted OUT of the JSX, as in PR #5030's suite: the sync effect re-seeds |
| 112 | + // internal state whenever the `value` PROP's identity changes, which would |
| 113 | + // undo the interaction under test on the next render. |
| 114 | + const value = { id: 'root', logic: 'and', conditions: [{ id: 'c1', ...condition }] }; |
| 115 | + const tree = ( |
| 116 | + <FilterBuilder fields={FIELDS as any} value={value as any} onChange={onChange} /> |
| 117 | + ); |
| 118 | + const utils = render( |
| 119 | + opts.dataSource ? ( |
| 120 | + <SchemaRendererContext.Provider value={{ dataSource: opts.dataSource } as any}> |
| 121 | + {tree} |
| 122 | + </SchemaRendererContext.Provider> |
| 123 | + ) : ( |
| 124 | + tree |
| 125 | + ), |
| 126 | + ); |
| 127 | + return { ...utils, onChange }; |
| 128 | +} |
| 129 | + |
| 130 | +/** A DataSource that answers `find` with {@link ACCOUNTS}. */ |
| 131 | +function fakeDataSource() { |
| 132 | + return { find: vi.fn(async () => ({ data: ACCOUNTS })) }; |
| 133 | +} |
| 134 | + |
| 135 | +/** |
| 136 | + * The row's value control, identified by what it IS rather than by position. |
| 137 | + * |
| 138 | + * The bug and the fix draw two structurally different controls, so the pins read |
| 139 | + * the DOM for the control that is actually there: `comboboxes` counts the Radix |
| 140 | + * triggers (field, operator, and — only for an option-driven column — value), |
| 141 | + * and the picker helpers look for `LookupValuePicker`'s own artifacts. |
| 142 | + */ |
| 143 | +function comboboxCount(): number { |
| 144 | + return screen.queryAllByRole('combobox').length; |
| 145 | +} |
| 146 | + |
| 147 | +function lastRow(onChange: ReturnType<typeof vi.fn>) { |
| 148 | + const calls = onChange.mock.calls; |
| 149 | + expect(calls.length, 'the builder never called onChange').toBeGreaterThan(0); |
| 150 | + return calls[calls.length - 1][0].conditions[0]; |
| 151 | +} |
| 152 | + |
| 153 | +describe('routes an unloaded lookup column to the remote picker', () => { |
| 154 | + it('opens a search box and real candidates for a `referenceTo` column with `options: []`', async () => { |
| 155 | + const dataSource = fakeDataSource(); |
| 156 | + renderRow( |
| 157 | + { field: 'account_pending', operator: 'equals', value: '' }, |
| 158 | + { dataSource }, |
| 159 | + ); |
| 160 | + |
| 161 | + // The card's literal complaint, asserted as the user meets it: a trigger |
| 162 | + // that opens a searchable list. On the old condition this column drew a |
| 163 | + // Radix Select whose only content was the row's own value. |
| 164 | + const trigger = screen.getByTestId('lookup-picker-account_pending'); |
| 165 | + fireEvent.click(trigger); |
| 166 | + |
| 167 | + expect(await screen.findByTestId('lookup-search-account_pending')).toBeInTheDocument(); |
| 168 | + await waitFor(() => { |
| 169 | + expect(screen.getByText('Initech')).toBeInTheDocument(); |
| 170 | + expect(screen.getByText('Umbrella')).toBeInTheDocument(); |
| 171 | + }); |
| 172 | + expect(dataSource.find).toHaveBeenCalledWith('accounts', expect.anything()); |
| 173 | + }); |
| 174 | + |
| 175 | + it('lets the user pick a NEW value there — the thing the empty Select made impossible', async () => { |
| 176 | + const dataSource = fakeDataSource(); |
| 177 | + const { onChange } = renderRow( |
| 178 | + { field: 'account_pending', operator: 'equals', value: '' }, |
| 179 | + { dataSource }, |
| 180 | + ); |
| 181 | + |
| 182 | + fireEvent.click(screen.getByTestId('lookup-picker-account_pending')); |
| 183 | + const candidate = await screen.findByText('Umbrella'); |
| 184 | + fireEvent.click(candidate); |
| 185 | + |
| 186 | + // "值可见了,用户依然挑不出别的记录" is what the card is about; this is the |
| 187 | + // assertion that the user CAN now, and that the choice reaches the row. |
| 188 | + expect(lastRow(onChange)).toMatchObject({ |
| 189 | + field: 'account_pending', |
| 190 | + value: 'acc_008', |
| 191 | + }); |
| 192 | + }); |
| 193 | + |
| 194 | + it('draws no option-driven value Select for that column at all', () => { |
| 195 | + renderRow({ field: 'account_pending', operator: 'equals', value: 'acc_007' }); |
| 196 | + |
| 197 | + // Field and operator only. A third combobox is the empty Select of the bug |
| 198 | + // report — the control with no search box and no candidates. |
| 199 | + expect(comboboxCount()).toBe(2); |
| 200 | + }); |
| 201 | + |
| 202 | + it('keeps the row value visible in the picker when no DataSource is configured', () => { |
| 203 | + const { onChange } = renderRow({ |
| 204 | + field: 'account_pending', |
| 205 | + operator: 'equals', |
| 206 | + value: 'acc_007', |
| 207 | + }); |
| 208 | + |
| 209 | + // `LookupValuePicker`'s by-hand fallback. The C half of objectui#4874's |
| 210 | + // ruling — the value is never invisible — is preserved by the reroute, it |
| 211 | + // is just the picker that honours it now instead of a temporary SelectItem. |
| 212 | + const input = document.querySelector<HTMLInputElement>( |
| 213 | + 'input[placeholder="Enter Account (pending) id"]', |
| 214 | + ); |
| 215 | + expect(input, 'the unloaded lookup drew no id input').toBeTruthy(); |
| 216 | + expect(input!.value).toBe('acc_007'); |
| 217 | + expect(onChange).not.toHaveBeenCalled(); |
| 218 | + }); |
| 219 | + |
| 220 | + it('gives the list operators the multi-value picker, not an empty checkbox list', () => { |
| 221 | + renderRow({ field: 'account_pending', operator: 'in', value: ['acc_007'] }); |
| 222 | + |
| 223 | + // `in`/`notIn` on this column used to render a checkbox list over zero |
| 224 | + // options: every existing term invisible, no term addable. The multiple |
| 225 | + // picker keeps list ARITY (objectui#3958) while restoring the search. |
| 226 | + expect(screen.getByTestId('lookup-ids-account_pending')).toBeInTheDocument(); |
| 227 | + expect(screen.queryAllByRole('checkbox')).toHaveLength(0); |
| 228 | + }); |
| 229 | + |
| 230 | + it('routes an `owner` column with `options: []` through the same criterion', () => { |
| 231 | + renderRow({ field: 'owner_pending', operator: 'equals', value: 'usr_1' }); |
| 232 | + |
| 233 | + // The branch's `type === "owner"` conjunct, which supplies its own |
| 234 | + // `referenceTo` ("users") downstream. `options: []` used to block it here |
| 235 | + // exactly as it blocked the `referenceTo` case. |
| 236 | + expect(comboboxCount()).toBe(2); |
| 237 | + const input = document.querySelector<HTMLInputElement>('input[placeholder="Enter Owner id"]'); |
| 238 | + expect(input, 'the owner column drew no id input').toBeTruthy(); |
| 239 | + expect(input!.value).toBe('usr_1'); |
| 240 | + }); |
| 241 | + |
| 242 | + it('matches the behaviour of the same column with `options` absent', () => { |
| 243 | + const absent = renderRow({ field: 'account', operator: 'equals', value: 'acc_007' }); |
| 244 | + const absentComboboxes = absent.container.querySelectorAll('[role="combobox"]').length; |
| 245 | + const absentIdInput = absent.container.querySelector<HTMLInputElement>( |
| 246 | + 'input[placeholder="Enter Account id"]', |
| 247 | + ); |
| 248 | + |
| 249 | + const pending = renderRow({ |
| 250 | + field: 'account_pending', |
| 251 | + operator: 'equals', |
| 252 | + value: 'acc_007', |
| 253 | + }); |
| 254 | + const pendingComboboxes = pending.container.querySelectorAll('[role="combobox"]').length; |
| 255 | + const pendingIdInput = pending.container.querySelector<HTMLInputElement>( |
| 256 | + 'input[placeholder="Enter Account (pending) id"]', |
| 257 | + ); |
| 258 | + |
| 259 | + // The card's own framing: `options: []` and no `options` key describe the |
| 260 | + // same state of the world (the domain has not arrived), so the column must |
| 261 | + // not be filterable in one spelling and unfilterable in the other. Compared |
| 262 | + // control-for-control rather than asserted twice, so the pin fails if either |
| 263 | + // side moves. |
| 264 | + expect(pendingComboboxes).toBe(absentComboboxes); |
| 265 | + expect(absentIdInput?.value).toBe('acc_007'); |
| 266 | + expect(pendingIdInput?.value).toBe('acc_007'); |
| 267 | + }); |
| 268 | +}); |
| 269 | + |
| 270 | +describe('a column that owns its option domain keeps the static Select', () => { |
| 271 | + it('draws the option-driven Select for a lookup with NON-EMPTY options', async () => { |
| 272 | + renderRow({ field: 'account_loaded', operator: 'equals', value: 'acc_001' }); |
| 273 | + |
| 274 | + // The half of the criterion that must NOT move: these options ARE the |
| 275 | + // column's domain, so the local Select is the right control and the remote |
| 276 | + // picker would be a regression. |
| 277 | + expect(comboboxCount()).toBe(3); |
| 278 | + expect(screen.queryByTestId('lookup-picker-account_loaded')).toBeNull(); |
| 279 | + |
| 280 | + const trigger = screen.getAllByRole('combobox')[2]; |
| 281 | + expect(trigger.textContent).toBe('Acme'); |
| 282 | + |
| 283 | + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); |
| 284 | + await waitFor(() => { |
| 285 | + expect( |
| 286 | + screen.getAllByRole('option').map((o) => o.textContent), |
| 287 | + ).toEqual(expect.arrayContaining(['Acme', 'Globex'])); |
| 288 | + }); |
| 289 | + }); |
| 290 | + |
| 291 | + it('keeps the checkbox list for a list operator on that column', () => { |
| 292 | + renderRow({ field: 'account_loaded', operator: 'in', value: ['acc_001'] }); |
| 293 | + |
| 294 | + expect(screen.queryByTestId('lookup-ids-account_loaded')).toBeNull(); |
| 295 | + expect( |
| 296 | + screen.getAllByRole('checkbox').map((box) => ({ |
| 297 | + label: box.parentElement?.textContent ?? '', |
| 298 | + checked: box.getAttribute('data-state') === 'checked', |
| 299 | + })), |
| 300 | + ).toEqual([ |
| 301 | + { label: 'Acme', checked: true }, |
| 302 | + { label: 'Globex', checked: false }, |
| 303 | + ]); |
| 304 | + }); |
| 305 | +}); |
| 306 | + |
| 307 | +describe("the branch's other conditions are untouched", () => { |
| 308 | + it('leaves a non-lookup `select` column with `options: []` exactly where it was', () => { |
| 309 | + renderRow({ field: 'stage_pending', operator: 'equals', value: 'draft' }); |
| 310 | + |
| 311 | + // `select` is not lookup-like: there is no remote search behind it, so its |
| 312 | + // empty Select is a metadata problem rather than this defect. Rerouting it |
| 313 | + // would be the fix over-reaching past the conjunct it replaced. |
| 314 | + expect(comboboxCount()).toBe(3); |
| 315 | + expect(screen.queryByTestId('lookup-picker-stage_pending')).toBeNull(); |
| 316 | + }); |
| 317 | + |
| 318 | + it('still rejects a lookup with `options: []` that has nothing to search against', () => { |
| 319 | + renderRow({ field: 'orphan_pending', operator: 'equals', value: 'x_1' }); |
| 320 | + |
| 321 | + // No `referenceTo`, and neither `user` nor `owner`: the picker would have no |
| 322 | + // object to query. The third conjunct is what says so, and it is unchanged — |
| 323 | + // this column's degradation is the card's adjacent observation, out of scope. |
| 324 | + expect(screen.queryByTestId('lookup-picker-orphan_pending')).toBeNull(); |
| 325 | + expect(comboboxCount()).toBe(3); |
| 326 | + }); |
| 327 | +}); |
0 commit comments