Skip to content

OM-412: ahora se puede enviar el address como null en createOrder - #76

Merged
CrisNAC merged 4 commits into
devfrom
OM-412
Apr 4, 2026
Merged

CrisNAC merged 4 commits into
devfrom
OM-412

Conversation

@leoAchu16

@leoAchu16 leoAchu16 commented Apr 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Nuevas Características
    • Se puede crear un pedido sin indicar dirección de envío (opción de recogida); la respuesta mostrará la dirección como nula cuando corresponda.
  • Correcciones
    • La verificación de existencia/propiedad de la dirección solo se ejecuta si se proporciona una dirección, evitando errores innecesarios.

@coderabbitai

coderabbitai Bot commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8a1934cf-549d-4025-aaf9-93dbeed0a4a3

📥 Commits

Reviewing files that changed from the base of the PR and between 5e2704c and 8eea835.

📒 Files selected for processing (2)
  • prisma/migrations/20260404000234_make_fk_address_optional/migration.sql
  • prisma/schema.prisma
✅ Files skipped from review due to trivial changes (1)
  • prisma/migrations/20260404000234_make_fk_address_optional/migration.sql

📝 Walkthrough

Walkthrough

Se hizo opcional la relación de dirección en pedidos: el servicio acepta addressId ausente (se interpreta como null) y la validación/búsqueda de dirección solo ocurre si hay addressId. El esquema y la migración DB permiten fk_address nulo; las respuestas de pedido muestran address: null cuando no hay dirección.

Changes

Cohort / File(s) Summary
Servicio de órdenes
src/modules/users/orders/order.service.js
Soporta addressId opcional: parsing que convierte valores falsy a null; la verificación de existencia/pertenencia de la dirección se ejecuta solo si resolvedAddressId no es null. La respuesta del pedido devuelve address: null cuando corresponde.
Esquema Prisma
prisma/schema.prisma
model Orders: fk_address cambiado de Int a Int? y la relación address de Addresses a Addresses? (relación mantenida).
Migración DB
prisma/migrations/20260404000234_make_fk_address_optional/migration.sql
Se altera Orders.fk_address a nullable; se recrea la FK Orders_fk_address_fkey con ON DELETE SET NULL, ON UPDATE CASCADE.

Esfuerzo estimado de revisión de código

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • SebaKisser

Poema

🐰 Brinco suave en la pradera del código,
una dirección que puede faltar,
el pedido sigue, ligero y risueño,
null en su nido, todo en su lugar. 🥕📦

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed El título describe claramente el cambio principal: permite enviar address como null en createOrder, lo cual se refleja en todas las modificaciones del changeset (schema, migración y servicio).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch OM-412

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/modules/users/orders/order.service.js (1)

52-116: ⚠️ Potential issue | 🟠 Major

No uses truthy/falsy para decidir si se envió addressId.

En Line 52 (addressId ? ... : null) y Line 109 (if (resolvedAddressId)), valores inválidos como 0 o "" quedan tratados como “no enviado”, se saltea validación y ownership check.

💡 Ajuste sugerido
- const resolvedAddressId = addressId ? parsePositiveInteger(addressId, "addressId") : null;
+ const resolvedAddressId = addressId == null
+   ? null
+   : parsePositiveInteger(addressId, "addressId");

- if (resolvedAddressId) {
+ if (resolvedAddressId !== null) {
    const address = await prisma.addresses.findFirst({
      where: { id_address: resolvedAddressId, fk_user: resolvedUserId, status: true }
    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/users/orders/order.service.js` around lines 52 - 116, The code
treats falsy values (like 0 or "") as "not sent" because resolvedAddressId is
set with addressId ? ... : null and later checked with if (resolvedAddressId);
change this to detect presence explicitly: assign resolvedAddressId using a null
check (e.g., resolvedAddressId = addressId === undefined || addressId === null ?
null : parsePositiveInteger(addressId, "addressId")) and replace the conditional
if (resolvedAddressId) with if (resolvedAddressId !== null) so zero or
empty-string inputs get validated/parsed instead of being skipped; reference
resolvedAddressId and parsePositiveInteger to locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/users/orders/order.service.js`:
- Line 142: La persistencia de fk_address como null rompe porque
Orders.fk_address en Prisma es requerido y mapOrderResponse (función
mapOrderResponse que accede a order.address.id_address) asume siempre una
address; arreglalo en dos pasos: 1) actualizar el modelo Prisma para que
fk_address sea Int? y la relación address opcional (migración correspondiente)
para permitir null en tx.orders.create cuando sea retiro en tienda; 2) hacer
mapOrderResponse null-safe: antes de acceder a order.address.id_address
comprueba que order.address exista (usar comprobación explícita u optional
chaining dentro de mapOrderResponse) y devuelve valores nulos o alternativos
apropiados cuando no haya address.

---

Outside diff comments:
In `@src/modules/users/orders/order.service.js`:
- Around line 52-116: The code treats falsy values (like 0 or "") as "not sent"
because resolvedAddressId is set with addressId ? ... : null and later checked
with if (resolvedAddressId); change this to detect presence explicitly: assign
resolvedAddressId using a null check (e.g., resolvedAddressId = addressId ===
undefined || addressId === null ? null : parsePositiveInteger(addressId,
"addressId")) and replace the conditional if (resolvedAddressId) with if
(resolvedAddressId !== null) so zero or empty-string inputs get validated/parsed
instead of being skipped; reference resolvedAddressId and parsePositiveInteger
to locate the changes.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef8e218c-1d19-4ac1-93f0-7d75bf538790

📥 Commits

Reviewing files that changed from the base of the PR and between 8d7fec9 and 12bd597.

📒 Files selected for processing (1)
  • src/modules/users/orders/order.service.js

Comment thread src/modules/users/orders/order.service.js
@CrisNAC
CrisNAC merged commit 179019d into dev Apr 4, 2026
1 of 2 checks passed
@Andoumeda
Andoumeda deleted the OM-412 branch April 11, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants