Skip to content

Commit 5cdeedc

Browse files
committed
feat(plugin-detail): row caps admit only a positive integer number, and warn on refusal
normalizeLimit (record:activity, record:chatter, record:discussion) and normalizeHistoryLimit (record:history) no longer run Number(value) before their integer check. The spec declares limit as z.number().int().positive(), so a string, boolean or array is refused and falls back to the block default. Each renderer states the refusal once through console.warn from an effect, naming the block and the raw value with its type. The four pins that asserted numeric-string coercion are flipped to refusal assertions with lit number controls; the two silence pins are flipped to warning pins.
1 parent 8b1f066 commit 5cdeedc

10 files changed

Lines changed: 436 additions & 157 deletions
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@object-ui/plugin-detail': minor
3+
---
4+
5+
feat(plugin-detail): row caps on `record:activity`, `record:history`, `record:chatter` and `record:discussion` admit only a positive integer number, and a refused one warns
6+
7+
`@objectstack/spec` declares `limit` on `RecordActivityProps` (which is also
8+
`RecordChatterProps.feed`) and on `RecordHistoryProps` as
9+
`z.number().int().positive()`, and refuses a string, a boolean or an array
10+
outright. The renderers used to run `Number(value)` before their integer check,
11+
so they also read `'5'` as 5, `' 5 '` as 5, `'0x10'` as 16, `true` as 1 and `[7]`
12+
as 7: a second accepted set, wider than the contract (objectui#10145, ruled
13+
STOP).
14+
15+
Both resolvers (`normalizeLimit`, shared by activity, chatter and discussion,
16+
and `normalizeHistoryLimit`) now admit only a value whose type is `number` and
17+
which is a positive integer. Every other authored value falls back to the
18+
block's default (20 for activity, chatter and discussion; 50 for history). The
19+
renderer then logs one `console.warn` that names the block and spells the raw
20+
value with its type (for example `declared limit: "5" (string)`). The warning
21+
also fires for the fractional, zero, negative, `NaN` and infinite values these
22+
blocks already refused silently. That matches the "always warn" ruling on
23+
objectui#10097 and the three objectui#9925 read points. An absent `limit` is not
24+
a refusal and stays silent.
25+
26+
**Migration.** Stored metadata that spells a row cap as a string (for example
27+
`limit: '5'`, or `feed: { limit: '5' }` on a chatter or discussion block) now
28+
drops to the block's default and warns in the console, where it used to be read
29+
as 5. Write the number instead (`limit: 5`). Publishing such metadata was
30+
already refused by the spec, so this affects only bodies stored before that
31+
check, or written around it.

