Merge pull request #3441 from luisramos0/2-0-shipments-api

[Spree Upgrade] Adapt api shipments controller to OFN by scoping variants before each action

app/controllers/spree/admin/line_items_controller_decorator.rb

@@ -1,60 +0,0 @@
-require 'open_food_network/scope_variant_to_hub'
-
-Spree::Admin::LineItemsController.class_eval do
-  prepend_before_filter :load_order, except: :index
-  around_filter :apply_enterprise_fees_with_lock, only: :update
-
-  def create
-    variant = Spree::Variant.find(params[:line_item][:variant_id])
-    OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant)
-
-    @line_item = @order.add_variant(variant, params[:line_item][:quantity].to_i)
-
-    if @order.save
-      respond_with(@line_item) do |format|
-        format.html { render :partial => 'spree/admin/orders/form', :locals => { :order => @order.reload } }
-      end
-    else
-      respond_with(@line_item) do |format|
-        format.js { render :action => 'create', :locals => { :order => @order.reload } }
-      end
-    end
-  end
-
-  # TODO: simplify this, 3 formats per action is too much
-  #       we need `js` format for admin/orders/edit (jquery-rails gem)
-  #       we don't know if `html` format is needed
-  def update
-    respond_to do |format|
-      format.html { render_order_form }
-      format.js {
-        if @line_item.update_attributes(params[:line_item])
-          render nothing: true, status: 204 # No Content, does not trigger ng resource auto-update
-        else
-          render json: { errors: @line_item.errors }, status: 412
-        end
-      }
-    end
-  end
-
-  private
-
-  def render_order_form
-    respond_to do |format|
-      format.html { render partial: 'spree/admin/orders/form', locals: {order: @order.reload} }
-    end
-  end
-
-  def load_order
-    @order = Spree::Order.find_by_number!(params[:order_id])
-    authorize! :update, @order
-  end
-
-  def apply_enterprise_fees_with_lock
-    authorize! :read, @order
-    @order.with_lock do
-      yield
-      @order.update_distribution_charge!
-    end
-  end
-end

app/controllers/spree/api/shipments_controller_decorator.rb

@@ -0,0 +1,47 @@
+require 'open_food_network/scope_variant_to_hub'
+
+Spree::Api::ShipmentsController.class_eval do
+  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)
+
+    @shipment.refresh_rates
+    @shipment.save!
+
+    respond_with(@shipment.reload, default_template: :show)
+  end
+
+  def add
+    variant = scoped_variant(params[:variant_id])
+    quantity = params[:quantity].to_i
+
+    @order.contents.add(variant, quantity, nil, @shipment)
+
+    respond_with(@shipment, default_template: :show)
+  end
+
+  def remove
+    variant = scoped_variant(params[:variant_id])
+    quantity = params[:quantity].to_i
+
+    @order.contents.remove(variant, quantity, @shipment)
+    @shipment.reload if @shipment.persisted?
+
+    respond_with(@shipment, default_template: :show)
+  end
+
+  private
+
+  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
+end

app/views/spree/admin/line_items/create.js.erb

@@ -1,6 +0,0 @@
-// The admin order form contains angular directives, so it needs to be
-// compiled before insertion into the DOM
-var scope = angular.element("#order-form-wrapper").scope();
-$("#order-form-wrapper").html(scope.$compile('<%= escape_javascript(render :partial => "spree/admin/orders/form") %>')(scope));
-scope.$apply();
-$('select.select2').select2({allowClear: true});

app/views/spree/admin/orders/edit.html.haml

@@ -26,9 +26,8 @@
       .no-objects-found
         = Spree.t(:your_order_is_empty_add_product)
 
-    %div{"data-hook" => "admin_order_edit_form"}
-      #order-form-wrapper
-        = render :partial => 'form', :locals => { :order => @order }
+    %div
+      = render :partial => 'form', :locals => { :order => @order }
 
 - content_for :head do
   = javascript_tag 'var expand_variants = true;'

app/views/spree/admin/orders/new.html.haml

@@ -11,7 +11,7 @@
 
 = csrf_meta_tags
 
-%div{"data-hook" => "admin_order_new_header"}
+%div
   = render 'spree/shared/error_messages', :target => @order
 
 %div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"}
