Skip to content

Commit a39b02a

Browse files
baozhoutaoclaude
andauthored
fix(lint): resolve app navigation viewName against the target object's list views (#14286)
* wip: nav viewName resolution * fix(lint): resolve app navigation viewName against the object's list views An app navigation entry's `viewName` was resolved by nothing at author time. The schema documents it as "Default list view to open", so an unresolvable name fell back rather than failing: the entry kept its authored label and icon and opened a different view, with `os validate --json` reporting valid: true and `os build` green. Extends #2554's `lintViewRefs` to the navigation door of the same `listViews` namespace. Resolution mirrors objectui's `resolveViewId` in all three directions so a name that works at runtime is never reported. Part of #14108 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8e03393 commit a39b02a

4 files changed

Lines changed: 450 additions & 2 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/lint': minor
3+
---
4+
5+
Resolve an app navigation entry's `viewName` against its object's list views at `validate` and `build`
6+
7+
`AppNavigationItemSchema.viewName` is documented as *"Default list view to open"*, so an
8+
unresolvable name never failed — it **fell back**. A nav entry keeping its authored label and
9+
icon would open a different view, and nothing said so: `os validate --json` reported
10+
`valid: true` and `os build` was green. The decay mode was worse than the typo mode — renaming
11+
a list view silently degraded every nav entry pointing at it, with every gate green and the
12+
diff reading correctly in review.
13+
14+
`lintViewRefs` now walks `app.navigation` (and the `areas[]` container) recursively and reports
15+
`view-ref-nav-view-missing` as an **error** when a `viewName` resolves to no list view on the
16+
object it names. This extends #2554's existing rule to the second, more travelled door into the
17+
same `listViews` namespace rather than adding a new rule class.
18+
19+
Resolution mirrors the runtime matcher (objectui's `resolveViewId`) in all three directions —
20+
exact id, short name retried as `<object>.<name>`, and qualified name with the prefix stripped —
21+
so a name that works at runtime is never reported. The accept set narrows only where the stack
22+
itself declares the object's list views: an entry is skipped when the `viewName` is interpolated,
23+
when `recordId` is set (the schema documents `viewName` as ignored there), when the item carries
24+
`requiresObject`, or when this stack contributed no list view for that object.

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ export {
755755
VIEW_KEY_COLLISION,
756756
VIEW_REF_FORM_TARGET_MISSING,
757757
VIEW_REF_FORM_TARGET_KIND,
758+
VIEW_REF_NAV_VIEW_MISSING,
758759
} from './lint-view-refs.js';
759760

760761
export {

packages/lint/src/lint-view-refs.test.ts

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import {
66
VIEW_KEY_COLLISION,
77
VIEW_REF_FORM_TARGET_MISSING,
88
VIEW_REF_FORM_TARGET_KIND,
9+
VIEW_REF_NAV_VIEW_MISSING,
910
} from './lint-view-refs.js';
11+
import { runAuthoringRules, splitBySeverity } from './authoring-rules.js';
1012

1113
const listView = (object: string) => ({
1214
type: 'grid',
@@ -174,3 +176,233 @@ describe('lintViewRefs — form action target resolution', () => {
174176
expect(out[0].where).toContain("object 'task'"); // object-nested context retained
175177
});
176178
});
179+
180+
// ─────────────────────────────────────────────────────────────────────────────
181+
// #14108 — app navigation `viewName`, the SECOND door into the same `listViews`
182+
// namespace the #2554 rules above guard.
183+
// ─────────────────────────────────────────────────────────────────────────────
184+
185+
/** Distinct configs so the expander does not dedupe the default `list` into a
186+
* structurally identical `listViews` entry — the fixture needs BOTH so the
187+
* default's real key is observable. */
188+
const navListView = (object: string, label: string, column: string) => ({
189+
type: 'grid',
190+
label,
191+
columns: [column],
192+
data: { provider: 'object', object },
193+
});
194+
195+
const NAV_OBJECT = {
196+
name: 'duly_task',
197+
list: navListView('duly_task', 'All tasks', 'title'),
198+
listViews: {
199+
schedule: navListView('duly_task', 'Schedule', 'due_at'),
200+
board: navListView('duly_task', 'Board', 'status'),
201+
},
202+
formViews: { edit: formView('duly_task') },
203+
};
204+
205+
const navStack = (nav: Record<string, unknown>, appExtra: Record<string, unknown> = {}) => ({
206+
objects: [NAV_OBJECT],
207+
apps: [{ name: 'duly', label: 'Duly', navigation: [nav], ...appExtra }],
208+
});
209+
210+
const navFindings = (stack: Record<string, unknown>) =>
211+
lintViewRefs(stack).filter((f) => f.rule === VIEW_REF_NAV_VIEW_MISSING);
212+
213+
describe('lintViewRefs — navigation viewName resolution (#14108)', () => {
214+
it('errors when a nav viewName names no list view on its object (the measured repro)', () => {
215+
const out = navFindings(
216+
navStack({ id: 'nav_schedule', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'Schedule' }),
217+
);
218+
expect(out).toHaveLength(1);
219+
expect(out[0].severity).toBe('error');
220+
expect(out[0].where).toBe("app 'duly' · nav 'nav_schedule'");
221+
// The available list views are listed the way the platform's other
222+
// unknown-name findings list theirs — short (authorable) keys, not
223+
// expanded `<object>.<key>` ids.
224+
expect(out[0].message).toContain('board, default, schedule');
225+
expect(out[0].message).not.toContain('duly_task.schedule');
226+
});
227+
228+
it('suggests the near-miss name a rename left behind', () => {
229+
const out = navFindings(
230+
navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'schedul', label: 'Schedule' }),
231+
);
232+
expect(out[0].hint).toContain('Did you mean "schedule"?');
233+
});
234+
235+
it('accepts a viewName that names a declared listViews key', () => {
236+
expect(navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'schedule', label: 'S' }))).toEqual([]);
237+
});
238+
239+
it("accepts the default `list`'s real expansion key — which is `default`, not `all`", () => {
240+
// The schema documents viewName as 'Defaults to "all"'. That describes the
241+
// CONVENTION of declaring a `listViews.all`, not a magic fallback name:
242+
// `expandViewContainer` keys a bare default `list` as `<object>.default`,
243+
// and objectui's `resolveViewId` has no special case for 'all' either.
244+
expect(navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'default', label: 'D' }))).toEqual([]);
245+
const undeclaredAll = navFindings(
246+
navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'all', label: 'A' }),
247+
);
248+
expect(undeclaredAll).toHaveLength(1);
249+
});
250+
251+
it("accepts 'all' once the object actually declares it (the app-crm convention)", () => {
252+
const stack = {
253+
objects: [{ ...NAV_OBJECT, listViews: { ...NAV_OBJECT.listViews, all: navListView('duly_task', 'All', 'title') } }],
254+
apps: [{ name: 'duly', navigation: [{ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'all', label: 'A' }] }],
255+
};
256+
expect(navFindings(stack)).toEqual([]);
257+
});
258+
259+
it('accepts a FULLY QUALIFIED viewName, exactly as the runtime matcher does', () => {
260+
// objectui's `resolveViewId` accepts `<object>.<key>` as well as the short
261+
// key. A lint stricter than the matcher would red a name that works.
262+
expect(
263+
navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'duly_task.schedule', label: 'S' })),
264+
).toEqual([]);
265+
});
266+
267+
it('resolves against an independent (already-expanded) top-level ViewItem', () => {
268+
const stack = {
269+
objects: [{ name: 'duly_task' }],
270+
views: [{ name: 'duly_task.mine', object: 'duly_task', viewKind: 'list', ...navListView('duly_task', 'Mine', 'title') }],
271+
apps: [{ name: 'duly', navigation: [{ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'mine', label: 'M' }] }],
272+
};
273+
expect(navFindings(stack)).toEqual([]);
274+
});
275+
276+
it('names the form-view case explicitly rather than reporting a bare miss', () => {
277+
const out = navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: 'edit', label: 'E' }));
278+
expect(out).toHaveLength(1);
279+
expect(out[0].message).toContain('resolves to a FORM view');
280+
expect(out[0].hint).toContain("type:'form' action target");
281+
});
282+
283+
it('recurses into `group` children AND into an `object` item’s own children', () => {
284+
const stack = {
285+
objects: [NAV_OBJECT],
286+
apps: [{
287+
name: 'duly',
288+
navigation: [{
289+
id: 'grp', type: 'group', label: 'Work',
290+
children: [{
291+
id: 'parent', type: 'object', objectName: 'duly_task', viewName: 'schedule', label: 'Tasks',
292+
children: [{ id: 'deep', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'Deep' }],
293+
}],
294+
}],
295+
}],
296+
};
297+
const out = navFindings(stack);
298+
expect(out).toHaveLength(1);
299+
expect(out[0].where).toContain("nav 'deep'");
300+
});
301+
302+
it('walks the `areas[]` nav container too, not only `navigation`', () => {
303+
const bad = { id: 'in_area', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'X' };
304+
for (const area of [{ name: 'a', items: [bad] }, { name: 'a', navigation: [bad] }]) {
305+
const out = navFindings({ objects: [NAV_OBJECT], apps: [{ name: 'duly', areas: [area] }] });
306+
expect(out).toHaveLength(1);
307+
expect(out[0].where).toContain("nav 'in_area'");
308+
}
309+
});
310+
311+
it('reports each distinct nav entry once', () => {
312+
const stack = {
313+
objects: [NAV_OBJECT],
314+
apps: [{
315+
name: 'duly',
316+
navigation: [
317+
{ id: 'a', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'A' },
318+
{ id: 'b', type: 'object', objectName: 'duly_task', viewName: 'A4_no_such_view', label: 'B' },
319+
],
320+
}],
321+
};
322+
expect(navFindings(stack).map((f) => f.where)).toEqual([
323+
"app 'duly' · nav 'a'",
324+
"app 'duly' · nav 'b'",
325+
]);
326+
});
327+
});
328+
329+
describe('lintViewRefs — navigation viewName exemptions (false positives stay near zero)', () => {
330+
const bogus = 'A4_no_such_view';
331+
332+
it('skips an interpolated viewName (resolved at render time)', () => {
333+
expect(navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: '${params.view}', label: 'X' }))).toEqual([]);
334+
});
335+
336+
it('skips an entry carrying `requiresObject` — another package provides the object', () => {
337+
expect(
338+
navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: bogus, requiresObject: 'duly_task', label: 'X' })),
339+
).toEqual([]);
340+
});
341+
342+
it('skips when `recordId` is set — the schema documents viewName as ignored there', () => {
343+
expect(
344+
navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', viewName: bogus, recordId: '{current_user_id}', label: 'X' })),
345+
).toEqual([]);
346+
});
347+
348+
it('says nothing about an object this stack declares no list views for', () => {
349+
// Indistinguishable from a cross-package object, so the rule stays silent
350+
// rather than guessing — the precondition that lets it be an ERROR at all.
351+
const stack = {
352+
objects: [{ name: 'other_object' }],
353+
apps: [{ name: 'duly', navigation: [{ id: 'n', type: 'object', objectName: 'not_here', viewName: bogus, label: 'X' }] }],
354+
};
355+
expect(navFindings(stack)).toEqual([]);
356+
});
357+
358+
it('says nothing when the nav entry names no view at all', () => {
359+
expect(navFindings(navStack({ id: 'n', type: 'object', objectName: 'duly_task', label: 'X' }))).toEqual([]);
360+
});
361+
362+
it('leaves the repo’s own shipped nav shapes alone (no error on a resolvable stack)', () => {
363+
const stack = {
364+
objects: [NAV_OBJECT],
365+
apps: [{
366+
name: 'duly',
367+
navigation: [
368+
{ id: 'a', type: 'object', objectName: 'duly_task', viewName: 'schedule', label: 'S', icon: 'calendar' },
369+
{ id: 'b', type: 'object', objectName: 'duly_task', label: 'Default landing' },
370+
{ id: 'c', type: 'page', pageName: 'pricing', label: 'Pricing' },
371+
{ id: 'd', type: 'url', url: 'https://example.com', label: 'Docs' },
372+
],
373+
}],
374+
};
375+
expect(navFindings(stack)).toEqual([]);
376+
});
377+
});
378+
379+
/**
380+
* The card's binding acceptance criterion, pinned end-to-end rather than
381+
* inferred from the registry entry (the #14148 / #14107 precedent): the
382+
* measured repro must fail `validate` AND `build`. The card measured
383+
* `os validate --json` returning `valid: true` and `os build` green, so a
384+
* validate-only fix was not acceptable — and nothing else in this file would
385+
* notice if the suite entry's `commands` were narrowed later.
386+
*/
387+
describe('#14108 acceptance — a nav viewName miss gates `validate` AND `build`', () => {
388+
const repro = navStack({
389+
id: 'nav_schedule', type: 'object', objectName: 'duly_task',
390+
viewName: 'A4_no_such_view', label: 'Schedule', icon: 'gantt-chart',
391+
});
392+
const clean = navStack({
393+
id: 'nav_schedule', type: 'object', objectName: 'duly_task',
394+
viewName: 'schedule', label: 'Schedule', icon: 'gantt-chart',
395+
});
396+
397+
for (const command of ['validate', 'build'] as const) {
398+
it(`the measured repro fails \`${command}\``, () => {
399+
const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: repro as never }));
400+
expect(errors.map((f) => f.rule)).toContain(VIEW_REF_NAV_VIEW_MISSING);
401+
});
402+
403+
it(`the corrected nav entry passes \`${command}\``, () => {
404+
const { errors, advisories } = splitBySeverity(runAuthoringRules(command, { normalized: clean as never }));
405+
expect([...errors, ...advisories].filter((f) => f.rule === VIEW_REF_NAV_VIEW_MISSING)).toEqual([]);
406+
});
407+
}
408+
});

0 commit comments

Comments
 (0)