Skip to content

Commit 7b2e557

Browse files
fix(drill): "Open in list" is offered only when the list can show exactly the drawer's records (#30735)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4Uu4M8GSzQt9g91oqAEXonaaD)_ Fixes #30654 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - 改了什么:仪表盘和报表下钻抽屉里的「在列表中打开」,以及 `drillDown.target: 'navigate'`,现在只有列表页的 URL 能把下钻筛选原样写出来时才给出口;写不全就不给,留在抽屉里。 - 为什么改:下钻筛选 = 点中的分组 ∧ 组件筛选 ∧ 度量的下钻范围。URL 一个参数只能放一个值、也写不了 `$in`,以前会丢掉条件,打开的列表比抽屉里的数字更宽(抽屉 0 张发票,列表 4 张)。 - 风险与回滚:只影响下钻出口是否显示;单条件等值和日期区间下钻的 URL 逐字节不变。布尔值分组的下钻现在也不再给出口(沿用相关列表「查看全部」的同一条精确规则)。回滚即 revert 本 PR。 - 先看:`ui/packages/app-shell/src/views/useOpenRecordList.ts`(`drillRecordListFilter`、`useRecordListDrillNavigation`)、`ui/packages/react/src/context/DrillNavigationContext.tsx`(新可选成员)、`ui/packages/plugin-charts/src/ObjectChart.tsx` 的两处守卫。 - 两分钟验证:app-showcase「Revenue Pulse (filtered)」→「Paid Rate by Status」表格 → Sent 行的 Paid Invoices 格子:抽屉 0 行,右上角没有「在列表中打开」;同一行的发票数格子仍有,点开 URL 是 `filter[status]=sent` 加组件的日期区间。 - 需要你做的:无,席位落地。 ## What changes - `@object-ui/react`: `DrillNavigationValue` gains an optional `canOpenRecordList(objectName, filter): boolean`. Absent means the host claims every filter it is handed has an exact destination, which is how hosts behaved before. - `@object-ui/app-shell`: `useRecordListDrillNavigation()` returns `{ openRecordList, canOpenRecordList }` for `DashboardView` and `ReportView`. Both go through the new pure `drillRecordListFilter`, which resolves placeholders first (`resolveFilterPlaceholders` with the same `useFilterScope` scope the navigate arm always used), then asks `exactRecordListFilter` (reused unchanged, with `usePermissions().checkField(object, field, 'read')`). `openRecordList` navigates to the exact filter and does nothing when there is none, so a caller that skipped the question still never lands on a wider list. `useOpenRecordList` stays as the navigate half. - `@object-ui/plugin-dashboard`: `OpenInListButton` renders nothing, and `DrillDownDrawer`'s `navigateOnly` is false, when `canOpenRecordList` answers `false`. The drill then stays in the drawer, the same fallback as a host with no `openRecordList`. - `@object-ui/plugin-charts`: `ObjectChart`'s navigate arm and its own drawer-header "Open in list" ask the same question (with `drillFilter` and `merged` respectively). - The 13 `DashboardView` / `ReportView` tests that mock `./useOpenRecordList` now mock `useRecordListDrillNavigation` instead (mechanical, same `vi.fn()` opener). - One changeset: patch for the four packages (`Metadata-protocol: none` ⇒ patch; `Clause-②: yes (widening)` for the new optional interface member, per the claim amendment). Answers to the claim's two questions: - A `drillDown.target: 'navigate'` click with no exact destination opens the in-place drawer (no navigation, no dead click). - Placeholders are resolved before the exactness check, inside `drillRecordListFilter`, with the scope `useOpenRecordList` already read; a `{current_user_id}` that resolves to the same value as a bucket on the same field is one exact param (pinned). ## Tests New pins: - `useOpenRecordList.placeholderScope-9022.test.tsx`, new top-level describe (6 cases): the repro shape `composeDrillFilter({ status: 'paid' }, { status: 'sent' })` answers `false` and `openRecordList` does not navigate; an `$in` scope answers `false`; a plain equality bucket answers `true` and opens byte-for-byte `/apps/crm/invoice/data?filter%5Bstatus%5D=sent`; the same condition twice is one param; placeholders resolve before judging; an unreadable field answers `false`. The file's existing CONTROL (equality + date range URL pinned as a whole string) stays green unchanged. - `DrillDownDrawer.escapeHatch.test.tsx`, new top-level describe (4 cases): host answers `false` ⇒ no button, asked with the drill filter; `navigate` stays in the drawer; host answers `true` ⇒ button; host without `canOpenRecordList` ⇒ old behaviour for both the button and `navigate`. - `ObjectChart.drillNavigate.test.tsx`, new top-level describe (2 cases): host answers `false` ⇒ drawer, no header button; host answers `true` ⇒ navigates. The file's existing four cases (no `canOpenRecordList`) are the unchanged-host control. Ablation (each on the committed fix, restored to the HEAD blob by `scripts/ablation-replace.mjs`; the tests import the source relatively, no `dist/` involved): | Mutation | Result | |:---|:---| | delete the `OpenInListButton` guard | 1 failed / 7 (host answers false case) | | `DrillDownDrawer` `navigateOnly` guard → `true` | 1 failed / 7 (navigate stays in drawer) | | `ObjectChart` navigate guard → `true` | 1 failed / 6 | | `ObjectChart` header-button guard → `true` | 1 failed / 6 | | `drillRecordListFilter` returns the resolved filter unjudged | 3 failed / 12 (repro, `$in`, unreadable field) | <details><summary>Test runs</summary> On `7643fb85` (before the main merge): - `vitest run packages/plugin-dashboard/ packages/plugin-charts/ packages/react/` + the 42 app-shell test files that name `useOpenRecordList`, `DrillNavigation`, `DashboardView`, `ReportView`, `OpenInList`, `DrillDownDrawer`, `drillUrlFilters`, `recordListPath` or `exactRecordListFilter`: `Test Files 485 passed (485)`, `Tests 5230 passed | 15 skipped (5245)`. - `type-check` for `@object-ui/react`, `@object-ui/plugin-dashboard`, `@object-ui/plugin-charts`, `@object-ui/app-shell`: all four `Done`, lock `VERDICT command-exit 0`. Declared narrowing: the rest of app-shell's 2208 test files are left to CI's `Test UI` (a full local run did not finish inside the box's budget at 2 workers). On `7519c053` (after merging `origin/main`, which touched `ui/packages/core` exports, and a rebuild of the app-shell closure): plugin-dashboard + plugin-charts full + the same 42 app-shell files, `Test Files 348 passed (348)`, `Tests 3662 passed | 15 skipped (3677)`; the four `type-check`s `Done`; lock `VERDICT command-exit 0`. </details> ## Acceptance notes - `exactRecordListFilter` is reused unchanged, so its refusals now apply to drills too: a drill on a boolean bucket (`{ is_paid: true }`), on `$ne`, on a text operator, or with a placeholder that cannot resolve to a comparand the route carries no longer offers "Open in list" (the drawer still lists the records). Before, those opened a list scoped by a string comparand or with the condition dropped. - `ReportView`'s group-key fallback drill (older server with no drill metadata) goes through the same host, so it is judged the same way. - The PR carries no assignee or reviewer: the seat sets them at hand-over. - Branch is the card thread's harness branch (`claude/project-thread-qgekwo`, claim amendment 6671583364), not `claude/issue-30654-*`. ## Verification Live, app-showcase on its own port (`pnpm dev -- --fresh`), Console via Vite, headless Chromium at 1440 px. "Revenue Pulse (filtered)" → "Paid Rate by Status", Sent row (`Sent 4 0 0.0%`): | | Paid Invoices cell drawer | "Open in list" there | Invoice count cell "Open in list" URL | |:---|:---|:---|:---| | before (the seven source files at `origin/main`) | 0 rows | shown | `/apps/com.example.showcase/showcase_invoice/data?filter%5Bissued_on%5D%5Bgte%5D=2026-08-07&filter%5Bissued_on%5D%5Blte%5D=2026-11-05&filter%5Bstatus%5D=sent` | | after | 0 rows | absent | identical string | <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands` on `7643fb85`: 35 families, all `exit 0`. `pnpm --dir ui exec vitest run scripts/` was killed by the container's memory limit while the live rig ran (exit 1, SIGKILL, no verdict) and re-run alone: `Test Files 170 passed | 2 skipped (172)`, exit 0. `--ran` with exit codes: "35 derived famil(ies) accounted for — 35 run, 0 NOT-MEASURED (a DERIVED zero — all 35 recorded an exit code and none of them is 3)". `check:nul-bytes` exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgTGQVA597MpPgAhmzK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 49c8568 commit 7b2e557

