Skip to content

feat: [TESIS-145] manage the company warehouses from their own screen - #62

Merged
LauAubert merged 2 commits into
masterfrom
TESIS-999004-warehouses-management
Oct 4, 2026
Merged

LauAubert merged 2 commits into
masterfrom
TESIS-999004-warehouses-management

Conversation

@LauAubert

@LauAubert LauAubert commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Ticket de Jira

https://proyectofinalfrlp.atlassian.net/browse/TESIS-145

ID provisorio: se reemplaza por la clave real al cargar la card en Jira. Card: cards/004.md.


Descripción

El alcance compromete que cada empresa registre y gestione sus depósitos físicos (E4b §2.2, Módulo B; RF-03 de E4a): nombre, dirección y código postal, que es el origen de la cotización de cada envío. La API tiene el CRUD completo de /api/v1/warehouses hace rato, pero el front no tenía ninguna pantalla: los depósitos sólo se cargaban por seeds o desde el backoffice. Este PR agrega la sección Depósitos (/warehouses) en el Sidebar, con listado, alta, edición y baja.

Decisiones que conviene mirar:

Sin maqueta. No hay pantalla de depósitos en docs/design/. Se arma con DataTable, ModalFrame, LabeledField y ConfirmDialog del DS y la misma cabecera que Inventario.

Feature nueva, features/warehouses. Inventario, órdenes y el panel ya piden /warehouses cada uno con su propia clave, y una feature no puede importar de otra. Por eso las mutaciones de esta pantalla invalidan todas las queries: refrescar de más ante algo tan poco frecuente como renombrar un depósito es más barato que dejar un nombre viejo en el select del alta manual. Está comentado en el hook.

Código postal argentino. La API sólo exige que esté, pero un CP mal escrito falla recién al cotizar el envío, lejos de donde se cargó. El formulario acepta 4 dígitos (1900) o el CPA completo (B1900ABC, se manda en mayúsculas).

El 409 de la baja, traducido. La API dice el motivo en el texto (existing stock, order lines taken from it, stock transfers from or to it). deleteBlockerFrom lo reconoce por fragmentos (el de transferencias se mira antes que el de stock, porque también contiene «stock») y cae en un motivo genérico si no lo reconoce, nunca en el texto crudo en inglés. Con el 409 la confirmación desaparece, igual que en la baja de producto.

Una aclaración en el mensaje de stock: la API bloquea por la fila de stock, no por la cantidad. Un producto asignado con cero unidades también impide borrar el depósito, y el mensaje lo dice. Corregirlo del lado de la API es una card aparte (la 005).

  • Agrega features/warehouses con api.ts, types.ts, queryKeys.ts, hooks/useWarehouses.ts y content.ts.
  • Agrega WarehousesPage, WarehouseFormModal (RHF + Zod) y DeleteWarehouseDialog.
  • Agrega utils/deleteBlocker.ts, que traduce el motivo del 409.
  • Registra la ruta /warehouses con su entrada «Depósitos» en el Sidebar (app/router/routes.tsx) y actualiza routes.test.tsx.
  • Suma la feature a la tabla de docs/guidelines/architecture.md.

Evidencia visual

Pendiente de captura con la API levantada. El comportamiento está cubierto por los tests de la página, el modal y la traducción del 409.


Cómo probar

Precondición: API de master, bin/rails db:seed, login con un usuario de Norte.

  1. Sidebar → «Depósitos» → se listan Depósito Central y Depósito Satélite Norte con dirección, CP y unidades guardadas.
  2. «Nuevo depósito» sin completar nada → los tres campos piden dato. CP 190 → mensaje de formato.
  3. Crear «Depósito Oeste», Av. Rivadavia 100, b1704abc → toast, aparece en la tabla con CP B1704ABC y 0 unidades. Ya se ofrece en el modal de producto y en el alta manual de órdenes sin recargar.
  4. Editar su nombre → se actualiza en la tabla.
  5. Eliminar «Depósito Oeste» → confirmación destructiva → desaparece.
  6. Eliminar Depósito Central → «No se puede eliminar: hay productos asignados a este depósito…» y sin botón de confirmar.

Verificación: npm run test (691 tests, 0 fallas), npm run lint, npm run format:check y npm run build limpios.


Impacto y consideraciones

¿Introduce breaking changes?
No

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No. Una feature nueva con la estructura estándar. Lo único distinto es la invalidación global, explicada arriba.

🤖 Generated with Claude Code

The scope commits every company to register and manage its physical
warehouses (RF-03, module B), and the API has had the full CRUD for a while,
but the front had no screen: warehouses could only come from the seeds or the
backoffice.

The new Depósitos section lists them with their address, postal code and
stored units, and creates, edits and deletes them. The form validates an
Argentine postal code, since every shipment quote starts from it. A 409 on
delete is translated to its concrete reason and the confirmation goes away,
because retrying would fail the same way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@TomasMartin2004

Copy link
Copy Markdown
Contributor

Revisado. Lo veo bien para implementar.

Este es el más importante de los que levantaste en el front. RF-03 («ABM de depósitos físicos») es alcance comprometido, tiene pantalla propia en la matriz de trazabilidad de E4a §3 («Panel de depósitos») y está en el módulo B de E4b. La API tiene el CRUD completo hace meses y verifiqué que en master no existe ninguna carpeta features/warehouses: los depósitos sólo se cargaban por seeds o desde Avo. Era un requisito comprometido sin front, y eso sí es un hueco si alguien va a buscarlo.

