From a21a4aba5d70d9e60afa14c494a8c012bcd0c667 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 31 Jul 2019 23:37:04 +0100 Subject: [PATCH 1/6] Convert spree/api/shipments from rabl to AMS and adapt its spec --- .../spree/api/shipments_controller.rb | 19 ++++++++--------- app/serializers/api/shipment_serializer.rb | 13 ++++++++++++ .../spree/api/shipments_controller_spec.rb | 21 +++++++++---------- 3 files changed, 32 insertions(+), 21 deletions(-) create mode 100644 app/serializers/api/shipment_serializer.rb diff --git a/app/controllers/spree/api/shipments_controller.rb b/app/controllers/spree/api/shipments_controller.rb index 4963c204a4..bf396287d3 100644 --- a/app/controllers/spree/api/shipments_controller.rb +++ b/app/controllers/spree/api/shipments_controller.rb @@ -2,7 +2,7 @@ require 'open_food_network/scope_variant_to_hub' module Spree module Api - class ShipmentsController < Spree::Api::BaseController + class ShipmentsController < ::Api::BaseController respond_to :json before_filter :find_order @@ -18,7 +18,7 @@ module Spree @shipment.refresh_rates @shipment.save! - respond_with(@shipment.reload, default_template: :show) + render json: @shipment.reload, serializer: ::Api::ShipmentSerializer, status: :ok end def update @@ -37,8 +37,7 @@ module Spree @shipment.adjustment.close end - @shipment.reload - respond_with(@shipment, default_template: :show) + render json: @shipment.reload, serializer: ::Api::ShipmentSerializer, status: :ok end def ready @@ -47,11 +46,11 @@ module Spree if @shipment.can_ready? @shipment.ready! else - render "spree/api/shipments/cannot_ready_shipment", status: :unprocessable_entity - return + render(json: { error: I18n.t(:cannot_ready, scope: "spree.api.shipment") }, + status: :unprocessable_entity) && return end end - respond_with(@shipment, default_template: :show) + render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok end def ship @@ -59,7 +58,7 @@ module Spree unless @shipment.shipped? @shipment.ship! end - respond_with(@shipment, default_template: :show) + render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok end def add @@ -68,7 +67,7 @@ module Spree @order.contents.add(variant, quantity, nil, @shipment) - respond_with(@shipment, default_template: :show) + render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok end def remove @@ -78,7 +77,7 @@ module Spree @order.contents.remove(variant, quantity, @shipment) @shipment.reload if @shipment.persisted? - respond_with(@shipment, default_template: :show) + render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok end private diff --git a/app/serializers/api/shipment_serializer.rb b/app/serializers/api/shipment_serializer.rb new file mode 100644 index 0000000000..7aefe826c1 --- /dev/null +++ b/app/serializers/api/shipment_serializer.rb @@ -0,0 +1,13 @@ +module Api + class ShipmentSerializer < ActiveModel::Serializer + attributes :number, :order_id, :stock_location_name + + def order_id + object.order.number + end + + def stock_location_name + object.stock_location.name + end + end +end diff --git a/spec/controllers/spree/api/shipments_controller_spec.rb b/spec/controllers/spree/api/shipments_controller_spec.rb index 2b02eb8534..f08f243f92 100644 --- a/spec/controllers/spree/api/shipments_controller_spec.rb +++ b/spec/controllers/spree/api/shipments_controller_spec.rb @@ -53,7 +53,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_post :create, params expect_valid_response - expect(json_response["inventory_units"].size).to eq 2 + expect(shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.variant.price).to eq(variant.price) end @@ -64,7 +64,7 @@ describe Spree::Api::ShipmentsController, type: :controller do expect(json_response["id"]). to eq(original_shipment_id) expect_valid_response - expect(json_response["inventory_units"].size).to eq 2 + expect(shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.variant.price).to eq(variant.price) end @@ -77,7 +77,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_post :create, params expect_valid_response - expect(json_response["inventory_units"].size).to eq 2 + expect(shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.price).to eq(variant_override.price) end @@ -114,7 +114,7 @@ describe Spree::Api::ShipmentsController, type: :controller do it 'adds a variant to a shipment' do api_put :add, variant_id: variant.to_param, quantity: 2 expect(response.status).to eq(200) - expect(json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size).to eq(2) + expect(shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }).to eq 2 end it 'removes a variant from a shipment' do @@ -122,7 +122,7 @@ describe Spree::Api::ShipmentsController, type: :controller do api_put :remove, variant_id: variant.to_param, quantity: 1 expect(response.status).to eq(200) - expect(json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size).to eq(1) + expect(shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq(1) end end @@ -156,7 +156,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_put :add, params expect_valid_response - expect(inventory_units_for(json_response["inventory_units"], variant).size).to eq 2 + expect(inventory_units_for(variant).size).to eq 2 end it 'returns error code when adding to order contents fails' do @@ -175,7 +175,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_put :add, params expect_valid_response - expect(inventory_units_for(json_response["inventory_units"], variant).size).to eq 2 + expect(inventory_units_for(variant).size).to eq 2 expect(order.reload.line_items.last.price).to eq(variant_override.price) end end @@ -190,7 +190,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_put :remove, params expect_valid_response - expect(inventory_units_for(json_response["inventory_units"], variant).size).to eq 0 + expect(inventory_units_for(variant).size).to eq 0 end it 'returns error code when removing from order contents fails' do @@ -203,14 +203,13 @@ describe Spree::Api::ShipmentsController, type: :controller do end end - def inventory_units_for(inventory_units, variant) - inventory_units.select { |unit| unit['variant_id'] == variant.id } + def inventory_units_for(variant) + shipment.inventory_units.select { |unit| unit['variant_id'] == variant.id } end def expect_valid_response expect(response.status).to eq 200 attributes.all?{ |attr| json_response.key? attr.to_s } - expect(json_response["shipping_method"]["name"]).to eq order.shipping_method.name end def make_order_contents_fail From 26f5ece7c0455f4374aebeeb2629745580de4217 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 11:04:29 +0100 Subject: [PATCH 2/6] Add a few relevant attributes to shipment serializer and fix some details in shipment controller spec --- app/serializers/api/shipment_serializer.rb | 2 +- .../spree/api/shipments_controller_spec.rb | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/serializers/api/shipment_serializer.rb b/app/serializers/api/shipment_serializer.rb index 7aefe826c1..71fc27450b 100644 --- a/app/serializers/api/shipment_serializer.rb +++ b/app/serializers/api/shipment_serializer.rb @@ -1,6 +1,6 @@ module Api class ShipmentSerializer < ActiveModel::Serializer - attributes :number, :order_id, :stock_location_name + attributes :id, :tracking, :number, :order_id, :cost, :shipped_at, :stock_location_name, :state def order_id object.order.number diff --git a/spec/controllers/spree/api/shipments_controller_spec.rb b/spec/controllers/spree/api/shipments_controller_spec.rb index f08f243f92..6bde1ff395 100644 --- a/spec/controllers/spree/api/shipments_controller_spec.rb +++ b/spec/controllers/spree/api/shipments_controller_spec.rb @@ -4,7 +4,7 @@ describe Spree::Api::ShipmentsController, type: :controller do render_views let!(:shipment) { create(:shipment) } - let!(:attributes) { [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id, :shipping_rates, :shipping_method, :inventory_units] } + let!(:attributes) { [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id] } let!(:resource_scoping) { { order_id: shipment.order.to_param, id: shipment.to_param } } let(:current_api_user) { build(:user) } @@ -53,7 +53,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_post :create, params expect_valid_response - expect(shipment.reload.inventory_units.size).to eq 2 + expect(order.shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.variant.price).to eq(variant.price) end @@ -64,7 +64,7 @@ describe Spree::Api::ShipmentsController, type: :controller do expect(json_response["id"]). to eq(original_shipment_id) expect_valid_response - expect(shipment.reload.inventory_units.size).to eq 2 + expect(order.shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.variant.price).to eq(variant.price) end @@ -77,7 +77,7 @@ describe Spree::Api::ShipmentsController, type: :controller do spree_post :create, params expect_valid_response - expect(shipment.reload.inventory_units.size).to eq 2 + expect(order.shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.price).to eq(variant_override.price) end @@ -113,16 +113,17 @@ describe Spree::Api::ShipmentsController, type: :controller do it 'adds a variant to a shipment' do api_put :add, variant_id: variant.to_param, quantity: 2 + expect(response.status).to eq(200) - expect(shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }).to eq 2 + expect(order.shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq 2 end it 'removes a variant from a shipment' do order.contents.add(variant, 2) - api_put :remove, variant_id: variant.to_param, quantity: 1 + expect(response.status).to eq(200) - expect(shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq(1) + expect(order.shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq(1) end end @@ -204,7 +205,7 @@ describe Spree::Api::ShipmentsController, type: :controller do end def inventory_units_for(variant) - shipment.inventory_units.select { |unit| unit['variant_id'] == variant.id } + order.shipment.reload.inventory_units.select { |unit| unit['variant_id'] == variant.id } end def expect_valid_response From 7cec24f1d41618ac29821af30fdfb948366b4513 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 11:07:06 +0100 Subject: [PATCH 3/6] Move shipments route, controller and ctrl spec from spree/api to api --- .../{spree => }/api/shipments_controller.rb | 0 config/routes/api.rb | 13 +++++++++++++ config/routes/spree.rb | 13 ------------- .../{spree => }/api/shipments_controller_spec.rb | 0 4 files changed, 13 insertions(+), 13 deletions(-) rename app/controllers/{spree => }/api/shipments_controller.rb (100%) rename spec/controllers/{spree => }/api/shipments_controller_spec.rb (100%) diff --git a/app/controllers/spree/api/shipments_controller.rb b/app/controllers/api/shipments_controller.rb similarity index 100% rename from app/controllers/spree/api/shipments_controller.rb rename to app/controllers/api/shipments_controller.rb diff --git a/config/routes/api.rb b/config/routes/api.rb index a81fdec770..b9b1385677 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -15,6 +15,19 @@ Openfoodnetwork::Application.routes.draw do resources :variants, :only => [:index] + resources :orders do + get :managed, on: :collection + + resources :shipments, :only => [:create, :update] do + member do + put :ready + put :ship + put :add + put :remove + end + end + end + resources :enterprises do post :update_image, on: :member get :managed, on: :collection diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 240ee39083..abdb1f477d 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -61,19 +61,6 @@ Spree::Core::Engine.routes.prepend do resources :users do get :authorise_api, on: :collection end - - resources :orders do - get :managed, on: :collection - - resources :shipments, :only => [:create, :update] do - member do - put :ready - put :ship - put :add - put :remove - end - end - end end namespace :admin do diff --git a/spec/controllers/spree/api/shipments_controller_spec.rb b/spec/controllers/api/shipments_controller_spec.rb similarity index 100% rename from spec/controllers/spree/api/shipments_controller_spec.rb rename to spec/controllers/api/shipments_controller_spec.rb From e9b5551c0f7bf2bda9d29c2747802c8662f003ef Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 11:08:16 +0100 Subject: [PATCH 4/6] Adpat shipment controller to move out of Spree namespace --- app/controllers/api/shipments_controller.rb | 156 +++++++++--------- .../api/shipments_controller_spec.rb | 2 +- 2 files changed, 78 insertions(+), 80 deletions(-) diff --git a/app/controllers/api/shipments_controller.rb b/app/controllers/api/shipments_controller.rb index bf396287d3..4c9bb047eb 100644 --- a/app/controllers/api/shipments_controller.rb +++ b/app/controllers/api/shipments_controller.rb @@ -1,107 +1,105 @@ require 'open_food_network/scope_variant_to_hub' -module Spree - module Api - class ShipmentsController < ::Api::BaseController - respond_to :json +module Api + class ShipmentsController < Api::BaseController + respond_to :json - before_filter :find_order - before_filter :find_and_update_shipment, only: [:ship, :ready, :add, :remove] + before_filter :find_order + before_filter :find_and_update_shipment, only: [:ship, :ready, :add, :remove] - def create - variant = scoped_variant(params[:variant_id]) - quantity = params[:quantity].to_i - @shipment = get_or_create_shipment(params[:stock_location_id]) + def create + variant = scoped_variant(params[:variant_id]) + quantity = params[:quantity].to_i + @shipment = get_or_create_shipment(params[:stock_location_id]) - @order.contents.add(variant, quantity, nil, @shipment) + @order.contents.add(variant, quantity, nil, @shipment) - @shipment.refresh_rates - @shipment.save! + @shipment.refresh_rates + @shipment.save! - render json: @shipment.reload, serializer: ::Api::ShipmentSerializer, status: :ok + render json: @shipment.reload, serializer: Api::ShipmentSerializer, status: :ok + end + + def update + authorize! :read, Spree::Shipment + @shipment = @order.shipments.find_by_number!(params[:id]) + params[:shipment] ||= [] + unlock = params[:shipment].delete(:unlock) + + if unlock == 'yes' + @shipment.adjustment.open end - def update - authorize! :read, Shipment - @shipment = @order.shipments.find_by_number!(params[:id]) - params[:shipment] ||= [] - unlock = params[:shipment].delete(:unlock) + @shipment.update_attributes(params[:shipment]) - if unlock == 'yes' - @shipment.adjustment.open + if unlock == 'yes' + @shipment.adjustment.close + end + + render json: @shipment.reload, serializer: Api::ShipmentSerializer, status: :ok + end + + def ready + authorize! :read, Spree::Shipment + unless @shipment.ready? + if @shipment.can_ready? + @shipment.ready! + else + render(json: { error: I18n.t(:cannot_ready, scope: "spree.api.shipment") }, + status: :unprocessable_entity) && return end - - @shipment.update_attributes(params[:shipment]) - - if unlock == 'yes' - @shipment.adjustment.close - end - - render json: @shipment.reload, serializer: ::Api::ShipmentSerializer, status: :ok end + render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok + end - def ready - authorize! :read, Shipment - unless @shipment.ready? - if @shipment.can_ready? - @shipment.ready! - else - render(json: { error: I18n.t(:cannot_ready, scope: "spree.api.shipment") }, - status: :unprocessable_entity) && return - end - end - render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok + def ship + authorize! :read, Spree::Shipment + unless @shipment.shipped? + @shipment.ship! end + render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok + end - def ship - authorize! :read, Shipment - unless @shipment.shipped? - @shipment.ship! - end - render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok - end + def add + variant = scoped_variant(params[:variant_id]) + quantity = params[:quantity].to_i - def add - variant = scoped_variant(params[:variant_id]) - quantity = params[:quantity].to_i + @order.contents.add(variant, quantity, nil, @shipment) - @order.contents.add(variant, quantity, nil, @shipment) + render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok + end - render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok - end + def remove + variant = scoped_variant(params[:variant_id]) + quantity = params[:quantity].to_i - def remove - variant = scoped_variant(params[:variant_id]) - quantity = params[:quantity].to_i + @order.contents.remove(variant, quantity, @shipment) + @shipment.reload if @shipment.persisted? - @order.contents.remove(variant, quantity, @shipment) - @shipment.reload if @shipment.persisted? + render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok + end - render json: @shipment, serializer: ::Api::ShipmentSerializer, status: :ok - end + private - private + def find_order + @order = Spree::Order.find_by_number!(params[:order_id]) + authorize! :read, @order + end - def find_order - @order = Spree::Order.find_by_number!(params[:order_id]) - authorize! :read, @order - end + def find_and_update_shipment + @shipment = @order.shipments.find_by_number!(params[:id]) + @shipment.update_attributes(params[:shipment]) + @shipment.reload + end - def find_and_update_shipment - @shipment = @order.shipments.find_by_number!(params[:id]) - @shipment.update_attributes(params[:shipment]) - @shipment.reload - end + def scoped_variant(variant_id) + variant = Spree::Variant.find(variant_id) + OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) + variant + end - def scoped_variant(variant_id) - variant = Spree::Variant.find(variant_id) - OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) - variant - end - - def get_or_create_shipment(stock_location_id) - @order.shipment || @order.shipments.create(stock_location_id: stock_location_id) - end + def get_or_create_shipment(stock_location_id) + @order.shipment || @order.shipments.create(stock_location_id: stock_location_id) end end end diff --git a/spec/controllers/api/shipments_controller_spec.rb b/spec/controllers/api/shipments_controller_spec.rb index 6bde1ff395..9e838617e3 100644 --- a/spec/controllers/api/shipments_controller_spec.rb +++ b/spec/controllers/api/shipments_controller_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Spree::Api::ShipmentsController, type: :controller do +describe Api::ShipmentsController, type: :controller do render_views let!(:shipment) { create(:shipment) } From f0586af1c7ed14f840545675d68c6c4babe8ad76 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 11:34:17 +0100 Subject: [PATCH 5/6] Re-organized api/orders resource routes --- config/routes/api.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config/routes/api.rb b/config/routes/api.rb index b9b1385677..94b9e180f0 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -15,7 +15,7 @@ Openfoodnetwork::Application.routes.draw do resources :variants, :only => [:index] - resources :orders do + resources :orders, only: [:index] do get :managed, on: :collection resources :shipments, :only => [:create, :update] do @@ -46,8 +46,6 @@ Openfoodnetwork::Application.routes.draw do get :accessible, on: :collection end - resources :orders, only: [:index] - resource :status do get :job_queue end From b4de8ef899ab6a5de60c23b4983da90581ba6781 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 3 Sep 2019 13:55:41 +0100 Subject: [PATCH 6/6] Make enterprises/index_spec a bit more resilient --- spec/features/admin/enterprises/index_spec.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/enterprises/index_spec.rb b/spec/features/admin/enterprises/index_spec.rb index 28d8843d1d..b4897cbb11 100644 --- a/spec/features/admin/enterprises/index_spec.rb +++ b/spec/features/admin/enterprises/index_spec.rb @@ -76,12 +76,20 @@ feature 'Enterprises Index' do visit admin_enterprises_path end + def enterprise_row_index(enterprise_name) + enterprise_row_number = all('tr').index { |tr| tr.text.include? enterprise_name } + enterprise_row_number - 1 + end + it "does not update the enterprises and displays errors" do + d_row_index = enterprise_row_index(d.name) within("tr.enterprise-#{d.id}") do - select d_manager.email, from: 'enterprise_set_collection_attributes_0_owner_id' + select d_manager.email, from: "enterprise_set_collection_attributes_#{d_row_index}_owner_id" end + + second_distributor_row_index = enterprise_row_index(second_distributor.name) within("tr.enterprise-#{second_distributor.id}") do - select d_manager.email, from: 'enterprise_set_collection_attributes_1_owner_id' + select d_manager.email, from: "enterprise_set_collection_attributes_#{second_distributor_row_index}_owner_id" end click_button "Update" expect(flash_message).to eq('Update failed')