Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions app/src/components/skills/MeetingBotsCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
type MascotMeetPlatform,
type MeetCallRecord,
} from '../../services/meetCallService';
import { useAppSelector } from '../../store/hooks';
import { selectPersonaDisplayName } from '../../store/personaSlice';

type Toast = { type: 'success' | 'error' | 'info'; title: string; message?: string };

Expand Down Expand Up @@ -131,7 +133,17 @@ export function MeetingBotsModal({ onClose, onToast }: ModalProps) {
// remote participant from issuing tool calls in the owner's
// name. Empty fails closed; the submit handler will surface an
// explicit error before opening the CEF window.
const [ownerDisplayName, setOwnerDisplayName] = useState('');
//
// Effective value = Persona display name (Settings → Persona) until
// the user types into the field — the "name prompt" UX complaint in
// #2945, so repeat callers don't retype the same value every meeting.
// Once the user edits the field, the dirty flag latches and the
// input becomes fully controlled — Persona changes no longer
// overwrite their input, and clearing the field stays empty.
const personaDisplayName = useAppSelector(selectPersonaDisplayName);
const [ownerDisplayNameDraft, setOwnerDisplayNameDraft] = useState('');
const [isOwnerNameEdited, setIsOwnerNameEdited] = useState(false);
const ownerDisplayName = isOwnerNameEdited ? ownerDisplayNameDraft : personaDisplayName;
const [submitting, setSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
// Recent-calls history loaded from core when the modal opens.
Expand Down Expand Up @@ -305,7 +317,10 @@ export function MeetingBotsModal({ onClose, onToast }: ModalProps) {
<input
type="text"
value={ownerDisplayName}
onChange={e => setOwnerDisplayName(e.target.value)}
onChange={e => {
setOwnerDisplayNameDraft(e.target.value);
setIsOwnerNameEdited(true);
}}
maxLength={64}
placeholder="As shown in Google Meet (e.g. Nikhil Bajaj)"
disabled={isComingSoon || submitting}
Expand Down
91 changes: 68 additions & 23 deletions app/src/components/skills/__tests__/MeetingBotsCard.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { cleanup, fireEvent, screen, waitFor } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import type { MeetCallRecord } from '../../../services/meetCallService';
import { renderWithProviders } from '../../../test/test-utils';
import MeetingBotsCard, { MeetingBotsModal } from '../MeetingBotsCard';

const joinMock = vi.fn();
Expand Down Expand Up @@ -32,26 +33,26 @@ describe('MeetingBotsCard', () => {
afterEach(() => cleanup());

it('renders the banner and hides the modal by default', () => {
render(<MeetingBotsCard />);
renderWithProviders(<MeetingBotsCard />);
expect(screen.getByTestId('meeting-bots-banner')).toBeInTheDocument();
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});

it('opens the modal when the banner is clicked', () => {
render(<MeetingBotsCard />);
renderWithProviders(<MeetingBotsCard />);
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
expect(screen.getByRole('dialog')).toBeInTheDocument();
});

it('closes the modal on Cancel', () => {
render(<MeetingBotsCard />);
renderWithProviders(<MeetingBotsCard />);
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});

it('closes the modal on Escape', () => {
render(<MeetingBotsCard />);
renderWithProviders(<MeetingBotsCard />);
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
fireEvent.keyDown(window, { key: 'Escape' });
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
Expand All @@ -60,7 +61,7 @@ describe('MeetingBotsCard', () => {
it('submits to joinMeetCall and fires a success toast', async () => {
joinMock.mockResolvedValueOnce({ requestId: 'req-1' });
const onToast = vi.fn();
render(<MeetingBotsCard onToast={onToast} />);
renderWithProviders(<MeetingBotsCard onToast={onToast} />);

fireEvent.click(screen.getByTestId('meeting-bots-banner'));
fireEvent.change(screen.getByLabelText(/meeting link/i), {
Expand Down Expand Up @@ -105,7 +106,7 @@ describe('MeetingBotsCard', () => {
it('surfaces a join error inline + as an error toast', async () => {
joinMock.mockRejectedValueOnce(new Error('Bad URL'));
const onToast = vi.fn();
render(<MeetingBotsCard onToast={onToast} />);
renderWithProviders(<MeetingBotsCard onToast={onToast} />);

fireEvent.click(screen.getByTestId('meeting-bots-banner'));
fireEvent.change(screen.getByLabelText(/meeting link/i), {
Expand All @@ -125,13 +126,57 @@ describe('MeetingBotsCard', () => {
});

it('disables the submit when the active platform is coming-soon', () => {
render(<MeetingBotsCard />);
renderWithProviders(<MeetingBotsCard />);
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
// Pick Zoom (coming soon)
fireEvent.click(screen.getByRole('button', { name: /Zoom/ }));
const submit = screen.getByRole('button', { name: /coming soon/i });
expect(submit).toBeDisabled();
});

// Issue #2945: users were being asked to retype "your name in the call"
// every meeting. When the Persona display name (Settings → Persona) is
// set, the owner-name field pre-fills from it so the user can submit
// without retyping.
it('pre-fills the owner display name from the Persona slice', () => {
const { store } = renderWithProviders(<MeetingBotsCard />, {
preloadedState: { persona: { displayName: 'Hemanth', description: '' } },
});
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
const ownerInput = screen.getByLabelText(/your name in the call/i) as HTMLInputElement;
expect(ownerInput.value).toBe('Hemanth');
// Sanity: the slice is wired so the assertion above isn't a no-op.
expect(store.getState().persona.displayName).toBe('Hemanth');
});

it('leaves the owner display name empty when no Persona name is set', () => {
// Default preloadedState — persona slice initial state is
// { displayName: '', description: '' } (see personaSlice.ts).
renderWithProviders(<MeetingBotsCard />);
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
const ownerInput = screen.getByLabelText(/your name in the call/i) as HTMLInputElement;
expect(ownerInput.value).toBe('');
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Dirty-flag contract: once the user has typed into the field, a
// subsequent Persona update from the slice must NOT overwrite their
// input. The pre-edit case is covered by the "pre-fills" test above.
it('does not overwrite user-typed owner name when Persona slice updates later', () => {
const { store } = renderWithProviders(<MeetingBotsCard />, {
preloadedState: { persona: { displayName: 'Hemanth', description: '' } },
});
fireEvent.click(screen.getByTestId('meeting-bots-banner'));
const ownerInput = screen.getByLabelText(/your name in the call/i) as HTMLInputElement;
// Sanity: pre-fill landed.
expect(ownerInput.value).toBe('Hemanth');
// User types over it.
fireEvent.change(ownerInput, { target: { value: 'Alice' } });
expect(ownerInput.value).toBe('Alice');
// Persona name changes underneath (e.g. user edits Settings in another window).
store.dispatch({ type: 'persona/setPersonaDisplayName', payload: 'Nova' });
// User's typed value wins — does NOT flip to 'Nova'.
expect(ownerInput.value).toBe('Alice');
});
});

// ── RecentCallsSection / RecentCallRow tests ──────────────────────────────────
Expand Down Expand Up @@ -160,15 +205,15 @@ describe('MeetingBotsModal — recent calls section', () => {
// Never resolves during this test — simulates a slow fetch.
listMock.mockReturnValue(new Promise(() => {}));

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

expect(screen.getByText(/loading…/i)).toBeInTheDocument();
});

it('shows an empty-state message when listMeetCalls returns an empty array', async () => {
listMock.mockResolvedValueOnce([]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/no previous calls yet/i)).toBeInTheDocument();
Expand All @@ -182,7 +227,7 @@ describe('MeetingBotsModal — recent calls section', () => {
];
listMock.mockResolvedValueOnce(records);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText('aaa-bbbb-ccc')).toBeInTheDocument();
Expand All @@ -196,7 +241,7 @@ describe('MeetingBotsModal — recent calls section', () => {
it('shows the count badge when there is at least one record', async () => {
listMock.mockResolvedValueOnce([makeCallRecord()]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
// The "(1)" count badge next to the "Recent calls" heading.
Expand All @@ -207,7 +252,7 @@ describe('MeetingBotsModal — recent calls section', () => {
it('shows an error hint and an empty list when listMeetCalls rejects', async () => {
listMock.mockRejectedValueOnce(new Error('Network timeout'));

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/network timeout/i)).toBeInTheDocument();
Expand All @@ -221,7 +266,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ meet_url: 'https://meet.google.com/xyz-1234-abc' }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText('xyz-1234-abc')).toBeInTheDocument();
Expand All @@ -235,7 +280,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ spoken_seconds: 40, listened_seconds: 20 }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/60s on call/i)).toBeInTheDocument();
Expand All @@ -248,7 +293,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ started_at_ms: Date.now() - 5 * 60 * 1000 }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/\dm ago/)).toBeInTheDocument();
Expand All @@ -258,7 +303,7 @@ describe('MeetingBotsModal — recent calls section', () => {
it('shows "—" for a zero started_at_ms timestamp', async () => {
listMock.mockResolvedValueOnce([makeCallRecord({ started_at_ms: 0 })]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText('—')).toBeInTheDocument();
Expand All @@ -270,7 +315,7 @@ describe('MeetingBotsModal — recent calls section', () => {
it('shows singular "turn" (not "turns") when turn_count is 1', async () => {
listMock.mockResolvedValueOnce([makeCallRecord({ turn_count: 1 })]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/1 turn$/)).toBeInTheDocument();
Expand All @@ -281,7 +326,7 @@ describe('MeetingBotsModal — recent calls section', () => {
it('falls back to the raw URL when it cannot be parsed', async () => {
listMock.mockResolvedValueOnce([makeCallRecord({ meet_url: 'not-a-valid-url' })]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText('not-a-valid-url')).toBeInTheDocument();
Expand All @@ -293,7 +338,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ started_at_ms: Date.now() - 3 * 60 * 60 * 1000 }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/3h ago/)).toBeInTheDocument();
Expand All @@ -305,7 +350,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ started_at_ms: Date.now() - 25 * 60 * 60 * 1000 }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText('yesterday')).toBeInTheDocument();
Expand All @@ -317,7 +362,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ started_at_ms: Date.now() - 3 * 24 * 60 * 60 * 1000 }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
expect(screen.getByText(/3d ago/)).toBeInTheDocument();
Expand All @@ -329,7 +374,7 @@ describe('MeetingBotsModal — recent calls section', () => {
makeCallRecord({ started_at_ms: Date.now() - 10 * 24 * 60 * 60 * 1000 }),
]);

render(<MeetingBotsModal onClose={() => {}} />);
renderWithProviders(<MeetingBotsModal onClose={() => {}} />);

await waitFor(() => {
// toLocaleDateString returns "Month Day" — just check it's not a relative label.
Expand Down
Loading