Skip to content

Commit fedefab

Browse files
hotlonghotlong
authored
Merge pull request #995 from objectstack-ai/copilot/fix-action-button-click-issue
fix: wire complete action execution chain for record detail page buttons
2 parents f65b757 + 24df5e5 commit fedefab

7 files changed

Lines changed: 655 additions & 14 deletions

File tree

‎ROADMAP.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,31 @@ All 313 `@object-ui/fields` tests pass.
13981398

13991399
---
14001400

1401+
### RecordDetailView — Action Button Full-Chain Integration (March 2026)
1402+
1403+
> **Issue #107:** All Action buttons on record detail pages (Change Stage, Mark as Won, etc.) clicked with zero response — no dialogs, no API calls, no toast, no data refresh.
1404+
1405+
**Root Causes (6 independent bugs):**
1406+
1407+
1. **Missing `ActionProvider`** — `RecordDetailView` didn't wrap `DetailView` with `ActionProvider`, so `useAction()` fell back to an empty `ActionRunner` with no handlers.
1408+
2. **Action type overwritten** — `action:bar` component overrode `action.type` (`'api'`) with the component type (`'action:button'`), so `ActionRunner` never matched the registered `'api'` handler.
1409+
3. **No API handler** — `api` action targets were logical names (e.g., `'opportunity_change_stage'`), not HTTP URLs. The built-in `executeAPI()` tried `fetch('opportunity_change_stage')` which failed silently.
1410+
4. **No param collection** — `ActionParam[]` was passed as `params` (values) instead of `actionParams` (definitions to collect), so the param collection dialog was never triggered.
1411+
5. **No confirm/toast handlers** — `confirmText` fell back to `window.confirm`, success/error messages were silently dropped.
1412+
6. **No visibility context** — `useCondition` evaluated `visible` expressions like `"stage !== 'closed_won'"` with empty context, always returning `true`.
1413+
1414+
**Fix:**
1415+
1416+
- **RecordDetailView** (`apps/console/src/components/RecordDetailView.tsx`): Wrapped `DetailView` with `<ActionProvider>` providing `onConfirm`, `onToast`, `onNavigate`, `onParamCollection` handlers and a custom `api` handler that maps logical action targets to `dataSource.update()` operations.
1417+
- **action-bar** (`packages/components/src/renderers/action/action-bar.tsx`): Preserves original `action.type` as `actionType` when overriding with component type. Forwards `data` prop to child action renderers for visibility context.
1418+
- **action-button** (`packages/components/src/renderers/action/action-button.tsx`): Uses `actionType` for execution. Detects `ActionParam[]` arrays and passes as `actionParams`. Passes record `data` to `useCondition` for visibility expressions.
1419+
- **ActionConfirmDialog** (`apps/console/src/components/ActionConfirmDialog.tsx`): Promise-based confirmation dialog using Shadcn `AlertDialog`.
1420+
- **ActionParamDialog** (`apps/console/src/components/ActionParamDialog.tsx`): Dynamic form dialog for collecting action parameters (select, text, textarea) using Shadcn `Dialog`.
1421+
1422+
**Tests:** 6 new integration tests covering: action button rendering, confirm dialog show/accept/cancel, param collection dialog, toast notification, dataSource.update invocation. All 764 console tests pass.
1423+
1424+
---
1425+
14011426
## ⚠️ Risk Management
14021427

14031428
| Risk | Mitigation |

