-
Notifications
You must be signed in to change notification settings - Fork 0
test(inbox): regression guards + security — ChatPanel P0 & Supabase RLS #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| /** | ||
| * Tests for useChatFilters — referential identity (memoization). | ||
| * | ||
| * Guard against regressions where derived arrays lose referential stability, | ||
| * causing unnecessary re-renders in virtualised message lists. Each useMemo | ||
| * derivation must return the same array reference when its inputs haven't changed. | ||
| */ | ||
|
|
||
| import { describe, it, expect } from 'vitest'; | ||
| import { renderHook, act } from '@testing-library/react'; | ||
| import { MemoryRouter } from 'react-router-dom'; | ||
| import React from 'react'; | ||
| import { useChatFilters } from '../hooks/useChatFilters'; | ||
| import type { Message } from '@/types/chat'; | ||
|
|
||
| // ── Helpers ─────────────────────────────────────────────────────────────────── | ||
|
|
||
| function makeMessage(overrides: Partial<Message> = {}): Message { | ||
| return { | ||
| id: 'msg-1', | ||
| content: 'hello', | ||
| status: 'sent', | ||
| timestamp: new Date().toISOString(), | ||
| type: 'text', | ||
| fromMe: true, | ||
| conversationId: 'conv-1', | ||
| ...overrides, | ||
| } as Message; | ||
| } | ||
|
|
||
| function makeWrapper() { | ||
| const Wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => | ||
| React.createElement(MemoryRouter, null, children); | ||
| return Wrapper; | ||
| } | ||
|
|
||
| const SENT = makeMessage({ id: 'ok-1', status: 'sent' }); | ||
| const FAILED = makeMessage({ id: 'fail-1', status: 'failed' }); | ||
| const FAILED_AUTH = makeMessage({ id: 'fail-2', status: 'failed_auth' }); | ||
| const FAILED_RETRIES = makeMessage({ id: 'fail-3', status: 'failed_retries' }); | ||
|
|
||
| const ALL_MESSAGES = [SENT, FAILED, FAILED_AUTH, FAILED_RETRIES]; | ||
|
|
||
| // ── Tests ───────────────────────────────────────────────────────────────────── | ||
|
|
||
| describe('useChatFilters — referential identity', () => { | ||
| it('failedMessages reference is stable across renders with same input', () => { | ||
| const { result, rerender } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| const ref1 = result.current.failedMessages; | ||
| rerender(); | ||
| const ref2 = result.current.failedMessages; | ||
|
|
||
| expect(ref1).toBe(ref2); | ||
| }); | ||
|
|
||
| it('visibleMessages reference is stable across renders with same input', () => { | ||
| const { result, rerender } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| const ref1 = result.current.visibleMessages; | ||
| rerender(); | ||
| const ref2 = result.current.visibleMessages; | ||
|
|
||
| expect(ref1).toBe(ref2); | ||
| }); | ||
|
|
||
| it('categoryCounts reference is stable across renders with same input', () => { | ||
| const { result, rerender } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| const ref1 = result.current.categoryCounts; | ||
| rerender(); | ||
| const ref2 = result.current.categoryCounts; | ||
|
|
||
| expect(ref1).toBe(ref2); | ||
| }); | ||
|
|
||
| it('categoryFilteredMessages reference is stable across renders with same input', () => { | ||
| const { result, rerender } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| const ref1 = result.current.categoryFilteredMessages; | ||
| rerender(); | ||
| const ref2 = result.current.categoryFilteredMessages; | ||
|
|
||
| expect(ref1).toBe(ref2); | ||
| }); | ||
| }); | ||
|
|
||
| describe('useChatFilters — correctness', () => { | ||
| it('failedMessages contains only failed-status messages', () => { | ||
| const { result } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| expect(result.current.failedMessages).toHaveLength(3); | ||
| expect(result.current.failedMessages.map((m) => m.id)).toEqual(['fail-1', 'fail-2', 'fail-3']); | ||
| }); | ||
|
|
||
| it('categoryCounts matches actual failure distribution', () => { | ||
| const { result } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| expect(result.current.categoryCounts).toEqual({ | ||
| failed: 1, | ||
| failed_auth: 1, | ||
| failed_retries: 1, | ||
| }); | ||
| }); | ||
|
|
||
| it('visibleMessages equals all messages when failuresOnly is false', () => { | ||
| const { result } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| expect(result.current.failuresOnly).toBe(false); | ||
| expect(result.current.visibleMessages).toHaveLength(ALL_MESSAGES.length); | ||
| }); | ||
|
|
||
| it('setFailuresOnly callback is stable across renders', () => { | ||
| const { result, rerender } = renderHook(() => useChatFilters(ALL_MESSAGES), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| const cb1 = result.current.setFailuresOnly; | ||
| rerender(); | ||
| expect(result.current.setFailuresOnly).toBe(cb1); | ||
| }); | ||
|
|
||
| it('failedMessages updates when new failed message added', () => { | ||
| let messages = [SENT, FAILED]; | ||
| const { result, rerender } = renderHook(() => useChatFilters(messages), { | ||
| wrapper: makeWrapper(), | ||
| }); | ||
|
|
||
| expect(result.current.failedMessages).toHaveLength(1); | ||
|
|
||
| act(() => { | ||
| messages = [...messages, FAILED_AUTH]; | ||
| }); | ||
| rerender(); | ||
|
|
||
| expect(result.current.failedMessages).toHaveLength(2); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| /** | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This patch adds only client-side tests even though the commit states that the scoped AGENTS.md reference: AGENTS.md:L24-L28 Useful? React with 👍 / 👎. |
||
| * Regression tests for the edit-message flow in useChatPanelHandlers. | ||
| * | ||
| * Critical invariant: editMessageApi MUST be awaited before any success toast. | ||
| * If the API call fails, no success toast should appear (no false-success). | ||
| * If preconditions are missing (no JID, no externalId, no instance), an error | ||
| * toast must appear instead of a success toast. | ||
| */ | ||
|
|
||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { renderHook, act } from '@testing-library/react'; | ||
| import { useChatPanelHandlers } from '../useChatPanelHandlers'; | ||
| import type { Message } from '@/types/chat'; | ||
|
|
||
| // ── Mocks ───────────────────────────────────────────────────────────────────── | ||
|
|
||
| const mockToast = vi.fn(); | ||
| vi.mock('@/hooks/use-toast', () => ({ toast: (p: unknown) => mockToast(p) })); | ||
|
|
||
| const mockDbUpdate = vi.fn(); | ||
| const mockDbEq = vi.fn(() => ({ | ||
| eq: vi.fn(() => ({ | ||
| select: vi.fn(() => Promise.resolve({ data: [{ id: 'msg-1' }], error: null })), | ||
| })), | ||
| })); | ||
| const mockDbFrom = vi.fn(() => ({ update: mockDbUpdate })); | ||
| mockDbUpdate.mockReturnValue({ | ||
| eq: vi.fn(() => ({ | ||
| select: vi.fn(() => Promise.resolve({ data: [{ id: 'msg-1' }], error: null })), | ||
| })), | ||
| }); | ||
|
|
||
| vi.mock('@/integrations/datasource/db', () => ({ | ||
| dbFrom: (...args: unknown[]) => mockDbFrom(...args), | ||
| })); | ||
| vi.mock('@/features/auth', () => ({ useAuth: () => ({ user: { id: 'user-1' } }) })); | ||
| vi.mock('@/lib/logger', () => ({ | ||
| getLogger: () => ({ warn: vi.fn(), error: vi.fn(), info: vi.fn() }), | ||
| })); | ||
| vi.mock('@/lib/undoToast', () => ({ undoToast: vi.fn() })); | ||
| vi.mock('../../hooks/useWhisperMessagesMutation', () => ({ insertWhisperMessage: vi.fn() })); | ||
| vi.mock('../useInputHandlers', async () => { | ||
| const { useState } = await import('react'); | ||
| return { | ||
| useInputHandlers: () => { | ||
| const [inputValue, setInputValue] = useState(''); | ||
| return { | ||
| inputValue, | ||
| setInputValue, | ||
| handleInputChange: vi.fn(), | ||
| applyTemplate: vi.fn(), | ||
| clearInput: vi.fn(), | ||
| }; | ||
| }, | ||
| }; | ||
| }); | ||
| vi.mock('../useProductHandlers', () => ({ | ||
| useProductHandlers: () => ({ handleSendProduct: vi.fn() }), | ||
| })); | ||
| vi.mock('../useAudioVoiceChange', () => ({ | ||
| useAudioVoiceChange: () => ({ handleAudioVoiceChange: vi.fn() }), | ||
| })); | ||
| vi.mock('../useMessageReactionHandlers', () => ({ | ||
| useMessageReactionHandlers: () => ({ handleReaction: vi.fn() }), | ||
| })); | ||
|
|
||
| // ── Test helpers ────────────────────────────────────────────────────────────── | ||
|
|
||
| const EDIT_JID = '5511999887766@s.whatsapp.net'; | ||
| const EDIT_MSG: Message = { | ||
| id: 'msg-1', | ||
| external_id: 'ext-abc123', | ||
| content: 'old text', | ||
| status: 'sent', | ||
| timestamp: new Date().toISOString(), | ||
| type: 'text', | ||
| fromMe: true, | ||
| conversationId: 'conv-1', | ||
| } as unknown as Message; | ||
|
|
||
| function makeHandlers(editMessageApi: ReturnType<typeof vi.fn>) { | ||
| return renderHook(() => | ||
| useChatPanelHandlers({ | ||
| conversationId: 'conv-1', | ||
| contactId: EDIT_JID, | ||
| contactPhone: '5511999887766', | ||
| instanceName: 'wpp2', | ||
| onSendMessage: vi.fn(), | ||
| editMessageApi, | ||
| applySignature: (t: string) => t, | ||
| handleTypingStart: vi.fn(), | ||
| handleTypingStop: vi.fn(), | ||
| openDialog: vi.fn(), | ||
| closeDialog: vi.fn(), | ||
| handleSetActiveTool: vi.fn(), | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| mockDbFrom.mockReturnValue({ | ||
| update: () => ({ | ||
| eq: () => ({ | ||
| select: () => Promise.resolve({ data: [{ id: 'msg-1' }], error: null }), | ||
| }), | ||
| }), | ||
| }); | ||
| }); | ||
|
|
||
| // ── Tests ───────────────────────────────────────────────────────────────────── | ||
|
|
||
| describe('edit message — API called before success toast', () => { | ||
| it('calls editMessageApi before showing success toast on happy path', async () => { | ||
| const callOrder: string[] = []; | ||
| const editMessageApi = vi.fn(async () => { | ||
| callOrder.push('api'); | ||
| }); | ||
|
Comment on lines
+116
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Useful? React with 👍 / 👎. |
||
| mockToast.mockImplementation(() => { | ||
| callOrder.push('toast'); | ||
| }); | ||
|
|
||
| const { result } = makeHandlers(editMessageApi); | ||
|
|
||
| act(() => { | ||
| result.current.handleEditStart(EDIT_MSG); | ||
| }); | ||
| act(() => { | ||
| result.current.setInputValue('new text'); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| await result.current.handleSend(); | ||
| }); | ||
|
|
||
| expect(editMessageApi).toHaveBeenCalledTimes(1); | ||
| const apiIdx = callOrder.indexOf('api'); | ||
| const toastIdx = callOrder.indexOf('toast'); | ||
| expect(apiIdx).toBeGreaterThanOrEqual(0); | ||
| expect(toastIdx).toBeGreaterThan(apiIdx); | ||
| }); | ||
|
|
||
| it('does NOT show success toast when editMessageApi throws', async () => { | ||
| const editMessageApi = vi.fn().mockRejectedValue(new Error('Network error')); | ||
|
|
||
| const { result } = makeHandlers(editMessageApi); | ||
|
|
||
| act(() => { | ||
| result.current.handleEditStart(EDIT_MSG); | ||
| }); | ||
| act(() => { | ||
| result.current.setInputValue('new text'); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| await result.current.handleSend(); | ||
| }); | ||
|
|
||
| const successToasts = mockToast.mock.calls.filter(([p]) => | ||
| typeof p === 'object' && p !== null && 'variant' in p | ||
| ? (p as { variant: string }).variant !== 'destructive' | ||
| : true | ||
| ); | ||
| // Only destructive toasts (errors) should appear when API throws | ||
| const destructiveToasts = mockToast.mock.calls.filter( | ||
| ([p]) => | ||
| typeof p === 'object' && p !== null && (p as { variant?: string }).variant === 'destructive' | ||
| ); | ||
| expect(successToasts.length).toBe(0); | ||
| expect(destructiveToasts.length).toBeGreaterThan(0); | ||
| }); | ||
| }); | ||
|
|
||
| describe('edit message — precondition guard', () => { | ||
| it('shows error toast and does not call API when message has no external_id', async () => { | ||
| const editMessageApi = vi.fn(); | ||
| const { result } = makeHandlers(editMessageApi); | ||
|
|
||
| const msgWithoutExternalId: Message = { | ||
| ...EDIT_MSG, | ||
| external_id: undefined, | ||
| } as unknown as Message; | ||
|
Comment on lines
+175
to
+182
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the Useful? React with 👍 / 👎. |
||
| act(() => { | ||
| result.current.handleEditStart(msgWithoutExternalId); | ||
| }); | ||
| act(() => { | ||
| result.current.setInputValue('new text'); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| await result.current.handleSend(); | ||
| }); | ||
|
|
||
| expect(editMessageApi).not.toHaveBeenCalled(); | ||
| expect(mockToast).toHaveBeenCalledWith(expect.objectContaining({ variant: 'destructive' })); | ||
| }); | ||
|
|
||
| it('calls editMessageApi with correct params when all preconditions are met', async () => { | ||
| const editMessageApi = vi.fn().mockResolvedValue(undefined); | ||
| const { result } = makeHandlers(editMessageApi); | ||
|
|
||
| act(() => { | ||
| result.current.handleEditStart(EDIT_MSG); | ||
| }); | ||
| act(() => { | ||
| result.current.setInputValue('updated text'); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| await result.current.handleSend(); | ||
| }); | ||
|
|
||
| expect(editMessageApi).toHaveBeenCalledWith('wpp2', { | ||
| number: EDIT_JID, | ||
| messageId: 'ext-abc123', | ||
| text: 'updated text', | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All identity checks mount
MemoryRouterwithout search parameters, sofailureCategoryandfailuresOnlyremain inactive and bothcategoryFilteredMessagesandvisibleMessagestake their pass-through branches. If the filtered branch starts allocating a new array on every render, these tests still pass; allow an initial route such as/?failuresOnly=1&failureCategory=failedand exercise the active filtering path.Useful? React with 👍 / 👎.