Feature entrega2 jpp#256
Open
jpeche1 wants to merge 3 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe PR establishes a Spanish-documented Next.js CRM MVP with Prisma and Supabase, including authentication, role-based access, client visibility, supplier and product management, customer order creation, mixed-supplier warnings, database seeding, validation schemas, and local setup tooling. ChangesCRM MVP
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
docs/v1-quickstart.md-23-23 (1)
23-23: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse relative paths for repository files and documentation links.
These lines use absolute file paths pointing to a specific local machine directory (
c:/Users/jppa_/Documents/...). This will cause broken links and confusion for anyone else cloning the repository.
docs/v1-quickstart.md#L23-L23: change[.env.local.example](c:/Users/jppa_/Documents/...)to a relative path like.env.local.example.docs/v1-quickstart.md#L77-L79: change the documentation file references to relative paths, e.g.,[docs/v1-technical-guide.md](./v1-technical-guide.md).🤖 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 `@docs/v1-quickstart.md` at line 23, Replace the machine-specific absolute link at docs/v1-quickstart.md lines 23-23 with a repository-relative .env.local.example link, and update the documentation references at docs/v1-quickstart.md lines 77-79 to use relative paths such as ./v1-technical-guide.md.src/app/layout.tsx-2-13 (1)
2-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse 'Inter' font family and adjust base font size.
As per coding guidelines, the 'Inter' font family must be used for typography and
1remmust be the base font size for primary body text content. Currently,layout.tsximports and appliesGeistandGeist_Mono, andglobals.cssspecifiesArial, Helvetica, sans-serifwithout a font-size declaration.
src/app/layout.tsx#L2-L13: replaceGeistandGeist_Monoimports and definitions withInter.src/app/layout.tsx#L26-L31: update theclassNameon the<html>tag to use theinter.variable. (Consider also updatinglang="en"tolang="es"for the Spanish-documented CRM).src/app/globals.css#L22-L26: update thefont-familyproperty to'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serifand addfont-size: 1rem;to meet the base font size guideline.🤖 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/app/layout.tsx` around lines 2 - 13, Replace Geist and Geist_Mono with the Inter font in src/app/layout.tsx lines 2-13, apply inter.variable to the html className in lines 26-31, and set the document language to es if appropriate for the Spanish CRM. In src/app/globals.css lines 22-26, use the specified Inter-first fallback font stack and add a 1rem base font-size.Source: Coding guidelines
src/app/login/page.tsx-80-82 (1)
80-82: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse primary blue and ensure visible focus indicators for links.
As per coding guidelines, Primary Blue (Indigo 600) should be used for links, and all interactive elements must be accessible via keyboard navigation with visible focus indicators.
🎨 Proposed fix to update link styles
- <Link className="mt-4 inline-flex text-sm font-medium text-stone-700 underline" href="/"> + <Link className="mt-4 inline-flex text-sm font-medium text-indigo-600 underline transition hover:text-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500" href="/">🤖 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/app/login/page.tsx` around lines 80 - 82, Update the Link element in the login page to use the primary blue/Indigo 600 text styling and add a visible keyboard focus indicator, such as an appropriate focus ring and offset, while preserving its existing layout, typography, destination, and label.Source: Coding guidelines
src/app/login/page.tsx-68-73 (1)
68-73: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winApply primary colors and focus indicators to the submit button.
As per coding guidelines, primary buttons and main actions should use Primary Blue (Indigo 600) with Primary Dark (Indigo 700) for hover states. Additionally, all buttons must implement the focus state with
focus:ring-2 focus:ring-indigo-500for keyboard accessibility.🎨 Proposed fix to update button styles
<button type="submit" - className="w-full rounded-2xl bg-stone-950 px-4 py-3 text-sm font-medium text-white transition hover:bg-stone-800" + className="w-full rounded-2xl bg-indigo-600 px-4 py-3 text-sm font-medium text-white transition hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500" >🤖 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/app/login/page.tsx` around lines 68 - 73, Update the submit button in the login page to use the primary Indigo 600 background with Indigo 700 on hover, replacing the existing stone color classes. Add the required focus:ring-2 and focus:ring-indigo-500 classes while preserving the button’s existing layout and typography styles.Source: Coding guidelines
src/app/page.tsx-71-91 (1)
71-91: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign link styles with the design system and add keyboard focus states.
As per coding guidelines:
- All interactive elements must be accessible via keyboard navigation with visible focus indicators.
- Primary Blue (Indigo 600) should be used for main actions.
- Warm Orange (Amber 500) should be used for warnings.
🎨 Proposed fix for styling and focus accessibility
{visibleActions.filter((action) => action.visible).map((action) => ( <Link key={action.href} href={action.href} className={ action.style === "primary" - ? "rounded-full border border-emerald-300/40 bg-emerald-300 px-5 py-3 text-sm font-medium text-stone-950 transition hover:bg-emerald-200" + ? "rounded-full border border-indigo-600/40 bg-indigo-600 px-5 py-3 text-sm font-medium text-white transition hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500" : action.style === "warning" - ? "rounded-full border border-amber-300/40 bg-amber-300 px-5 py-3 text-sm font-medium text-stone-950 transition hover:bg-amber-200" - : "rounded-full border border-white/15 px-5 py-3 text-sm font-medium text-white transition hover:bg-white/5" + ? "rounded-full border border-amber-500/40 bg-amber-500 px-5 py-3 text-sm font-medium text-white transition hover:bg-amber-600 focus:outline-none focus:ring-2 focus:ring-amber-500" + : "rounded-full border border-white/15 px-5 py-3 text-sm font-medium text-white transition hover:bg-white/5 focus:outline-none focus:ring-2 focus:ring-white" } > {action.label} </Link> ))} <a href="/docs/mvp-backlog.md" - className="rounded-full border border-white/15 px-5 py-3 text-sm font-medium text-white transition hover:bg-white/5" + className="rounded-full border border-white/15 px-5 py-3 text-sm font-medium text-white transition hover:bg-white/5 focus:outline-none focus:ring-2 focus:ring-white" > Ver backlog MVP </a>🤖 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/app/page.tsx` around lines 71 - 91, Update the action Link className styling and the backlog anchor in the visibleActions rendering to match the design system: use Indigo 600 for primary actions and Amber 500 for warning actions. Add visible keyboard focus states to every interactive link, including the backlog link, using the project’s focus-visible styling conventions while preserving existing hover and layout styles.Source: Coding guidelines
src/app/login/page.tsx-34-44 (1)
34-44: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winApply correct color states and accessibility roles for alert messages.
As per coding guidelines:
- Use
role="alert"for dynamic alert messages.- Use Warm Orange (Amber 500) for warnings and reminders.
- Error/Danger state must use
#EF4444(Red 500) for error messages.🎨 Proposed fix for styling and accessibility
{reasonMessage ? ( - <p className="mt-6 rounded-2xl border border-amber-200 bg-amber-50 px-4 py-3 text-sm text-amber-900"> + <p role="alert" className="mt-6 rounded-2xl border border-amber-500 bg-amber-50 px-4 py-3 text-sm text-amber-500"> {reasonMessage} </p> ) : null} {errorMessage ? ( - <p className="mt-6 rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900"> + <p role="alert" className="mt-6 rounded-2xl border border-red-500 bg-red-50 px-4 py-3 text-sm text-red-500"> {errorMessage} </p> ) : null}🤖 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/app/login/page.tsx` around lines 34 - 44, Update the conditional alert paragraphs in the login page: add role="alert" to both reasonMessage and errorMessage elements, use Amber 500 styling for the warning/reason message, and change the errorMessage styling to the Red 500 (`#EF4444`) danger state while preserving their existing conditional rendering.Source: Coding guidelines
src/app/clients/new/submit-button.tsx-9-13 (1)
9-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPrimary button doesn't follow the mandated color/focus-ring guideline.
bg-stone-950with no focus ring instead of the required Indigo-600 primary color andfocus:ring-2 focus:ring-indigo-500. See consolidated comment for the shared fix across this button and other CTA links.🤖 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/app/clients/new/submit-button.tsx` around lines 9 - 13, Update the submit button styling in the button element to use the mandated Indigo-600 primary background instead of bg-stone-950, and add the required focus:ring-2 focus:ring-indigo-500 classes while preserving the existing hover and disabled states.Source: Coding guidelines
src/app/clients/new/client-form.tsx-56-58 (1)
56-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
commercialNameis a required field but has norequired/aria-requiredor visual asterisk.
prisma/schema.prismadeclarescommercialName String(non-nullable), so this field is required, yet the form gives no indication until the server round-trip fails. As per coding guidelines, "Required form fields must include an asterisk (*) and havearia-required="true"attribute for accessibility."🤖 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/app/clients/new/client-form.tsx` around lines 56 - 58, Update the commercialName Field in the client form to mark it as required, including the required visual asterisk and aria-required="true" accessibility attribute, while leaving the optional legalName Field unchanged.Source: Coding guidelines
src/app/clients/new/client-form.tsx-21-43 (1)
21-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winForm controls don't match the mandated input styling.
Labels use
text-stone-800instead of gray-700, and inputs/select use a 1px border, 16px radius, and a black focus border with no focus shadow. As per coding guidelines, "Form inputs must have 2px border, 8px border radius, and 12px 16px padding with focus state showing blue border and focus shadow" and "Form labels must use Medium (500) font weight, gray-700 color..."🎨 Proposed fix
- <label className="block text-sm font-medium text-stone-800"> + <label className="block text-sm font-medium text-gray-700"> {label} <input name={name} type={type} placeholder={placeholder} - className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" + className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500/30" /> </label>Also applies to: 59-72
🤖 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/app/clients/new/client-form.tsx` around lines 21 - 43, Update the Field component’s label and input styling to follow the mandated form controls: use gray-700 with medium (500) label weight, a 2px input border, 8px radius, 12px 16px padding, and a blue focus border with focus shadow. Apply the same styling to the additional form controls referenced in the comment, including the select.Source: Coding guidelines
src/app/clients/new/client-form.tsx-50-54 (1)
50-54: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDynamic status messages don't use
role="alert". Both the create-client error and the post-creation success banner render plain<p>tags that screen readers won't announce as alerts. As per coding guidelines, "Userole="alert"for dynamic alert messages... to communicate semantic meaning to screen readers."
src/app/clients/new/client-form.tsx#L50-L54: addrole="alert"to the error<p>and switch the rose palette to the guideline's danger color (red-*/#EF4444).src/app/clients/[clientId]/page.tsx#L43-L47: addrole="alert"(orrole="status"if treated as polite) to the success confirmation<p>.🤖 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/app/clients/new/client-form.tsx` around lines 50 - 54, The dynamic client messages lack accessible announcement semantics. In src/app/clients/new/client-form.tsx lines 50-54, add role="alert" to the error paragraph and replace the rose color classes with the guideline’s red-* danger palette. In src/app/clients/[clientId]/page.tsx lines 43-47, add role="alert" or role="status" to the post-creation success paragraph.Source: Coding guidelines
src/app/clients/page.tsx-46-77 (1)
46-77: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClient listing renders tabular data without
role="table".The grid mimics a data table (column headers + rows) but has no table semantics, so screen readers won't announce it as tabular. As per coding guidelines, "Use ... role="table" for data tables to communicate semantic meaning to screen readers."
♿ Proposed fix
- <section className="rounded-[2rem] bg-white shadow-sm"> - <div className="grid grid-cols-[2fr_1fr_1fr_1fr] gap-4 border-b border-stone-200 px-6 py-4 text-xs font-semibold uppercase tracking-[0.2em] text-stone-500"> + <section role="table" aria-label="Clientes visibles" className="rounded-[2rem] bg-white shadow-sm"> + <div role="row" className="grid grid-cols-[2fr_1fr_1fr_1fr] gap-4 border-b border-stone-200 px-6 py-4 text-xs font-semibold uppercase tracking-[0.2em] text-stone-500">Apply
role="row"/role="cell"similarly to the list items and their spans.🤖 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/app/clients/page.tsx` around lines 46 - 77, Update the client listing section around visibleClients to expose table semantics: add role="table" to the outer container, role="row" to the header and each client row, role="columnheader" to the header spans, and role="cell" to the corresponding data spans. Preserve the existing layout, links, empty state, and client content.Source: Coding guidelines
src/modules/clients/create-client.ts-6-34 (1)
6-34: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSurface server-action failures in the client form
createClientActiononly returns form state for validation errors. A Prisma/runtime failure fromcreateClientForUser()will still throw, so add atry/catchhere and return a user-facing error message instead of letting the form fall through to a generic error page.🤖 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/modules/clients/create-client.ts` around lines 6 - 34, The createClientAction flow must catch Prisma/runtime failures from createClientForUser and return form state containing a user-facing error instead of allowing the exception to propagate. Add the try/catch around the createClientForUser call, preserve the existing validation-error handling, and use the action’s established error-state shape.
🧹 Nitpick comments (3)
prompts.md (1)
7-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse horizontal rules instead of headings for visual separators.
Using
######and####as decorative dividers harms the semantic structure of the markdown document and triggers linter warnings (MD001, MD024). Consider replacing them with standard horizontal rules (---).💡 Proposed refactor
-###### +--- Usa el archivo README que voy a utilizar para implementar este proyecto. Me gustaría que crearas la versión MVP utilizando entre 3 y 5 historias de usuario que representen los requisitos mínimos necesarios para implementar este proyecto. Evalúalas y propón las mejores candidatas para que yo pueda decidir. -###### +--- Empieza la implementación de los tickets del backlog uno por uno según se haya definido en el documento `@file`:mvp-backlog.md # Multiples iteraciones de Revisión de código y validación como: -- Revisa que todo esté funcionando en lint. -- Revisa porque la impleme -#### +--- Realiza la validación de lo implementado -#### +--- crea documentos técnicos para esta version. Indica lo necesario para que pueda ser instalado, ejecutado y validado por el usuario🤖 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 `@prompts.md` around lines 7 - 26, Replace the decorative ###### and #### separators in the prompt content with standard Markdown horizontal rules (---), while preserving the surrounding Spanish instructions and their order.Source: Linters/SAST tools
src/app/clients/new/submit-button.tsx (1)
9-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winButtons/CTAs across the client module don't follow the mandated color + focus-ring guideline. All three sites use the neutral
stonepalette with no keyboard focus indicator instead of the required Indigo-600 primary color andfocus:ring-2 focus:ring-indigo-500. As per coding guidelines, "Primary buttons must have blue background (#4F46E5)..." and "All buttons must implement focus state with focus:ring-2 focus:ring-indigo-500 for keyboard accessibility."
src/app/clients/new/submit-button.tsx#L9-L13: changebg-stone-950/hover:bg-stone-800tobg-indigo-600/hover:bg-indigo-700and addfocus:outline-none focus:ring-2 focus:ring-indigo-500.src/app/clients/new/client-form.tsx#L82-L89: addfocus:outline-none focus:ring-2 focus:ring-indigo-500to the CancelLinkand align its border/text color with the secondary-button spec (transparent background, indigo border/text).src/app/clients/page.tsx#L30-L43: change the "Nuevo cliente" primary CTA tobg-indigo-600/hover:bg-indigo-700with a focus ring, and add a focus ring to the "Panel" secondary link.🤖 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/app/clients/new/submit-button.tsx` around lines 9 - 13, The client module buttons and links use noncompliant colors and lack keyboard focus indicators. In src/app/clients/new/submit-button.tsx lines 9-13, update the submit button to indigo primary colors and add the required focus ring; in src/app/clients/new/client-form.tsx lines 82-89, update the Cancel Link to transparent indigo secondary styling and add the focus ring; in src/app/clients/page.tsx lines 30-43, apply indigo primary styling and a focus ring to the “Nuevo cliente” CTA, and add a focus ring to the “Panel” secondary link.Source: Coding guidelines
src/modules/orders/queries.ts (1)
3-15: 🚀 Performance & Scalability | 🔵 TrivialConsider pagination for
listOrdersas data grows.
listOrders()fetches every order with nested includes on each page load. Fine for an MVP dataset, but will not scale; considertake/skipor cursor-based pagination ahead of production traffic growth.🤖 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/modules/orders/queries.ts` around lines 3 - 15, Add pagination to listOrders so it no longer fetches every order and its nested relations on each request. Accept the established pagination inputs, apply them through the Prisma findMany take/skip or cursor options, and preserve the existing descending createdAt ordering and includes.
🤖 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/app/products/new/product-form.tsx`:
- Around line 13-35: Update the Field component to associate each label and
input using a stable id and htmlFor, switch label styling to text-gray-700, and
apply the required-field convention by displaying an asterisk and
aria-required="true" when appropriate. Replace the input styling with a 2px
border, rounded-lg corners, px-4 py-3 padding, focus:border-indigo-500, and
focus:ring-2 focus:ring-indigo-500, while preserving the existing field types
and values.
- Around line 46-105: Update the product form around the error block,
supplier/status selects, Field components, and action buttons to follow the UI
guidelines: use role="alert" with text-red-500 and border-red-500 for
state.error; apply 2px borders, rounded-lg, and blue focus states to every
input/select. Mark supplierId, sku, name, basePrice, and status as required with
visible asterisks and aria-required="true", and ensure labels use text-gray-700
with htmlFor references. Style Guardar producto as the indigo primary button and
Cancelar as the transparent indigo secondary action, both with the specified
padding and focus rings.
In `@src/app/suppliers/new/supplier-form.tsx`:
- Around line 29-80: Update the Field and SelectField components so labels use
text-gray-700 and associate with controls via matching htmlFor/id values. Apply
the required 2px border, rounded-lg radius, px-4 py-3 padding,
focus:border-indigo-500, and focus:ring-2 focus:ring-indigo-500 classes to both
controls; add required-field indicators and aria-required="true" where
applicable, preserving existing field props and option behavior.
- Around line 87-121: Update the supplier form’s error message to use
text-red-500 and border-red-500 with role="alert"; mark commercialName,
supplierType, and status as required through the Field/SelectField props. Update
the submit button to use bg-indigo-600, px-6 py-3, and focus:ring-2
focus:ring-indigo-500, and update the Cancel Link with a transparent background,
border-indigo-600 text-indigo-600, and the same focus ring.
In `@src/app/suppliers/page.tsx`:
- Around line 51-55: Add role="alert" to the dynamic notification banner
paragraphs in src/app/suppliers/page.tsx lines 51-55, src/app/products/page.tsx
lines 42-46, and both success and mixed-suppliers warning banners in
src/app/orders/[orderId]/page.tsx lines 41-45 and 47-51. Preserve their existing
content and styling.
- Around line 36-48: Update the links in src/app/suppliers/page.tsx lines 36-48
and src/app/products/page.tsx lines 33-39 to add focus:ring-2
focus:ring-indigo-500 to both actions and use bg-indigo-600 hover:bg-indigo-700
for each primary creation link. Update the links in
src/app/orders/[orderId]/page.tsx lines 33-39 with the same focus ring classes,
and change the “Volver a órdenes” link color from stone-600 to text-indigo-600;
preserve the existing link roles and layout.
- Around line 57-84: Apply table semantics consistently in the suppliers section
at src/app/suppliers/page.tsx lines 57-84, the products section at
src/app/products/page.tsx lines 48-74, and the order details section at
src/app/orders/[orderId]/page.tsx lines 64-84: add role="table" to each section,
role="rowgroup" to the list/header containers, role="row" to each grid wrapper
and list item, role="columnheader" to header cells, and role="cell" to data
cells.
In `@src/lib/auth/get-current-user.ts`:
- Around line 7-27: Wrap the db.userProfile.findUnique call in getCurrentUser
with error handling, and return null when the Prisma lookup fails. Leave the
existing unauthenticated and successful profile mapping behavior unchanged so
requireActiveUser and requireRole fail safely.
In `@src/lib/supabase/server.ts`:
- Around line 15-19: Update the setAll callback in createSupabaseServerClient to
wrap cookieStore.set mutations in a try/catch, swallowing render-phase cookie
mutation errors while preserving normal cookie updates. Leave session refresh
handling to the existing middleware.
In `@src/lib/validations/product.ts`:
- Around line 13-15: The product validation fields estimatedCost, leadTimeDays,
and minimumOrderQuantity currently allow empty strings to be coerced to 0;
reorder each z.union so z.literal("") is evaluated before the coerced numeric
schema, preserving the existing transforms. Apply the same ordering to
averageLeadTimeDays in src/lib/validations/product.ts lines 13-15 and
src/lib/validations/supplier.ts lines 15-17.
In `@src/modules/suppliers/create-supplier.ts`:
- Around line 4-21: Update createSupplier so the averageLeadTimeDays value
passed to Prisma is null when input.averageLeadTimeDays is an empty string,
while preserving valid numeric values unchanged.
---
Minor comments:
In `@docs/v1-quickstart.md`:
- Line 23: Replace the machine-specific absolute link at docs/v1-quickstart.md
lines 23-23 with a repository-relative .env.local.example link, and update the
documentation references at docs/v1-quickstart.md lines 77-79 to use relative
paths such as ./v1-technical-guide.md.
In `@src/app/clients/new/client-form.tsx`:
- Around line 56-58: Update the commercialName Field in the client form to mark
it as required, including the required visual asterisk and aria-required="true"
accessibility attribute, while leaving the optional legalName Field unchanged.
- Around line 21-43: Update the Field component’s label and input styling to
follow the mandated form controls: use gray-700 with medium (500) label weight,
a 2px input border, 8px radius, 12px 16px padding, and a blue focus border with
focus shadow. Apply the same styling to the additional form controls referenced
in the comment, including the select.
- Around line 50-54: The dynamic client messages lack accessible announcement
semantics. In src/app/clients/new/client-form.tsx lines 50-54, add role="alert"
to the error paragraph and replace the rose color classes with the guideline’s
red-* danger palette. In src/app/clients/[clientId]/page.tsx lines 43-47, add
role="alert" or role="status" to the post-creation success paragraph.
In `@src/app/clients/new/submit-button.tsx`:
- Around line 9-13: Update the submit button styling in the button element to
use the mandated Indigo-600 primary background instead of bg-stone-950, and add
the required focus:ring-2 focus:ring-indigo-500 classes while preserving the
existing hover and disabled states.
In `@src/app/clients/page.tsx`:
- Around line 46-77: Update the client listing section around visibleClients to
expose table semantics: add role="table" to the outer container, role="row" to
the header and each client row, role="columnheader" to the header spans, and
role="cell" to the corresponding data spans. Preserve the existing layout,
links, empty state, and client content.
In `@src/app/layout.tsx`:
- Around line 2-13: Replace Geist and Geist_Mono with the Inter font in
src/app/layout.tsx lines 2-13, apply inter.variable to the html className in
lines 26-31, and set the document language to es if appropriate for the Spanish
CRM. In src/app/globals.css lines 22-26, use the specified Inter-first fallback
font stack and add a 1rem base font-size.
In `@src/app/login/page.tsx`:
- Around line 80-82: Update the Link element in the login page to use the
primary blue/Indigo 600 text styling and add a visible keyboard focus indicator,
such as an appropriate focus ring and offset, while preserving its existing
layout, typography, destination, and label.
- Around line 68-73: Update the submit button in the login page to use the
primary Indigo 600 background with Indigo 700 on hover, replacing the existing
stone color classes. Add the required focus:ring-2 and focus:ring-indigo-500
classes while preserving the button’s existing layout and typography styles.
- Around line 34-44: Update the conditional alert paragraphs in the login page:
add role="alert" to both reasonMessage and errorMessage elements, use Amber 500
styling for the warning/reason message, and change the errorMessage styling to
the Red 500 (`#EF4444`) danger state while preserving their existing conditional
rendering.
In `@src/app/page.tsx`:
- Around line 71-91: Update the action Link className styling and the backlog
anchor in the visibleActions rendering to match the design system: use Indigo
600 for primary actions and Amber 500 for warning actions. Add visible keyboard
focus states to every interactive link, including the backlog link, using the
project’s focus-visible styling conventions while preserving existing hover and
layout styles.
In `@src/modules/clients/create-client.ts`:
- Around line 6-34: The createClientAction flow must catch Prisma/runtime
failures from createClientForUser and return form state containing a user-facing
error instead of allowing the exception to propagate. Add the try/catch around
the createClientForUser call, preserve the existing validation-error handling,
and use the action’s established error-state shape.
---
Nitpick comments:
In `@prompts.md`:
- Around line 7-26: Replace the decorative ###### and #### separators in the
prompt content with standard Markdown horizontal rules (---), while preserving
the surrounding Spanish instructions and their order.
In `@src/app/clients/new/submit-button.tsx`:
- Around line 9-13: The client module buttons and links use noncompliant colors
and lack keyboard focus indicators. In src/app/clients/new/submit-button.tsx
lines 9-13, update the submit button to indigo primary colors and add the
required focus ring; in src/app/clients/new/client-form.tsx lines 82-89, update
the Cancel Link to transparent indigo secondary styling and add the focus ring;
in src/app/clients/page.tsx lines 30-43, apply indigo primary styling and a
focus ring to the “Nuevo cliente” CTA, and add a focus ring to the “Panel”
secondary link.
In `@src/modules/orders/queries.ts`:
- Around line 3-15: Add pagination to listOrders so it no longer fetches every
order and its nested relations on each request. Accept the established
pagination inputs, apply them through the Prisma findMany take/skip or cursor
options, and preserve the existing descending createdAt ordering and includes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1eb073e6-4cc2-445f-ba41-8284e4b056b1
⛔ Files ignored due to path filters (7)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/file.svgis excluded by!**/*.svgpublic/globe.svgis excluded by!**/*.svgpublic/next.svgis excluded by!**/*.svgpublic/vercel.svgis excluded by!**/*.svgpublic/window.svgis excluded by!**/*.svgsrc/app/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (60)
.gitignoredocs/mvp-backlog.mddocs/mvp-validation-checklist.mddocs/v1-quickstart.mddocs/v1-technical-guide.mdeslint.config.mjsnext.config.tspackage.jsonpostcss.config.mjsprisma/schema.prismaprisma/seed.tsprompts.mdreadme.mdsrc/app/clients/[clientId]/page.tsxsrc/app/clients/new/actions.tssrc/app/clients/new/client-form.tsxsrc/app/clients/new/page.tsxsrc/app/clients/new/submit-button.tsxsrc/app/clients/page.tsxsrc/app/globals.csssrc/app/layout.tsxsrc/app/login/actions.tssrc/app/login/page.tsxsrc/app/orders/[orderId]/page.tsxsrc/app/orders/new/actions.tssrc/app/orders/new/order-form.tsxsrc/app/orders/new/page.tsxsrc/app/orders/page.tsxsrc/app/page.tsxsrc/app/products/new/actions.tssrc/app/products/new/page.tsxsrc/app/products/new/product-form.tsxsrc/app/products/page.tsxsrc/app/suppliers/new/actions.tssrc/app/suppliers/new/page.tsxsrc/app/suppliers/new/supplier-form.tsxsrc/app/suppliers/page.tsxsrc/lib/auth/get-current-user.tssrc/lib/auth/require-active-user.tssrc/lib/auth/require-role.tssrc/lib/auth/roles.tssrc/lib/auth/types.tssrc/lib/db.tssrc/lib/env.tssrc/lib/supabase/browser.tssrc/lib/supabase/server.tssrc/lib/validations/client.tssrc/lib/validations/order.tssrc/lib/validations/product.tssrc/lib/validations/supplier.tssrc/modules/clients/create-client.tssrc/modules/clients/queries.tssrc/modules/orders/create-order.tssrc/modules/orders/queries.tssrc/modules/products/create-product.tssrc/modules/products/queries.tssrc/modules/suppliers/active-suppliers.tssrc/modules/suppliers/create-supplier.tssrc/modules/suppliers/queries.tstsconfig.json
Comment on lines
+13
to
+35
| function Field({ | ||
| label, | ||
| name, | ||
| placeholder, | ||
| type = "text", | ||
| }: { | ||
| label: string; | ||
| name: string; | ||
| placeholder?: string; | ||
| type?: "text" | "number"; | ||
| }) { | ||
| return ( | ||
| <label className="block text-sm font-medium text-stone-800"> | ||
| {label} | ||
| <input | ||
| name={name} | ||
| type={type} | ||
| placeholder={placeholder} | ||
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | ||
| /> | ||
| </label> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Update form field definitions to meet UI, styling, and accessibility guidelines.
As per coding guidelines:
- Form inputs must have 2px border, 8px border radius (
rounded-lg), and 12px 16px padding with focus state showing blue border (focus:border-indigo-500) and focus shadow (focus:ring-2 focus:ring-indigo-500). - Form labels must use
text-gray-700and be associated with inputs using thehtmlFor/idattributes. - Required form fields must include an asterisk (*) and have
aria-required="true".
✨ Proposed fixes for the Field component
function Field({
label,
name,
placeholder,
type = "text",
+ required = false,
}: {
label: string;
name: string;
placeholder?: string;
type?: "text" | "number";
+ required?: boolean;
}) {
return (
- <label className="block text-sm font-medium text-stone-800">
- {label}
+ <label htmlFor={name} className="block text-sm font-medium text-gray-700">
+ {label}{required && " *"}
<input
+ id={name}
name={name}
type={type}
placeholder={placeholder}
- className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950"
+ required={required}
+ aria-required={required ? "true" : undefined}
+ className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500"
/>
</label>
);
}📝 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
| function Field({ | |
| label, | |
| name, | |
| placeholder, | |
| type = "text", | |
| }: { | |
| label: string; | |
| name: string; | |
| placeholder?: string; | |
| type?: "text" | "number"; | |
| }) { | |
| return ( | |
| <label className="block text-sm font-medium text-stone-800"> | |
| {label} | |
| <input | |
| name={name} | |
| type={type} | |
| placeholder={placeholder} | |
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | |
| /> | |
| </label> | |
| ); | |
| } | |
| function Field({ | |
| label, | |
| name, | |
| placeholder, | |
| type = "text", | |
| required = false, | |
| }: { | |
| label: string; | |
| name: string; | |
| placeholder?: string; | |
| type?: "text" | "number"; | |
| required?: boolean; | |
| }) { | |
| return ( | |
| <label htmlFor={name} className="block text-sm font-medium text-gray-700"> | |
| {label}{required && " *"} | |
| <input | |
| id={name} | |
| name={name} | |
| type={type} | |
| placeholder={placeholder} | |
| required={required} | |
| aria-required={required ? "true" : undefined} | |
| className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500" | |
| /> | |
| </label> | |
| ); | |
| } |
🤖 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/app/products/new/product-form.tsx` around lines 13 - 35, Update the Field
component to associate each label and input using a stable id and htmlFor,
switch label styling to text-gray-700, and apply the required-field convention
by displaying an asterisk and aria-required="true" when appropriate. Replace the
input styling with a 2px border, rounded-lg corners, px-4 py-3 padding,
focus:border-indigo-500, and focus:ring-2 focus:ring-indigo-500, while
preserving the existing field types and values.
Source: Coding guidelines
Comment on lines
+46
to
+105
| {state.error ? ( | ||
| <p className="rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900"> | ||
| {state.error} | ||
| </p> | ||
| ) : null} | ||
|
|
||
| <div className="grid gap-6 md:grid-cols-2"> | ||
| <label className="block text-sm font-medium text-stone-800"> | ||
| Proveedor principal | ||
| <select | ||
| name="supplierId" | ||
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | ||
| required | ||
| > | ||
| <option value="">Selecciona un proveedor activo</option> | ||
| {suppliers.map((supplier) => ( | ||
| <option key={supplier.id} value={supplier.id}> | ||
| {supplier.commercialName} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </label> | ||
| <Field label="SKU" name="sku" placeholder="SKU-001" /> | ||
| <Field label="SKU proveedor" name="supplierSku" placeholder="SUP-7781" /> | ||
| <Field label="Nombre" name="name" placeholder="Filtro hidráulico" /> | ||
| <Field label="Marca" name="brand" placeholder="Atlas" /> | ||
| <Field label="Categoría" name="category" placeholder="Repuestos" /> | ||
| <Field label="Presentación" name="presentation" placeholder="Caja de 10" /> | ||
| <Field label="Unidad" name="unit" placeholder="unidad" /> | ||
| <Field label="Precio base" name="basePrice" type="number" placeholder="250" /> | ||
| <Field label="Costo estimado" name="estimatedCost" type="number" placeholder="170" /> | ||
| <Field label="Lead time (días)" name="leadTimeDays" type="number" placeholder="30" /> | ||
| <Field label="Cantidad mínima" name="minimumOrderQuantity" type="number" placeholder="1" /> | ||
| <label className="block text-sm font-medium text-stone-800"> | ||
| Estado | ||
| <select | ||
| name="status" | ||
| defaultValue="ACTIVE" | ||
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | ||
| > | ||
| <option value="ACTIVE">Activo</option> | ||
| <option value="INACTIVE">Inactivo</option> | ||
| </select> | ||
| </label> | ||
| </div> | ||
|
|
||
| <div className="flex items-center justify-end gap-3"> | ||
| <Link | ||
| href="/products" | ||
| className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-stone-50" | ||
| > | ||
| Cancelar | ||
| </Link> | ||
| <button | ||
| type="submit" | ||
| className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800" | ||
| > | ||
| Guardar producto | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply UI guidelines to form elements, error messages, and buttons.
As per coding guidelines:
- Form error messages must use red text (
text-red-500) and red border (border-red-500). Dynamic alert messages must userole="alert". - All form inputs (including native
selectfields) must use 2px borders, 8px radius (rounded-lg), and blue focus states. - Required fields (
supplierId,sku,name,basePrice,status) must be marked with an asterisk and includearia-required="true". - Form labels must use
text-gray-700and reference inputs withhtmlFor. - Primary buttons must have a blue background (
bg-indigo-600), white text, padding 12px 24px (px-6 py-3), andfocus:ring-2 focus:ring-indigo-500. - Secondary buttons must have a transparent background with blue border and blue text (
border-indigo-600 text-indigo-600), plusfocus:ring-2 focus:ring-indigo-500.
✨ Proposed form updates
- {state.error ? (
- <p className="rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900">
- {state.error}
- </p>
- ) : null}
+ {state.error ? (
+ <p className="rounded-lg border-2 border-red-500 bg-red-50 px-4 py-3 text-sm text-red-500" role="alert">
+ {state.error}
+ </p>
+ ) : null}
<div className="grid gap-6 md:grid-cols-2">
- <label className="block text-sm font-medium text-stone-800">
- Proveedor principal
+ <label htmlFor="supplierId" className="block text-sm font-medium text-gray-700">
+ Proveedor principal *
<select
+ id="supplierId"
name="supplierId"
- className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950"
+ className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500"
required
+ aria-required="true"
>
<option value="">Selecciona un proveedor activo</option>
{suppliers.map((supplier) => (
<option key={supplier.id} value={supplier.id}>
{supplier.commercialName}
</option>
))}
</select>
</label>
- <Field label="SKU" name="sku" placeholder="SKU-001" />
+ <Field label="SKU" name="sku" placeholder="SKU-001" required />
<Field label="SKU proveedor" name="supplierSku" placeholder="SUP-7781" />
- <Field label="Nombre" name="name" placeholder="Filtro hidráulico" />
+ <Field label="Nombre" name="name" placeholder="Filtro hidráulico" required />
<Field label="Marca" name="brand" placeholder="Atlas" />
<Field label="Categoría" name="category" placeholder="Repuestos" />
<Field label="Presentación" name="presentation" placeholder="Caja de 10" />
<Field label="Unidad" name="unit" placeholder="unidad" />
- <Field label="Precio base" name="basePrice" type="number" placeholder="250" />
+ <Field label="Precio base" name="basePrice" type="number" placeholder="250" required />
<Field label="Costo estimado" name="estimatedCost" type="number" placeholder="170" />
<Field label="Lead time (días)" name="leadTimeDays" type="number" placeholder="30" />
<Field label="Cantidad mínima" name="minimumOrderQuantity" type="number" placeholder="1" />
- <label className="block text-sm font-medium text-stone-800">
- Estado
+ <label htmlFor="status" className="block text-sm font-medium text-gray-700">
+ Estado *
<select
+ id="status"
name="status"
defaultValue="ACTIVE"
- className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950"
+ required
+ aria-required="true"
+ className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500"
>
<option value="ACTIVE">Activo</option>
<option value="INACTIVE">Inactivo</option>
</select>
</label>
</div>
<div className="flex items-center justify-end gap-3">
<Link
href="/products"
- className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-stone-50"
+ className="rounded-lg border border-indigo-600 bg-transparent px-6 py-3 text-sm font-medium text-indigo-600 transition hover:bg-indigo-50 focus:outline-none focus:ring-2 focus:ring-indigo-500"
>
Cancelar
</Link>
<button
type="submit"
- className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800"
+ className="rounded-lg bg-indigo-600 px-6 py-3 text-sm font-medium text-white transition hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500"
>
Guardar producto
</button>
</div>📝 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
| {state.error ? ( | |
| <p className="rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900"> | |
| {state.error} | |
| </p> | |
| ) : null} | |
| <div className="grid gap-6 md:grid-cols-2"> | |
| <label className="block text-sm font-medium text-stone-800"> | |
| Proveedor principal | |
| <select | |
| name="supplierId" | |
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | |
| required | |
| > | |
| <option value="">Selecciona un proveedor activo</option> | |
| {suppliers.map((supplier) => ( | |
| <option key={supplier.id} value={supplier.id}> | |
| {supplier.commercialName} | |
| </option> | |
| ))} | |
| </select> | |
| </label> | |
| <Field label="SKU" name="sku" placeholder="SKU-001" /> | |
| <Field label="SKU proveedor" name="supplierSku" placeholder="SUP-7781" /> | |
| <Field label="Nombre" name="name" placeholder="Filtro hidráulico" /> | |
| <Field label="Marca" name="brand" placeholder="Atlas" /> | |
| <Field label="Categoría" name="category" placeholder="Repuestos" /> | |
| <Field label="Presentación" name="presentation" placeholder="Caja de 10" /> | |
| <Field label="Unidad" name="unit" placeholder="unidad" /> | |
| <Field label="Precio base" name="basePrice" type="number" placeholder="250" /> | |
| <Field label="Costo estimado" name="estimatedCost" type="number" placeholder="170" /> | |
| <Field label="Lead time (días)" name="leadTimeDays" type="number" placeholder="30" /> | |
| <Field label="Cantidad mínima" name="minimumOrderQuantity" type="number" placeholder="1" /> | |
| <label className="block text-sm font-medium text-stone-800"> | |
| Estado | |
| <select | |
| name="status" | |
| defaultValue="ACTIVE" | |
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | |
| > | |
| <option value="ACTIVE">Activo</option> | |
| <option value="INACTIVE">Inactivo</option> | |
| </select> | |
| </label> | |
| </div> | |
| <div className="flex items-center justify-end gap-3"> | |
| <Link | |
| href="/products" | |
| className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-stone-50" | |
| > | |
| Cancelar | |
| </Link> | |
| <button | |
| type="submit" | |
| className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800" | |
| > | |
| Guardar producto | |
| </button> | |
| </div> | |
| {state.error ? ( | |
| <p className="rounded-lg border-2 border-red-500 bg-red-50 px-4 py-3 text-sm text-red-500" role="alert"> | |
| {state.error} | |
| </p> | |
| ) : null} | |
| <div className="grid gap-6 md:grid-cols-2"> | |
| <label htmlFor="supplierId" className="block text-sm font-medium text-gray-700"> | |
| Proveedor principal * | |
| <select | |
| id="supplierId" | |
| name="supplierId" | |
| className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500" | |
| required | |
| aria-required="true" | |
| > | |
| <option value="">Selecciona un proveedor activo</option> | |
| {suppliers.map((supplier) => ( | |
| <option key={supplier.id} value={supplier.id}> | |
| {supplier.commercialName} | |
| </option> | |
| ))} | |
| </select> | |
| </label> | |
| <Field label="SKU" name="sku" placeholder="SKU-001" required /> | |
| <Field label="SKU proveedor" name="supplierSku" placeholder="SUP-7781" /> | |
| <Field label="Nombre" name="name" placeholder="Filtro hidráulico" required /> | |
| <Field label="Marca" name="brand" placeholder="Atlas" /> | |
| <Field label="Categoría" name="category" placeholder="Repuestos" /> | |
| <Field label="Presentación" name="presentation" placeholder="Caja de 10" /> | |
| <Field label="Unidad" name="unit" placeholder="unidad" /> | |
| <Field label="Precio base" name="basePrice" type="number" placeholder="250" required /> | |
| <Field label="Costo estimado" name="estimatedCost" type="number" placeholder="170" /> | |
| <Field label="Lead time (días)" name="leadTimeDays" type="number" placeholder="30" /> | |
| <Field label="Cantidad mínima" name="minimumOrderQuantity" type="number" placeholder="1" /> | |
| <label htmlFor="status" className="block text-sm font-medium text-gray-700"> | |
| Estado * | |
| <select | |
| id="status" | |
| name="status" | |
| defaultValue="ACTIVE" | |
| required | |
| aria-required="true" | |
| className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500" | |
| > | |
| <option value="ACTIVE">Activo</option> | |
| <option value="INACTIVE">Inactivo</option> | |
| </select> | |
| </label> | |
| </div> | |
| <div className="flex items-center justify-end gap-3"> | |
| <Link | |
| href="/products" | |
| className="rounded-lg border border-indigo-600 bg-transparent px-6 py-3 text-sm font-medium text-indigo-600 transition hover:bg-indigo-50 focus:outline-none focus:ring-2 focus:ring-indigo-500" | |
| > | |
| Cancelar | |
| </Link> | |
| <button | |
| type="submit" | |
| className="rounded-lg bg-indigo-600 px-6 py-3 text-sm font-medium text-white transition hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500" | |
| > | |
| Guardar producto | |
| </button> | |
| </div> |
🤖 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/app/products/new/product-form.tsx` around lines 46 - 105, Update the
product form around the error block, supplier/status selects, Field components,
and action buttons to follow the UI guidelines: use role="alert" with
text-red-500 and border-red-500 for state.error; apply 2px borders, rounded-lg,
and blue focus states to every input/select. Mark supplierId, sku, name,
basePrice, and status as required with visible asterisks and
aria-required="true", and ensure labels use text-gray-700 with htmlFor
references. Style Guardar producto as the indigo primary button and Cancelar as
the transparent indigo secondary action, both with the specified padding and
focus rings.
Source: Coding guidelines
Comment on lines
+29
to
+80
| function Field({ | ||
| label, | ||
| name, | ||
| placeholder, | ||
| type = "text", | ||
| }: { | ||
| label: string; | ||
| name: string; | ||
| placeholder?: string; | ||
| type?: "text" | "email" | "number"; | ||
| }) { | ||
| return ( | ||
| <label className="block text-sm font-medium text-stone-800"> | ||
| {label} | ||
| <input | ||
| name={name} | ||
| type={type} | ||
| placeholder={placeholder} | ||
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | ||
| /> | ||
| </label> | ||
| ); | ||
| } | ||
|
|
||
| function SelectField({ | ||
| label, | ||
| name, | ||
| defaultValue, | ||
| options, | ||
| }: { | ||
| label: string; | ||
| name: string; | ||
| defaultValue: string; | ||
| options: ReadonlyArray<{ value: string; label: string }>; | ||
| }) { | ||
| return ( | ||
| <label className="block text-sm font-medium text-stone-800"> | ||
| {label} | ||
| <select | ||
| name={name} | ||
| defaultValue={defaultValue} | ||
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | ||
| > | ||
| {options.map((option) => ( | ||
| <option key={option.value} value={option.value}> | ||
| {option.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </label> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Update form field definitions to meet UI, styling, and accessibility guidelines.
As per coding guidelines:
- Form inputs must have 2px border, 8px border radius (
rounded-lg), and 12px 16px padding with focus state showing blue border (focus:border-indigo-500) and focus shadow (focus:ring-2 focus:ring-indigo-500). - Form labels must use
text-gray-700and be associated with inputs using thehtmlFor/idattributes. - Required form fields must include an asterisk (*) and have
aria-required="true".
✨ Proposed fixes for Field and SelectField
function Field({
label,
name,
placeholder,
type = "text",
+ required = false,
}: {
label: string;
name: string;
placeholder?: string;
type?: "text" | "email" | "number";
+ required?: boolean;
}) {
return (
- <label className="block text-sm font-medium text-stone-800">
- {label}
+ <label htmlFor={name} className="block text-sm font-medium text-gray-700">
+ {label}{required && " *"}
<input
+ id={name}
name={name}
type={type}
placeholder={placeholder}
- className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950"
+ required={required}
+ aria-required={required ? "true" : undefined}
+ className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500"
/>
</label>
);
}
function SelectField({
label,
name,
defaultValue,
options,
+ required = false,
}: {
label: string;
name: string;
defaultValue: string;
options: ReadonlyArray<{ value: string; label: string }>;
+ required?: boolean;
}) {
return (
- <label className="block text-sm font-medium text-stone-800">
- {label}
+ <label htmlFor={name} className="block text-sm font-medium text-gray-700">
+ {label}{required && " *"}
<select
+ id={name}
name={name}
defaultValue={defaultValue}
- className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950"
+ required={required}
+ aria-required={required ? "true" : undefined}
+ className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500"
>
{options.map((option) => (
<option key={option.value} value={option.value}>📝 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
| function Field({ | |
| label, | |
| name, | |
| placeholder, | |
| type = "text", | |
| }: { | |
| label: string; | |
| name: string; | |
| placeholder?: string; | |
| type?: "text" | "email" | "number"; | |
| }) { | |
| return ( | |
| <label className="block text-sm font-medium text-stone-800"> | |
| {label} | |
| <input | |
| name={name} | |
| type={type} | |
| placeholder={placeholder} | |
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | |
| /> | |
| </label> | |
| ); | |
| } | |
| function SelectField({ | |
| label, | |
| name, | |
| defaultValue, | |
| options, | |
| }: { | |
| label: string; | |
| name: string; | |
| defaultValue: string; | |
| options: ReadonlyArray<{ value: string; label: string }>; | |
| }) { | |
| return ( | |
| <label className="block text-sm font-medium text-stone-800"> | |
| {label} | |
| <select | |
| name={name} | |
| defaultValue={defaultValue} | |
| className="mt-2 w-full rounded-2xl border border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-stone-950" | |
| > | |
| {options.map((option) => ( | |
| <option key={option.value} value={option.value}> | |
| {option.label} | |
| </option> | |
| ))} | |
| </select> | |
| </label> | |
| ); | |
| } | |
| function Field({ | |
| label, | |
| name, | |
| placeholder, | |
| type = "text", | |
| required = false, | |
| }: { | |
| label: string; | |
| name: string; | |
| placeholder?: string; | |
| type?: "text" | "email" | "number"; | |
| required?: boolean; | |
| }) { | |
| return ( | |
| <label htmlFor={name} className="block text-sm font-medium text-gray-700"> | |
| {label} | |
| {required && " *"} | |
| <input | |
| id={name} | |
| name={name} | |
| type={type} | |
| placeholder={placeholder} | |
| required={required} | |
| aria-required={required ? "true" : undefined} | |
| className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500" | |
| /> | |
| </label> | |
| ); | |
| } | |
| function SelectField({ | |
| label, | |
| name, | |
| defaultValue, | |
| options, | |
| required = false, | |
| }: { | |
| label: string; | |
| name: string; | |
| defaultValue: string; | |
| options: ReadonlyArray<{ value: string; label: string }>; | |
| required?: boolean; | |
| }) { | |
| return ( | |
| <label htmlFor={name} className="block text-sm font-medium text-gray-700"> | |
| {label} | |
| {required && " *"} | |
| <select | |
| id={name} | |
| name={name} | |
| defaultValue={defaultValue} | |
| required={required} | |
| aria-required={required ? "true" : undefined} | |
| className="mt-2 w-full rounded-lg border-2 border-stone-300 px-4 py-3 text-sm text-stone-950 outline-none transition focus:border-indigo-500 focus:ring-2 focus:ring-indigo-500" | |
| > | |
| {options.map((option) => ( | |
| <option key={option.value} value={option.value}> | |
| {option.label} | |
| </option> | |
| ))} | |
| </select> | |
| </label> | |
| ); | |
| } |
🤖 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/app/suppliers/new/supplier-form.tsx` around lines 29 - 80, Update the
Field and SelectField components so labels use text-gray-700 and associate with
controls via matching htmlFor/id values. Apply the required 2px border,
rounded-lg radius, px-4 py-3 padding, focus:border-indigo-500, and focus:ring-2
focus:ring-indigo-500 classes to both controls; add required-field indicators
and aria-required="true" where applicable, preserving existing field props and
option behavior.
Source: Coding guidelines
Comment on lines
+87
to
+121
| {state.error ? ( | ||
| <p className="rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900"> | ||
| {state.error} | ||
| </p> | ||
| ) : null} | ||
|
|
||
| <div className="grid gap-6 md:grid-cols-2"> | ||
| <Field label="Nombre comercial" name="commercialName" placeholder="Ocean Parts" /> | ||
| <Field label="Razón social" name="legalName" placeholder="Ocean Parts Ltd." /> | ||
| <SelectField label="Tipo de proveedor" name="supplierType" defaultValue="OTHER" options={supplierTypeOptions} /> | ||
| <SelectField label="Estado" name="status" defaultValue="UNDER_REVIEW" options={statusOptions} /> | ||
| <Field label="País" name="country" placeholder="China" /> | ||
| <Field label="Ciudad" name="city" placeholder="Shenzhen" /> | ||
| <Field label="Email" name="email" type="email" placeholder="sales@supplier.com" /> | ||
| <Field label="Teléfono" name="phone" placeholder="+86 755 5555 5555" /> | ||
| <Field label="Condiciones de pago" name="paymentTerms" placeholder="50% anticipado / 50% embarque" /> | ||
| <Field label="Moneda" name="currency" placeholder="USD" /> | ||
| <Field label="Incoterm" name="incoterm" placeholder="FOB" /> | ||
| <Field label="Lead time promedio (días)" name="averageLeadTimeDays" type="number" placeholder="30" /> | ||
| </div> | ||
|
|
||
| <div className="flex items-center justify-end gap-3"> | ||
| <Link | ||
| href="/suppliers" | ||
| className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-stone-50" | ||
| > | ||
| Cancelar | ||
| </Link> | ||
| <button | ||
| type="submit" | ||
| className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800" | ||
| > | ||
| Guardar proveedor | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply UI guidelines to form elements, error messages, and buttons.
As per coding guidelines:
- Form error messages must use red text (
text-red-500) and red border (border-red-500). Dynamic alert messages must userole="alert". - Required fields (
commercialName,supplierType,status) must be marked appropriately. - Primary buttons must have a blue background (
bg-indigo-600), white text, padding 12px 24px (px-6 py-3), andfocus:ring-2 focus:ring-indigo-500. - Secondary buttons must have a transparent background with blue border and blue text (
border-indigo-600 text-indigo-600), plusfocus:ring-2 focus:ring-indigo-500.
✨ Proposed form updates
- {state.error ? (
- <p className="rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900">
- {state.error}
- </p>
- ) : null}
+ {state.error ? (
+ <p className="rounded-lg border-2 border-red-500 bg-red-50 px-4 py-3 text-sm text-red-500" role="alert">
+ {state.error}
+ </p>
+ ) : null}
<div className="grid gap-6 md:grid-cols-2">
- <Field label="Nombre comercial" name="commercialName" placeholder="Ocean Parts" />
+ <Field label="Nombre comercial" name="commercialName" placeholder="Ocean Parts" required />
<Field label="Razón social" name="legalName" placeholder="Ocean Parts Ltd." />
- <SelectField label="Tipo de proveedor" name="supplierType" defaultValue="OTHER" options={supplierTypeOptions} />
- <SelectField label="Estado" name="status" defaultValue="UNDER_REVIEW" options={statusOptions} />
+ <SelectField label="Tipo de proveedor" name="supplierType" defaultValue="OTHER" options={supplierTypeOptions} required />
+ <SelectField label="Estado" name="status" defaultValue="UNDER_REVIEW" options={statusOptions} required />
<Field label="País" name="country" placeholder="China" />
<Field label="Ciudad" name="city" placeholder="Shenzhen" />
<Field label="Email" name="email" type="email" placeholder="sales@supplier.com" />
<Field label="Teléfono" name="phone" placeholder="+86 755 5555 5555" />
<Field label="Condiciones de pago" name="paymentTerms" placeholder="50% anticipado / 50% embarque" />
<Field label="Moneda" name="currency" placeholder="USD" />
<Field label="Incoterm" name="incoterm" placeholder="FOB" />
<Field label="Lead time promedio (días)" name="averageLeadTimeDays" type="number" placeholder="30" />
</div>
<div className="flex items-center justify-end gap-3">
<Link
href="/suppliers"
- className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-stone-50"
+ className="rounded-lg border border-indigo-600 bg-transparent px-6 py-3 text-sm font-medium text-indigo-600 transition hover:bg-indigo-50 focus:outline-none focus:ring-2 focus:ring-indigo-500"
>
Cancelar
</Link>
<button
type="submit"
- className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800"
+ className="rounded-lg bg-indigo-600 px-6 py-3 text-sm font-medium text-white transition hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500"
>
Guardar proveedor
</button>
</div>📝 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
| {state.error ? ( | |
| <p className="rounded-2xl border border-rose-200 bg-rose-50 px-4 py-3 text-sm text-rose-900"> | |
| {state.error} | |
| </p> | |
| ) : null} | |
| <div className="grid gap-6 md:grid-cols-2"> | |
| <Field label="Nombre comercial" name="commercialName" placeholder="Ocean Parts" /> | |
| <Field label="Razón social" name="legalName" placeholder="Ocean Parts Ltd." /> | |
| <SelectField label="Tipo de proveedor" name="supplierType" defaultValue="OTHER" options={supplierTypeOptions} /> | |
| <SelectField label="Estado" name="status" defaultValue="UNDER_REVIEW" options={statusOptions} /> | |
| <Field label="País" name="country" placeholder="China" /> | |
| <Field label="Ciudad" name="city" placeholder="Shenzhen" /> | |
| <Field label="Email" name="email" type="email" placeholder="sales@supplier.com" /> | |
| <Field label="Teléfono" name="phone" placeholder="+86 755 5555 5555" /> | |
| <Field label="Condiciones de pago" name="paymentTerms" placeholder="50% anticipado / 50% embarque" /> | |
| <Field label="Moneda" name="currency" placeholder="USD" /> | |
| <Field label="Incoterm" name="incoterm" placeholder="FOB" /> | |
| <Field label="Lead time promedio (días)" name="averageLeadTimeDays" type="number" placeholder="30" /> | |
| </div> | |
| <div className="flex items-center justify-end gap-3"> | |
| <Link | |
| href="/suppliers" | |
| className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-stone-50" | |
| > | |
| Cancelar | |
| </Link> | |
| <button | |
| type="submit" | |
| className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800" | |
| > | |
| Guardar proveedor | |
| </button> | |
| </div> | |
| {state.error ? ( | |
| <p className="rounded-lg border-2 border-red-500 bg-red-50 px-4 py-3 text-sm text-red-500" role="alert"> | |
| {state.error} | |
| </p> | |
| ) : null} | |
| <div className="grid gap-6 md:grid-cols-2"> | |
| <Field label="Nombre comercial" name="commercialName" placeholder="Ocean Parts" required /> | |
| <Field label="Razón social" name="legalName" placeholder="Ocean Parts Ltd." /> | |
| <SelectField label="Tipo de proveedor" name="supplierType" defaultValue="OTHER" options={supplierTypeOptions} required /> | |
| <SelectField label="Estado" name="status" defaultValue="UNDER_REVIEW" options={statusOptions} required /> | |
| <Field label="País" name="country" placeholder="China" /> | |
| <Field label="Ciudad" name="city" placeholder="Shenzhen" /> | |
| <Field label="Email" name="email" type="email" placeholder="sales@supplier.com" /> | |
| <Field label="Teléfono" name="phone" placeholder="+86 755 5555 5555" /> | |
| <Field label="Condiciones de pago" name="paymentTerms" placeholder="50% anticipado / 50% embarque" /> | |
| <Field label="Moneda" name="currency" placeholder="USD" /> | |
| <Field label="Incoterm" name="incoterm" placeholder="FOB" /> | |
| <Field label="Lead time promedio (días)" name="averageLeadTimeDays" type="number" placeholder="30" /> | |
| </div> | |
| <div className="flex items-center justify-end gap-3"> | |
| <Link | |
| href="/suppliers" | |
| className="rounded-lg border border-indigo-600 bg-transparent px-6 py-3 text-sm font-medium text-indigo-600 transition hover:bg-indigo-50 focus:outline-none focus:ring-2 focus:ring-indigo-500" | |
| > | |
| Cancelar | |
| </Link> | |
| <button | |
| type="submit" | |
| className="rounded-lg bg-indigo-600 px-6 py-3 text-sm font-medium text-white transition hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-indigo-500" | |
| > | |
| Guardar proveedor | |
| </button> | |
| </div> |
🤖 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/app/suppliers/new/supplier-form.tsx` around lines 87 - 121, Update the
supplier form’s error message to use text-red-500 and border-red-500 with
role="alert"; mark commercialName, supplierType, and status as required through
the Field/SelectField props. Update the submit button to use bg-indigo-600, px-6
py-3, and focus:ring-2 focus:ring-indigo-500, and update the Cancel Link with a
transparent background, border-indigo-600 text-indigo-600, and the same focus
ring.
Source: Coding guidelines
Comment on lines
+36
to
+48
| <Link | ||
| href="/" | ||
| className="rounded-full border border-stone-300 px-5 py-3 text-sm font-medium text-stone-700 transition hover:bg-white" | ||
| > | ||
| Panel | ||
| </Link> | ||
| <Link | ||
| href="/suppliers/new" | ||
| className="rounded-full bg-stone-950 px-5 py-3 text-sm font-medium text-white transition hover:bg-stone-800" | ||
| > | ||
| Nuevo proveedor | ||
| </Link> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply focus states and Primary Blue color to interactive actions. As per coding guidelines, all buttons/links must implement focus state with focus:ring-2 focus:ring-indigo-500 for keyboard accessibility, and Primary Blue (indigo-600) should be used for main actions and links instead of stone.
src/app/suppliers/page.tsx#L36-L48: Update the "Nuevo proveedor" and "Panel" links with focus ring classes, and usebg-indigo-600 hover:bg-indigo-700for the primary action.src/app/products/page.tsx#L33-L39: Update the "Nuevo producto" and "Panel" links with focus ring classes, and usebg-indigo-600 hover:bg-indigo-700for the primary action.src/app/orders/[orderId]/page.tsx#L33-L39: Update the "Volver a órdenes" and "Panel" links with focus ring classes, and usetext-indigo-600instead oftext-stone-600.
📍 Affects 3 files
src/app/suppliers/page.tsx#L36-L48(this comment)src/app/products/page.tsx#L33-L39src/app/orders/[orderId]/page.tsx#L33-L39
🤖 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/app/suppliers/page.tsx` around lines 36 - 48, Update the links in
src/app/suppliers/page.tsx lines 36-48 and src/app/products/page.tsx lines 33-39
to add focus:ring-2 focus:ring-indigo-500 to both actions and use bg-indigo-600
hover:bg-indigo-700 for each primary creation link. Update the links in
src/app/orders/[orderId]/page.tsx lines 33-39 with the same focus ring classes,
and change the “Volver a órdenes” link color from stone-600 to text-indigo-600;
preserve the existing link roles and layout.
Source: Coding guidelines
Comment on lines
+57
to
+84
| <section className="rounded-[2rem] bg-white shadow-sm"> | ||
| <div className="grid grid-cols-[2fr_1fr_1fr_1fr] gap-4 border-b border-stone-200 px-6 py-4 text-xs font-semibold uppercase tracking-[0.2em] text-stone-500"> | ||
| <span>Proveedor</span> | ||
| <span>Estado</span> | ||
| <span>País</span> | ||
| <span>Lead time</span> | ||
| </div> | ||
|
|
||
| {suppliers.length === 0 ? ( | ||
| <div className="px-6 py-10 text-sm text-stone-600">Todavía no hay proveedores registrados.</div> | ||
| ) : ( | ||
| <ul> | ||
| {suppliers.map((supplier) => ( | ||
| <li key={supplier.id} className="grid grid-cols-[2fr_1fr_1fr_1fr] gap-4 border-b border-stone-100 px-6 py-5 text-sm text-stone-700 last:border-b-0"> | ||
| <span> | ||
| <span className="block font-medium text-stone-950">{supplier.commercialName}</span> | ||
| <span className="block text-stone-500">{supplier.email ?? supplier.phone ?? "Sin contacto"}</span> | ||
| </span> | ||
| <span>{formatEnumLabel(supplier.status)}</span> | ||
| <span>{supplier.country ?? "No informado"}</span> | ||
| <span> | ||
| {supplier.averageLeadTimeDays ? `${supplier.averageLeadTimeDays} días` : "No informado"} | ||
| </span> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| )} | ||
| </section> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add role="table" and nested ARIA roles to data lists. As per coding guidelines, data tables must use role="table" to communicate semantic meaning to screen readers. To ensure valid HTML/ARIA semantics, also apply role="rowgroup", role="row", role="columnheader", and role="cell" to the corresponding nested elements.
src/app/suppliers/page.tsx#L57-L84: Apply table ARIA roles to the<section>, grid wrappers, and list items.src/app/products/page.tsx#L48-L74: Apply table ARIA roles to the<section>, grid wrappers, and list items.src/app/orders/[orderId]/page.tsx#L64-L84: Apply table ARIA roles to the<section>, grid wrappers, and list items.
📍 Affects 3 files
src/app/suppliers/page.tsx#L57-L84(this comment)src/app/products/page.tsx#L48-L74src/app/orders/[orderId]/page.tsx#L64-L84
🤖 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/app/suppliers/page.tsx` around lines 57 - 84, Apply table semantics
consistently in the suppliers section at src/app/suppliers/page.tsx lines 57-84,
the products section at src/app/products/page.tsx lines 48-74, and the order
details section at src/app/orders/[orderId]/page.tsx lines 64-84: add
role="table" to each section, role="rowgroup" to the list/header containers,
role="row" to each grid wrapper and list item, role="columnheader" to header
cells, and role="cell" to data cells.
Source: Coding guidelines
Comment on lines
+7
to
+27
| export const getCurrentUser = cache(async (): Promise<CurrentUser | null> => { | ||
| const supabase = await createSupabaseServerClient(); | ||
| const { | ||
| data: { user }, | ||
| } = await supabase.auth.getUser(); | ||
|
|
||
| if (!user) { | ||
| return null; | ||
| } | ||
|
|
||
| const profile = await db.userProfile.findUnique({ | ||
| where: { id: user.id }, | ||
| }); | ||
|
|
||
| return { | ||
| authUser: user, | ||
| profile, | ||
| role: profile?.role ?? null, | ||
| isActive: profile?.isActive ?? false, | ||
| }; | ||
| }); No newline at end of file |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File map for src/lib/auth/get-current-user.ts and call sites\n'
git ls-files 'src/lib/auth/*' 'src/**/*require*' 'src/**/*user*' | sed -n '1,200p'
printf '\n## Outline: src/lib/auth/get-current-user.ts\n'
ast-grep outline src/lib/auth/get-current-user.ts --view expanded || true
printf '\n## Search for getCurrentUser / requireActiveUser / requireRole usages\n'
rg -n "getCurrentUser|requireActiveUser|requireRole" src || true
printf '\n## Read target file with line numbers\n'
cat -n src/lib/auth/get-current-user.ts | sed -n '1,120p'Repository: LIDR-academy/AI4Devs-finalproject
Length of output: 4690
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Read auth wrapper files\n'
cat -n src/lib/auth/require-active-user.ts | sed -n '1,120p'
printf '\n---\n'
cat -n src/lib/auth/require-role.ts | sed -n '1,120p'
printf '\n---\n'
cat -n src/lib/supabase/server.ts | sed -n '1,220p'
printf '\n---\n'
cat -n src/lib/auth/types.ts | sed -n '1,160p'
printf '\n## Search for error handling around getCurrentUser callers\n'
rg -n "try \{|catch \(|redirect\(|notFound\(|console\.error|auth\.getUser|userProfile\.findUnique" src/lib src/app || trueRepository: LIDR-academy/AI4Devs-finalproject
Length of output: 3442
🌐 Web query:
Supabase auth.getUser return shape error field docs
💡 Result:
In the Supabase JavaScript client, the auth.getUser method returns an object containing both data and error fields [1][2][3]. It is critical to check the error field to determine if the operation succeeded, as relying solely on the user property can lead to incorrect assumptions [2][3]. The return structure follows this pattern: { data: { user: User | null }, error: AuthError | null } Key points regarding this return shape: 1. Handling Success and Failure: When the request is successful, the error field will be null, and the data object will contain the authenticated user [4]. If there is an issue—such as a missing session—the error field will be populated with an AuthError, and the user property within data will typically be null [2][3]. 2. Common Error Cases: A frequent source of confusion is receiving an AuthSessionMissingError (with a status of 400) when calling auth.getUser in environments like Next.js middleware or server-side functions if the session is not properly established or provided via the appropriate headers [2][5][3]. 3. Recommended Destructuring: To safely access the user, you should destructure the response and verify the absence of an error: const { data: { user }, error } = await supabase.auth.getUser; if (error) { // Handle the error (e.g., redirect or return early) console.error('Error fetching user:', error.message); } else { console.log('Authenticated user:', user); } This pattern ensures that you correctly distinguish between a successful request, a network or auth-level error, and a case where a user is simply not logged in [3][4].
Citations:
- 1: v2 docs show auth.getUser returning user. New return is data so data.user is needed. supabase/supabase#8394
- 2: auth.getUser() returns null user and AuthSessionMissingError in supabase-js@2.42.5 supabase/supabase-js#1024
- 3: "supabase.auth.getUser()" is returning a null user value in nextjs middleware.ts using @supabase/supabase-js @supabase/ssr only supabase/supabase#24194
- 4: https://github.com/supabase/auth-js/blob/master/src/lib/types.ts
- 5: fix:
getUserreturns null if there is no session supabase/auth-js#876
Guard the Prisma lookup in getCurrentUser. supabase.auth.getUser() already reports auth-service/session issues via error, but db.userProfile.findUnique(...) can still throw and bubble through requireActiveUser/requireRole, taking down every protected route. Catch that failure and return null so the guard fails safe.
🤖 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/lib/auth/get-current-user.ts` around lines 7 - 27, Wrap the
db.userProfile.findUnique call in getCurrentUser with error handling, and return
null when the Prisma lookup fails. Leave the existing unauthenticated and
successful profile mapping behavior unchanged so requireActiveUser and
requireRole fail safely.
Comment on lines
+15
to
+19
| setAll(cookiesToSet) { | ||
| cookiesToSet.forEach(({ name, options, value }) => { | ||
| cookieStore.set(name, value, options); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Prevent Server Component crashes on implicit token refreshes.
When createSupabaseServerClient is used inside a Server Component (e.g., resolving the current user in a layout), any background session refresh triggered by supabase.auth.getUser() will invoke this setAll method. Next.js strictly forbids mutating cookies during the render phase and will throw an error, crashing the application.
Wrap the cookie mutation in a try/catch block to safely swallow this error. (Session cookie refreshes should be handled by a dedicated Next.js middleware).
🔒️ Proposed fix
- setAll(cookiesToSet) {
- cookiesToSet.forEach(({ name, options, value }) => {
- cookieStore.set(name, value, options);
- });
- },
+ setAll(cookiesToSet) {
+ try {
+ cookiesToSet.forEach(({ name, options, value }) => {
+ cookieStore.set(name, value, options);
+ });
+ } catch {
+ // The `setAll` method was called from a Server Component.
+ // This can be ignored if you have middleware refreshing user sessions.
+ }
+ },📝 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
| setAll(cookiesToSet) { | |
| cookiesToSet.forEach(({ name, options, value }) => { | |
| cookieStore.set(name, value, options); | |
| }); | |
| }, | |
| setAll(cookiesToSet) { | |
| try { | |
| cookiesToSet.forEach(({ name, options, value }) => { | |
| cookieStore.set(name, value, options); | |
| }); | |
| } catch { | |
| // The `setAll` method was called from a Server Component. | |
| // This can be ignored if you have middleware refreshing user sessions. | |
| } | |
| }, |
🤖 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/lib/supabase/server.ts` around lines 15 - 19, Update the setAll callback
in createSupabaseServerClient to wrap cookieStore.set mutations in a try/catch,
swallowing render-phase cookie mutation errors while preserving normal cookie
updates. Leave session refresh handling to the existing middleware.
Comment on lines
+13
to
+15
| estimatedCost: z.union([z.coerce.number().nonnegative(), z.literal("")]).transform((value) => (value === "" ? undefined : value)), | ||
| leadTimeDays: z.union([z.coerce.number().int().positive(), z.literal("")]).transform((value) => (value === "" ? undefined : value)), | ||
| minimumOrderQuantity: z.union([z.coerce.number().int().positive(), z.literal("")]).transform((value) => (value === "" ? 1 : value)), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reorder z.union to prevent empty inputs from being incorrectly saved as 0.
Zod’s z.coerce.number() natively coerces an empty string "" into 0. Because z.union evaluates left-to-right, an empty input sent to estimatedCost will be coerced to 0, successfully pass the .nonnegative() check, and return 0 instead of falling back to the z.literal("") branch. This will incorrectly save empty costs as $0 and skew downstream margin calculations.
Moving z.literal("") to the start of the union safely intercepts empty strings before they can be coerced into numbers.
src/lib/validations/product.ts#L13-L15: Reorder the unions soz.literal("")comes first:estimatedCost: z.union([z.literal(""), z.coerce.number().nonnegative()]).transform((value) => (value === "" ? undefined : value)), leadTimeDays: z.union([z.literal(""), z.coerce.number().int().positive()]).transform((value) => (value === "" ? undefined : value)), minimumOrderQuantity: z.union([z.literal(""), z.coerce.number().int().positive()]).transform((value) => (value === "" ? 1 : value)),
src/lib/validations/supplier.ts#L15-L17: Apply the exact same reordering toaverageLeadTimeDays:averageLeadTimeDays: z .union([z.literal(""), z.coerce.number().int().positive()]) .transform((value) => (value === "" ? undefined : value)),
📍 Affects 2 files
src/lib/validations/product.ts#L13-L15(this comment)src/lib/validations/supplier.ts#L15-L17
🤖 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/lib/validations/product.ts` around lines 13 - 15, The product validation
fields estimatedCost, leadTimeDays, and minimumOrderQuantity currently allow
empty strings to be coerced to 0; reorder each z.union so z.literal("") is
evaluated before the coerced numeric schema, preserving the existing transforms.
Apply the same ordering to averageLeadTimeDays in src/lib/validations/product.ts
lines 13-15 and src/lib/validations/supplier.ts lines 15-17.
Comment on lines
+4
to
+21
| export async function createSupplier(input: CreateSupplierInput) { | ||
| return db.supplier.create({ | ||
| data: { | ||
| commercialName: input.commercialName, | ||
| legalName: input.legalName || null, | ||
| supplierType: input.supplierType, | ||
| status: input.status, | ||
| country: input.country || null, | ||
| city: input.city || null, | ||
| email: input.email || null, | ||
| phone: input.phone || null, | ||
| paymentTerms: input.paymentTerms || null, | ||
| currency: input.currency || null, | ||
| incoterm: input.incoterm || null, | ||
| averageLeadTimeDays: input.averageLeadTimeDays, | ||
| }, | ||
| }); | ||
| } No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Ensure empty string lead times do not crash Prisma.
The upstream Zod schema allows averageLeadTimeDays to evaluate to "" (empty string) if left blank. Passing "" directly to a Prisma Int field will cause a runtime error. Fall back to null to safely handle empty inputs.
🐛 Proposed fix to handle empty inputs safely
currency: input.currency || null,
incoterm: input.incoterm || null,
- averageLeadTimeDays: input.averageLeadTimeDays,
+ averageLeadTimeDays: input.averageLeadTimeDays || null,
},
});📝 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
| export async function createSupplier(input: CreateSupplierInput) { | |
| return db.supplier.create({ | |
| data: { | |
| commercialName: input.commercialName, | |
| legalName: input.legalName || null, | |
| supplierType: input.supplierType, | |
| status: input.status, | |
| country: input.country || null, | |
| city: input.city || null, | |
| email: input.email || null, | |
| phone: input.phone || null, | |
| paymentTerms: input.paymentTerms || null, | |
| currency: input.currency || null, | |
| incoterm: input.incoterm || null, | |
| averageLeadTimeDays: input.averageLeadTimeDays, | |
| }, | |
| }); | |
| } | |
| export async function createSupplier(input: CreateSupplierInput) { | |
| return db.supplier.create({ | |
| data: { | |
| commercialName: input.commercialName, | |
| legalName: input.legalName || null, | |
| supplierType: input.supplierType, | |
| status: input.status, | |
| country: input.country || null, | |
| city: input.city || null, | |
| email: input.email || null, | |
| phone: input.phone || null, | |
| paymentTerms: input.paymentTerms || null, | |
| currency: input.currency || null, | |
| incoterm: input.incoterm || null, | |
| averageLeadTimeDays: input.averageLeadTimeDays || null, | |
| }, | |
| }); | |
| } |
🤖 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/modules/suppliers/create-supplier.ts` around lines 4 - 21, Update
createSupplier so the averageLeadTimeDays value passed to Prisma is null when
input.averageLeadTimeDays is an empty string, while preserving valid numeric
values unchanged.
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
1 participant
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.
Entrega del segundo deliverable del proyecto
Summary by CodeRabbit
New Features
Documentation