From 4178eb39c44dd9fa886c226e3dc6781152270b65 Mon Sep 17 00:00:00 2001 From: Tomas Martin Date: Sun, 4 Oct 2026 21:01:20 -0300 Subject: [PATCH] feat: [TESIS-164] let the shipments listing be searched by tracking and order The listing had tabs and pagination but no way in. When a buyer calls, the operator holds one of two codes: the tracking number the courier issued, or the id the sales channel gave the order. Neither was reachable, so finding a shipment meant walking the pages by eye. GET /api/v1/shipments now takes `search` and runs it against both columns. ILIKE so case does not matter, and the term goes through sanitize_sql_like so a typed % is searched as text instead of returning the whole table. The join with orders is left_outer and only happens when there is a term. A shipment always has an order -- the FK is NOT NULL -- but an INNER here would make Postgres resolve the join on every unsearched listing, which is the normal case. For the same reason the blank term returns early: `NULL ILIKE '%%'` is NULL and not true, so applying the condition anyway would drop every shipment that has neither code yet from the plain listing. meta.total counts the matching rows, because the paginator and the table have to agree. A malformed `?search[foo]=bar` leaves through scalar_param with a 400, like the other filters of the endpoint. Out of scope, as in the orders search: ignoring accents and a GIN index with pg_trgm. Both need their own extension and travel together in their own card. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp --- .../api/v1/shipments_controller.rb | 33 +++++- spec/requests/api/v1/shipments_spec.rb | 105 +++++++++++++++++- 2 files changed, 135 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/shipments_controller.rb b/app/controllers/api/v1/shipments_controller.rb index 6cba8a1..b164561 100644 --- a/app/controllers/api/v1/shipments_controller.rb +++ b/app/controllers/api/v1/shipments_controller.rb @@ -23,6 +23,15 @@ class ShipmentsController < ApplicationController # Cuánto del cuerpo del courier se propaga en el mensaje de error. COURIER_ERROR_LIMIT = 300 + # Las dos columnas que recorre el buscador del listado (TESIS-164): el + # número de seguimiento y el id con el que el canal nombra la venta. Son + # los dos códigos que el operador tiene en la mano cuando lo llaman a + # preguntar por un paquete. + SEARCH_CONDITION = <<~SQL.squish + shipments.tracking_number ILIKE :pattern + OR orders.external_order_id ILIKE :pattern + SQL + def index # La precarga es load-bearing: ShipmentListSerializer lee el nombre del # courier a través de la plantilla del Service, y sin ella son dos @@ -91,7 +100,29 @@ def filtered_shipments shipments = policy_scope(Shipment) shipments = shipments.where(status: status) if status.present? shipments = shipments.where(order_id: order_id) if order_id.present? - shipments + apply_search(shipments) + end + + # El buscador del listado (TESIS-164). Busca por lo único que el operador + # tiene en la mano cuando lo llaman a preguntar por un paquete: el número + # de seguimiento que le dio al comprador, o el id con el que el canal + # nombra la venta. + # + # El join con `orders` es `left_outer`: un envío sin orden no existe —la + # FK es NOT NULL—, pero un INNER acá obligaría a Postgres a resolver el + # join también cuando no hay término, y el listado sin buscar es el caso + # normal. Con el `return` de arriba, el join sólo entra cuando se busca. + # + # Mismo criterio que el buscador de órdenes: ILIKE para ignorar + # mayúsculas, `sanitize_sql_like` para que un `%` tipeado se busque + # literalmente, y sin índice —un `%term%` necesita GIN con `pg_trgm`, que + # es la misma card que `unaccent`—. + def apply_search(shipments) + term = scalar_param(:search).to_s.strip + return shipments if term.blank? + + pattern = "%#{Shipment.sanitize_sql_like(term)}%" + shipments.left_outer_joins(:order).where(SEARCH_CONDITION, pattern: pattern) end # find y no find_by dentro del scope del tenant: el default_scope de diff --git a/spec/requests/api/v1/shipments_spec.rb b/spec/requests/api/v1/shipments_spec.rb index c9621b2..aba6469 100644 --- a/spec/requests/api/v1/shipments_spec.rb +++ b/spec/requests/api/v1/shipments_spec.rb @@ -15,9 +15,9 @@ def auth_headers(user) { 'Authorization' => "Bearer #{response.parsed_body['token']}" } end - def order_for(customer, owner: company) + def order_for(customer, owner: company, external_id: nil) Order.create!(company: owner, customer_name: customer, customer_zip_code: '5000', - customer_address: 'Av. Siempreviva 742') + customer_address: 'Av. Siempreviva 742', external_order_id: external_id) end # El alta del courier sale de spec/support/courier_builders.rb: estaba copiada @@ -88,6 +88,12 @@ def foreign_shipment expect(response).to have_http_status(:bad_request) end + it 'returns 400 for a search that is not a single value' do + get '/api/v1/shipments', params: { search: { foo: 'bar' } }, headers: headers + + expect(response).to have_http_status(:bad_request) + end + it 'says which parameter is wrong' do get '/api/v1/shipments', params: { order_id: %w[1 2] }, headers: headers @@ -196,6 +202,101 @@ def foreign_shipment end end + # El buscador del listado (TESIS-164): los dos códigos que el operador tiene + # en la mano cuando lo llaman a preguntar por un paquete. + describe 'search' do + # Propio y no `shipment_for`: acá hace falta el id externo de la orden, que + # es la mitad de lo que recorre el buscador. + def searchable(customer, status: 'pending', tracking: nil, external_id: nil, owner: company) + Shipment.create!(company: owner, status: status, tracking_number: tracking, + order: order_for(customer, owner: owner, external_id: external_id)) + end + + before do + searchable('Ana', status: 'in_transit', + tracking: 'AND-9920-X8829-Z', external_id: 'ML-1001') + searchable('Beto', external_id: 'TN-2002') + # Alta manual sin despachar: no tiene ninguno de los dos códigos. En SQL + # `NULL ILIKE '%%'` no es `true` sino `NULL`, así que si el buscador se + # aplicara igual con el término vacío, esta fila desaparecería del + # listado normal. + searchable('Cora') + end + + def ids_for(term) + get '/api/v1/shipments', params: { search: term }, headers: headers + response.parsed_body['data'].pluck('tracking_number') + end + + # Mismo número de seguimiento, otra empresa. Current a nil para que el + # fixture nazca afuera: assign_current_company pisaría el company manual. + def twin_of_another_company + Current.set(company_id: nil) do + other = Company.create!(name: 'Tenant C', tax_id: '30-33333333-3') + searchable('Ajena', status: 'in_transit', + tracking: 'AND-9920-X8829-Z', owner: other) + end + end + + it 'matches by tracking number' do + expect(ids_for('9920')).to eq(['AND-9920-X8829-Z']) + end + + # El envío todavía no tiene seguimiento hasta que se despacha, así que el + # id del canal es la única forma de llegar a uno recién abierto. + it 'matches by the external order id of its order' do + expect(ids_for('TN-20')).to eq([nil]) + end + + it 'ignores case' do + expect(ids_for('and-9920')).to eq(['AND-9920-X8829-Z']) + end + + # El paginador y la tabla tienen que decir lo mismo: si el total ignorara + # el término, diría que hay más envíos de los que se ven. + it 'counts only the matching rows in meta.total', :aggregate_failures do + get '/api/v1/shipments', params: { search: '9920' }, headers: headers + + expect(response.parsed_body['data'].length).to eq(1) + expect(response.parsed_body['meta']['total']).to eq(1) + end + + it 'returns every shipment when the term is blank, including the one with no codes' do + get '/api/v1/shipments', params: { search: ' ' }, headers: headers + + expect(response.parsed_body['meta']['total']).to eq(3) + end + + # Un `%` tipeado por el usuario es texto a buscar, no un comodín: sin + # escaparlo, buscar "%" devolvería la tabla entera. + it 'treats a literal % as text and not as a wildcard' do + get '/api/v1/shipments', params: { search: '%' }, headers: headers + + expect(response.parsed_body['data']).to be_empty + end + + it 'narrows the active tab instead of replacing it', :aggregate_failures do + get '/api/v1/shipments', params: { status: 'pending', search: '9920' }, headers: headers + + expect(response.parsed_body['data']).to be_empty + expect(response.parsed_body['meta']['total']).to eq(0) + end + + # El default_scope de CompanyScoped acota `shipments`, pero la condición + # del buscador toca `orders`. El seguimiento lo asigna el courier, no el + # OMS: dos empresas que usan el mismo operador pueden terminar con el + # mismo código, y ahí el buscador no puede ser la vía de entrada. + it 'never reaches the shipment of another company', :aggregate_failures do + mine = Shipment.find_by(tracking_number: 'AND-9920-X8829-Z') + twin_of_another_company + + get '/api/v1/shipments', params: { search: '9920' }, headers: headers + + expect(response.parsed_body['data'].pluck('id')).to eq([mine.id]) + expect(response.parsed_body['meta']['total']).to eq(1) + end + end + # La precarga de company_integration: :service es lo único que sostiene la # columna Carrier: sin ella, el nombre del courier cuesta dos queries por fila. describe 'query count' do