‎apps/console/src/__tests__/RecordDetailActions.test.tsx‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
/**
2+
* RecordDetailView — Action button integration tests
3+
*
4+
* Validates that action buttons in the detail page header are wired to
5+
* ActionProvider and that the full action chain works:
6+
* - API actions use dataSource.update() via the registered handler
7+
* - Actions with params show a param collection dialog
8+
* - Actions with confirmText show a confirmation dialog
9+
* - Toast notifications are displayed on success/error
10+
* - Data refreshes after successful action execution
11+
*/
12+
13+
import { describe, it, expect, vi, beforeEach } from 'vitest';
14+
import { render, screen, waitFor } from '@testing-library/react';
15+
import userEvent from '@testing-library/user-event';
16+
import '@testing-library/jest-dom';
17+
import { MemoryRouter, Routes, Route } from 'react-router-dom';
18+
import { RecordDetailView } from '../components/RecordDetailView';
19+
20+
// Mock sonner toast
21+
const mockToastSuccess = vi.fn();
22+
const mockToastError = vi.fn();
23+
vi.mock('sonner', () => ({
24+
toast: Object.assign(vi.fn(), {
25+
success: (...args: any[]) => mockToastSuccess(...args),
26+
error: (...args: any[]) => mockToastError(...args),
27+
}),
28+
}));
29+
30+
// ─── Test Data ───────────────────────────────────────────────────────────────
31+
32+
function createMockDataSource() {
33+
return {
34+
async getObjectSchema() {
35+
return {
36+
name: 'opportunity',
37+
label: 'Opportunity',
38+
fields: {
39+
name: { name: 'name', label: 'Name', type: 'text' },
40+
stage: { name: 'stage', label: 'Stage', type: 'select' },
41+
amount: { name: 'amount', label: 'Amount', type: 'currency' },
42+
},
43+
};
44+
},
45+
findOne: vi.fn().mockResolvedValue({
46+
id: 'opp-1',
47+
name: 'Acme Deal',
48+
stage: 'proposal',
49+
amount: 50000,
50+
}),
51+
find: vi.fn().mockResolvedValue({ data: [] }),
52+
create: vi.fn().mockResolvedValue({ id: '1' }),
53+
update: vi.fn().mockResolvedValue({ id: 'opp-1' }),
54+
delete: vi.fn().mockResolvedValue(true),
55+
} as any;
56+
}
57+
58+
const opportunityActions = [
59+
{
60+
name: 'opportunity_change_stage',
61+
label: 'Change Stage',
62+
icon: 'arrow-right-circle',
63+
type: 'api' as const,
64+
target: 'opportunity_change_stage',
65+
locations: ['record_header' as const],
66+
params: [
67+
{
68+
name: 'new_stage', label: 'New Stage', type: 'select' as const, required: true,
69+
options: [
70+
{ label: 'Prospecting', value: 'prospecting' },
71+
{ label: 'Qualification', value: 'qualification' },
72+
{ label: 'Proposal', value: 'proposal' },
73+
{ label: 'Negotiation', value: 'negotiation' },
74+
{ label: 'Closed Won', value: 'closed_won' },
75+
{ label: 'Closed Lost', value: 'closed_lost' },
76+
],
77+
},
78+
],
79+
refreshAfter: true,
80+
successMessage: 'Stage updated successfully',
81+
},
82+
{
83+
name: 'opportunity_mark_won',
84+
label: 'Mark as Won',
85+
icon: 'trophy',
86+
type: 'api' as const,
87+
target: 'opportunity_mark_won',
88+
locations: ['record_header' as const],
89+
variant: 'primary' as const,
90+
confirmText: 'Mark this opportunity as Closed Won?',
91+
refreshAfter: true,
92+
successMessage: 'Opportunity marked as won!',
93+
},
94+
];
95+
96+
const mockObjectsWithActions = [
97+
{
98+
name: 'opportunity',
99+
label: 'Opportunity',
100+
fields: {
101+
name: { name: 'name', label: 'Name', type: 'text' },
102+
stage: { name: 'stage', label: 'Stage', type: 'select' },
103+
amount: { name: 'amount', label: 'Amount', type: 'currency' },
104+
},
105+
actions: opportunityActions,
106+
},
107+
];
108+
109+
function renderDetailWithActions(ds?: any) {
110+
const dataSource = ds ?? createMockDataSource();
111+
const onEdit = vi.fn();
112+
return {
113+
dataSource,
114+
onEdit,
115+
...render(
116+
<MemoryRouter initialEntries={['/opportunity/record/opp-1']}>
117+
<Routes>
118+
<Route
119+
path="/:objectName/record/:recordId"
120+
element={
121+
<RecordDetailView
122+
dataSource={dataSource}
123+
objects={mockObjectsWithActions}
124+
onEdit={onEdit}
125+
/>
126+
}
127+
/>
128+
</Routes>
129+
</MemoryRouter>,
130+
),
131+
};
132+
}
133+
134+
// ─── Tests ───────────────────────────────────────────────────────────────────
135+
136+
describe('RecordDetailView — Action buttons', () => {
137+
beforeEach(() => {
138+
vi.clearAllMocks();
139+
});
140+
141+
it('renders action buttons in the detail header', async () => {
142+
renderDetailWithActions();
143+
144+
// Wait for record to load
145+
await waitFor(() => {
146+
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Acme Deal');
147+
});
148+
149+
// Action buttons should be visible
150+
expect(screen.getByRole('button', { name: /Change Stage/i })).toBeInTheDocument();
151+
expect(screen.getByRole('button', { name: /Mark as Won/i })).toBeInTheDocument();
152+
});
153+
154+
it('shows confirmation dialog for Mark as Won action', async () => {
155+
const user = userEvent.setup();
156+
renderDetailWithActions();
157+
158+
await waitFor(() => {
159+
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Acme Deal');
160+
});
161+
162+
// Click Mark as Won
163+
await user.click(screen.getByRole('button', { name: /Mark as Won/i }));
164+
165+
// Confirmation dialog should appear
166+
await waitFor(() => {
167+
expect(screen.getByText('Mark this opportunity as Closed Won?')).toBeInTheDocument();
168+
});
169+
});
170+
171+
it('calls dataSource.update when confirmation is accepted', async () => {
172+
const user = userEvent.setup();
173+
const ds = createMockDataSource();
174+
renderDetailWithActions(ds);
175+
176+
await waitFor(() => {
177+
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Acme Deal');
178+
});
179+
180+
// Click Mark as Won
181+
await user.click(screen.getByRole('button', { name: /Mark as Won/i }));
182+
183+
// Wait for confirmation dialog
184+
await waitFor(() => {
185+
expect(screen.getByText('Mark this opportunity as Closed Won?')).toBeInTheDocument();
186+
});
187+
188+
// Click Continue (confirm)
189+
await user.click(screen.getByRole('button', { name: /Continue/i }));
190+
191+
// dataSource.update should be called with the correct params
192+
await waitFor(() => {
193+
expect(ds.update).toHaveBeenCalledWith('opportunity', 'opp-1', { stage: 'closed_won' });
194+
}, { timeout: 3000 });
195+
});
196+
197+
it('does not call dataSource.update when confirmation is cancelled', async () => {
198+
const user = userEvent.setup();
199+
const ds = createMockDataSource();
200+
renderDetailWithActions(ds);
201+
202+
await waitFor(() => {
203+
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Acme Deal');
204+
});
205+
206+
// Click Mark as Won
207+
await user.click(screen.getByRole('button', { name: /Mark as Won/i }));
208+
209+
// Wait for confirmation dialog
210+
await waitFor(() => {
211+
expect(screen.getByText('Mark this opportunity as Closed Won?')).toBeInTheDocument();
212+
});
213+
214+
// Click Cancel
215+
await user.click(screen.getByRole('button', { name: /Cancel/i }));
216+
217+
// dataSource.update should NOT be called
218+
// (wait a tick to ensure no async calls happen)
219+
await new Promise(r => setTimeout(r, 100));
220+
expect(ds.update).not.toHaveBeenCalled();
221+
});
222+
223+
it('shows param collection dialog for Change Stage action', async () => {
224+
const user = userEvent.setup();
225+
renderDetailWithActions();
226+
227+
await waitFor(() => {
228+
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Acme Deal');
229+
});
230+
231+
// Click Change Stage
232+
await user.click(screen.getByRole('button', { name: /Change Stage/i }));
233+
234+
// Param dialog should appear with the select field
235+
await waitFor(() => {
236+
expect(screen.getByText('Action Parameters')).toBeInTheDocument();
237+
expect(screen.getByText('New Stage')).toBeInTheDocument();
238+
});
239+
});
240+
241+
it('shows toast on successful action execution', async () => {
242+
const user = userEvent.setup();
243+
const ds = createMockDataSource();
244+
renderDetailWithActions(ds);
245+
246+
await waitFor(() => {
247+
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Acme Deal');
248+
});
249+
250+
// Click Mark as Won
251+
await user.click(screen.getByRole('button', { name: /Mark as Won/i }));
252+
253+
// Confirm
254+
await waitFor(() => {
255+
expect(screen.getByText('Mark this opportunity as Closed Won?')).toBeInTheDocument();
256+
});
257+
await user.click(screen.getByRole('button', { name: /Continue/i }));
258+
259+
// Toast should be called with success message
260+
await waitFor(() => {
261+
expect(mockToastSuccess).toHaveBeenCalledWith('Opportunity marked as won!');
262+
});
263+
});
264+
});