‎content/docs/plugins/plugin-detail.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ itself, so everything above describes that page too.
255255
| `types` | Allow-list of feed item types. Recognised entries narrow the feed; unrecognised ones are dropped and named in one `console.warn`. A list that keeps nothing — `types: []`, or one whose every entry is unrecognised — filters to **nothing** and the panel renders an empty timeline, rather than widening back to every kind. Omit `types` for no filter; that is the only value that means it. A recognised entry that **no producer emits** is honoured but named too — see [Feed kinds with no producer](#feed-kinds-with-no-producer). | — |
256256
| `filterMode` | Which slice the filter dropdown opens on (`all` / `comments_only` / `changes_only` / `tasks_only`). The user can still change it. An unrecognised value falls back to `all`. | — |
257257
| `showFilterToggle` | Shows/hides that dropdown. | — |
258-
| `limit` | Items per page; also caps the scoped `sys_activity` read. "Load more" grows the window by this much and re-reads. | — |
258+
| `limit` | Items per page; also caps the scoped `sys_activity` read. "Load more" grows the window by this much and re-reads. Must be a positive integer **number**: any other value (a string such as `"5"`, a boolean, a fraction, zero or a negative) is ignored, the block uses its default of 20, and one `console.warn` names the value. The same rule applies to `feed.limit` on `record:chatter` / `record:discussion` and to `limit` on `record:history` (default 50). | — |
259259
| `showCompleted` | Include completed activities (`sys_activity.type = 'completed'`, which surfaces as a `task` item). Off by default, per spec. | — |
260260
| `unifiedTimeline` | Default `true` mixes field changes and comments in one timeline (Airtable style). `false` un-mixes them: the panel becomes a discussion stream and field changes stay in `record:history`. | — |
261261
| `showCommentInput` | Shows the composer. | A host discussion context to persist the comment. Without one the feed is read-only and no composer is rendered. |
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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+
* `record:chatter` / `record:discussion` REFUSE a `feed.limit` the contract
11+
* rejects, and say so (objectui#10145, ruled STOP).
12+
*
13+
* `RecordChatterProps.feed` is `RecordActivityProps`, whose `limit` is
14+
* `z.number().int().positive().default(20)`, so these two blocks resolve their
15+
* page size through the same `normalizeLimit` as `record:activity`. Until
16+
* objectui#10145 that resolver ran `Number(value)` first and read `'2'` as a
17+
* two-row page; the spec refuses a string outright, so the page now falls back
18+
* to the default and the renderer names the refused value on the developer
19+
* channel.
20+
*
21+
* The observable is the number of ROOT rows the panel shows: with six comments
22+
* on hand, a two-row page shows two, and the default page (20) shows all six.
23+
*/
24+
25+
import * as React from 'react';
26+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
27+
import { render, screen, cleanup } from '@testing-library/react';
28+
import { DiscussionContextProvider } from '@object-ui/react';
29+
import type { FeedItem } from '@object-ui/types';
30+
import { RecordChatterRenderer } from '../record-chatter';
31+
32+
/** Six root comments, oldest first. */
33+
const SIX: FeedItem[] = Array.from({ length: 6 }, (_, i) => ({
34+
id: `c${i + 1}`,
35+
type: 'comment',
36+
actor: 'Ada',
37+
body: `Comment number ${i + 1}`,
38+
createdAt: `2026-01-0${i + 1}T00:00:00.000Z`,
39+
})) as FeedItem[];
40+
41+
const handlers = { onAddComment: vi.fn(), onAddReply: vi.fn(), onToggleReaction: vi.fn() };
42+
43+
function mount(type: string, limit: unknown) {
44+
return render(
45+
<DiscussionContextProvider items={SIX as any} loading={false} {...handlers}>
46+
<RecordChatterRenderer schema={{ type, position: 'bottom', feed: { limit } } as any} />
47+
</DiscussionContextProvider>,
48+
);
49+
}
50+
51+
const shownRows = () => screen.queryAllByText(/Comment number \d/).length;
52+
53+
describe.each(['record:chatter', 'record:discussion'])('%s — `feed.limit` admits only a positive integer number (objectui#10145)', (blockName) => {
54+
let warn: ReturnType<typeof vi.spyOn>;
55+
56+
beforeEach(() => {
57+
cleanup();
58+
warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
59+
});
60+
61+
afterEach(() => {
62+
warn.mockRestore();
63+
});
64+
65+
const rowCapWarnings = () =>
66+
(warn.mock.calls as unknown[][])
67+
.map((args) => args.map((a) => (typeof a === 'string' ? a : String(a))).join(' '))
68+
.filter((text) => text.includes('row cap'));
69+
70+
it('CONTROL — a usable NUMBER cap narrows the page, silently', () => {
71+
mount(blockName, 2);
72+
expect(shownRows()).toBe(2);
73+
expect(rowCapWarnings()).toEqual([]);
74+
});
75+
76+
it.each([
77+
['a numeric string', '2', '"2" (string)'],
78+
['a padded numeric string', ' 2 ', '" 2 " (string)'],
79+
['a boolean', true, 'true (boolean)'],
80+
['an array', [2], '[2] (array)'],
81+
])('refuses %s — the page falls back to the default and one warning names the block', (_label, authored, spelled) => {
82+
mount(blockName, authored);
83+
// Default page (20) over six rows: all six, ⛔ not the two `Number()` read.
84+
expect(shownRows()).toBe(6);
85+
const hits = rowCapWarnings();
86+
expect(hits).toHaveLength(1);
87+
expect(hits[0]).toContain(`${blockName} feed row cap`);
88+
expect(hits[0]).toContain(`declared limit: ${spelled}`);
89+
});
90+
});

‎packages/plugin-detail/src/renderers/__tests__/record-history.rowLimitRefusal-10005.test.tsx‎

Lines changed: 61 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,25 @@ describe('record:history — a refused row cap falls back, it is not repaired (o
107107
expect(await topFor({ limit: 10 })).toBe(10);
108108
});
109109

110-
it('CONTROL — a numeric string still resolves, as it does on the sibling', async () => {
111-
// `normalizeLimit('5')` is pinned to `5` on `record:activity`; this
112-
// refusal narrows what is admitted to the contract's value set, it does
113-
// not change how a node's value is read.
114-
expect(await topFor({ limit: '5' })).toBe(5);
110+
it('refuses a numeric STRING — ⛔ no `Number()` coercion (objectui#10145, flipped)', async () => {
111+
// FLIPPED from `.toBe(5)`: while the family kept `Number(value)`, `'5'`
112+
// resolved to a five-row window. The spec refuses a string outright
113+
// (`z.number()`), and the ruling on objectui#10145 is STOP.
114+
const top = await topFor({ limit: '5' });
115+
expect(top).toBe(RENDERER_DEFAULT);
116+
expect(top).not.toBe(5);
117+
});
118+
119+
it('refuses the other shapes `Number()` used to admit (objectui#10145)', async () => {
120+
expect(await topFor({ limit: ' 5 ' })).toBe(RENDERER_DEFAULT); // was 5
121+
expect(await topFor({ limit: '0x10' })).toBe(RENDERER_DEFAULT); // was 16
122+
expect(await topFor({ limit: true })).toBe(RENDERER_DEFAULT); // was 1
123+
expect(await topFor({ limit: [7] })).toBe(RENDERER_DEFAULT); // was 7
124+
});
125+
126+
it('CONTROL — the same cap as a NUMBER still passes through', async () => {
127+
// Lit control beside the flip above.
128+
expect(await topFor({ limit: 5 })).toBe(5);
115129
});
116130

117131
it('the `properties` read point answers identically — refused', async () => {
@@ -123,61 +137,64 @@ describe('record:history — a refused row cap falls back, it is not repaired (o
123137
});
124138
});
125139

126-
describe('record:history — the refusal is SILENT, matching its sibling (objectui#10005)', () => {
140+
describe('record:history — the refusal is LOUD (objectui#10097, objectui#10145)', () => {
127141
let warn: ReturnType<typeof vi.spyOn>;
128-
let error: ReturnType<typeof vi.spyOn>;
129142

130143
beforeEach(() => {
131144
cleanup();
132145
warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
133-
error = vi.spyOn(console, 'error').mockImplementation(() => {});
134146
});
135147

136148
afterEach(() => {
137149
warn.mockRestore();
138-
error.mockRestore();
139150
});
140151

141152
/**
142-
* Calls on a developer channel that are ABOUT this renderer's row cap.
143-
*
144-
* ⚠️ Deliberately a FILTER rather than a blanket `not.toHaveBeenCalled()`.
145-
* MEASURED on this harness: rendering the timeline emits an unrelated
146-
* `react-i18next` `NO_I18NEXT_INSTANCE` warning, and whether it lands inside
147-
* this spy's window depends on which OTHER tests in the file ran first — so a
148-
* blanket assertion pins test ORDER, not this renderer's silence, and goes red
149-
* for anyone who runs this one test alone. The filter is kept wide enough that
150-
* a real diagnostic cannot slip through it: any message naming the authored
151-
* value, `limit`, a row cap, or this renderer counts.
153+
* Calls on the developer channel that are ABOUT this block's row cap. A
154+
* FILTER rather than a blanket count: MEASURED on this harness, rendering the
155+
* timeline emits an unrelated `react-i18next` `NO_I18NEXT_INSTANCE` warning
156+
* whose presence in this spy's window depends on test order.
152157
*/
153-
function rowCapDiagnostics(spy: ReturnType<typeof vi.spyOn>): unknown[][] {
154-
return (spy.mock.calls as unknown[][]).filter((args) => {
155-
const text = args
156-
.map((a) => {
157-
if (typeof a === 'string') return a;
158-
try {
159-
return JSON.stringify(a);
160-
} catch {
161-
return String(a);
162-
}
163-
})
164-
.join(' ');
165-
return /limit|row cap|record:history|RecordHistory|-5/i.test(text);
166-
});
158+
function rowCapWarnings(): string[] {
159+
return (warn.mock.calls as unknown[][])
160+
.map((args) => args.map((a) => (typeof a === 'string' ? a : String(a))).join(' '))
161+
.filter((text) => text.includes('record:history row cap'));
167162
}
168163

169164
/**
170-
* ⭐ This is a DECISION recorded as a pin, not an inevitability. The sibling
171-
* this card was told to match — `normalizeLimit` in `recordActivityFeed` —
172-
* refuses in silence, so this renderer does too. The three read points
173-
* objectui#9925 repaired (`object-kanban`, `object-timeline`,
174-
* `record:reference_rail`) refuse LOUDLY instead, so the family currently
175-
* holds two answers on loudness. Whoever rules that question changes this
176-
* pin deliberately rather than discovering the silence by accident.
165+
* FLIPPED from "names nothing on the developer channel when it refuses":
166+
* that pin recorded the family's silence as a decision awaiting a ruling.
167+
* objectui#10097 ruled "always warn"; objectui#10145 ruled STOP with a loud
168+
* fallback. The warning names the block and spells the raw value WITH its
169+
* type, so `'5'` and `5` cannot be confused in the message.
177170
*/
178-
it('names nothing on the developer channel when it refuses', async () => {
179-
expect(await topFor({ limit: -5 })).toBe(RENDERER_DEFAULT);
180-
expect(rowCapDiagnostics(warn)).toEqual([]);
181-
expect(rowCapDiagnostics(error)).toEqual([]);
171+
it.each([
172+
['a numeric string', '5', '"5" (string)'],
173+
['a padded numeric string', ' 5 ', '" 5 " (string)'],
174+
['a hex string', '0x10', '"0x10" (string)'],
175+
['a boolean', true, 'true (boolean)'],
176+
['an array', [7], '[7] (array)'],
177+
['a fraction', 1.5, '1.5 (number)'],
178+
['zero', 0, '0 (number)'],
179+
['a negative', -5, '-5 (number)'],
180+
['NaN', Number.NaN, 'NaN (number)'],
181+
['Infinity', Number.POSITIVE_INFINITY, 'Infinity (number)'],
182+
])('warns once, naming the block and the raw value, for %s', async (_label, authored, spelled) => {
183+
expect(await topFor({ limit: authored })).toBe(RENDERER_DEFAULT);
184+
const hits = rowCapWarnings();
185+
expect(hits).toHaveLength(1);
186+
expect(hits[0]).toContain('record:history');
187+
expect(hits[0]).toContain(`declared limit: ${spelled}`);
188+
});
189+
190+
it('CONTROL — a usable cap and an absent cap say nothing', async () => {
191+
expect(await topFor({ limit: 10 })).toBe(10);
192+
expect(await topFor({})).toBe(RENDERER_DEFAULT);
193+
expect(rowCapWarnings()).toEqual([]);
194+
});
195+
196+
it('the `properties` read point warns identically', async () => {
197+
expect(await topFor({ properties: { limit: '5' } })).toBe(RENDERER_DEFAULT);
198+
expect(rowCapWarnings()).toHaveLength(1);
182199
});
183200
});

0 commit comments

Comments
 (0)