Lo que verifiqué

  • La estructura sigue la convención de las demás features al pie de la letra (api.ts, components/, hooks/, pages/, queryKeys.ts, types.ts, content.ts, utils/), y docs/guidelines/architecture.md queda actualizado con la fila de la feature nueva. No es una pantalla pegada al costado.
  • routes.test.tsx actualiza la lista de rutas del sidebar. Ese spec es el que impide que una ruta nueva aparezca o desaparezca de la navegación sin que nadie lo note, así que está bien que se toque acá.
  • La frontera con Rails queda en api.ts: ningún componente ve snake_case, y toPayload normaliza (trim y CPA en mayúsculas). El comentario explica por qué el listado no pagina de verdad (WHOLE_LIST_PER_PAGE, hasta 100), que es correcto contra la API.
  • deleteBlockerFrom cubre los tres motivos del 409 y cae en 'unknown' ante cualquier otra cosa, con test para los cuatro casos.

Dos cosas que dejaría anotadas

  1. El motivo del 409 se deduce del texto en inglés del mensaje. Funciona, está testeado y hoy no hay alternativa, pero es un acople frágil: si alguien reescribe el mensaje en Rails, la UI pasa a mostrar el texto genérico sin que ningún test del back se ponga en rojo. Lo correcto a futuro es que la API devuelva un código en el cuerpo. Vale la línea en la card.
  2. Las mutaciones invalidan todas las queries. Está documentado y el motivo es válido (los depósitos llenan selects de otras features cuyas claves esta no puede importar), pero es un martillo: alta, edición o baja de un depósito disparan un refetch de todo lo que haya montado. No lo bloquearía; si molesta, se resuelve exportando las claves afectadas desde shared/.

Antes de mergear

Conviene que vaya junto o después de proyecto-api#100: sin ese PR, un depósito al que el modal de producto le «quitó» todos los productos (filas en quantity: 0) no se puede borrar nunca, y la pantalla nueva va a mostrar un 409 de «existing stock» sobre un depósito que no guarda una sola unidad. La pantalla funciona igual, pero el botón de borrar queda con un caso raro bien a la vista.

@LauAubert LauAubert changed the title feat: [TESIS-999004] manage the company warehouses from their own screen feat: [TESIS-145] manage the company warehouses from their own screen Oct 3, 2026
@LauAubert LauAubert closed this Oct 3, 2026
@LauAubert
LauAubert deleted the TESIS-999004-warehouses-management branch October 3, 2026 23:18
@LauAubert
LauAubert restored the TESIS-999004-warehouses-management branch October 3, 2026 23:22
@LauAubert LauAubert reopened this Oct 3, 2026
@LauAubert
LauAubert marked this pull request as ready for review October 3, 2026 23:25
@LauAubert
LauAubert requested a review from a team as a code owner October 3, 2026 23:25
@LauAubert
LauAubert requested review from TomasMartin2004 and removed request for a team October 3, 2026 23:25

@TomasMartin2004 TomasMartin2004 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.

Revisión del diff completo. El código es idéntico al que leí cuando los PRs estaban en draft —ningún commit nuevo—, así que lo que sigue es el veredicto formal.

✅ Aprobado

El más importante de la tanda en el front. RF-03 («ABM de depósitos físicos») es alcance comprometido, tiene pantalla propia en la matriz de trazabilidad de E4a §3 («Panel de depósitos») y está en el módulo B de E4b. La API tiene el CRUD completo hace meses y verifiqué que en master no existe ninguna carpeta features/warehouses: los depósitos sólo se cargaban por seeds o desde Avo. Era un requisito comprometido sin front.

Lo que verifiqué

  • La estructura sigue la convención de las demás features al pie de la letra, y architecture.md queda actualizado con la fila de la feature nueva. No es una pantalla pegada al costado.
  • routes.test.tsx actualiza la lista del sidebar. Ese spec es el que impide que una ruta aparezca o desaparezca de la navegación sin que nadie lo note.
  • La frontera con Rails queda en api.ts: ningún componente ve snake_case, y toPayload normaliza. El comentario sobre WHOLE_LIST_PER_PAGE es correcto contra la API.
  • deleteBlockerFrom cubre los tres motivos del 409 y cae en 'unknown' ante cualquier otra cosa, con test para los cuatro casos.

🟡 Dos cosas para anotar, no bloqueantes

  1. El motivo del 409 se deduce del texto en inglés del mensaje. Funciona y está testeado, pero es un acople frágil: si alguien reescribe el mensaje en Rails, la UI pasa a mostrar el texto genérico sin que ningún test del back se ponga en rojo. Lo correcto a futuro es que la API devuelva un código en el cuerpo.
  2. Las mutaciones invalidan todas las queries. Está documentado y el motivo es válido, pero es un martillo: alta, edición o baja disparan un refetch de todo lo montado. Si molesta, se resuelve exportando las claves afectadas desde shared/.

⚠️ Orden de merge

Conviene junto o después de proyecto-api#100: sin ese PR, un depósito al que el modal de producto le «quitó» todos los productos no se puede borrar nunca, y la pantalla nueva muestra un 409 de «existing stock» sobre un depósito que no guarda una sola unidad.

@LauAubert
LauAubert merged commit ef68b64 into master Oct 4, 2026
4 checks passed
LauAubert added a commit that referenced this pull request Oct 5, 2026
…#62)

The scope commits every company to register and manage its physical
warehouses (RF-03, module B), and the API has had the full CRUD for a while,
but the front had no screen: warehouses could only come from the seeds or the
backoffice.

The new Depósitos section lists them with their address, postal code and
stored units, and creates, edits and deletes them. The form validates an
Argentine postal code, since every shipment quote starts from it. A 409 on
delete is translated to its concrete reason and the confirmation goes away,
because retrying would fail the same way.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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