24 files changed

Lines changed: 422 additions & 50 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@object-ui/react': patch
3+
'@object-ui/plugin-dashboard': patch
4+
'@object-ui/plugin-charts': patch
5+
'@object-ui/app-shell': patch
6+
---
7+
8+
fix(drill): "Open in list" from a dashboard or report drill is offered only when the list can show exactly the drawer's records
9+
10+
Clause-②: yes (widening)
11+
Metadata-protocol: none
12+
13+
A drill filter is the clicked bucket AND the widget's filter AND the measure's drill scope. The list route holds one value per filter param and has no spelling for `$in`, so a drill with two conditions on one field (a `status = sent` bucket under a `status = paid` measure scope) or an `$in` scope opened a wider list than the drawer: the drawer showed 0 invoices and the list showed every sent one. The Console now offers "Open in list", and follows `drillDown.target: 'navigate'`, only when the route can write every condition of the drill filter, after resolving its placeholders; otherwise the drill stays in the drawer. A plain equality or date-range drill opens the same URL as before.
14+
15+
`DrillNavigationValue` (`@object-ui/react`) gains an optional `canOpenRecordList(objectName, filter)`. The drill drawers (`DrillDownDrawer`, `OpenInListButton`, `ObjectChart`) offer the escape hatch only when `openRecordList` is present and `canOpenRecordList` is absent or answers `true`. A host that does not provide it behaves exactly as before.

