fix: harden product simulation hooks lint#166
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughRefatoração distribuída de 8 hooks React com foco em null safety, consolidação de padrão Map aggregate, e enriquecimento de imagens de produtos. Removidas non-null assertions (!) e substituídas por guards explícitos retornando defaults. Integração de product images em simulator com agrupamento seguro por product_id. ChangesRefatoração de hooks: null safety e agregação
Sequence Diagram(s)Não aplicável — mudanças consistem em refatorações internas de hooks, removals de assertions, consolidações de pattern, sem novas integrações multi-componente significativas. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutos Justificativa: Mix de refatorações de pattern (Map aggregation), guarding defensivo (null checks), e uma integração substancial de imagens em simulator. Requer verificação de type safety em múltiplos arquivos, validação de padrão Map em 3+ hooks, e atenção especial a 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Este PR endurece e estabiliza hooks relacionados a simulação/produtos/cotações, com foco em eliminar non-null assertions, melhorar guards de queries condicionais e ajustar dependências/memoização para satisfazer regras de lint (especialmente hooks/exhaustive-deps) sem alterar a intenção funcional.
Changes:
- Adiciona type guards (
hasMaxColors/hasAreaSize) para remover asserts e tornar o acesso a campos nullable mais seguro em pricing tables. - Remove non-null assertions em agrupamentos via
Map(imagens, fornecedores e replies de comentários), substituindo por inicialização segura. - Adiciona guards explícitos em
queryFnquandoenableddepende deproductId/clientId, e estabiliza dependências (ex.: key de múltiplas técnicas).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/simulation/useTechniquePricingOptions.ts | Type guards para tabelas de preço e estabilização de dependência no hook multi-técnica. |
| src/hooks/simulation/useExternalSimulator.ts | Remove non-null assertions e padroniza agrupamento de imagens por Map. |
| src/hooks/quotes/useQuoteComments.ts | Remove non-null assertions ao montar árvore de replies, com guard para parent_id. |
| src/hooks/products/useVariantStock.ts | Memoização de arrays derivados de data para estabilizar dependências e remove ! em filtro. |
| src/hooks/products/useSupplierComparison.ts | Remove non-null assertion ao agrupar produtos por fornecedor em Map. |
| src/hooks/products/useProductImages.ts | Remove non-null assertion ao agrupar imagens em batch por Map. |
| src/hooks/products/useProductFreshnessOverride.ts | Guard explícito de productId no queryFn (alinhado ao enabled). |
| src/hooks/bi/useClientOrdersHistory.ts | Guard explícito de clientId no queryFn (alinhado ao enabled) e ajustes de lint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 84dca07eb6
ℹ️ 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".
| } | ||
|
|
||
| function hasMaxColors(table: PriceTableEntry): table is PriceTableEntry & { max_colors: number } { | ||
| return table.price_by_color && typeof table.max_colors === 'number'; |
There was a problem hiding this comment.
The new guard typeof table.max_colors === 'number' filters out rows where external-db-bridge serializes numeric DB fields as strings (a case already handled elsewhere in the repo, e.g. by parsing max_cores before use). In that scenario, valid customization_price_tables rows are dropped, so colorOptions/sizeOptions become incomplete or empty and the simulator can hide valid pricing choices for a technique.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/simulation/useTechniquePricingOptions.ts (1)
193-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEvite sobrescrever estado com resposta antiga no
useEffectassíncrono
QuandotechniqueCodesKeymuda rápido, uma execução anterior pode terminar depois e sobrescreversetAllTables(results)esetIsLoading(false)com dados obsoletos. Adicione cleanup/cancelamento antes de atualizar o estado.💡 Sugestão de ajuste
useEffect(() => { + let cancelled = false; const codes = techniqueCodesKey ? techniqueCodesKey.split(',').filter(Boolean) : []; if (codes.length === 0) { setAllTables({}); return; } const fetchAll = async () => { setIsLoading(true); const results: Record<string, PriceTableEntry[]> = {}; await Promise.all( codes.map(async (code) => { try { const { data, error } = await supabase.functions.invoke('external-db-bridge', { body: { table: 'customization_price_tables', operation: 'select', filters: { table_code: code }, limit: 100, }, }); if (!error && data?.success && data?.data?.records) { results[code] = data.data.records; } else if (!error && Array.isArray(data?.data)) { results[code] = data.data; } } catch (err) { console.error(`Error fetching pricing for ${code}:`, err); } }), ); - setAllTables(results); - setIsLoading(false); + if (!cancelled) { + setAllTables(results); + setIsLoading(false); + } }; fetchAll(); + return () => { + cancelled = true; + }; }, [techniqueCodesKey]);🤖 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/hooks/simulation/useTechniquePricingOptions.ts` around lines 193 - 233, The async useEffect in useTechniquePricingOptions may set state from stale fetches when techniqueCodesKey changes quickly; modify fetchAll to support cancellation by creating a local cancelled flag or AbortController tied to the effect (inside useEffect) and check it before calling setAllTables and setIsLoading (and before assigning to results) so returned results from prior invocations are ignored; ensure the effect returns a cleanup that flips the cancelled flag or aborts the controller and that you check cancellation after each await inside fetchAll (references: useEffect, fetchAll, setAllTables, setIsLoading, techniqueCodesKey).
🤖 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.
Inline comments:
In `@src/hooks/quotes/useQuoteComments.ts`:
- Line 173: The insert into workspace_notifications currently ignores Supabase
errors; change the call in useQuoteComments (where notifications is inserted) to
capture the result and handle errors: const { data, error } = await
supabase.from('workspace_notifications').insert(notifications); then if (error)
either throw error or log/report it (e.g., processLogger.error /
Sentry.captureException) so failures (RLS/invalid payload) are not swallowed by
the surrounding try/catch that is non-blocking; ensure you reference the same
notifications variable and preserve existing behavior when data is returned.
In `@src/hooks/simulation/useExternalSimulator.ts`:
- Around line 357-365: A consulta atual chama
invokeExternalDb<ProductImageRecord>('product_images','select',...) sem filtrar
por product_id e depois filtra imagens em memória (imagesResult), o que pode
causar perda por limit e I/O desnecessário; update a chamada a invokeExternalDb
para incluir um filtro de product_id usando os ids de produtos em contexto (ex.:
filters: { is_active: true, product_id: { in: productIds } }) e ajustar/remover
o limit ou torná-lo suficientemente grande por produto, para que a filtragem por
product_id ocorra na query e não apenas em memória; referências-chave:
invokeExternalDb, 'product_images', ProductImageRecord, imagesResult e o código
que atualmente faz o .filter em memória.
In `@src/hooks/simulation/useTechniquePricingOptions.ts`:
- Around line 191-195: The current techniqueCodesKey uses join(',')/split(',')
which breaks if a technique code contains a comma; replace that with an
unambiguous serialization: set techniqueCodesKey =
JSON.stringify(techniqueCodes) and inside the useEffect derive codes via
JSON.parse(techniqueCodesKey) (or fallback to []), and keep the effect
dependency on techniqueCodesKey; update references to techniqueCodesKey,
useEffect, and techniqueCodes accordingly.
---
Outside diff comments:
In `@src/hooks/simulation/useTechniquePricingOptions.ts`:
- Around line 193-233: The async useEffect in useTechniquePricingOptions may set
state from stale fetches when techniqueCodesKey changes quickly; modify fetchAll
to support cancellation by creating a local cancelled flag or AbortController
tied to the effect (inside useEffect) and check it before calling setAllTables
and setIsLoading (and before assigning to results) so returned results from
prior invocations are ignored; ensure the effect returns a cleanup that flips
the cancelled flag or aborts the controller and that you check cancellation
after each await inside fetchAll (references: useEffect, fetchAll, setAllTables,
setIsLoading, techniqueCodesKey).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e763b545-76b0-4165-9636-2cda59a438d6
📒 Files selected for processing (8)
src/hooks/bi/useClientOrdersHistory.tssrc/hooks/products/useProductFreshnessOverride.tssrc/hooks/products/useProductImages.tssrc/hooks/products/useSupplierComparison.tssrc/hooks/products/useVariantStock.tssrc/hooks/quotes/useQuoteComments.tssrc/hooks/simulation/useExternalSimulator.tssrc/hooks/simulation/useTechniquePricingOptions.ts
| })); | ||
|
|
||
| await supabase.from("workspace_notifications").insert(notifications); | ||
| await supabase.from('workspace_notifications').insert(notifications); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verifica chamadas de escrita no hook sem checagem explícita de `error`
rg -n -C2 "supabase\.from\('.*'\)\.(insert|update|delete|upsert)\(" src/hooks/quotes/useQuoteComments.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 604
🏁 Script executed:
#!/bin/bash
# Checa chamadas específicas de escrita sem destructurar `error`
rg -n "workspace_notifications" src/hooks/quotes/useQuoteComments.ts
rg -n "await supabase\.from\('.*'\)\.insert\(" -n src/hooks/quotes/useQuoteComments.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 300
🏁 Script executed:
#!/bin/bash
# Verifica se há algum outro insert/delete/update/upsert aguardado sem `{ error }`
rg -n -C1 "await supabase\.from\('.*'\)\.(insert|update|delete|upsert)\(" src/hooks/quotes/useQuoteComments.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 407
🏁 Script executed:
#!/bin/bash
# Checa ocorrências do insert que estava sendo apontado
rg -n "workspace_notifications" src/hooks/quotes/useQuoteComments.ts
# Mostra todos os awaited inserts no arquivo (para comparar com checagem de `error`)
rg -n "await supabase\.from\('.*'\)\.insert\(" src/hooks/quotes/useQuoteComments.ts
# Verifica contexto de writes aguardadas (insert/update/delete/upsert)
rg -n -C2 "await supabase\.from\('.*'\)\.(insert|update|delete|upsert)\(" src/hooks/quotes/useQuoteComments.tsRepository: adm01-debug/promo-gifts-v4
Length of output: 834
Valide o erro do insert em workspace_notifications
O await supabase.from('workspace_notifications').insert(notifications); (linha 173) não checa { error }; mesmo estando em try/catch com catch { // Non-blocking }, falhas do Supabase (RLS/payload inválido) viram falha silenciosa porque não há throw.
🔧 Ajuste sugerido
- await supabase.from('workspace_notifications').insert(notifications);
+ const { error: notifyError } = await supabase
+ .from('workspace_notifications')
+ .insert(notifications);
+ if (notifyError) throw notifyError;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await supabase.from('workspace_notifications').insert(notifications); | |
| const { error: notifyError } = await supabase | |
| .from('workspace_notifications') | |
| .insert(notifications); | |
| if (notifyError) throw notifyError; |
🤖 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/hooks/quotes/useQuoteComments.ts` at line 173, The insert into
workspace_notifications currently ignores Supabase errors; change the call in
useQuoteComments (where notifications is inserted) to capture the result and
handle errors: const { data, error } = await
supabase.from('workspace_notifications').insert(notifications); then if (error)
either throw error or log/report it (e.g., processLogger.error /
Sentry.captureException) so failures (RLS/invalid payload) are not swallowed by
the surrounding try/catch that is non-blocking; ensure you reference the same
notifications variable and preserve existing behavior when data is returned.
| const imagesResult = await invokeExternalDb<ProductImageRecord>( | ||
| 'product_images', | ||
| 'select', | ||
| { | ||
| filters: { is_active: true }, | ||
| select: 'product_id, url_cdn, image_type, is_primary, display_order', | ||
| orderBy: { column: 'display_order', ascending: true }, | ||
| limit: 2000, | ||
| }, |
There was a problem hiding this comment.
Filtre product_images por product_id na query para evitar perda de imagens e custo desnecessário.
Em Line 361 você busca todas as imagens ativas e só filtra em memória depois (Line 372). Com limit: 2000 (Line 364), isso pode excluir imagens de produtos retornados quando a tabela crescer, além de aumentar I/O sem necessidade.
Diff sugerido
const imagesResult = await invokeExternalDb<ProductImageRecord>(
'product_images',
'select',
{
- filters: { is_active: true },
+ filters: { product_id: productIds, is_active: true },
select: 'product_id, url_cdn, image_type, is_primary, display_order',
orderBy: { column: 'display_order', ascending: true },
- limit: 2000,
+ limit: Math.max(productIds.length * 8, 100),
},
);Also applies to: 372-374
🤖 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/hooks/simulation/useExternalSimulator.ts` around lines 357 - 365, A
consulta atual chama
invokeExternalDb<ProductImageRecord>('product_images','select',...) sem filtrar
por product_id e depois filtra imagens em memória (imagesResult), o que pode
causar perda por limit e I/O desnecessário; update a chamada a invokeExternalDb
para incluir um filtro de product_id usando os ids de produtos em contexto (ex.:
filters: { is_active: true, product_id: { in: productIds } }) e ajustar/remover
o limit ou torná-lo suficientemente grande por produto, para que a filtragem por
product_id ocorra na query e não apenas em memória; referências-chave:
invokeExternalDb, 'product_images', ProductImageRecord, imagesResult e o código
que atualmente faz o .filter em memória.
| const techniqueCodesKey = techniqueCodes.join(','); | ||
|
|
||
| useEffect(() => { | ||
| if (techniqueCodes.length === 0) { | ||
| const codes = techniqueCodesKey ? techniqueCodesKey.split(',').filter(Boolean) : []; | ||
|
|
There was a problem hiding this comment.
join(',')/split(',') pode quebrar código de técnica contendo vírgula.
A chave derivada por vírgula não é robusta para todos os valores possíveis. Prefira manter techniqueCodes como fonte e usar uma serialização sem ambiguidade para a dependência (ex.: JSON.stringify(techniqueCodes)).
🤖 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/hooks/simulation/useTechniquePricingOptions.ts` around lines 191 - 195,
The current techniqueCodesKey uses join(',')/split(',') which breaks if a
technique code contains a comma; replace that with an unambiguous serialization:
set techniqueCodesKey = JSON.stringify(techniqueCodes) and inside the useEffect
derive codes via JSON.parse(techniqueCodesKey) (or fallback to []), and keep the
effect dependency on techniqueCodesKey; update references to techniqueCodesKey,
useEffect, and techniqueCodes accordingly.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
adm01-debug
deleted the
fix/batch-products-simulation-hooks-lint-hardening
branch
Resumo
useVariantStockpara corrigir dependências de hooks e removeminQuantityNeeded!.Mapem imagens, fornecedores e simulação externa.productId/clientIdem queries habilitadas condicionalmente.Validação
npx.cmd eslint src/hooks/products/useVariantStock.ts src/hooks/simulation/useTechniquePricingOptions.ts src/hooks/simulation/useExternalSimulator.ts src/hooks/products/useProductImages.ts src/hooks/products/useSupplierComparison.ts src/hooks/products/useProductFreshnessOverride.ts src/hooks/bi/useClientOrdersHistory.ts src/hooks/quotes/useQuoteComments.tsnpx.cmd eslintno conjunto ampliado usado no levantamento: 0 errors; warnings reduziram de 47 para 17git diff --checkVITE_SUPABASE_URL=https://doufsxqlfjyuvxuezpln.supabase.co VITE_SUPABASE_PUBLISHABLE_KEY=sb_publishable_test npm.cmd run buildObservação: o push normal foi bloqueado pelo hook local já conhecido
scripts/check-eslint-baseline.mjscomeslint falhou com status null; após validação manual acima, a branch foi publicada comHUSKY=0.Summary by cubic
Hardened product and simulation hooks by removing non-null assertions, adding query guards, and stabilizing memoized arrays to satisfy ESLint and prevent edge-case crashes. Also improved pricing option logic with typed guards and fixed dependency keys for multi-technique flows.
Bug Fixes
useVariantStockand removed unsafeminQuantityNeeded!.productId/clientIdguards for conditionally enabled@tanstack/react-queryqueries; return safe defaults when absent.useQuoteCommentsand Map usages in images/external data to avoid non-null assertions.Refactors
Written for commit 84dca07. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
Refactor