‎apps/console/src/components/ActionConfirmDialog.tsx‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* ActionConfirmDialog — Promise-based confirmation dialog for action execution.
3+
*
4+
* Uses Shadcn AlertDialog to replace window.confirm with a styled, accessible
5+
* confirmation dialog. Renders only when state.open is true.
6+
*/
7+
8+
import {
9+
AlertDialog,
10+
AlertDialogAction,
11+
AlertDialogCancel,
12+
AlertDialogContent,
13+
AlertDialogDescription,
14+
AlertDialogFooter,
15+
AlertDialogHeader,
16+
AlertDialogTitle,
17+
} from '@object-ui/components';
18+
19+
export interface ConfirmDialogState {
20+
open: boolean;
21+
message: string;
22+
options?: { title?: string; confirmText?: string; cancelText?: string };
23+
resolve?: (value: boolean) => void;
24+
}
25+
26+
interface ActionConfirmDialogProps {
27+
state: ConfirmDialogState;
28+
onOpenChange: (open: boolean) => void;
29+
}
30+
31+
export function ActionConfirmDialog({ state, onOpenChange }: ActionConfirmDialogProps) {
32+
const handleConfirm = () => {
33+
state.resolve?.(true);
34+
onOpenChange(false);
35+
};
36+
37+
const handleCancel = () => {
38+
state.resolve?.(false);
39+
onOpenChange(false);
40+
};
41+
42+
return (
43+
<AlertDialog open={state.open} onOpenChange={(open) => {
44+
if (!open) handleCancel();
45+
}}>
46+
<AlertDialogContent>
47+
<AlertDialogHeader>
48+
<AlertDialogTitle>{state.options?.title || 'Confirm Action'}</AlertDialogTitle>
49+
<AlertDialogDescription>{state.message}</AlertDialogDescription>
50+
</AlertDialogHeader>
51+
<AlertDialogFooter>
52+
<AlertDialogCancel onClick={handleCancel}>
53+
{state.options?.cancelText || 'Cancel'}
54+
</AlertDialogCancel>
55+
<AlertDialogAction onClick={handleConfirm}>
56+
{state.options?.confirmText || 'Continue'}
57+
</AlertDialogAction>
58+
</AlertDialogFooter>
59+
</AlertDialogContent>
60+
</AlertDialog>
61+
);
62+
}

0 commit comments

Comments
 (0)