@@ -19,9 +19,8 @@
     = render 'add_product'
 
   - unless @order.line_items.any?
-    %div{"data-hook" => "admin_order_new_form"}
-      #order-form-wrapper
-        = render 'form'
+    %div
+      = render 'form'
 
 - content_for :head do
   = javascript_tag 'var expand_variants = true;'

app/views/spree/admin/orders/set_distribution.html.haml

@@ -11,7 +11,7 @@
 
 = csrf_meta_tags
 
-%div{"data-hook" => "admin_order_new_header"}
+%div
   = render 'spree/shared/error_messages', :target => @order
 
 %div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"}

spec/controllers/spree/admin/line_items_controller_spec.rb

@@ -1,113 +0,0 @@
-require 'spec_helper'
-
-describe Spree::Admin::LineItemsController, type: :controller do
-  include AuthenticationWorkflow
-
-  describe "#create" do
-    let!(:variant) { create(:variant, price: 88) }
-    let!(:vo) { create(:variant_override, hub: distributor, variant: variant, price: 11.11) }
-    let!(:distributor) { create(:distributor_enterprise) }
-    let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: [variant]) }
-    let!(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
-    let(:params) { { order_id: order.number, line_item: { variant_id: variant.id, quantity: 1 } } }
-
-    before { login_as_admin }
-
-    it "takes variant overrides into account for price" do
-      spree_post :create, params
-
-      order.line_items(:reload).last.price.should == 11.11
-    end
-  end
-
-  describe '#update' do
-    let(:supplier) { create(:supplier_enterprise) }
-    let(:distributor1) { create(:distributor_enterprise) }
-    let(:coordinator) { create(:distributor_enterprise) }
-    let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) }
-    let!(:order1) { FactoryBot.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryBot.create(:address) ) }
-    let!(:line_item1) { FactoryBot.create(:line_item_with_shipment, order: order1, product: FactoryBot.create(:product, supplier: supplier)) }
-    let(:line_item_params) { { quantity: 3, final_weight_volume: 3000, price: 3.00 } }
-    let(:params) { { id: line_item1.id, order_id: order1.number, line_item: line_item_params } }
-
-    context "as an enterprise user" do
-      context "producer enterprise" do
-        before do
-          controller.stub spree_current_user: supplier.owner
-          spree_put :update, params
-        end
-
-        it "does not allow access" do
-          expect(response).to redirect_to spree.unauthorized_path
-        end
-      end
-
-      context "coordinator enterprise" do
-        render_views
-
-        before do
-          controller.stub spree_current_user: coordinator.owner
-        end
-
-        # Used in admin/orders/edit
-        context 'when the request is JS/XHR (jquery-rails gem)' do
-          it "updates the line item" do
-            xhr :put, :update, params
-            line_item1.reload
-            expect(line_item1.quantity).to eq 3
-            expect(line_item1.final_weight_volume).to eq 3000
-            expect(line_item1.price).to eq 3.00
-          end
-
-          it "returns an empty JSON response" do
-            xhr :put, :update, params
-            expect(response.body).to eq ' '
-          end
-
-          it 'returns a 204 response' do
-            xhr :put, :update, params
-            expect(response.status).to eq 204
-          end
-
-          context 'when the line item params are not correct' do
-            let(:line_item_params) { { price: 'hola' } }
-            let(:errors) { { 'price' => ['is not a number'] } }
-
-            it 'returns a JSON with the errors' do
-              xhr :put, :update, params
-              expect(JSON.parse(response.body)['errors']).to eq(errors)
-            end
-
-            it 'returns a 412 response' do
-              xhr :put, :update, params
-              expect(response.status).to eq 412
-            end
-          end
-        end
-
-        context 'when the request is HTML' do
-          before { params[:format] = :html }
-
-          it 'returns an HTML response with the order form' do
-            spree_put :update, params
-            expect(response.body).to match(/edit_order/)
-          end
-        end
-      end
-
-      context "hub enterprise" do
-        before do
-          controller.stub spree_current_user: distributor1.owner
-          xhr :put, :update, params
-        end
-
-        it "updates the line item" do
-          line_item1.reload
-          expect(line_item1.quantity).to eq 3
-          expect(line_item1.final_weight_volume).to eq 3000
-          expect(line_item1.price).to eq 3.00
-        end
-      end
-    end
-  end
-end

