fix(contacts): atualizar total na branch search + remover countData morto (Onda 2 PR 2.2)#102
Conversation
…orto (Onda 2 PR 2.2) Resolve issue do CodeRabbit no PR #99 (Onda 1.3): `countData` declarado mas nunca usado, e `setTotal` não chamado na branch de search → UI mostra valores stale do query anterior. ## Fixes em src/components/contacts/useContactsPagination.ts ### 1) Removida variável morta ```diff - const countData: { count: number } | null = null; ``` Linha 75 declarava mas nunca usava. Era um vestígio (provavelmente de uma tentativa anterior de implementar count que ficou pelo caminho). ### 2) setTotal adicionado na branch search de loadContacts ```diff data = (searchData ?? []).map(sanitizeRow); + // TODO: search_contacts RPC não retorna count exato. Por ora, setamos total = qtd carregada + // (acumulado em loadMore). Resolver criando count_search_contacts() RPC. + setTotal(data.length); ``` ### 3) setTotal acumulando em loadMore (branch search) ```diff data = (searchData ?? []).map(sanitizeRow); + // Search RPC: acumular total conforme loadMore traz mais resultados + setTotal((prev) => prev + data.length); setContacts((prev) => [...prev, ...data]); ``` ## Por que essa estratégia (e não count exato) O RPC `search_contacts` retorna apenas a `TABLE` de resultados, sem coluna count. As alternativas seriam: - **A) Query paralela de count**: complexa porque o RPC usa `websearch_to_tsquery('portuguese', unaccent(...))` — duplicar essa lógica em outra query é frágil - **B) Modificar o RPC pra adicionar window function `count(*) over()`**: mudança de DB schema, vai pra Onda 5 (TS hardening + DB) - **C) Setar total = qtd carregada acumulada (esta solução)**: aproximação razoável, cresce conforme o usuário rola, e é correto quando `data.length < PAGE_SIZE` (resultado completo cabe) Estratégia C foi escolhida pelo trade-off: resolve o bug de stale (impacto direto na UX) sem mudar DB nem duplicar lógica de search. TODO documentado pra resolver definitivamente quando criarmos `count_search_contacts()` RPC. ## Trade-off: o que muda na UX | Cenário | Antes (bug) | Depois | |---|---|---| | Search retorna 30 resultados | "5000 contatos encontrados" (stale) | "30 contatos encontrados" ✓ | | Search retorna 200, paginação 50/pg | "5000 contatos encontrados" (stale) | "50 → 100 → 150 → 200 conforme rola" ✓ | | Filtro normal (sem search) | exato (`count: 'exact'`) | exato (sem mudança) | Vale lembrar que `hasMore` continua exato (`data.length === PAGE_SIZE` indica se há mais). Apenas o display de "X encontrados" mostra o quanto foi carregado, não o total absoluto. ## Stress-test - bun run build: OK (1m 6s) ✓ - TypeScript: sem novos erros ✓ - Comportamento em filtros normais: preservado (count exato continua funcionando) ✓ ## Refs - /workspace/notes/coderabbit-feedback-pr99.md (issue #1) - PR #99 (#issuecomment do CodeRabbit Pro)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA função AlteraçõesFluxo de Paginação com Search
Esforço de Revisão Estimado🎯 2 (Simples) | ⏱️ ~8 minutos Justificativa: Alteração isolada em um único arquivo afetando duas funções relacionadas. A lógica é direta—inicializar e incrementar um contador. Sem mudanças de contrato, sem efeitos secundários complexos. Requer apenas verificação de que a semântica da paginação está correta e que o acesso aos tamanhos dos resultados é seguro. PRs Potencialmente Relacionadas
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b83c772231
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| data = (searchData ?? []).map(sanitizeRow); | ||
| // TODO: search_contacts RPC não retorna count exato. Por ora, setamos total = qtd carregada | ||
| // (acumulado em loadMore). Resolver criando count_search_contacts() RPC. | ||
| setTotal(data.length); |
There was a problem hiding this comment.
When a search matches more than one page, setting total to only data.length makes the displayed count under-report the real result set and then change as the user scrolls (for example 200 matches show as 50 after the first page). The RPC overload being called here is the search_term/page_size one, and both the generated Supabase types and SQL function expose total_count on each returned row (src/integrations/supabase/types.ts:14944-14972, supabase/migrations-from-lovable/03_functions.sql:4430-4467), so the hook can keep the exact search total instead of replacing it with the loaded-page count.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/contacts/useContactsPagination.ts (1)
126-169:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRisco de corrida:
loadMoreantigo pode contaminarcontactsetotalapós troca de filtro.Se uma paginação antiga terminar depois de um
updateFilters/loadContacts, ela ainda executasetContactsesetTotal, misturando resultados de buscas diferentes.💡 Sugestão objetiva (invalidar respostas antigas com requestId)
@@ const offsetRef = useRef(0); const abortRef = useRef<AbortController | null>(null); + const requestIdRef = useRef(0); @@ const loadContacts = useCallback( async (overrideFilters?: Partial<FiltersState>) => { + const requestId = ++requestIdRef.current; abortRef.current?.abort(); abortRef.current = new AbortController(); @@ - setContacts(data); - setHasMore(data.length === PAGE_SIZE); - offsetRef.current = PAGE_SIZE; + if (requestId !== requestIdRef.current) return; + setContacts(data); + setHasMore(data.length === PAGE_SIZE); + offsetRef.current = PAGE_SIZE; @@ const loadMore = useCallback(async () => { if (loadingMore || !hasMore) return; + const requestId = requestIdRef.current; setLoadingMore(true); @@ - setContacts((prev) => [...prev, ...data]); - setHasMore(data.length === PAGE_SIZE); - offsetRef.current = offset + PAGE_SIZE; + if (requestId !== requestIdRef.current) return; + setContacts((prev) => [...prev, ...data]); + setHasMore(data.length === PAGE_SIZE); + offsetRef.current = offset + PAGE_SIZE;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/contacts/useContactsPagination.ts` around lines 126 - 169, The loadMore callback can apply stale results after filters change; add a request-id guard to ignore outdated responses: introduce a mutable currentRequestIdRef (e.g., useRef<number>()), increment it when starting a new logical load (both in loadMore and anywhere filters change such as updateFilters/loadContacts), capture the id in loadMore (const reqId = ++currentRequestIdRef.current) and before each state update (setContacts, setTotal, setHasMore, offsetRef.current = ...) check that reqId === currentRequestIdRef.current; only apply updates when they match so completed old loadMore calls are ignored and cannot contaminate contacts/total.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/contacts/useContactsPagination.ts`:
- Around line 126-169: The loadMore callback can apply stale results after
filters change; add a request-id guard to ignore outdated responses: introduce a
mutable currentRequestIdRef (e.g., useRef<number>()), increment it when starting
a new logical load (both in loadMore and anywhere filters change such as
updateFilters/loadContacts), capture the id in loadMore (const reqId =
++currentRequestIdRef.current) and before each state update (setContacts,
setTotal, setHasMore, offsetRef.current = ...) check that reqId ===
currentRequestIdRef.current; only apply updates when they match so completed old
loadMore calls are ignored and cannot contaminate contacts/total.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78251f43-0bf5-4e7e-a331-998e44b838c0
📒 Files selected for processing (1)
src/components/contacts/useContactsPagination.ts
There was a problem hiding this comment.
Pull request overview
Corrige a paginação/contagem exibida no módulo de contatos quando a listagem está usando a branch de search (RPC search_contacts), garantindo que o total reflita o número de resultados carregados, e remove um countData que estava morto.
Changes:
- Remove variável
countDatanão utilizada. - Atualiza
totalao carregar a primeira página viasearch_contacts. - Passa a acumular
totala cadaloadMorena branch de search.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
↩️ Resposta ao CodeRabbitCodeRabbit identificou um issue legítimo (race condition em Escopo desta PR
Issue identificado (fora de escopo)
Por que não fixar agoraEste é um bug de concorrência que envolve refatorar a lógica de pagination (afeta tanto PlanoVai pra Onda 2.4+ (limpeza incremental de useContactsPagination):
Documentado em |
…gination (Onda 2 PR 2.4) (#105) * fix(contacts): race condition guard com requestIdRef em useContactsPagination (Onda 2 PR 2.4) Issue identificada pelo CodeRabbit no PR #102: > Race condition: if loadMore from a previous filter completes after > updateFilters/loadContacts triggers a new fetch, the stale loadMore > still calls setContacts/setTotal — corrupting the list with > contacts from the previous filter. AbortController já cancela requests HTTP, mas chamadas .rpc() do supabase-js nem sempre respeitam o abort signal. Defesa em profundidade com ID monotônico de request: - requestIdRef: useRef(0) — incrementado a cada loadContacts - loadContacts: const requestId = ++requestIdRef.current; antes de qualquer await - loadMore: const requestId = requestIdRef.current; (captura sem incrementar) - Antes de setContacts/setTotal/setHasMore: guard 'if (requestId !== requestIdRef.current) return;' - catch: guard antes de setContacts([]) - finally: guard antes de setLoading(false)/setLoadingMore(false) Cenários cobertos: 1. loadContacts → user troca filter → novo loadContacts: resposta antiga descartada (requestId != atual) 2. loadMore → user troca filter → novo loadContacts: resposta do loadMore antigo descartada 3. Erro de rede em request antigo: setContacts([]) só dispara se ainda for o atual 4. Loading state: setLoading(false) tardio não esconde spinner do request novo Mudanças estruturais: - setTotal/setContacts movidos pra DEPOIS do guard único no try - totalCount/isSearchBranch como variáveis locais para diferir o set até depois do guard - sanitizeRow PRESERVADO 100% idêntico ao main (mantém defesa XSS via sanitizeText) Validação: - TypeScript: 0 erros - ESLint: 174 errors (igual main) - Build production: ✓ 57.90s - 1 arquivo, +48 -10 linhas * fix(contacts): atender 3 issues do CodeRabbit no PR #105 3 fixes aplicados conforme review: 1. AbortSignal AGORA realmente anexado (era ilusório antes) - Helper withAbortSignal<T>() encapsula o cast tipado - Aplicado nos 4 awaits: rpc + query do loadContacts e loadMore - Padrão idêntico ao usado em useMessagesCursor.ts:114 - Verificação 'controller.signal.aborted' após cada await re-throw como AbortError 2. setLoading(false) e setLoadingMore(false) SEMPRE no finally - Removido o guard 'if (requestId === requestIdRef.current)' do finally - Antes: loadingMore podia ficar stuck=true permanentemente quando filter trocava durante um loadMore (cf. CodeRabbit) 3. catch do loadContacts limpa total e hasMore além de contacts - Antes: erro de rede deixava 'total: N' e 'hasMore: true' do filter anterior com lista vazia → UI inconsistente - Agora: setTotal(0) + setHasMore(false) junto com setContacts([]) 4. (BÔNUS) loadMore agora encadeia abort do parent controller - Listener com {once: true} + cleanup no finally evita leak Validação: - TypeScript: 0 erros - ESLint: 164 errors (≤ main 174, sem regressão)
Resumo
Resolve issue do CodeRabbit no PR #99:
countDatamorto +totalnão atualizado na branch de search.Bug
Por que essa estratégia (e não count exato)
search_contactsé um RPC do Postgres que retornaTABLE, sem coluna count. Alternativas:count(*) over()window function ao RPC: mudança de schema → Onda 5data.length < PAGE_SIZEe cresce com loadMoreTODO documentado no código pra resolver com
count_search_contacts()RPC dedicado no futuro.Stress-test
bun run build(1m 6s)Métricas Onda 2
Summary by CodeRabbit
Notas de Lançamento