proyectoFinalFRLP · TomasMartin2004 · Sep 29, 2026 · Sep 28, 2026 · Sep 29, 2026
@@ -251,8 +251,18 @@ module Api
         authorize @product
       end
 
+      # expect y no require + permit: require devuelve lo que haya bajo la
+      # clave, y permit sobre un String levanta NoMethodError -> 500. expect
+      # responde 400 a cualquier cosa que no sea un objeto (TESIS-133).
+      #
+      # Ojo con el ProductsController real, que NO usa expect: cuando el body
+      # trae una parte que se recorre a mano —las `stocks` de un producto, las
+      # `items` de una orden—, esa clave no entra en la lista, y expect toma
+      # como faltante un filtrado que queda vacío. Un PUT que sólo manda stocks
+      # terminaba en 400. Ahí el envoltorio se valida con `body_of`
+      # (ApplicationController) y el permit va sobre lo que devuelve.
       def product_params
-        params.require(:product).permit(:sku, :name, :stock)
+        params.expect(product: %i[sku name stock])
       end
     end
   end

@@ -10,10 +10,6 @@ module V1
     # que el asistente ya juntó: depósito de origen, destino y líneas. La orden se
     # crea una sola vez, cuando el operador elige y confirma.
     class DraftQuotesController < ApplicationController
-      # El parámetro que falta es un 400 de contrato, con el mismo cuerpo
-      # `{ error }` que el resto de la API.
-      rescue_from ActionController::ParameterMissing, with: :render_bad_request
-
       # Es el primer endpoint autenticado que sale a los couriers sin dejar nada
       # en la base: antes cotizar exigía crear la orden, que era un freno natural.
       # Sin tope, un cliente en loop —un `useEffect` mal puesto que cotiza en

@@ -8,17 +8,18 @@ class OrdersController < ApplicationController
 
       rescue_from ActiveRecord::RecordNotSaved, with: :render_unprocessable
       rescue_from Catalog::InsufficientStockError, with: :render_insufficient_stock
-      # ParameterMissing no es 422 de negocio: es un 400 de contrato. Rescatarlo
-      # acá mantiene la forma del body ({error: ...}) consistente con el resto
-      # de la API en vez del default de Rails.
-      rescue_from ActionController::ParameterMissing, with: :render_bad_request
       # Una orden cancelada o con el envío ya despachado: no hay body que haga
       # pasar el mismo PUT, así que es 409 y no 422 (TESIS-126).
       rescue_from Orders::OrderNotEditableError, with: :render_conflict
       rescue_from Orders::StaleOrderError, with: :render_precondition_failed
 
       MAX_ITEMS = 100
 
+      # Los datos del cliente que acepta el body. `items` queda afuera: se arma
+      # aparte, línea por línea, en `items_params`.
+      ORDER_FIELDS = %i[customer_name customer_document customer_address
+                        customer_zip_code customer_city customer_province].freeze
+
       # Campos sobre los que corre el buscador del listado (TESIS-52). Son las
       # formas en que un operador nombra una venta: el id con el que la conoce el
       # canal externo, el nombre con el que la cargó, y a dónde va.
@@ -161,34 +162,36 @@ def apply_search(orders)
         orders.where(condition, pattern: pattern)
       end
 
-      def order_params(*extra_keys)
-        order = params.require(:order)
-        unless order.is_a?(ActionController::Parameters)
-          raise ActiveRecord::RecordNotSaved, 'order must be an object'
-        end
-
-        order.permit(:customer_name, :customer_document,
-                     :customer_address, :customer_zip_code,
-                     :customer_city, :customer_province, *extra_keys)
+      def order_params
+        order_body.permit(*ORDER_FIELDS)
       end
 
       # Lo mismo que el alta más el estado. Qué valores de estado se aceptan lo
       # decide Orders::UpdateOrder (sólo pending y paid).
       def update_params
-        order_params(:status)
+        order_body.permit(*ORDER_FIELDS, :status)
+      end
+
+      # El envoltorio, validado como objeto por `body_of`. La forma es contrato,
+      # así que un `order` String o Array es 400 y no el 422 de negocio que
+      # devolvía el RecordNotSaved que había acá (TESIS-133). Las `items` se leen
+      # de este mismo objeto: cada línea tiene sus propios 422 con mensaje —y el
+      # tope de MAX_ITEMS—, que un filtrado de strong params no sabe dar.
+      def order_body
+        body_of(:order)
       end
 
       # Sin `items` en el body, las líneas no se tocan: un PUT que sólo corrige la
       # dirección no tiene por qué mandar la orden entera. `id` identifica una
       # línea que ya existe; sin él, la línea es nueva.
       def update_items_params
