fix: harden admin magic runtime guards#172
Merged
Merged
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. |
WalkthroughPR refatora 13 componentes para melhorar null-safety em operações Map/Array, adiciona validação com fallback seguro em validadores PIX, refatora data flows em ProductPersonalizationManager/NoveltyProductGrid/MultiEngravingResult, aprimora error handling em PromptGenerator com Promise.all para async pricing, e atualiza UI visual em AdImageResult/LogoPreviewCanvas/TechniqueSelector. ChangesNull-Safety Hardening
Validation & Error Handling
Component Data Flow Refactoring
UI Visual Refinement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens several admin, pricing simulator, Magic Up, and novelty UI flows by removing targeted non-null assertions and adding null-safe guards/fallbacks to reduce runtime crashes when optional data is missing.
Changes:
- Replaced
!assertions with guarded access (null-safe map access, safe refs, and safer collection fallbacks). - Improved handler safety in selectors (component/location/technique selection) and canvas/signature flows.
- Cleaned up imports/formatting and minor UI/layout adjustments while maintaining behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/quotes/QuoteSignaturePad.tsx | Removes unsafe canvasRef.current! usage and guards canvas interactions. |
| src/components/pricing/simulator/TechniqueSelector.tsx | Guards selection handlers and fixes effect deps for safer technique picking. |
| src/components/pricing/simulator/ProductVariantSelector.tsx | Makes variant grouping logic null-safe and adjusts UI classes. |
| src/components/pricing/simulator/MultiEngravingResult.tsx | Removes non-null assertions around calculation results / codes and cleans imports. |
| src/components/novelties/NoveltyProductGrid.tsx | Adds safer defaults and refactors loading/progress/filter rendering for resilience. |
| src/components/mockup/logo-editor/LogoPreviewCanvas.tsx | Avoids non-null assertions in logo rendering and improves conditional rendering safety. |
| src/components/magic-up/PromptGenerator.tsx | Uses safe print area fallbacks and removes non-null assertions in customization panel wiring. |
| src/components/magic-up/AdImageResult.tsx | Removes unsafe imageUrl! and improves guarded rendering around optional props. |
| src/components/kit-builder/VariantSelector.tsx | Makes variant grouping null-safe and cleans up imports/props usage. |
| src/components/admin/suppliers-manager/useSuppliersManager.ts | Replaces assertions and improves PIX validation error fallback. |
| src/components/admin/products/new-supplier/useNewSupplierForm.ts | Improves PIX validation error fallback (no !). |
| src/components/admin/products/CategorySelect.tsx | Makes tree building null-safe and refactors breadcrumb/search logic formatting. |
| src/components/admin/products/CategoryCascadeSelector.tsx | Makes tree building null-safe and cleans import formatting. |
| src/components/admin/products/BulkImportDialog.tsx | Uses flatMap to avoid data! assertions when building import rows. |
| src/components/admin/personalization-manager/ProductPersonalizationManager.tsx | Adds guard to avoid rendering with selectedProduct! and refactors props formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/admin/products/BulkImportDialog.tsx (1)
263-288:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTrate falha de
executeBatchImportpara evitar UI travada em “importing”.Se a Promise falhar em Line 278, hoje não há
try/catchlocal: o usuário pode ficar sem feedback e sem rollback de estado.✅ Sugestão objetiva de correção
const executeImport = useCallback(async () => { let rowsToImport: ImportRow[]; if (importMode === 'insert') { rowsToImport = validationResults.flatMap((r) => r.valid && r.data && !r.existsInDb ? [r.data] : [], ); } else { rowsToImport = validationResults.flatMap((r) => (r.valid && r.data ? [r.data] : [])); } if (rowsToImport.length === 0) { toast.error('Nenhuma linha para importar'); return; } - setStep('importing'); - const result = await executeBatchImport(rowsToImport, importMode, (p) => setProgress({ ...p })); - setImportResult(result); - setStep('complete'); - if (result.succeeded > 0) { - toast.success(`${result.succeeded} produto(s) importado(s)!`); - onComplete(); - } - if (result.failed > 0) { - toast.error(`${result.failed} produto(s) falharam`); - } + setStep('importing'); + try { + const result = await executeBatchImport(rowsToImport, importMode, (p) => setProgress({ ...p })); + setImportResult(result); + setStep('complete'); + if (result.succeeded > 0) { + toast.success(`${result.succeeded} produto(s) importado(s)!`); + onComplete(); + } + if (result.failed > 0) { + toast.error(`${result.failed} produto(s) falharam`); + } + } catch (err) { + logger.error('Falha na importação em lote:', err); + toast.error('Falha ao importar produtos. Tente novamente.'); + setStep('preview'); + } }, [validationResults, importMode, onComplete]);🤖 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/admin/products/BulkImportDialog.tsx` around lines 263 - 288, A Promise rejection from executeBatchImport can leave the UI stuck in the "importing" step; inside the executeImport function wrap the await executeBatchImport(rowsToImport, importMode, ...) call in a try/catch (and optionally a finally) so that on error you setStep('complete') (or a dedicated error state), setImportResult to an error/empty result, reset/setProgress appropriately, and call toast.error with the caught error message; reference the executeImport function, the executeBatchImport call, setStep, setImportResult, setProgress and toast so you ensure the UI state is always updated and onComplete is not called on failures.src/components/pricing/simulator/MultiEngravingResult.tsx (1)
61-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEvitar sobrescrita de estado por resposta assíncrona obsoleta.
O fluxo atual pode aplicar no estado o resultado de um cálculo antigo (corrida entre chamadas), exibindo total/prazo incorretos quando
quantityouengravingsmudam rápido.Diff sugerido
useEffect(() => { + let cancelled = false; + const calculateAll = async () => { if (engravings.length === 0) { setCalculations([]); + setIsCalculating(false); return; } setIsCalculating(true); @@ const results = await Promise.all( engravings.map(async (engraving) => { @@ }), ); + if (cancelled) return; setCalculations(results); setIsCalculating(false); }; const debounce = setTimeout(calculateAll, 300); - return () => clearTimeout(debounce); + return () => { + cancelled = true; + clearTimeout(debounce); + }; }, [engravings, quantity, calculatePrice]);🤖 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/pricing/simulator/MultiEngravingResult.tsx` around lines 61 - 102, The effect can apply stale async results to state when multiple calculateAll runs overlap; modify the useEffect/calculateAll flow to ignore outdated responses by introducing a request token/ID (e.g., a useRef like currentRequestId) or an AbortController so each run captures a unique id/controller, checks it before calling setCalculations/setIsCalculating, and/or aborts in-flight calculatePrice calls on cleanup; update the calculateAll mapping (the async engraving loop) to bail out if the token has changed and only setCalculations/setIsCalculating for the latest token, referencing the existing useEffect, calculateAll function, setCalculations and setIsCalculating identifiers.
🤖 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/components/novelties/NoveltyProductGrid.tsx`:
- Line 399: No componente NoveltyProductGrid atualize o valor do placeholder
usado no campo de busca: substitua a string placeholder="Buscar novidades… /"
por uma versão sem a barra e sem o espaço extra (ex: "Buscar novidades…") no
elemento que define o atributo placeholder (procure pelo atributo placeholder
dentro de NoveltyProductGrid.tsx).
In `@src/components/pricing/simulator/TechniqueSelector.tsx`:
- Around line 171-175: Add explicit type="button" to the clickable card buttons
in the TechniqueSelector component to prevent accidental form submissions;
locate the button elements rendered in TechniqueSelector.tsx (the ones using
onClick={() => handleComponentSelect(comp)}) and any other similar card buttons
around the other occurrences referenced, and update their JSX to include
type="button" on each <button> element so clicks do not act as submits when
inside a form.
---
Outside diff comments:
In `@src/components/admin/products/BulkImportDialog.tsx`:
- Around line 263-288: A Promise rejection from executeBatchImport can leave the
UI stuck in the "importing" step; inside the executeImport function wrap the
await executeBatchImport(rowsToImport, importMode, ...) call in a try/catch (and
optionally a finally) so that on error you setStep('complete') (or a dedicated
error state), setImportResult to an error/empty result, reset/setProgress
appropriately, and call toast.error with the caught error message; reference the
executeImport function, the executeBatchImport call, setStep, setImportResult,
setProgress and toast so you ensure the UI state is always updated and
onComplete is not called on failures.
In `@src/components/pricing/simulator/MultiEngravingResult.tsx`:
- Around line 61-102: The effect can apply stale async results to state when
multiple calculateAll runs overlap; modify the useEffect/calculateAll flow to
ignore outdated responses by introducing a request token/ID (e.g., a useRef like
currentRequestId) or an AbortController so each run captures a unique
id/controller, checks it before calling setCalculations/setIsCalculating, and/or
aborts in-flight calculatePrice calls on cleanup; update the calculateAll
mapping (the async engraving loop) to bail out if the token has changed and only
setCalculations/setIsCalculating for the latest token, referencing the existing
useEffect, calculateAll function, setCalculations and setIsCalculating
identifiers.
🪄 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: c626bee7-55e4-433f-9716-4539ff98e3ea
📒 Files selected for processing (15)
src/components/admin/personalization-manager/ProductPersonalizationManager.tsxsrc/components/admin/products/BulkImportDialog.tsxsrc/components/admin/products/CategoryCascadeSelector.tsxsrc/components/admin/products/CategorySelect.tsxsrc/components/admin/products/new-supplier/useNewSupplierForm.tssrc/components/admin/suppliers-manager/useSuppliersManager.tssrc/components/kit-builder/VariantSelector.tsxsrc/components/magic-up/AdImageResult.tsxsrc/components/magic-up/PromptGenerator.tsxsrc/components/mockup/logo-editor/LogoPreviewCanvas.tsxsrc/components/novelties/NoveltyProductGrid.tsxsrc/components/pricing/simulator/MultiEngravingResult.tsxsrc/components/pricing/simulator/ProductVariantSelector.tsxsrc/components/pricing/simulator/TechniqueSelector.tsxsrc/components/quotes/QuoteSignaturePad.tsx
| {searchQuery && <button onClick={() => setSearchQuery("")} className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"><X className="h-3 w-3" /></button>} | ||
| <Search className="absolute left-2.5 top-1/2 h-3.5 w-3.5 -translate-y-1/2 text-muted-foreground" /> | ||
| <Input | ||
| placeholder="Buscar novidades… /" |
There was a problem hiding this comment.
Remover caractere extra no placeholder de busca.
Na Line 399, o placeholder tem uma barra (/) sobrando ("Buscar novidades… /"), o que aparece como ruído na UI.
Diff sugerido
- placeholder="Buscar novidades… /"
+ placeholder="Buscar novidades…"📝 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.
Suggested change
| placeholder="Buscar novidades… /" | |
| placeholder="Buscar novidades…" |
🤖 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/novelties/NoveltyProductGrid.tsx` at line 399, No componente
NoveltyProductGrid atualize o valor do placeholder usado no campo de busca:
substitua a string placeholder="Buscar novidades… /" por uma versão sem a barra
e sem o espaço extra (ex: "Buscar novidades…") no elemento que define o atributo
placeholder (procure pelo atributo placeholder dentro de
NoveltyProductGrid.tsx).
Comment on lines
171
to
175
| <button | ||
| key={`${comp.code}-${idx}`} | ||
| onClick={() => handleComponentSelect(comp)} | ||
| className="p-4 rounded-lg border bg-card hover:bg-accent hover:border-primary/50 transition-all text-left group" | ||
| className="group rounded-lg border bg-card p-4 text-left transition-all hover:border-primary/50 hover:bg-accent" | ||
| > |
There was a problem hiding this comment.
Defina type="button" nos botões de seleção
Sem type explícito, o default é submit; se este componente estiver dentro de <form>, clique em card pode disparar submit acidental.
✅ Correção rápida
- <button
+ <button
+ type="button"
key={`${comp.code}-${idx}`}
onClick={() => handleComponentSelect(comp)}
className="group rounded-lg border bg-card p-4 text-left transition-all hover:border-primary/50 hover:bg-accent"
>
...
- <button
+ <button
+ type="button"
key={`${loc.code}-${idx}`}
onClick={() => handleLocationSelect(loc)}
className="group rounded-lg border bg-card p-4 text-left transition-all hover:border-primary/50 hover:bg-accent"
>
...
- <button
+ <button
+ type="button"
key={tech.id}
onClick={() => handleTechniqueSelect(tech)}
className="group rounded-lg border bg-card p-4 text-left transition-all hover:border-primary/50 hover:bg-accent"
>Also applies to: 213-216, 249-253
🤖 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/pricing/simulator/TechniqueSelector.tsx` around lines 171 -
175, Add explicit type="button" to the clickable card buttons in the
TechniqueSelector component to prevent accidental form submissions; locate the
button elements rendered in TechniqueSelector.tsx (the ones using onClick={() =>
handleComponentSelect(comp)}) and any other similar card buttons around the
other occurrences referenced, and update their JSX to include type="button" on
each <button> element so clicks do not act as submits when inside a form.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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/magic-up/AdImageResult.tsx (1)
405-405:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPromise do clipboard sem tratamento de erro.
A chamada
navigator.clipboard?.writeText()retorna uma Promise que pode falhar (ex: permissões negadas, contexto não-HTTPS), mas não há.catch()nemawaitpara tratar rejeições. Isso pode gerar unhandled rejection e o usuário não recebe feedback caso a cópia falhe.Como per coding guidelines, Promises devem ter
.catch()ouawaitcom try/catch.🛡️ Fix proposto com tratamento de erro
<Button size="sm" variant="ghost" className="h-7 gap-1 text-xs" - onClick={() => navigator.clipboard?.writeText(copyPack.whatsapp)} + onClick={async () => { + try { + await navigator.clipboard?.writeText(copyPack.whatsapp); + } catch { + // Silent fail ou adicionar toast de erro + } + }} > <Copy className="h-3.5 w-3.5" /> Copiar WhatsApp </Button>Alternativa com feedback ao usuário (requer import do toast):
onClick={async () => { try { await navigator.clipboard?.writeText(copyPack.whatsapp); toast.success('Copiado para área de transferência'); } catch { toast.error('Erro ao copiar texto'); } }}🤖 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/magic-up/AdImageResult.tsx` at line 405, The inline onClick currently calls navigator.clipboard?.writeText(copyPack.whatsapp) without handling the returned Promise; change the handler to an async function that awaits navigator.clipboard?.writeText(copyPack.whatsapp) inside a try/catch, and in the try call toast.success('Copiado para área de transferência') and in the catch call toast.error('Erro ao copiar texto') so rejections are handled and the user gets feedback; update the onClick referencing navigator.clipboard?.writeText(copyPack.whatsapp) accordingly.
🤖 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/magic-up/AdImageResult.tsx`:
- Line 405: The inline onClick currently calls
navigator.clipboard?.writeText(copyPack.whatsapp) without handling the returned
Promise; change the handler to an async function that awaits
navigator.clipboard?.writeText(copyPack.whatsapp) inside a try/catch, and in the
try call toast.success('Copiado para área de transferência') and in the catch
call toast.error('Erro ao copiar texto') so rejections are handled and the user
gets feedback; update the onClick referencing
navigator.clipboard?.writeText(copyPack.whatsapp) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bb24bd4-69e2-445c-acb8-990cd2d998a1
📒 Files selected for processing (1)
src/components/magic-up/AdImageResult.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
rgsearch for the removed assertion patterns returned no matches.npx.cmd eslinton the 15 touched files passed.git diff --check HEAD~1..HEADpassed.npx.cmd vitest run src/components/products/customization/__tests__/LocationPanelPrice.test.tsx src/components/products/ProductGrid.test.tsxpassed: 2 files, 6 tests.npm.cmd run buildpassed with existing Vite/Tailwind warnings.Notes
npx.cmd tsc -p tsconfig.app.json --noEmit --pretty falsewas attempted and timed out after 4 minutes without emitting errors.HUSKY=0because the repository pre-pushlint:baselinehook is currently failing locally before remote validation.Summary by cubic
Hardened runtime guards across admin, personalization manager, and Magic flows to prevent crashes when optional data is missing. Replaced non-null assertions with safe checks and fallbacks across selectors, imports, novelty grid, simulator, and signature pad.
Bug Fixes
flatMapfor import rows, safer PIX validation fallback with default message, and trimmed payload fields without!.Refactors
VariantSelector: removed unuseditemNameprop.Written for commit edfc49b. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
Melhorias de UI/UX