Repository navigation
fix(menu,admin): category order and visibility, end to end - #94
Merged
Merged
Conversation
The public menu asked PostgreSQL for categories and products with no ORDER BY, so what the customer saw was the heap order: the insertion order until someone edits a row, because an UPDATE rewrites the row at the end of the table. The carta groups products in payload order (groupByCategory in Menu.tsx), so editing one product moved its whole category. The two public reads now order by categories.orden, and so do the two panel reads, so the table and the carta agree. The id is the tie-break: ties are reachable today because the panel computes the next value as rowCount + 1, so deleting a middle category repeats a number. PublicMenuCategoryOrderTests pins the three cases (owner order over insertion order, a tie broken by id with a real UPDATE in the fixture, a null orden going last); all three were red on the previous code. Verified against the dev database: the UPDATE that used to move "Clasica" to the end no longer changes the carta order.
The public read path filtered products by products.active but never looked at categories.active, so turning a category off in the panel removed it from the panel's own list and changed nothing a customer could see. The products query now requires p.category.active = true, and the menu payload lists only active categories, so the carta never advertises a category it does not offer. The pre-existing half of the rule (an inactive product in an active category stays hidden) is pinned by the sibling case in InactiveCategoryNotOfferedTests; both were red on the previous code.
GET /api/categorias serves the category selectors of the product and
combo forms, so it offers only active categories — and that is the only
list the categories table was reading. Turning a category off therefore
removed the row from the table and the switch was one-way: there was no
way to turn it back on.
GET /api/categorias/todas is the admin table read: every category of the
tenant, in the owner's order, wired to the service method that already
existed for it but had no caller
(findAllByBusinessIdAndActiveTrueOrFalse, now named
findAllIncludingInactive). It is a new route instead of a change to the
existing one, so the pickers keep their active-only contract; the
literal path wins over the /{id} template, and id is a Long, so the two
never compete.
Backend: AdminCategoryListHttpTests (the inactive row travels in order,
and the pickers' list keeps offering only active ones). The frontend
table tests move their fixture route to the new URL.
Creating a category stored whatever order the client sent, and the panel sent "table rows + 1". With a category deleted in the middle, rows + 1 repeats a number that already exists: the new category tied with an older one and landed in the middle of the list instead of at the end, which is where the owner just added it. The server owns the place now: orden is max(orden) + 1 for the business (the same query count ignores nulls and COALESCE covers a business with no rows), and the client's order is ignored on create. The panel stops sending it, and the lastOrder/categoryRowCount plumbing that only existed to compute it is gone. CategoryCreateOrderHttpTests pins both halves (a colliding client order ends at the end; the first category of a business opens the list at 1); both were red on the previous code, and the panel's payload assertion moved with the contract.
The row switch sent order: index + 1 along with name and active. The
service only applies non-null fields, so most of the time it was a
no-op, but with gaps in orden it normalized the value to the row
position and could repeat a number that already existed — a tie, which
the list then breaks by id instead of by the owner's order.
The switch now sends only what it changes. Order belongs to the drag and
drop (PUT /api/categorias/orden); the inline rename already worked this
way, this was the odd one out.
The panel depends on PUT /api/categorias/{id} only touching the fields
it receives, so AdminCategoryListHttpTests now pins that too: a PUT
without order leaves the category where it was.
The categories create test waited for the modal to disappear and then asserted that the refresh GET had already happened. The reload is fired by an effect after the modal unmounts, so under load the assertion won the race: it went red once during a full suite run at HEAD. Both waits that my earlier commits touched now wait for the second list request with the same deadline: the create one (categories table) and the delete one (admin-delete-and-toggle). Three consecutive full suite runs are green: 501/501.
This was referenced Nov 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue relacionada
Sin issue vinculada, a propósito. El reporte llegó por prueba manual del dueño — «el orden de las categorías de los productos no siempre es la misma en las cartas» — y los tres arreglos restantes salieron de esa investigación. Ninguna de las issues abiertas (#61, #65, #68, #87, #88, #91) describe este trabajo, así que no las referencio para no asociarles cambios ajenos ni cerrarlas de rebote. Si querés cumplir la convención estricta, decime y abro una issue para esto (con su
status:approved) y cambio el cuerpo aCloses #N.Tipo de PR
type:feature) — además de tres correcciones, el branch agrega un endpoint (GET /api/categorias/todas). El repo no tiene labeltype:bug, por eso el label disponible que corresponde es este.Resumen
categories.orden(desempate por id) y deja de ofrecer las desactivadas con sus productos.orden = max(orden) + 1en el servidor y el switch del panel ya no mandaorder: el cliente dejó de administrar el orden.Cambios
repository/.../jpa/CategoryRepository.javafindByBusinessIdOrdered/findActiveByBusinessIdOrdered(orden del dueño + desempate por id) ynextOrden(COALESCE(MAX(orden),0)); se retiran los tres finders sinORDER BYrepository/.../jpa/ProductRepository.javafindPublicMenuProductsordena por categoría (ese orden ES el de la carta) y exigep.category.active = trueservice/.../impl/MenuServiceImpl.javaservice/.../impl/CategoryServiceImpl.javamax+1;findAllByBusinessIdAndActiveTrueOrFalsepasa afindAllIncludingInactive(existía sin llamador)service/.../interfaces/ICategoryService.javaapi/.../api/CategoryController.javaGET /api/categorias/todas(ruta aditiva: el listado de los selectores sigue devolviendo sólo activas)frontend/src/pages/admin/CategoriesTable.tsx/api/categorias/todas; el switch manda sólo{name, active}; se va el cableado de conteo de filasfrontend/src/pages/admin/CreateCategoryModal.tsx{name, active}(el orden lo asigna el servidor)frontend/src/pages/Admin.tsxcategoryRowCount/lastOrderapi/src/test/.../PublicMenuCategoryOrderTests.javaUPDATEreal en el fixture) yordenNULL al finalapi/src/test/.../InactiveCategoryNotOfferedTests.javaapi/src/test/.../AdminCategoryListHttpTests.java/todasincluye inactivas en orden,/api/categoriassigue con sólo activas, y un PUT sinorderno mueve la filaapi/src/test/.../CategoryCreateOrderHttpTests.javaordercolisionado del cliente se ignora y la categoría va última; primera categoría = 1frontend/tests/admin-categories-table.test.tsxfrontend/tests/admin-delete-and-toggle.test.tsx/todas+ la misma espera del reload después del DELETEPlan de pruebas
git stashdel código y correr los tests nuevos): 3 casos de orden, 1 de categoría inactiva,/todasdevolvía 400 (caía en/{id}), alta con$.order1 en vez de 3 ynullsin orden.mvn -B --no-transfer-progress verify→ 510/510, BUILD SUCCESS (500 previos + 10 nuevos).npm test→ 501/501, tres corridas consecutivas.npm run lint→ exit 0 (sólo warnings preexistentes en archivos que no toca este PR).npm run build:spring→ exit 0.java -jar api/target/api-0.0.1-SNAPSHOT.jar, DB de desarrollo): la carta devuelveHamburguesas(1) → Bebidas(2) → Postres(3) → Lomitos(4)agrupadas en ese orden (antes Lomitos encabezaba por orden físico);GET /api/categoriasvs/todas; alta conorder: 1colisionado →order: 5y última, borrada con 204.review-b0484bb596de677e(15 archivos / 922 líneas, lentereliability) → aprobada y confirmada; 3 hallazgos informativos no bloqueantes (R3-client-order-ignored,R3-nextorden-race,R3-todas-auth-parity) que quedan como trabajo posterior.9f931cf8d9849be96bf547c25839a9152051a143antes y después.Checklist
type:*Co-Authored-By