-        return nil unless params[:order].key?(:items)
+        return nil unless order_body.key?(:items)
 
         items_params(:id)
       end
 
       def items_params(*extra_keys)
-        raw = params[:order][:items]
+        raw = order_body[:items]
         raise ActiveRecord::RecordNotSaved, 'items must be an array' unless raw.is_a?(Array)
         if raw.size > MAX_ITEMS
           raise ActiveRecord::RecordNotSaved,

@@ -74,10 +74,13 @@ def company_integration
 
       # El body va anidado bajo `product_mapping`, igual que `product` en
       # ProductsController: los dos endpoints del mismo árbol comparten contrato.
+      #
+      # `expect` y no `require` + `permit`: un `product_mapping` que no sea un
+      # objeto responde 400 en vez de reventar con 500 (TESIS-133).
       def mapping_params
-        # rubocop:disable-next Rails/StrongParametersExpect
-        params.require(:product_mapping)
-              .permit(:company_integration_id, :external_product_id, :external_price)
+        params.expect(
+          product_mapping: %i[company_integration_id external_product_id external_price]
+        )
       end
 
       # El índice único (company_integration_id, external_product_id) no tiene

@@ -6,6 +6,10 @@ class ProductsController < ApplicationController
       include OptimisticLocking
       include Paginatable
 
+      # Qué acepta el body del producto. `stocks` no entra: se arma aparte en
+      # `stock_params`, línea por línea.
+      PRODUCT_FIELDS = %i[sku name description category weight dimensions].freeze
+
       before_action :set_product, only: %i[show update destroy]
       rescue_from ActiveRecord::RecordNotUnique, with: :render_conflict
       rescue_from ActiveRecord::RecordNotSaved, with: :render_unprocessable
@@ -110,16 +114,23 @@ def set_product
         authorize @product
       end
 
+      # Un company_id en el body se sigue descartando en silencio: `permit` sólo
+      # deja pasar lo que lista.
       def product_params
-        # permit (no expect) es intencional y load-bearing: expect usa
-        # on_unpermitted: :raise, así que un body con company_id daría 400 en
-        # vez de ignorarlo — rompiendo el requisito de la card.
-        # rubocop:disable-next Rails/StrongParametersExpect
-        params.require(:product).permit(:sku, :name, :description, :category, :weight, :dimensions)
+        product_body.permit(*PRODUCT_FIELDS)
+      end
+
+      # El envoltorio, validado como objeto por `body_of` (TESIS-133). Antes esto
+      # era `params.require(:product)`, que devolvía el String tal cual y hacía
+      # reventar al `permit` de abajo con un 500.
+      def product_body
+        body_of(:product)
       end
 
+      # `stocks` queda afuera de `expect`: cada línea necesita sus propios 422
+      # con mensaje, que `expect` no sabe dar.
       def stock_params
-        raw_stocks = params[:product][:stocks]
+        raw_stocks = product_body[:stocks]
         return [] if raw_stocks.nil?
 
         # Si stocks viene presente pero no es un array (ej. un objeto), es un

@@ -17,8 +17,6 @@ class ShipmentsController < ApplicationController
       rescue_from Shipments::DispatchResponseError, with: :render_bad_gateway
       rescue_from Shipments::InvalidShippingCostError, with: :render_bad_request
       rescue_from Integrations::AdapterExecutionError, with: :render_courier_failure
-      # El parámetro que falta es un 400 de contrato, no un 422 de negocio.
-      rescue_from ActionController::ParameterMissing, with: :render_bad_request
 
       include Paginatable
 

@@ -77,12 +77,13 @@ def warehouse_for(key)
         Warehouse.find(transfer_params[key])
       end
 
+      # `expect` y no `require` + `permit`, igual que en productos: un
+      # `stock_transfer` que no sea un objeto es 400 y no un 500 (TESIS-133).
+      # Un company_id en el body se sigue descartando en silencio.
       def transfer_params
-        # permit y no expect, igual que en productos: un body con company_id se
-        # ignora en lugar de devolver 400.
-        # rubocop:disable-next Rails/StrongParametersExpect
-        params.require(:stock_transfer)
-              .permit(:product_id, :origin_warehouse_id, :destination_warehouse_id, :quantity)
+        params.expect(
+          stock_transfer: %i[product_id origin_warehouse_id destination_warehouse_id quantity]
+        )
       end
 
       def render_conflict(exception)

@@ -23,6 +23,12 @@ class MalformedParameterError < StandardError; end
   rescue_from Catalog::LockTimeoutError, with: :render_lock_conflict
   rescue_from ActiveRecord::CheckViolation, with: :render_constraint_violation
   rescue_from MalformedParameterError, with: :render_bad_request
+  # Lo que levanta `params.expect` cuando el body no trae el recurso o no es un
+  # objeto. Rails ya lo mapea a 400, pero con su propio cuerpo; acá se rescata
+  # para que el error viaje como {"error": "..."} igual que todos los demás
+  # (ADR-015). Vive en el padre porque los cuatro controllers que migraron a
+  # `expect` en TESIS-133 lo necesitan por igual.
+  rescue_from ActionController::ParameterMissing, with: :render_bad_request
 
   # Las acciones index usan policy_scope; el resto deben llamar authorize.
   # Si una acción futura olvida el authorize, falla en vez de pasar sin ruido.
@@ -91,6 +97,23 @@ def scalar_param(name)
     raise MalformedParameterError, "#{name} must be a single value"
   end
 
+  # El envoltorio del body (`params[:product]`, `params[:order]`), validado como
+  # objeto y no vacío. `params.expect` hace esto mismo y además filtra, y es lo
+  # que usan los controllers cuyo body no tiene partes que se arman a mano.
+  #
+  # Acá no alcanza: `expect` considera faltante un filtrado que queda vacío, y
+  # un PUT que sólo manda `stocks` o `items` —las dos partes que se recorren
+  # línea por línea, cada una con sus propios 422— es un request válido que no
+  # puede terminar en 400. El chequeo de forma, que es lo que evitaba el 500,
+  # es el mismo: `permit` sobre un String levanta NoMethodError (TESIS-133).
+  def body_of(key)
+    body = params[key]
+    raise ActionController::ParameterMissing, key unless body.is_a?(ActionController::Parameters)
+    raise ActionController::ParameterMissing, key if body.empty?
+
+    body
+  end
+
   def render_bad_request(exception)
     render json: { error: exception.message }, status: :bad_request
   end

@@ -265,5 +265,22 @@ def foreign_order
 
       expect(response).to have_http_status(:bad_request)
     end
+
+    # `expect` corta por la forma del envoltorio antes de tocar la orden
+    # (TESIS-133); antes de eso, `permit` sobre un String era un 500.
+    it 'returns 400 and changes nothing when order is not an object', :aggregate_failures do
+      put "/api/v1/orders/#{order.id}", params: { order: 'Otro' }, headers: headers, as: :json
+
+      expect(response).to have_http_status(:bad_request)
+      expect(order.reload.customer_name).to eq('Juan Pérez')
+    end
+
+    it 'returns 400 and changes nothing when order is a list', :aggregate_failures do
+      put "/api/v1/orders/#{order.id}", params: { order: [{ customer_name: 'Otro' }] },
+                                        headers: headers, as: :json
+
+      expect(response).to have_http_status(:bad_request)
+      expect(order.reload.customer_name).to eq('Juan Pérez')
+    end
   end
 end
@@ -402,11 +402,24 @@ def order_of_another_company
         expect(response).to have_http_status(:unprocessable_content)
       end
 
-      it 'rejects when order is not an object' do
-        post '/api/v1/orders',
-             params: { order: 'not_an_object' },
-             headers: headers, as: :json
-        expect(response).to have_http_status(:unprocessable_content)
+      # 400 y no 422: la forma del envoltorio es contrato, no negocio. Lo da
+      # `params.expect` desde TESIS-133; antes era un RecordNotSaved a mano.
+      it 'rejects when order is not an object with a 400', :aggregate_failures do
+        expect do
+          post '/api/v1/orders', params: { order: 'not_an_object' },
+                                 headers: headers, as: :json
+        end.not_to change(Order, :count)
+
+        expect(response).to have_http_status(:bad_request)
+      end
+
+      it 'rejects when order is a list with a 400', :aggregate_failures do
+        expect do
+          post '/api/v1/orders', params: { order: [{ customer_name: 'X' }] },
+                                 headers: headers, as: :json
+        end.not_to change(Order, :count)
+
+        expect(response).to have_http_status(:bad_request)
       end
 
       it 'rejects when items is not an array' do

@@ -197,6 +197,26 @@ def post_mapping(product_id: product.id, **attrs)
       expect(response).to have_http_status(:not_found)
     end
 
+    # Mismo contrato que POST /products: un envoltorio que no es un objeto es
+    # 400 y no el 500 que daba `permit` sobre un String (TESIS-133).
+    it 'returns 400 when product_mapping is not an object', :aggregate_failures do
+      expect do
+        post mappings_url(product.id), params: { product_mapping: 'MLA-123' },
+                                       headers: headers, as: :json
+      end.not_to change(ProductMapping, :count)
+
+      expect(response).to have_http_status(:bad_request)
+    end
+
+    it 'returns 400 when product_mapping is a list', :aggregate_failures do
+      expect do
+        post mappings_url(product.id), params: { product_mapping: [valid_params[:product_mapping]] },
+                                       headers: headers, as: :json
+      end.not_to change(ProductMapping, :count)
+
+      expect(response).to have_http_status(:bad_request)
+    end
+
     it 'returns 409 when the external id is already linked in that integration', :aggregate_failures do
       link_external_id_to_another_product('MLA-123')
 

@@ -702,6 +702,29 @@ def stale
       post '/api/v1/products', params: { product: product_attrs }, headers: headers, as: :json
       expect(response).to have_http_status(:unprocessable_content)
     end
+
+    # Hallazgo de la QA de TESIS-82, que acá seguía vivo: `require` devolvía el
+    # String tal cual y `permit` reventaba sobre él, con un 500 (TESIS-133).
+    it 'returns 400 when product is not an object', :aggregate_failures do
+      expect do
+        post '/api/v1/products', params: { product: 'Teclado' }, headers: headers, as: :json
+      end.not_to change(Product, :count)
+
+      expect(response).to have_http_status(:bad_request)
+    end
+
+    it 'returns 400 when product is a list', :aggregate_failures do
+      expect do
+        post '/api/v1/products', params: { product: [product_attrs] }, headers: headers, as: :json
+      end.not_to change(Product, :count)
+
+      expect(response).to have_http_status(:bad_request)
+    end
+
+    it 'returns 400 when the product key is missing' do
+      post '/api/v1/products', params: {}, headers: headers, as: :json
+      expect(response).to have_http_status(:bad_request)
+    end
   end
 
   describe 'PUT /api/v1/products/:id' do
@@ -732,6 +755,22 @@ def stale
       expect(response).to have_http_status(:not_found)
     end
 
+    it 'returns 400 and changes nothing when product is not an object', :aggregate_failures do
+      put "/api/v1/products/#{product.id}", params: { product: 'Updated' },
+                                            headers: headers, as: :json
+
+      expect(response).to have_http_status(:bad_request)
+      expect(product.reload.name).to eq('Original')
+    end
+
+    it 'returns 400 and changes nothing when product is a list', :aggregate_failures do
+      put "/api/v1/products/#{product.id}", params: { product: [{ name: 'Updated' }] },
+                                            headers: headers, as: :json
+
+      expect(response).to have_http_status(:bad_request)
+      expect(product.reload.name).to eq('Original')
+    end
+
     it 'rejects stocks with a warehouse from another company' do
       params = { product: { stocks: stocks_for(other_warehouse.id, quantity: 5) } }
       put "/api/v1/products/#{product.id}", params: params, headers: headers, as: :json

@@ -69,6 +69,26 @@ def dispatch_one(quantity: 4)
       expect(response).to have_http_status(:unprocessable_content)
     end
 
+    # Mismo hallazgo que en warehouses y productos: `permit` sobre un String
+    # era un 500; con `expect` es un 400 de contrato (TESIS-133).
+    it 'returns 400 when stock_transfer is not an object', :aggregate_failures do
+      expect do
+        post '/api/v1/stock-transfers', params: { stock_transfer: 'PROD-1' },
+                                        headers: headers, as: :json
+      end.not_to change(StockTransfer, :count)
+
+      expect(response).to have_http_status(:bad_request)
+    end
+
+    it 'returns 400 when stock_transfer is a list', :aggregate_failures do
+      expect do
+        post '/api/v1/stock-transfers', params: { stock_transfer: [body_for(4)[:stock_transfer]] },
+                                        headers: headers, as: :json
+      end.not_to change(StockTransfer, :count)
+
+      expect(response).to have_http_status(:bad_request)
+    end
+
     it 'returns 404 for a product of another company' do
       params = body_for(1).deep_merge(stock_transfer: { product_id: foreign_product.id })
       post '/api/v1/stock-transfers', params: params, headers: headers, as: :json