spec/controllers/spree/api/shipments_controller_spec.rb

@@ -0,0 +1,154 @@
+require 'spec_helper'
+
+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] }
+
+  before do
+    allow(controller).to receive(:spree_current_user) { current_api_user }
+  end
+
+  context "as an admin" do
+    let!(:order) { shipment.order }
+    let(:order_ship_address) { create(:address) }
+    let!(:stock_location) { create(:stock_location_with_items) }
+    let!(:variant) { create(:variant) }
+    let(:params) do
+      { quantity: 2,
+        variant_id: variant.to_param,
+        order_id: order.number,
+        stock_location_id: stock_location.to_param,
+        format: :json }
+    end
+    let(:error_message) { "broken shipments creation" }
+
+    before do
+      order.update_attribute(:ship_address_id, order_ship_address.id)
+    end
+
+    sign_in_as_admin!
+
+    context '#create' do
+      it 'creates a shipment if order does not have a shipment' do
+        order.shipment.destroy
+        order.reload
+
+        spree_post :create, params
+
+        expect_valid_response
+        expect(json_response["inventory_units"].size).to eq 2
+        expect(order.reload.line_items.first.variant.price).to eq(variant.price)
+      end
+
+      it 'updates and returns exiting shipment, if order already has a shipment' do
+        original_shipment_id = order.shipment.id
+
+        spree_post :create, params
+
+        expect(json_response["id"]). to eq(original_shipment_id)
+        expect_valid_response
+        expect(json_response["inventory_units"].size).to eq 2
+        expect(order.reload.line_items.first.variant.price).to eq(variant.price)
+      end
+
+      it 'updates existing shipment with variant override if an VO is sent' do
+        hub = create(:distributor_enterprise)
+        order.update_attribute(:distributor, hub)
+        variant_override = create(:variant_override, hub: hub, variant: variant)
+
+        spree_post :create, params
+
+        expect_valid_response
+        expect(json_response["inventory_units"].size).to eq 2
+        expect(order.reload.line_items.first.price).to eq(variant_override.price)
+      end
+
+      it 'returns error code when adding to order contents fails' do
+        make_order_contents_fail
+
+        spree_post :create, params
+
+        expect_error_response
+      end
+    end
+
+    context 'for a completed order with shipment' do
+      let(:order) { create :completed_order_with_totals }
+
+      before { params[:id] = order.shipments.first.to_param }
+
+      context '#add' do
+        it 'adds a variant to the shipment' do
+          spree_put :add, params
+
+          expect_valid_response
+          expect(inventory_units_for(json_response["inventory_units"], variant).size).to eq 2
+        end
+
+        it 'returns error code when adding to order contents fails' do
+          make_order_contents_fail
+
+          spree_put :add, params
+
+          expect_error_response
+        end
+
+        it 'adds a variant override to the shipment' do
+          hub = create(:distributor_enterprise)
+          order.update_attribute(:distributor, hub)
+          variant_override = create(:variant_override, hub: hub, variant: variant)
+
+          spree_put :add, params
+
+          expect_valid_response
+          expect(inventory_units_for(json_response["inventory_units"], variant).size).to eq 2
+          expect(order.reload.line_items.last.price).to eq(variant_override.price)
+        end
+      end
+
+      context '#remove' do
+        before do
+          params[:variant_id] = order.line_items.first.variant.to_param
+          params[:quantity] = 1
+        end
+
+        it 'removes a variant from the shipment' do
+          spree_put :remove, params
+
+          expect_valid_response
+          expect(inventory_units_for(json_response["inventory_units"], variant).size).to eq 0
+        end
+
+        it 'returns error code when removing from order contents fails' do
+          make_order_contents_fail
+
+          spree_put :remove, params
+
+          expect_error_response
+        end
+      end
+    end
+
+    def inventory_units_for(inventory_units, variant)
+      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
+      expect(Spree::Order).to receive(:find_by_number!) { order }
+      expect(order).to receive(:contents) { raise error_message }
+    end
+
+    def expect_error_response
+      expect(response.status).to eq 422
+      expect(json_response["exception"]).to eq error_message
+    end
+  end
+end