feat: [TESIS-125] search the catalog against the backend - #49
Conversation
The picker pulled one page of a hundred products and filtered it in memory, because when it was written `GET /products` only paginated. Past that cut a product simply never appeared, and the operator had no way to tell that apart from "it does not exist". TESIS-62 added `search`, so the filter moves to where the whole catalog is. `filterOptions` now returns the options untouched. Without that MUI filters the server's answer a second time by the visible label, which would hide matches the backend did find. What the term does is debounced, so the three keystrokes of "sen" are one request, and it travels in the query key, so going back to a term already typed comes from the cache. `keepPreviousData` keeps the list from blinking empty between searches. An empty term does not send `search` at all: the backend would read it as a filter by empty string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
LoLoo03
left a comment
There was a problem hiding this comment.
Review de TESIS-125: el buscador de productos del alta manual pasa a buscar en el backend
Veredicto: pedir cambios. El enfoque es correcto y sigue el mismo patrón que el buscador del inventario. Hay un bug al elegir una opción, un test que no prueba lo que dice y un comentario con una justificación que no es cierta.
En la rama del PR corrí lint, prettier --check, test y build, y pasaron todos: 454 tests, 0 fallas, igual que dice la descripción. Después volví a TESIS-64-reports-analytics-overview, que quedó limpia.
- Bug: al elegir un producto, su etiqueta se manda como búsqueda
ProductPicker.tsx:81
Cuando se elige una opción, MUI escribe la etiqueta en el campo y llama a onInputChange con reason: 'reset'. Como el campo está controlado, ese texto llega a search y, pasado el debounce, al backend. Lo probé con un test temporal en la rama: tras elegir PX-9021, la búsqueda que viaja es:
"PX-9021-LRG · Router industrial de alta densidad"
En el backend, search_catalog hace sku ILIKE … OR name ILIKE …, así que ese texto no encuentra nada. Consecuencias:
Cada vez que se elige un producto se hace un request que no sirve.
La lista queda vacía. Si el operador reabre el buscador para cambiar de producto, ve «Ningún producto coincide.» en lugar de las coincidencias de lo que había tipeado.
Arreglo sugerido (no lo probé): que el campo lo maneje MUI y que solo lo que el operador tipea llegue como término.
// sin inputValue={search}
onInputChange={(_event, value, reason) => {
// Al elegir una opción MUI escribe su etiqueta en el campo: eso no es una búsqueda.
if (reason === 'input' || value === '') onSearchChange(value)
}}
El caso value === '' cubre la cruz de borrar y el reinicio que hace MUI al pasar product a null después de agregar la línea. Con ese cambio, la prop search deja de hacer falta. Convendría agregar un test que cubra esto: elegir un producto y comprobar que lastSearch() sigue siendo lo que se tipeó.
- El test de «no volver a filtrar» pasaría también sin el cambio
NewOrderPage.test.tsx:168
El test tipea sensor y espera ver «Sensor de presión X4». Esa opción aparecería igual con el filtro que MUI usa por defecto, porque sensor está en la etiqueta, así que el test pasaría aunque se sacara filterOptions={(options) => options}. Hay que tipear algo que no esté en la etiqueta. Con zzz lo probé y la opción sí aparece, o sea que el comportamiento funciona; lo que falta es que el test lo cubra.
- El comentario y el «Caso 3» explican el cambio con algo que no existe
ProductPicker.tsx:84-85
El backend busca solo por sku y name (proyecto-api → app/models/product.rb:94), y los dos ya aparecen en la etiqueta visible. No hay ningún producto que coincida «por su descripción», así que el «Caso 3» del plan de pruebas no se puede reproducir.
Igual está bien no dejar que MUI filtre, porque ese filtro sobra cuando busca el servidor. Hay que corregir el motivo en el comentario y en la descripción del PR. Un efecto real que conviene mencionar: con keepPreviousData, mientras se escribe se ven sin filtrar los resultados de la búsqueda anterior hasta que llega la nueva. Es un costo aceptable, pero debería quedar escrito.
Menores (nits)
Término sin recortar en la query key (queryKeys.ts:36): api.ts recorta el término, pero la key usa el término sin recortar. Entonces "cab" y "cab " son dos entradas de caché y dos requests que devuelven lo mismo. Se resuelve recortando en el hook antes de armar la key.
loading={isPending || isFetching} casi nunca se ve: MUI solo muestra loadingText cuando no hay opciones, y con keepPreviousData casi siempre las hay. Además el texto «Cargando el catálogo…» ya no describe lo que pasa: ahora se está buscando.
Evidencia sin completar: la sección de evidencia todavía dice «(adjuntar captura…)».
Picking an option made MUI write its label into the field, and the controlled input sent that label to the backend as the search term. `search_catalog` matches on sku and name, so it found nothing and the list came back empty for the next time the picker was opened. MUI owns the field now; only `reason: 'input'` and an empty value reach the search, which also covers the clear button and the reset that follows adding a line. Three more things the review found: - The test for "do not filter twice" typed a term that was in the label, so it passed with MUI's own filter too. It now types one that is not, and fails if `filterOptions` is removed. - The comment justified skipping MUI's filter with a match "by description" that does not exist. The real effect is that with keepPreviousData the previous matches stay visible while the new search travels. - The query key used the untrimmed term, so "cab" and "cab " were two cache entries and two identical requests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
Respuesta — TESIS-125 (review de Lorenzo, 24/sep)Commit en Los tres puntos eran reales y los tres están resueltos. Suite 461 tests, 0 fallas (54 archivos) · 🔴 1. El bug: la etiqueta de la opción elegida viajaba como búsquedaLo reproduje antes de tocar nada, con una sonda propia sobre la rama, y sale exactamente lo que decís: Tomé el arreglo que proponías: el campo lo maneja MUI y sólo lo tipeado se propaga. onInputChange={(_event, value, reason) => {
if (reason === 'input' || value === '') onSearchChange(value)
}}Con eso la prop Tres ejemplos nuevos, y uno tiene una trampa que vale la pena contar: los escribí, pasaron, y no probaban nada. Lo que viaja pasa por un debounce de 300 ms, así que al afirmar justo después del click el término todavía no había llegado — el ejemplo pasaba por llegar antes, no por el comportamiento. Con la regresión puesta (volver a propagar el
Con 🟡 2. El test de «no volver a filtrar» pasaba igual sin el cambioCierto: 🟡 3. El comentario explicaba el cambio con algo que no existeTambién cierto, y el error era mío al leer el backend: Reescribí el comentario con el efecto real, que es el que mencionás:
La descripción del PR quedó corregida igual, y el Caso 3 reemplazado por uno que sí se puede hacer. Nits
Gracias — el primero era un bug de verdad y los otros dos eran un test y un comentario que decían más de lo que hacían. 🤖 Generated with Claude Code |
TESIS-58 and TESIS-61 landed and both touch orders. Three conflicts, all of them "keep both sides": - queryKeys.ts: the catalog key keeps the search term, and the keys that TESIS-61 added for stocks and warehouses stay. - api.test.ts: one import list with the helpers of both cards. - architecture.md: the orders row names the search of this card and the steps of the wizard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
The merge of TESIS-61 brought a second user of ProductPicker: the "add line" toolbar of the edit screen. It called the catalog with no term, which the type checker caught — the tests did not, because vitest does not type check. It now holds its own debounced term and hands it to the same hook, the way step 1 of the wizard does, with an example that types in that search and asserts what travels. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
|
Actualizado contra Tres conflictos, los tres de «conservar las dos partes»: la clave del catálogo —que acá lleva el término y allá sumó las de stocks y depósitos—, la lista de imports de Y una cosa que el merge de texto no podía ver: TESIS-61 trajo un segundo consumidor de Así que esa pantalla ahora también busca contra el backend: su propio término debounceado, el mismo hook, el mismo patrón que el paso 1. Va con un ejemplo que tipea en ese buscador y verifica qué viaja. 545 tests, 0 fallas (59 archivos) · @LoLoo03 los tres puntos de tu review están resueltos desde 🤖 Generated with Claude Code |
Sanntinat
left a comment
There was a problem hiding this comment.
Revisión — TESIS-125 (PR #49) · El buscador del alta manual busca en el backend
Revisión contra origin/master de proyecto-web, sobre TESIS-125-catalog-search-from-the-backend (9d2167d), con la card TESIS-125 (sin comentarios de alcance), la review de Lorenzo (CHANGES_REQUESTED) y las dos respuestas de Tomás al lado.
| Check | Resultado |
|---|---|
npm run test (local, sobre la rama) |
59 archivos · 545 tests · 0 failures |
npm run lint · prettier --check . · npm run build (local) |
Limpios |
| CI (GitHub Actions) | Lint & Format · Branch Naming Convention · Unit Tests · Build: success |
Rama vs master |
Al día (ya trae TESIS-58 y TESIS-61) |
| Conflictos con los demás PRs abiertos | Ninguno (#50 sólo toca un comentario de orders/api.ts, en otra zona) |
✅ Los tres puntos de Lorenzo están resueltos, y los verifiqué rompiéndolos
No me quedé con que la suite pase. Deshice cada arreglo, de a uno, y corrí NewOrderPage.test.tsx + useCatalogProducts.test.tsx:
| Rotura | Resultado |
|---|---|
1. Volver a propagar el reset de MUI (onInputChange={(_e, value) => onSearchChange(value)}), que era el bug |
2 failures: does not search for the label of the option that was picked y keeps searching for what was typed after picking an option |
2. Sacar filterOptions={(options) => options} |
1 failure: lists what the backend returned without filtering it again, el que ahora tipea zzz |
| 3. Armar la clave con el término sin recortar (el nit) | 2 failures: treats a term with surrounding spaces as the same search y files it in the cache under the trimmed term |
Sacar keepPreviousData |
1 failure: keeps the previous matches while the new search travels |
Cada rotura hace fallar sólo sus ejemplos y ninguno pasa por construcción. El detalle del settle() que cuenta Tomás es real: sin dejar vencer el debounce, los ejemplos de la selección pasarían aunque el bug estuviera.
El comentario de ProductPicker.tsx ahora da el motivo verdadero para no refiltrar (con keepPreviousData, el filtro de MUI escondería las coincidencias anteriores mientras viaja la búsqueda nueva), y el «Caso 3» que no se podía reproducir salió de la descripción.
✅ El segundo consumidor que trajo TESIS-61 quedó bien resuelto
Al actualizarse contra master apareció NewLineToolbar, el buscador de la pantalla de modificación, que usaba el catálogo sin término. Lo detectó npm run build y no la suite, porque Vitest no chequea tipos. La solución repite el patrón del paso 1 (término propio con debounce, mismo hook) y va con su test. También tiene dientes: si OrderEditForm vuelve a pedir el catálogo con '', falla asks the backend for what was typed in the line search.
✅ El criterio de la card, contra la API
La card pide que, con más de 100 productos, buscar uno lo encuentre. Hice una sonda de request spec sobre la API con los parámetros exactos que ahora manda el front (page=1&per_page=20&search=…) y 106 productos cargados:
antes (page=1, per_page=100, filtrado en memoria) contiene SKU-000: false
ahora (page=1, per_page=20, search=SKU-000) -> ["SKU-000"]
Detalle: el listado ordena por created_at descendente, así que el producto que quedaba afuera era el primero creado, no «el creado en último lugar» como dice la card. El criterio se cumple igual. Sólo lo aclaro para quien lo pruebe a mano.
🟡 Menores
- El estado de carga no es el mismo en las dos pantallas.
NewOrderPagepasaloading={catalog.isPending || catalog.isFetching}, yOrderEditFormpasaproductsLoading={catalog.isPending}. Hay un caso donde se nota: si la búsqueda anterior no trajo nada (zzz) y se tipea otra,keepPreviousDatasostiene la lista vacía. Mientras viaja el request, el paso 1 dice «Buscando…» y la pantalla de modificación dice «Ningún producto coincide.». Es una línea. - Formalidad: la review de Lorenzo sigue en
CHANGES_REQUESTED. Los tres puntos están resueltos desdec9b9321, pero hace falta que él la actualice para que el PR quede habilitado.
Los criterios de la card
-
fetchCatalogProductsmanda el término comosearch. Un término vacío no viaja. - Salen
CATALOG_PAGE_SIZE,filterCatalogy elfilterOptionspropio. El nuevofilterOptions={(o) => o}es lo contrario de un filtro y está justificado. - La clave depende del término (recortado) y hay debounce.
- Los tests de
filterCatalogsalen y el de «busca por nombre» se reemplazó por los que prueban lo que viaja. - Con más de 100 productos, el buscador encuentra el que antes quedaba afuera. Verificado contra la API.
Veredicto
APPROVE.
La review de Lorenzo encontró un bug real y las correcciones lo cierran con tests que lo detectan si vuelve. La actualización contra master resolvió además un problema que el merge de texto no mostraba. El menor del estado de carga no bloquea. Para mergear falta que Lorenzo actualice su review.
The edit screen passed only isPending, so a search that came back empty kept saying "no product matches" while the next one was still in flight. Step 1 already used isPending || isFetching. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
Sanntinat
left a comment
There was a problem hiding this comment.
Re-revisión — TESIS-125 (PR #49) · Buscar el catálogo en el backend
Tercera vuelta sobre TESIS-125-catalog-search-from-the-backend (f6bee4b), contra origin/master de proyecto-web. La vuelta anterior la aprobé sobre 9d2167d. Después entró un commit, y la review de Lorenzo sigue en CHANGES_REQUESTED.
| Check | Resultado |
|---|---|
| CI (GitHub Actions) | Los cuatro jobs en verde |
Rama vs master |
2 commits atrás (TESIS-82 y TESIS-104). Mergea limpio |
Sobre el merge con master (local) |
61 archivos · 565 tests · 0 failures · lint, prettier y build limpios |
| Conflictos con otros PRs | Con #52 (TESIS-59): docs/guidelines/architecture.md y src/features/orders/api.test.ts. #52 es mío y está bloqueado, así que lo resuelvo yo cuando éste entre. Con #53, ninguno |
✅ El commit nuevo es el menor que había marcado
f6bee4b hace que la pantalla de modificación use isPending || isFetching, como el paso 1. Con eso, una búsqueda que no trajo nada ya no dice «Ningún producto coincide.» mientras la siguiente todavía viaja. Es el cambio exacto, y el comentario explica por qué hace falta con keepPreviousData.
Sobre el merge con master: TESIS-104 pasó los estilos a theme.vars. Revisé el diff de esta rama y no agrega ninguna lectura de theme.palette, así que no queda nada por convertir.
🟡 Menor: ese estado no lo prueba ningún test
Volví la línea a catalog.isPending y los 215 tests de features/orders siguen en verde. Tampoco hay un test del «Buscando…» en el paso 1, así que las dos pantallas están igual de descubiertas. No lo pido para este PR, pero es el tipo de diferencia que se reintroduce sin que nadie lo note.
Los puntos de Lorenzo
Siguen resueltos, como verifiqué en la vuelta anterior (rompiendo cada uno): la etiqueta de la opción elegida ya no viaja como búsqueda, el test de «no volver a filtrar» tipea algo que no está en la etiqueta y el comentario ya no habla de una búsqueda por descripción que no existe. Lo único que falta es formal: que @LoLoo03 actualice su review, que es lo que mantiene el PR en CHANGES_REQUESTED.
Los criterios de la card
-
fetchCatalogProductsmanda el término comosearch; un término vacío no viaja. - Salen el tope de página,
filterCatalogy el filtro propio del Autocomplete. - La clave depende del término recortado y hay debounce.
- Con más de 100 productos, el buscador encuentra el que antes quedaba afuera (verificado contra la API en la vuelta anterior).
Veredicto
APPROVE.
Sigue en pie la aprobación anterior, y el commit nuevo cierra el menor que había dejado. Del lado del PR no falta nada: sólo la actualización de la review de Lorenzo.
|
@LoLoo03 ¿podés volver a mirar este? Tu review del 24 sigue en Los tres puntos que marcaste están resueltos desde 1. El bug — la etiqueta de la opción elegida viajaba como búsqueda. Lo reproduje antes de tocar nada con una sonda propia: Tomé tu arreglo: el campo lo maneja MUI y sólo se propaga 2. El test sin dientes. Cierto: 3. El comentario con el motivo falso. También cierto, y el error de lectura fue mío: el backend busca por Los nits también: el término se recorta en el hook (con cuatro ejemplos nuevos en Después de tu review entró TESIS-61 y trajo un segundo consumidor del buscador —la pantalla de modificación— que llamaba al catálogo sin término; lo detectó Si encontrás algo más, decilo y lo corrijo; si no, con que actualices la review alcanza para mergearlo. 🤖 Generated with Claude Code |
TESIS-59 landed, so step 3 of the wizard exists now. Two conflicts, both of them "keep both sides": - architecture.md: the orders row names the three steps, as master does, and keeps the note that the search filters against the backend. - api.test.ts: master's helpers plus this card's block for fetchCatalogProducts. Rebuilt from master's file rather than merged line by line, which broke the imports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
🔗 Link
📝 Descripción
El buscador de productos del paso 1 del alta manual pasa a filtrar contra el backend. Antes traía una página de cien productos y filtraba en memoria, porque cuando se escribió
GET /productssólo paginaba.El problema que cierra. Más allá de ese corte, un producto no aparecía nunca en el buscador — y el operador no tenía cómo distinguirlo de «no existe». TESIS-62 (proyecto-api#76, mergeado) agregó
search, así que el filtro se muda a donde está el catálogo entero.Decisiones:
filterOptions={(options) => options}es lo que hace que esto funcione de verdad: sin eso elAutocompletefiltra otra vez la respuesta del servidor por la etiqueta visible, y esconde coincidencias que el backend sí encontró. Es el detalle que convierte «pedirle al backend» en «mostrar lo que el backend contestó».useDebouncedValue, el mismo del buscador del catálogo.keepPreviousDataevita que la lista parpadee vacía entre búsquedas: mientras llega la nueva se siguen viendo las coincidencias de la anterior.search. El backend lo leería como un filtro por cadena vacía; mismo criterio quetoFiltersdel listado de órdenes. Con el buscador recién abierto se ve la primera página, que es algo en vez de nada.CATALOG_PAGE_SIZE(100) pasa aCATALOG_MATCHES(20). Ya no es «el catálogo entero» sino «cuántas coincidencias se muestran», y veinte alcanzan de sobra para elegir una: el operador acota tipeando.ProductPickerqueda controlado. El término vive en la página porque de ahí sale la consulta: el componente no decide cuándo se busca ni con qué demora.🛠️ Cambios realizados
features/orders/api.ts—fetchCatalogProducts(search)manda el término;CATALOG_MATCHESreemplaza aCATALOG_PAGE_SIZE.features/orders/hooks/useCatalogProducts.ts— recibe el término, lo pone en la key y usakeepPreviousData.features/orders/queryKeys.ts— el término entra en la clave.features/orders/components/ProductPicker/— buscador controlado, sinfilterOptionspropio.features/orders/pages/NewOrderPage.tsx— el estado del término y su debounce.features/orders/utils/draft.ts— se eliminafilterCatalogy sus tests: el filtro ya no vive acá.docs/guidelines/architecture.md— la línea deorders.api.test.ts(el término viaja, se recorta, no viaja vacío) y 3 enNewOrderPage.test.tsx(pide al backend lo tipeado, no pide por pulsación, muestra sin volver a filtrar).🧪 Cómo probarlo (Opcional)
Precondiciones: backend con master actual corriendo en
localhost:3000, seeds del tenantnorte, sesión iniciada.Caso 1 — la búsqueda es del backend
cab: en la pestaña de red se ve un soloGET /products?page=1&per_page=20&search=cab, no uno por letra.search.Caso 2 — lo que antes no se encontraba
Caso 3 — que no filtre dos veces
caby esperar los resultados.cable industrial. Mientras viaja la búsqueda nueva, la lista sigue mostrando las coincidencias decaben vez de vaciarse: eso es lo que el filtro de MUI escondería.(La versión anterior de este caso decía «un producto que matchea por un campo que la etiqueta no muestra». No existe: el backend busca por
skuy porname, y los dos están en la etiqueta. Corregido tras la review de Lorenzo.)Caso 5 — elegir una opción no es buscar
PX-9021y elegir la opción.GET /products?...&search=PX-9021-LRG+·+Router+industrial...: lo único que viajó es lo que se tipeó.Caso 4 — bordes
📸 Evidencia (Opcional)
La sonda con la que reproduje el bug de la etiqueta, sobre la rama antes del arreglo:
¿Afecta la arquitectura o genera un nuevo patrón?
No. Es el patrón que ya usa el buscador del catálogo: término debounceado, en la query key, filtrado por el backend.
Verificación:
npm run lint,prettier --check .,npm run buildynpm run test(461 tests, 0 fallas).Dientes: volver a propagar el
resetde MUI pone en rojo los dos ejemplos de la selección; quitarfilterOptionspone en rojo el de «no filtrar dos veces». Los escribí primero sin esperar al debounce y pasaban igual: la espera es parte del ejemplo, no un detalle.🤖 Generated with Claude Code
https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp