Skip to content

fix: [TESIS-155] bound the page number of every listing - #110

Merged
LauAubert merged 7 commits into
masterfrom
TESIS-999022-bounded-page-number
Oct 5, 2026
Merged

LauAubert merged 7 commits into
masterfrom
TESIS-999022-bounded-page-number

Conversation

@LauAubert

@LauAubert LauAubert commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Ticket de Jira

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

ID provisorio: se reemplaza por la clave real al cargar la card en Jira. Card: cards/022.md. Sale de una auditoría de código (TESIS-89).


Descripción

Api::V1::Paginatable#page_number acotaba la página sólo por abajo ([page.to_i, 1].max). Con un ?page= de veinte dígitos, el OFFSET desbordaba bigint, Postgres levantaba NumericValueOutOfRange (ActiveRecord::RangeError) y cualquier listado de la API respondía 500: órdenes, productos, envíos, transferencias, depósitos, eventos fallidos.

  • Agrega Paginatable::MAX_PAGE = 1_000_000 y acota la página con clamp(1, MAX_PAGE). Un millón de páginas de cien filas está muy por encima de cualquier listado real y muy lejos del límite de la base.
  • Una página así se lee como la última aceptada y responde vacía, con el total real: el mismo criterio que ya tenía una página pasada del final, y el mismo criterio «acotar en vez de romper» que el concern aplica a page=0 y a per_page.
  • Spec: ?page=99999999999999999999 responde 200 con data vacío y meta.page == MAX_PAGE.

Evidencia visual

N/A


Cómo probar

  1. GET /api/v1/orders?page=99999999999999999999 → 200 {"data": [], "meta": {"page": 1000000, "per_page": 20, "total": N}}. En master, 500.
  2. GET /api/v1/orders?page=2 → igual que antes.

Verificación: bundle exec rspec (1568 ejemplos, 0 fallas, cobertura 99.89% de línea), rubocop y brakeman limpios.

Dientes: con el page_number de master, el spec nuevo falla con ActiveRecord::RangeError.


Impacto y consideraciones

¿Introduce breaking changes?
No

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No

🤖 Generated with Claude Code

Paginatable only bounded the page number from below. A page of twenty
digits made the OFFSET overflow bigint, PostgreSQL raised
NumericValueOutOfRange and every listing answered 500.

The page is now clamped between 1 and a million, far above any real listing
and far from the limit of the database. A page that large reads as the last
accepted one and answers empty, like any page past the end, with the real
total.

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

Copy link
Copy Markdown
Contributor

Revisado. Lo veo bien para implementar.

20 líneas que sacan un 500 de todos los listados de la API —órdenes, productos, envíos, transferencias, depósitos, eventos fallidos— con sólo mandar un ?page= largo: el OFFSET desbordaba bigint y Postgres levantaba NumericValueOutOfRange. Es el tipo de hallazgo que corresponde a la tarea de QA de seguridad del entorno desplegado, y ahora que el sistema está sirviendo en la VPS vale más que cuando se escribió.

Lo que verifiqué

  • clamp(1, MAX_PAGE) mantiene el criterio que el método ya tenía por abajo (page=0 y page=-3 se acotan en vez de dar 400) y lo aplica por arriba con la misma lógica. Coherente con lo que ya estaba escrito y comentado.
  • MAX_PAGE = 1_000_000 por MAX_PER_PAGE = 100 da un offset máximo de 100.000.000: muy por debajo del límite de bigint. Holgado.
  • El spec verifica las tres cosas que importan: 200, data vacía y meta.page acotado. Que meta devuelva MAX_PAGE y no el número pedido está bien y está comentado.

Sin objeciones. scan_ruby en rojo es la corrida vieja del 02/10, no el cambio.

@LauAubert LauAubert changed the title fix: [TESIS-999022] bound the page number of every listing fix: [TESIS-155] bound the page number of every listing Oct 3, 2026
@LauAubert LauAubert closed this Oct 3, 2026
@LauAubert
LauAubert deleted the TESIS-999022-bounded-page-number branch October 3, 2026 23:19
@LauAubert
LauAubert restored the TESIS-999022-bounded-page-number branch October 3, 2026 23:23
@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 a review from LoLoo03 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

20 líneas que sacan un 500 de todos los listados de la API —órdenes, productos, envíos, transferencias, depósitos, eventos fallidos— con sólo mandar un ?page= largo: el OFFSET desbordaba bigint y Postgres levantaba NumericValueOutOfRange. Ahora que el sistema está sirviendo en la VPS vale más que cuando se escribió.

Lo que verifiqué

  • clamp(1, MAX_PAGE) mantiene el criterio que el método ya tenía por abajo (page=0 y page=-3 se acotan en vez de dar 400) y lo aplica por arriba con la misma lógica.
  • MAX_PAGE = 1_000_000 por MAX_PER_PAGE = 100 da un offset máximo de 100.000.000, muy por debajo del límite de bigint.
  • El spec verifica las tres cosas que importan: 200, data vacía y meta.page acotado. Que meta devuelva MAX_PAGE y no el número pedido está bien y está comentado.

Sin objeciones.

LauAubert and others added 6 commits October 4, 2026 20:45
…er warehouse in the product detail (#99)

The detail did not carry stock_status, so the front recomputed it with its own
thresholds and the same product read "available" in the catalog and "critical"
in the detail. The rule now lives in Product.stock_status_for and is applied
to the product total and to each stock row.

in_transit_by_warehouse breaks the units in flight down by destination with a
single grouped query, including warehouses that have no stock row yet.

The warehouse nested in each stock drops stored_units through a reference
view: it cost one SUM per warehouse and nobody reads it there.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… zero (#100)

Removing a warehouse from a product in the edit modal sends quantity 0,
because Products::UpdateProduct upserts and never deletes. The row stays, and
restrict_with_error on stocks then blocked deleting the warehouse forever,
even though it held no units.

The warehouse now drops its empty rows right before the restriction runs, in
the same transaction: if orders or transfers still block the deletion, the
rows come back. The 409 reason looks only at rows holding units, so it names
the real blocker.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ad of 500 (#105)

OrderItem validated the quantity as greater than zero on the raw value, and
the integer column then cast it: 0.5 passed, was stored as 0 and made
DeductStock raise ArgumentError, which nobody rescues, so POST /orders
answered 500. 2.7 was silently truncated to 2. The edition already refused
both through ReplaceOrderLines#positive_integer; the creation did not.

The quantity of an order item is now validated as an integer, so the
creation and the webhook ingestion answer 422 with the reason before any
stock moves. Integers that come as text from a form are still accepted.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…106)

The products POROs compared the warehouse ids of the stock rows raw against
the ids in the database. A row without warehouse_id made the sort raise
ArgumentError (nil against Integer), so POST and PUT /products answered 500;
ids that came as text ("5") never matched and gave a false 422 saying the
warehouse did not belong to the company. The order edition had the same
false 422 with the same warehouse sent as "1" and 1.

The rows now need a positive integer warehouse_id, as a number or as digits,
and are compared as unique integers by count. A missing one answers 422
naming the row. Warehouses of another company are still refused with the
same generic message.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@LauAubert
LauAubert merged commit 7f8d32b into master Oct 5, 2026
4 checks passed
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