‎ui/packages/app-shell/src/views/DashboardView.autoRefresh-11062.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ vi.mock('react-router-dom', () => ({
4545
useLocation: () => ({ pathname: '/dashboard/pipeline', search: '' }),
4646
}));
4747

48-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
48+
vi.mock('./useOpenRecordList', () => ({
49+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
50+
}));
4951
vi.mock('./MetadataInspector', () => ({
5052
MetadataPanel: () => null,
5153
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.modalTarget.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ vi.mock('react-router-dom', () => ({
7070
useLocation: () => ({ pathname: '/dashboards/sales_overview', search: '' }),
7171
}));
7272

73-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
73+
vi.mock('./useOpenRecordList', () => ({
74+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
75+
}));
7476
vi.mock('./MetadataInspector', () => ({
7577
MetadataPanel: () => null,
7678
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.pageHeader-12526.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ vi.mock('react-router-dom', () => ({
4242
useLocation: () => ({ pathname: '/apps/crm/dashboard/ops_board', search: '' }),
4343
}));
4444

45-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
45+
vi.mock('./useOpenRecordList', () => ({
46+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
47+
}));
4648
vi.mock('./MetadataInspector', () => ({
4749
MetadataPanel: () => null,
4850
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.phoneTouchTarget-29078.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ vi.mock('react-router-dom', () => ({
2929
useNavigate: () => vi.fn(),
3030
useLocation: () => ({ pathname: '/apps/crm/dashboard/ops_board', search: '' }),
3131
}));
32-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
32+
vi.mock('./useOpenRecordList', () => ({
33+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
34+
}));
3335
vi.mock('./MetadataInspector', () => ({
3436
MetadataPanel: () => null,
3537
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.refreshInHeader-12554.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ vi.mock('react-router-dom', () => ({
4747
useLocation: () => ({ pathname: '/apps/crm/dashboard/ops_board', search: '' }),
4848
}));
4949

50-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
50+
vi.mock('./useOpenRecordList', () => ({
51+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
52+
}));
5153
vi.mock('./MetadataInspector', () => ({
5254
MetadataPanel: () => null,
5355
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.refusal-12112.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ vi.mock('react-router-dom', () => ({
4343
useLocation: () => ({ pathname: `/dashboards/${route.dashboardName}`, search: '' }),
4444
}));
4545

46-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
46+
vi.mock('./useOpenRecordList', () => ({
47+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
48+
}));
4749
vi.mock('./MetadataInspector', () => ({
4850
MetadataPanel: () => null,
4951
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.rootTitleRetired.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ vi.mock('react-router-dom', () => ({
5959
useLocation: () => ({ pathname: '/dashboards/sales_overview', search: '' }),
6060
}));
6161

62-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
62+
vi.mock('./useOpenRecordList', () => ({
63+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
64+
}));
6365
vi.mock('./MetadataInspector', () => ({
6466
MetadataPanel: () => null,
6567
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.servedLocalized-11295.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ vi.mock('react-router-dom', () => ({
4848
useLocation: () => ({ pathname: '/dashboards/system_overview', search: '' }),
4949
}));
5050

51-
vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() }));
51+
vi.mock('./useOpenRecordList', () => ({
52+
useRecordListDrillNavigation: () => ({ openRecordList: vi.fn(), canOpenRecordList: () => true }),
53+
}));
5254
vi.mock('./MetadataInspector', () => ({
5355
MetadataPanel: () => null,
5456
useMetadataInspector: () => ({ showDebug: false }),

‎ui/packages/app-shell/src/views/DashboardView.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { useState, useEffect, useMemo, type ComponentProps } from 'react';
1313
import { useParams } from 'react-router-dom';
1414
import { DashboardRenderer } from '@object-ui/plugin-dashboard';
1515
import { DrillNavigationProvider, notifyDataChanged } from '@object-ui/react';
16-
import { useOpenRecordList } from './useOpenRecordList.js';
16+
import { useRecordListDrillNavigation } from './useOpenRecordList.js';
1717
import { toast } from 'sonner';
1818
import type { ActionDef, ActionContext, ActionResult } from '@object-ui/core';
1919
import {
@@ -97,7 +97,7 @@ type DashboardRefreshState = Parameters<
9797

9898
export function DashboardView({ dataSource }: { dataSource?: any }) {
9999
// Drill "escape hatch": lets the drill drawers open an object's full list page.
100-
const openRecordList = useOpenRecordList();
100+
const drillNavigation = useRecordListDrillNavigation();
101101
const { dashboardName } = useParams<{ dashboardName: string }>();
102102
const { showDebug } = useMetadataInspector();
103103
const adapter = useAdapter();
@@ -380,7 +380,7 @@ export function DashboardView({ dataSource }: { dataSource?: any }) {
380380
{/* ── Main area ────────────────────────────────────────────── */}
381381
<div className="flex-1 overflow-hidden flex flex-col sm:flex-row relative">
382382
<div className="flex-1 min-w-0 overflow-auto p-2 sm:p-4 md:p-6">
383-
<DrillNavigationProvider value={{ openRecordList }}>
383+
<DrillNavigationProvider value={drillNavigation}>
384384
{/* `localized`: this document is the `/meta` read's, already
385385
translated for this language, so the renderer draws its
386386
title, description and sub-caption as served (objectui#11295). */}

0 commit comments

Comments
 (0)