Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion app/controllers/api/v1/shipments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
105 changes: 103 additions & 2 deletions spec/requests/api/v1/shipments_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Loading