From 69afcf751050d4dc4dac8d2a99fbbdda0d3cf88c Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Tue, 17 Sep 2019 14:30:11 +0100
Subject: [PATCH 01/12] Improve readability in order permissions

---
 app/models/spree/ability_decorator.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb
index 72f7986a38..f71c7c9d7e 100644
--- a/app/models/spree/ability_decorator.rb
+++ b/app/models/spree/ability_decorator.rb
@@ -207,12 +207,15 @@ class AbilityDecorator
   end
 
   def add_order_management_abilities(user)
-    # Enterprise User can only access orders that they are a distributor for
     can [:index, :create], Spree::Order
     can [:read, :update, :fire, :resend, :invoice, :print, :print_ticket], Spree::Order do |order|
       # We allow editing orders with a nil distributor as this state occurs
       # during the order creation process from the admin backend
-      order.distributor.nil? || user.enterprises.include?(order.distributor) || order.order_cycle.andand.coordinated_by?(user)
+      order.distributor.nil? ||
+        # Enterprise User can access orders that they are a distributor for
+        user.enterprises.include?(order.distributor) ||
+        # Enterprise User can access orders that are placed inside a OC they coordinate
+        order.order_cycle.andand.coordinated_by?(user)
     end
     can [:admin, :bulk_management, :managed], Spree::Order do
       user.admin? || user.enterprises.any?(&:is_distributor)

From 6796d91a07fb667be3e0e93f9935f20e6ac33b7e Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Tue, 17 Sep 2019 14:32:00 +0100
Subject: [PATCH 02/12] Add some basic attributes to address and order
 serializers that will be used in the order show api endpoint

---
 app/serializers/api/address_serializer.rb     | 6 +++++-
 app/serializers/api/admin/order_serializer.rb | 3 ++-
 app/serializers/api/payment_serializer.rb     | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/serializers/api/address_serializer.rb b/app/serializers/api/address_serializer.rb
index 73069822c4..4ef6d7d790 100644
--- a/app/serializers/api/address_serializer.rb
+++ b/app/serializers/api/address_serializer.rb
@@ -4,7 +4,11 @@ class Api::AddressSerializer < ActiveModel::Serializer
 
   attributes :id, :zipcode, :city, :state_name, :state_id,
              :phone, :firstname, :lastname, :address1, :address2, :city, :country_id,
-             :zipcode
+             :zipcode, :country_name
+
+  def country_name
+    object.country.andand.name
+  end
 
   def state_name
     object.state.andand.abbr
diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb
index 1836fcfa44..18661a0923 100644
--- a/app/serializers/api/admin/order_serializer.rb
+++ b/app/serializers/api/admin/order_serializer.rb
@@ -1,8 +1,9 @@
 class Api::Admin::OrderSerializer < ActiveModel::Serializer
-  attributes :id, :number, :full_name, :email, :phone, :completed_at, :display_total
+  attributes :id, :number, :user_id, :full_name, :email, :phone, :completed_at, :display_total
   attributes :edit_path, :state, :payment_state, :shipment_state
   attributes :payments_path, :ship_path, :ready_to_ship, :created_at
   attributes :distributor_name, :special_instructions, :payment_capture_path
+  attributes :item_total, :adjustment_total, :payment_total, :total
 
   has_one :distributor, serializer: Api::Admin::IdSerializer
   has_one :order_cycle, serializer: Api::Admin::IdSerializer
diff --git a/app/serializers/api/payment_serializer.rb b/app/serializers/api/payment_serializer.rb
index 8b956ecb1f..7172e21f12 100644
--- a/app/serializers/api/payment_serializer.rb
+++ b/app/serializers/api/payment_serializer.rb
@@ -1,6 +1,7 @@
 module Api
   class PaymentSerializer < ActiveModel::Serializer
     attributes :amount, :updated_at, :payment_method, :state
+
     def payment_method
       object.payment_method.try(:name)
     end

From 2921ee19e198d9c48519b6da9cd53789bd13a6f0 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Tue, 17 Sep 2019 14:33:39 +0100
Subject: [PATCH 03/12] Add api/order/{order_number} ednpoint and its new order
 detailed serializer

---
 app/controllers/api/orders_controller.rb      |   9 ++
 app/serializers/api/adjustment_serializer.rb  |   8 ++
 .../api/order_detailed_serializer.rb          |  12 ++
 config/routes/api.rb                          |   2 +-
 .../controllers/api/orders_controller_spec.rb | 125 +++++++++++++++++-
 5 files changed, 149 insertions(+), 7 deletions(-)
 create mode 100644 app/serializers/api/adjustment_serializer.rb
 create mode 100644 app/serializers/api/order_detailed_serializer.rb

diff --git a/app/controllers/api/orders_controller.rb b/app/controllers/api/orders_controller.rb
index a03eb3b36b..e214575ea1 100644
--- a/app/controllers/api/orders_controller.rb
+++ b/app/controllers/api/orders_controller.rb
@@ -1,5 +1,10 @@
 module Api
   class OrdersController < BaseController
+    def show
+      authorize! :read, order
+      render json: @order, serializer: Api::OrderDetailedSerializer
+    end
+
     def index
       authorize! :admin, Spree::Order
 
@@ -19,5 +24,9 @@ module Api
         each_serializer: Api::Admin::OrderSerializer
       )
     end
+
+    def order
+      @order ||= Spree::Order.find_by_number!(params[:id])
+    end
   end
 end
diff --git a/app/serializers/api/adjustment_serializer.rb b/app/serializers/api/adjustment_serializer.rb
new file mode 100644
index 0000000000..5f0938cbcb
--- /dev/null
+++ b/app/serializers/api/adjustment_serializer.rb
@@ -0,0 +1,8 @@
+module Api
+  class AdjustmentSerializer < ActiveModel::Serializer
+    attributes :id, :amount, :label, :eligible
+    attributes :source_type, :source_id
+    attributes :adjustable_type, :adjustable_id
+    attributes :originator_type, :originator_id
+  end
+end
diff --git a/app/serializers/api/order_detailed_serializer.rb b/app/serializers/api/order_detailed_serializer.rb
new file mode 100644
index 0000000000..424f7d91b7
--- /dev/null
+++ b/app/serializers/api/order_detailed_serializer.rb
@@ -0,0 +1,12 @@
+module Api
+  class OrderDetailedSerializer < Api::Admin::OrderSerializer
+    has_one :shipping_method, serializer: Api::ShippingMethodSerializer
+    has_one :ship_address, serializer: Api::AddressSerializer
+    has_one :bill_address, serializer: Api::AddressSerializer
+
+    has_many :line_items, serializer: Api::LineItemSerializer
+    has_many :adjustments, serializer: Api::AdjustmentSerializer
+
+    has_many :payments, serializer: Api::PaymentSerializer
+  end
+end
diff --git a/config/routes/api.rb b/config/routes/api.rb
index 94b9e180f0..75d3638016 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, only: [:index] do
+    resources :orders, only: [:index, :show] do
       get :managed, on: :collection
 
       resources :shipments, :only => [:create, :update] do
diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb
index 0819733e82..50a0614bb5 100644
--- a/spec/controllers/api/orders_controller_spec.rb
+++ b/spec/controllers/api/orders_controller_spec.rb
@@ -5,13 +5,17 @@ module Api
     include AuthenticationWorkflow
     render_views
 
+    let!(:regular_user) { create(:user) }
+    let!(:admin_user) { create(:admin_user) }
+
+    let!(:distributor) { create(:distributor_enterprise) }
+    let!(:coordinator) { create(:distributor_enterprise) }
+    let!(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) }
+
     describe '#index' do
-      let!(:distributor) { create(:distributor_enterprise) }
       let!(:distributor2) { create(:distributor_enterprise) }
-      let!(:supplier) { create(:supplier_enterprise) }
-      let!(:coordinator) { create(:distributor_enterprise) }
       let!(:coordinator2) { create(:distributor_enterprise) }
-      let!(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) }
+      let!(:supplier) { create(:supplier_enterprise) }
       let!(:order_cycle2) { create(:simple_order_cycle, coordinator: coordinator2) }
       let!(:order1) do
         create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now,
@@ -45,8 +49,6 @@ module Api
         create(:line_item_with_shipment, order: order3,
                                          product: create(:product, supplier: supplier))
       end
-      let!(:regular_user) { create(:user) }
-      let!(:admin_user) { create(:admin_user) }
 
       context 'as a regular user' do
         before do
@@ -156,6 +158,109 @@ module Api
       end
     end
 
+    describe "#show" do
+      let!(:order) { create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor ) }
+
+      context "Resource not found" do
+        before { allow(controller).to receive(:spree_current_user) { admin_user } }
+
+        it "when no order number is given" do
+          get :show, id: nil
+          expect_resource_not_found
+        end
+
+        it "when order number given is not in the systen" do
+          get :show, id: "X1321313232"
+          expect_resource_not_found
+        end
+
+        def expect_resource_not_found
+          expect(json_response).to eq( "error" => "The resource you were looking for could not be found." )
+          expect(response.status).to eq(404)
+        end
+      end
+
+      context "access" do
+        it "returns unauthorized, as a regular user" do
+          allow(controller).to receive(:spree_current_user) { regular_user }
+          get :show, id: order.number
+          assert_unauthorized!
+        end
+
+        it "returns the order, as an admin user" do
+          allow(controller).to receive(:spree_current_user) { admin_user }
+          get :show, id: order.number
+          expect_order
+        end
+
+        it "returns the order, as the order distributor owner" do
+          allow(controller).to receive(:spree_current_user) { order.distributor.owner }
+          get :show, id: order.number
+          expect_order
+        end
+
+        it "returns unauthorized, as the order product's supplier owner" do
+          allow(controller).to receive(:spree_current_user) { order.line_items.first.variant.product.supplier.owner }
+          get :show, id: order.number
+          assert_unauthorized!
+        end
+
+        it "returns the order, as the Order Cycle coorinator owner" do
+          allow(controller).to receive(:spree_current_user) { order.order_cycle.coordinator.owner }
+          get :show, id: order.number
+          expect_order
+        end
+      end
+
+      context "as distributor owner" do
+        let!(:order) { create(:completed_order_with_fees, order_cycle: order_cycle, distributor: distributor ) }
+
+        before { allow(controller).to receive(:spree_current_user) { order.distributor.owner } }
+
+        it "can view an order not in a standard state" do
+          order.update_attributes(completed_at: nil, state: 'shipped')
+          get :show, id: order.number
+          expect_order
+        end
+
+        it "returns an order with all required fields" do
+          get :show, id: order.number
+          expect_order
+          expect_detailed_attributes_to_be_present(json_response)
+
+          expect(json_response[:bill_address][:address1]).to eq(order.bill_address.address1)
+          expect(json_response[:bill_address][:lastname]).to eq(order.bill_address.lastname)
+          expect(json_response[:ship_address][:address1]).to eq(order.ship_address.address1)
+          expect(json_response[:ship_address][:lastname]).to eq(order.ship_address.lastname)
+          expect(json_response[:shipping_method][:name]).to eq(order.shipping_method.name)
+          json_response[:adjustments].each do |adjustment|
+            if adjustment[:label] == "Transaction fee"
+              expect(adjustment[:amount]).to eq(order.adjustments.payment_fee.first.amount.to_s)
+            elsif json_response[:adjustments].first[:label] == "Shipping"
+              expect(adjustment[:amount]).to eq(order.adjustments.shipping.first.amount.to_s)
+            end
+          end
+          expect(json_response[:payments].first[:amount]).to eq(order.payments.first.amount.to_s)
+          expect(json_response[:line_items].size).to eq(order.line_items.size)
+          expect(json_response[:line_items].first[:variant][:product_name]). to eq(order.line_items.first.variant.product.name)
+        end
+      end
+
+      def expect_order
+        expect(response.status).to eq 200
+        expect_correct_order(json_response, order)
+      end
+
+      def expect_correct_order(json_response, order)
+        expect(json_response[:number]).to eq(order.number)
+        expect(json_response[:email]).to eq(order.email)
+      end
+
+      def expect_detailed_attributes_to_be_present(json_response)
+        expect(order_detailed_attributes.all?{ |attr| json_response.key? attr.to_s }).to eq(true)
+      end
+    end
+
     private
 
     def serialized_orders(orders)
@@ -181,5 +286,13 @@ module Api
         :distributor_name, :special_instructions, :payment_capture_path
       ]
     end
+
+    def order_detailed_attributes
+      [
+        :number, :item_total, :total, :state, :adjustment_total, :payment_total,
+        :completed_at, :shipment_state, :payment_state, :email, :special_instructions,
+        :adjustments, :payments, :bill_address, :ship_address, :line_items, :shipping_method
+      ]
+    end
   end
 end

From a44a251d960026600fed8ecae18a1ed4446d916d Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 19 Sep 2019 10:37:40 +0100
Subject: [PATCH 04/12] Remove duplicated attributes tag from all serializers
 to create consistency

---
 app/serializers/api/adjustment_serializer.rb       |  8 ++++----
 .../api/admin/basic_enterprise_serializer.rb       |  4 ++--
 app/serializers/api/admin/customer_serializer.rb   |  4 ++--
 .../api/admin/enterprise_fee_serializer.rb         |  4 ++--
 app/serializers/api/admin/enterprise_serializer.rb | 14 +++++++-------
 app/serializers/api/admin/exchange_serializer.rb   |  4 ++--
 .../admin/for_order_cycle/enterprise_serializer.rb |  6 +++---
 .../api/admin/index_enterprise_serializer.rb       |  5 ++---
 .../api/admin/index_order_cycle_serializer.rb      |  6 +++---
 .../api/admin/order_cycle_serializer.rb            |  8 ++++----
 app/serializers/api/admin/order_serializer.rb      | 10 +++++-----
 .../api/admin/subscription_serializer.rb           |  6 +++---
 .../api/admin/variant_override_serializer.rb       |  4 ++--
 app/serializers/api/admin/variant_serializer.rb    |  8 ++++----
 app/serializers/api/order_serializer.rb            |  8 ++++----
 app/serializers/api/variant_serializer.rb          | 10 +++++-----
 16 files changed, 54 insertions(+), 55 deletions(-)

diff --git a/app/serializers/api/adjustment_serializer.rb b/app/serializers/api/adjustment_serializer.rb
index 5f0938cbcb..254b173777 100644
--- a/app/serializers/api/adjustment_serializer.rb
+++ b/app/serializers/api/adjustment_serializer.rb
@@ -1,8 +1,8 @@
 module Api
   class AdjustmentSerializer < ActiveModel::Serializer
-    attributes :id, :amount, :label, :eligible
-    attributes :source_type, :source_id
-    attributes :adjustable_type, :adjustable_id
-    attributes :originator_type, :originator_id
+    attributes :id, :amount, :label, :eligible,
+               :source_type, :source_id,
+               :adjustable_type, :adjustable_id,
+               :originator_type, :originator_id
   end
 end
diff --git a/app/serializers/api/admin/basic_enterprise_serializer.rb b/app/serializers/api/admin/basic_enterprise_serializer.rb
index 854b9b4019..c82eb8994d 100644
--- a/app/serializers/api/admin/basic_enterprise_serializer.rb
+++ b/app/serializers/api/admin/basic_enterprise_serializer.rb
@@ -1,4 +1,4 @@
 class Api::Admin::BasicEnterpriseSerializer < ActiveModel::Serializer
-  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :payment_method_ids, :shipping_method_ids
-  attributes :producer_profile_only, :permalink
+  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :payment_method_ids, :shipping_method_ids,
+             :producer_profile_only, :permalink
 end
diff --git a/app/serializers/api/admin/customer_serializer.rb b/app/serializers/api/admin/customer_serializer.rb
index f5fb4405be..2bc9f5a7db 100644
--- a/app/serializers/api/admin/customer_serializer.rb
+++ b/app/serializers/api/admin/customer_serializer.rb
@@ -1,6 +1,6 @@
 class Api::Admin::CustomerSerializer < ActiveModel::Serializer
-  attributes :id, :email, :enterprise_id, :user_id, :code, :tags, :tag_list, :name
-  attributes :allow_charges, :default_card_present?
+  attributes :id, :email, :enterprise_id, :user_id, :code, :tags, :tag_list, :name,
+             :allow_charges, :default_card_present?
 
   has_one :ship_address, serializer: Api::AddressSerializer
   has_one :bill_address, serializer: Api::AddressSerializer
diff --git a/app/serializers/api/admin/enterprise_fee_serializer.rb b/app/serializers/api/admin/enterprise_fee_serializer.rb
index 2f7f7ca10e..6c59a3d482 100644
--- a/app/serializers/api/admin/enterprise_fee_serializer.rb
+++ b/app/serializers/api/admin/enterprise_fee_serializer.rb
@@ -1,6 +1,6 @@
 class Api::Admin::EnterpriseFeeSerializer < ActiveModel::Serializer
-  attributes :id, :enterprise_id, :fee_type, :name, :tax_category_id, :inherits_tax_category, :calculator_type
-  attributes :enterprise_name, :calculator_description, :calculator_settings
+  attributes :id, :enterprise_id, :fee_type, :name, :tax_category_id, :inherits_tax_category, :calculator_type,
+             :enterprise_name, :calculator_description, :calculator_settings
 
   def enterprise_name
     object.enterprise.andand.name
diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb
index a4bba2b3d2..702f39aa29 100644
--- a/app/serializers/api/admin/enterprise_serializer.rb
+++ b/app/serializers/api/admin/enterprise_serializer.rb
@@ -1,11 +1,11 @@
 class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer
-  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :payment_method_ids, :shipping_method_ids
-  attributes :producer_profile_only, :long_description, :permalink
-  attributes :preferred_shopfront_message, :preferred_shopfront_closed_message, :preferred_shopfront_taxon_order, :preferred_shopfront_order_cycle_order
-  attributes :preferred_product_selection_from_inventory_only
-  attributes :owner, :contact, :users, :tag_groups, :default_tag_group
-  attributes :require_login, :allow_guest_orders, :allow_order_changes
-  attributes :logo, :promo_image
+  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :payment_method_ids, :shipping_method_ids,
+             :producer_profile_only, :long_description, :permalink,
+             :preferred_shopfront_message, :preferred_shopfront_closed_message, :preferred_shopfront_taxon_order, :preferred_shopfront_order_cycle_order,
+             :preferred_product_selection_from_inventory_only,
+             :owner, :contact, :users, :tag_groups, :default_tag_group,
+             :require_login, :allow_guest_orders, :allow_order_changes,
+             :logo, :promo_image
 
   has_one :owner, serializer: Api::Admin::UserSerializer
   has_many :users, serializer: Api::Admin::UserSerializer
diff --git a/app/serializers/api/admin/exchange_serializer.rb b/app/serializers/api/admin/exchange_serializer.rb
index 43d7a08ac8..483f490f47 100644
--- a/app/serializers/api/admin/exchange_serializer.rb
+++ b/app/serializers/api/admin/exchange_serializer.rb
@@ -1,6 +1,6 @@
 class Api::Admin::ExchangeSerializer < ActiveModel::Serializer
-  attributes :id, :sender_id, :receiver_id, :incoming, :variants, :receival_instructions, :pickup_time, :pickup_instructions
-  attributes :tags, :tag_list
+  attributes :id, :sender_id, :receiver_id, :incoming, :variants, :receival_instructions, :pickup_time, :pickup_instructions,
+             :tags, :tag_list
 
   has_many :enterprise_fees, serializer: Api::Admin::BasicEnterpriseFeeSerializer
 
diff --git a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
index 4d2afc9ed5..0724778ba7 100644
--- a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
+++ b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
@@ -1,9 +1,9 @@
 require 'open_food_network/enterprise_issue_validator'
 
 class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer
-  attributes :id, :name, :managed, :supplied_products
-  attributes :issues_summary_supplier, :issues_summary_distributor
-  attributes :is_primary_producer, :is_distributor, :sells
+  attributes :id, :name, :managed, :supplied_products,
+             :issues_summary_supplier, :issues_summary_distributor,
+             :is_primary_producer, :is_distributor, :sells
 
   def issues_summary_supplier
     issues = OpenFoodNetwork::EnterpriseIssueValidator.new(object).issues_summary confirmation_only: true
diff --git a/app/serializers/api/admin/index_enterprise_serializer.rb b/app/serializers/api/admin/index_enterprise_serializer.rb
index d1d315d63d..6cf877aa45 100644
--- a/app/serializers/api/admin/index_enterprise_serializer.rb
+++ b/app/serializers/api/admin/index_enterprise_serializer.rb
@@ -1,9 +1,8 @@
 require 'open_food_network/enterprise_issue_validator'
 
 class Api::Admin::IndexEnterpriseSerializer < ActiveModel::Serializer
-  attributes :name, :id, :permalink, :is_primary_producer, :sells, :producer_profile_only, :owned, :edit_path
-
-  attributes :issues, :warnings
+  attributes :name, :id, :permalink, :is_primary_producer, :sells, :producer_profile_only, :owned, :edit_path,
+             :issues, :warnings
 
   def owned
     return true if options[:spree_current_user].admin?
diff --git a/app/serializers/api/admin/index_order_cycle_serializer.rb b/app/serializers/api/admin/index_order_cycle_serializer.rb
index 144bc38a7c..97ff415f24 100644
--- a/app/serializers/api/admin/index_order_cycle_serializer.rb
+++ b/app/serializers/api/admin/index_order_cycle_serializer.rb
@@ -5,9 +5,9 @@ module Api
     class IndexOrderCycleSerializer < ActiveModel::Serializer
       include OrderCyclesHelper
 
-      attributes :id, :name, :orders_open_at, :orders_close_at, :status, :variant_count, :deletable
-      attributes :coordinator, :producers, :shops, :viewing_as_coordinator
-      attributes :edit_path, :clone_path, :delete_path, :subscriptions_count
+      attributes :id, :name, :orders_open_at, :orders_close_at, :status, :variant_count, :deletable,
+                 :coordinator, :producers, :shops, :viewing_as_coordinator,
+                 :edit_path, :clone_path, :delete_path, :subscriptions_count
 
       has_many :schedules, serializer: Api::Admin::IdNameSerializer
 
diff --git a/app/serializers/api/admin/order_cycle_serializer.rb b/app/serializers/api/admin/order_cycle_serializer.rb
index 4989530e37..1001908ded 100644
--- a/app/serializers/api/admin/order_cycle_serializer.rb
+++ b/app/serializers/api/admin/order_cycle_serializer.rb
@@ -1,10 +1,10 @@
 require 'open_food_network/order_cycle_permissions'
 
 class Api::Admin::OrderCycleSerializer < ActiveModel::Serializer
-  attributes :id, :name, :orders_open_at, :orders_close_at, :coordinator_id, :exchanges
-  attributes :editable_variants_for_incoming_exchanges, :editable_variants_for_outgoing_exchanges
-  attributes :visible_variants_for_outgoing_exchanges
-  attributes :viewing_as_coordinator, :schedule_ids, :subscriptions_count
+  attributes :id, :name, :orders_open_at, :orders_close_at, :coordinator_id, :exchanges,
+             :editable_variants_for_incoming_exchanges, :editable_variants_for_outgoing_exchanges,
+             :visible_variants_for_outgoing_exchanges,
+             :viewing_as_coordinator, :schedule_ids, :subscriptions_count
 
   has_many :coordinator_fees, serializer: Api::IdSerializer
 
diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb
index 18661a0923..83707e8af5 100644
--- a/app/serializers/api/admin/order_serializer.rb
+++ b/app/serializers/api/admin/order_serializer.rb
@@ -1,9 +1,9 @@
 class Api::Admin::OrderSerializer < ActiveModel::Serializer
-  attributes :id, :number, :user_id, :full_name, :email, :phone, :completed_at, :display_total
-  attributes :edit_path, :state, :payment_state, :shipment_state
-  attributes :payments_path, :ship_path, :ready_to_ship, :created_at
-  attributes :distributor_name, :special_instructions, :payment_capture_path
-  attributes :item_total, :adjustment_total, :payment_total, :total
+  attributes :id, :number, :user_id, :full_name, :email, :phone, :completed_at, :display_total,
+             :edit_path, :state, :payment_state, :shipment_state,
+             :payments_path, :ship_path, :ready_to_ship, :created_at,
+             :distributor_name, :special_instructions, :payment_capture_path,
+             :item_total, :adjustment_total, :payment_total, :total
 
   has_one :distributor, serializer: Api::Admin::IdSerializer
   has_one :order_cycle, serializer: Api::Admin::IdSerializer
diff --git a/app/serializers/api/admin/subscription_serializer.rb b/app/serializers/api/admin/subscription_serializer.rb
index 91b30e4e8c..1ffd4d3eed 100644
--- a/app/serializers/api/admin/subscription_serializer.rb
+++ b/app/serializers/api/admin/subscription_serializer.rb
@@ -1,9 +1,9 @@
 module Api
   module Admin
     class SubscriptionSerializer < ActiveModel::Serializer
-      attributes :id, :shop_id, :customer_id, :schedule_id, :payment_method_id, :shipping_method_id, :begins_at, :ends_at
-      attributes :customer_email, :schedule_name, :edit_path, :canceled_at, :paused_at, :state
-      attributes :shipping_fee_estimate, :payment_fee_estimate
+      attributes :id, :shop_id, :customer_id, :schedule_id, :payment_method_id, :shipping_method_id, :begins_at, :ends_at,
+                 :customer_email, :schedule_name, :edit_path, :canceled_at, :paused_at, :state,
+                 :shipping_fee_estimate, :payment_fee_estimate
 
       has_many :subscription_line_items, serializer: Api::Admin::SubscriptionLineItemSerializer
       has_many :closed_proxy_orders, serializer: Api::Admin::ProxyOrderSerializer
diff --git a/app/serializers/api/admin/variant_override_serializer.rb b/app/serializers/api/admin/variant_override_serializer.rb
index c63e77c068..683ad0101e 100644
--- a/app/serializers/api/admin/variant_override_serializer.rb
+++ b/app/serializers/api/admin/variant_override_serializer.rb
@@ -1,6 +1,6 @@
 class Api::Admin::VariantOverrideSerializer < ActiveModel::Serializer
-  attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand, :default_stock, :resettable
-  attributes :tag_list, :tags, :import_date
+  attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand, :default_stock, :resettable,
+             :tag_list, :tags, :import_date
 
   def tag_list
     object.tag_list.join(",")
diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb
index 479780064f..4ab886b270 100644
--- a/app/serializers/api/admin/variant_serializer.rb
+++ b/app/serializers/api/admin/variant_serializer.rb
@@ -1,8 +1,8 @@
 class Api::Admin::VariantSerializer < ActiveModel::Serializer
-  attributes :id, :name, :producer_name, :image, :sku, :import_date
-  attributes :options_text, :unit_value, :unit_description, :unit_to_display
-  attributes :display_as, :display_name, :name_to_display
-  attributes :price, :on_demand, :on_hand, :in_stock, :stock_location_id, :stock_location_name
+  attributes :id, :name, :producer_name, :image, :sku, :import_date,
+             :options_text, :unit_value, :unit_description, :unit_to_display,
+             :display_as, :display_name, :name_to_display,
+             :price, :on_demand, :on_hand, :in_stock, :stock_location_id, :stock_location_name
 
   has_many :variant_overrides
 
diff --git a/app/serializers/api/order_serializer.rb b/app/serializers/api/order_serializer.rb
index 68c8f6a549..017f32bbf5 100644
--- a/app/serializers/api/order_serializer.rb
+++ b/app/serializers/api/order_serializer.rb
@@ -1,9 +1,9 @@
 module Api
   class OrderSerializer < ActiveModel::Serializer
-    attributes :number, :completed_at, :total, :state, :shipment_state, :payment_state
-    attributes :outstanding_balance, :payments, :path, :cancel_path
-    attributes :changes_allowed, :changes_allowed_until, :item_count
-    attributes :shop_id
+    attributes :number, :completed_at, :total, :state, :shipment_state, :payment_state,
+               :outstanding_balance, :payments, :path, :cancel_path,
+               :changes_allowed, :changes_allowed_until, :item_count,
+               :shop_id
 
     has_many :payments, serializer: Api::PaymentSerializer
 
diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb
index 518dd60d7c..54eae7aed5 100644
--- a/app/serializers/api/variant_serializer.rb
+++ b/app/serializers/api/variant_serializer.rb
@@ -1,9 +1,9 @@
 class Api::VariantSerializer < ActiveModel::Serializer
-  attributes :id, :is_master, :product_name, :sku
-  attributes :options_text, :unit_value, :unit_description, :unit_to_display
-  attributes :display_as, :display_name, :name_to_display
-  attributes :price, :on_demand, :on_hand, :fees, :price_with_fees
-  attributes :tag_list
+  attributes :id, :is_master, :product_name, :sku,
+             :options_text, :unit_value, :unit_description, :unit_to_display,
+             :display_as, :display_name, :name_to_display,
+             :price, :on_demand, :on_hand, :fees, :price_with_fees,
+             :tag_list
 
   delegate :price, to: :object
 

From 19e28cb14a4a90e318ce8187091a05806c0d8ebe Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 19 Sep 2019 10:46:57 +0100
Subject: [PATCH 05/12] Make spec/controllers/api/orders_controller_spec more
 simple assuming adjustments will always come in the same order

---
 .../controllers/api/orders_controller_spec.rb | 34 +++++++++----------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb
index 50a0614bb5..8eb80d3aa3 100644
--- a/spec/controllers/api/orders_controller_spec.rb
+++ b/spec/controllers/api/orders_controller_spec.rb
@@ -228,21 +228,20 @@ module Api
           expect_order
           expect_detailed_attributes_to_be_present(json_response)
 
-          expect(json_response[:bill_address][:address1]).to eq(order.bill_address.address1)
-          expect(json_response[:bill_address][:lastname]).to eq(order.bill_address.lastname)
-          expect(json_response[:ship_address][:address1]).to eq(order.ship_address.address1)
-          expect(json_response[:ship_address][:lastname]).to eq(order.ship_address.lastname)
-          expect(json_response[:shipping_method][:name]).to eq(order.shipping_method.name)
-          json_response[:adjustments].each do |adjustment|
-            if adjustment[:label] == "Transaction fee"
-              expect(adjustment[:amount]).to eq(order.adjustments.payment_fee.first.amount.to_s)
-            elsif json_response[:adjustments].first[:label] == "Shipping"
-              expect(adjustment[:amount]).to eq(order.adjustments.shipping.first.amount.to_s)
-            end
-          end
-          expect(json_response[:payments].first[:amount]).to eq(order.payments.first.amount.to_s)
-          expect(json_response[:line_items].size).to eq(order.line_items.size)
-          expect(json_response[:line_items].first[:variant][:product_name]). to eq(order.line_items.first.variant.product.name)
+          expect(json_response[:bill_address][:address1]).to eq order.bill_address.address1
+          expect(json_response[:bill_address][:lastname]).to eq order.bill_address.lastname
+          expect(json_response[:ship_address][:address1]).to eq order.ship_address.address1
+          expect(json_response[:ship_address][:lastname]).to eq order.ship_address.lastname
+          expect(json_response[:shipping_method][:name]).to eq order.shipping_method.name
+
+          expect(json_response[:adjustments].first[:label]).to eq "Transaction fee"
+          expect(json_response[:adjustments].first[:amount]).to eq order.adjustments.payment_fee.first.amount.to_s
+          expect(json_response[:adjustments].second[:label]).to eq "Shipping"
+          expect(json_response[:adjustments].second[:amount]).to eq order.adjustments.shipping.first.amount.to_s
+
+          expect(json_response[:payments].first[:amount]).to eq order.payments.first.amount.to_s
+          expect(json_response[:line_items].size).to eq order.line_items.size
+          expect(json_response[:line_items].first[:variant][:product_name]). to eq order.line_items.first.variant.product.name
         end
       end
 
@@ -252,12 +251,11 @@ module Api
       end
 
       def expect_correct_order(json_response, order)
-        expect(json_response[:number]).to eq(order.number)
-        expect(json_response[:email]).to eq(order.email)
+        expect(json_response[:number]).to eq order.number
       end
 
       def expect_detailed_attributes_to_be_present(json_response)
-        expect(order_detailed_attributes.all?{ |attr| json_response.key? attr.to_s }).to eq(true)
+        expect(order_detailed_attributes.all?{ |attr| json_response.key? attr.to_s }).to eq true
       end
     end
 

From 4d37aaac640ade4ab1043914e75ecd1cf6d8d131 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 19 Sep 2019 10:52:00 +0100
Subject: [PATCH 06/12] Use have_http_status and remove check for error
 message, that's something for the base_controller test to test

---
 spec/controllers/api/orders_controller_spec.rb | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb
index 8eb80d3aa3..7786536635 100644
--- a/spec/controllers/api/orders_controller_spec.rb
+++ b/spec/controllers/api/orders_controller_spec.rb
@@ -166,17 +166,12 @@ module Api
 
         it "when no order number is given" do
           get :show, id: nil
-          expect_resource_not_found
+          expect(response).to have_http_status(:not_found)
         end
 
         it "when order number given is not in the systen" do
           get :show, id: "X1321313232"
-          expect_resource_not_found
-        end
-
-        def expect_resource_not_found
-          expect(json_response).to eq( "error" => "The resource you were looking for could not be found." )
-          expect(response.status).to eq(404)
+          expect(response).to have_http_status(:not_found)
         end
       end
 

From 25fbab2e3703548033e02299622625e41604afa3 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 19 Sep 2019 10:56:31 +0100
Subject: [PATCH 07/12] Use memoized order method

---
 app/controllers/api/orders_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/api/orders_controller.rb b/app/controllers/api/orders_controller.rb
index e214575ea1..03d4e8caa4 100644
--- a/app/controllers/api/orders_controller.rb
+++ b/app/controllers/api/orders_controller.rb
@@ -2,7 +2,7 @@ module Api
   class OrdersController < BaseController
     def show
       authorize! :read, order
-      render json: @order, serializer: Api::OrderDetailedSerializer
+      render json: order, serializer: Api::OrderDetailedSerializer
     end
 
     def index

From bdb3dd5aaf14c3f3d0d7ead24a8d503b4d0f4492 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 19 Sep 2019 16:23:01 +0100
Subject: [PATCH 08/12] Fix long lines in app/serializers

---
 .rubocop_manual_todo.yml                       | 12 ------------
 .../api/admin/basic_enterprise_serializer.rb   |  4 ++--
 .../api/admin/enterprise_fee_serializer.rb     |  8 +++++---
 .../api/admin/enterprise_serializer.rb         | 18 ++++++++++++------
 .../api/admin/exchange_serializer.rb           |  3 ++-
 .../for_order_cycle/enterprise_serializer.rb   |  9 +++++++--
 .../api/admin/index_enterprise_serializer.rb   |  4 ++--
 .../api/admin/index_order_cycle_serializer.rb  |  5 ++++-
 .../api/admin/line_item_serializer.rb          |  6 ++++--
 .../api/admin/order_cycle_serializer.rb        | 18 ++++++++++++++----
 .../api/admin/subscription_serializer.rb       |  3 ++-
 .../api/admin/tag_rule_serializer.rb           |  6 ++++--
 .../api/admin/variant_override_serializer.rb   |  4 ++--
 13 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 0804602b70..b4cf580789 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -110,18 +110,6 @@ Metrics/LineLength:
   - app/models/variant_override.rb
   - app/models/variant_override_set.rb
   - app/overrides/add_enterprise_fees_to_admin_configurations_menu.rb
-  - app/serializers/api/admin/basic_enterprise_serializer.rb
-  - app/serializers/api/admin/enterprise_fee_serializer.rb
-  - app/serializers/api/admin/enterprise_serializer.rb
-  - app/serializers/api/admin/exchange_serializer.rb
-  - app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
-  - app/serializers/api/admin/index_enterprise_serializer.rb
-  - app/serializers/api/admin/index_order_cycle_serializer.rb
-  - app/serializers/api/admin/line_item_serializer.rb
-  - app/serializers/api/admin/order_cycle_serializer.rb
-  - app/serializers/api/admin/subscription_serializer.rb
-  - app/serializers/api/admin/tag_rule_serializer.rb
-  - app/serializers/api/admin/variant_override_serializer.rb
   - app/services/cart_service.rb
   - app/services/default_stock_location.rb
   - app/services/embedded_page_service.rb
diff --git a/app/serializers/api/admin/basic_enterprise_serializer.rb b/app/serializers/api/admin/basic_enterprise_serializer.rb
index c82eb8994d..880147e296 100644
--- a/app/serializers/api/admin/basic_enterprise_serializer.rb
+++ b/app/serializers/api/admin/basic_enterprise_serializer.rb
@@ -1,4 +1,4 @@
 class Api::Admin::BasicEnterpriseSerializer < ActiveModel::Serializer
-  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :payment_method_ids, :shipping_method_ids,
-             :producer_profile_only, :permalink
+  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category,
+             :payment_method_ids, :shipping_method_ids, :producer_profile_only, :permalink
 end
diff --git a/app/serializers/api/admin/enterprise_fee_serializer.rb b/app/serializers/api/admin/enterprise_fee_serializer.rb
index 6c59a3d482..dec43261d4 100644
--- a/app/serializers/api/admin/enterprise_fee_serializer.rb
+++ b/app/serializers/api/admin/enterprise_fee_serializer.rb
@@ -1,6 +1,6 @@
 class Api::Admin::EnterpriseFeeSerializer < ActiveModel::Serializer
-  attributes :id, :enterprise_id, :fee_type, :name, :tax_category_id, :inherits_tax_category, :calculator_type,
-             :enterprise_name, :calculator_description, :calculator_settings
+  attributes :id, :enterprise_id, :fee_type, :name, :tax_category_id, :inherits_tax_category,
+             :calculator_type, :enterprise_name, :calculator_description, :calculator_settings
 
   def enterprise_name
     object.enterprise.andand.name
@@ -16,7 +16,9 @@ class Api::Admin::EnterpriseFeeSerializer < ActiveModel::Serializer
     result = nil
 
     options[:controller].__send__(:with_format, :html) do
-      result = options[:controller].render_to_string partial: 'admin/enterprise_fees/calculator_settings', locals: { enterprise_fee: object }
+      result = options[:controller].
+        render_to_string(partial: 'admin/enterprise_fees/calculator_settings',
+                         locals: { enterprise_fee: object })
     end
 
     result.gsub('[0]', '[{{ $index }}]').gsub('_0_', '_{{ $index }}_')
diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb
index 702f39aa29..1d1e45e4e8 100644
--- a/app/serializers/api/admin/enterprise_serializer.rb
+++ b/app/serializers/api/admin/enterprise_serializer.rb
@@ -1,7 +1,8 @@
 class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer
-  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :payment_method_ids, :shipping_method_ids,
-             :producer_profile_only, :long_description, :permalink,
-             :preferred_shopfront_message, :preferred_shopfront_closed_message, :preferred_shopfront_taxon_order, :preferred_shopfront_order_cycle_order,
+  attributes :name, :id, :is_primary_producer, :is_distributor, :sells, :category, :permalink,
+             :payment_method_ids, :shipping_method_ids, :producer_profile_only, :long_description,
+             :preferred_shopfront_message, :preferred_shopfront_closed_message,
+             :preferred_shopfront_taxon_order, :preferred_shopfront_order_cycle_order,
              :preferred_product_selection_from_inventory_only,
              :owner, :contact, :users, :tag_groups, :default_tag_group,
              :require_login, :allow_guest_orders, :allow_order_changes,
@@ -21,7 +22,9 @@ class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer
 
   def tag_groups
     object.tag_rules.prioritised.reject(&:is_default).each_with_object([]) do |tag_rule, tag_groups|
-      tag_group = find_match(tag_groups, tag_rule.preferred_customer_tags.split(",").map{ |t| { text: t } })
+      tag_group = find_match(tag_groups, tag_rule.preferred_customer_tags.
+                                           split(",").
+                                           map{ |t| { text: t } })
       if tag_group[:rules].empty?
         tag_groups << tag_group
         tag_group[:position] = tag_groups.count
@@ -32,13 +35,16 @@ class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer
 
   def default_tag_group
     default_rules = object.tag_rules.select(&:is_default)
-    serialized_rules = ActiveModel::ArraySerializer.new(default_rules, each_serializer: Api::Admin::TagRuleSerializer)
+    serialized_rules =
+      ActiveModel::ArraySerializer.new(default_rules,
+                                       each_serializer: Api::Admin::TagRuleSerializer)
     { tags: [], rules: serialized_rules }
   end
 
   def find_match(tag_groups, tags)
     tag_groups.each do |tag_group|
-      return tag_group if tag_group[:tags].length == tags.length && (tag_group[:tags] & tags) == tag_group[:tags]
+      return tag_group if tag_group[:tags].length == tags.length &&
+                          (tag_group[:tags] & tags) == tag_group[:tags]
     end
     { tags: tags, rules: [] }
   end
diff --git a/app/serializers/api/admin/exchange_serializer.rb b/app/serializers/api/admin/exchange_serializer.rb
index 483f490f47..ad81cfa9f1 100644
--- a/app/serializers/api/admin/exchange_serializer.rb
+++ b/app/serializers/api/admin/exchange_serializer.rb
@@ -1,5 +1,6 @@
 class Api::Admin::ExchangeSerializer < ActiveModel::Serializer
-  attributes :id, :sender_id, :receiver_id, :incoming, :variants, :receival_instructions, :pickup_time, :pickup_instructions,
+  attributes :id, :sender_id, :receiver_id, :incoming, :variants,
+             :receival_instructions, :pickup_time, :pickup_instructions,
              :tags, :tag_list
 
   has_many :enterprise_fees, serializer: Api::Admin::BasicEnterpriseFeeSerializer
diff --git a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
index 0724778ba7..0d15bbb80b 100644
--- a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
+++ b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
@@ -6,7 +6,11 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer
              :is_primary_producer, :is_distributor, :sells
 
   def issues_summary_supplier
-    issues = OpenFoodNetwork::EnterpriseIssueValidator.new(object).issues_summary confirmation_only: true
+    issues =
+      OpenFoodNetwork::EnterpriseIssueValidator.
+        new(object).
+        issues_summary(confirmation_only: true)
+
     if issues.nil? && products.empty?
       issues = "no products in inventory"
     end
@@ -23,7 +27,8 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer
 
   def supplied_products
     serializer = Api::Admin::ForOrderCycle::SuppliedProductSerializer
-    ActiveModel::ArraySerializer.new(products, each_serializer: serializer, order_cycle: order_cycle)
+    ActiveModel::ArraySerializer.new(products, each_serializer: serializer,
+                                               order_cycle: order_cycle)
   end
 
   private
diff --git a/app/serializers/api/admin/index_enterprise_serializer.rb b/app/serializers/api/admin/index_enterprise_serializer.rb
index 6cf877aa45..8946686c5f 100644
--- a/app/serializers/api/admin/index_enterprise_serializer.rb
+++ b/app/serializers/api/admin/index_enterprise_serializer.rb
@@ -1,8 +1,8 @@
 require 'open_food_network/enterprise_issue_validator'
 
 class Api::Admin::IndexEnterpriseSerializer < ActiveModel::Serializer
-  attributes :name, :id, :permalink, :is_primary_producer, :sells, :producer_profile_only, :owned, :edit_path,
-             :issues, :warnings
+  attributes :name, :id, :permalink, :is_primary_producer, :sells, :producer_profile_only, :owned,
+             :edit_path, :issues, :warnings
 
   def owned
     return true if options[:spree_current_user].admin?
diff --git a/app/serializers/api/admin/index_order_cycle_serializer.rb b/app/serializers/api/admin/index_order_cycle_serializer.rb
index 97ff415f24..f621f32dc0 100644
--- a/app/serializers/api/admin/index_order_cycle_serializer.rb
+++ b/app/serializers/api/admin/index_order_cycle_serializer.rb
@@ -68,7 +68,10 @@ module Api
       private
 
       def visible_enterprises
-        @visible_enterprises ||= OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object).visible_enterprises
+        @visible_enterprises ||=
+          OpenFoodNetwork::OrderCyclePermissions.
+            new(options[:current_user], object).
+            visible_enterprises
       end
     end
   end
diff --git a/app/serializers/api/admin/line_item_serializer.rb b/app/serializers/api/admin/line_item_serializer.rb
index 5feeddc43a..63ae22a6fb 100644
--- a/app/serializers/api/admin/line_item_serializer.rb
+++ b/app/serializers/api/admin/line_item_serializer.rb
@@ -1,5 +1,6 @@
 class Api::Admin::LineItemSerializer < ActiveModel::Serializer
-  attributes :id, :quantity, :max_quantity, :price, :supplier, :final_weight_volume, :units_product, :units_variant
+  attributes :id, :quantity, :max_quantity, :price, :supplier, :final_weight_volume,
+             :units_product, :units_variant
 
   has_one :order, serializer: Api::Admin::IdSerializer
 
@@ -20,7 +21,8 @@ class Api::Admin::LineItemSerializer < ActiveModel::Serializer
   end
 
   def max_quantity
-    return object.quantity unless object.max_quantity.present? && object.max_quantity > object.quantity
+    return object.quantity unless object.max_quantity.present? &&
+                                  object.max_quantity > object.quantity
     object.max_quantity
   end
 end
diff --git a/app/serializers/api/admin/order_cycle_serializer.rb b/app/serializers/api/admin/order_cycle_serializer.rb
index 1001908ded..52e6f52711 100644
--- a/app/serializers/api/admin/order_cycle_serializer.rb
+++ b/app/serializers/api/admin/order_cycle_serializer.rb
@@ -25,8 +25,14 @@ class Api::Admin::OrderCycleSerializer < ActiveModel::Serializer
   end
 
   def exchanges
-    scoped_exchanges = OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object).visible_exchanges.by_enterprise_name
-    ActiveModel::ArraySerializer.new(scoped_exchanges, each_serializer: Api::Admin::ExchangeSerializer, current_user: options[:current_user])
+    scoped_exchanges =
+      OpenFoodNetwork::OrderCyclePermissions.
+        new(options[:current_user], object).
+        visible_exchanges.by_enterprise_name
+
+    ActiveModel::ArraySerializer.
+      new(scoped_exchanges, each_serializer: Api::Admin::ExchangeSerializer,
+                            current_user: options[:current_user])
   end
 
   def editable_variants_for_incoming_exchanges
@@ -66,9 +72,13 @@ class Api::Admin::OrderCycleSerializer < ActiveModel::Serializer
       # for shops. We need this here to allow hubs to restrict visible variants to only those in
       # their inventory if they so choose
       variants = if enterprise.prefers_product_selection_from_inventory_only?
-                   permissions.visible_variants_for_outgoing_exchanges_to(enterprise).visible_for(enterprise)
+                   permissions.
+                     visible_variants_for_outgoing_exchanges_to(enterprise).
+                     visible_for(enterprise)
                  else
-                   permissions.visible_variants_for_outgoing_exchanges_to(enterprise).not_hidden_for(enterprise)
+                   permissions.
+                     visible_variants_for_outgoing_exchanges_to(enterprise).
+                     not_hidden_for(enterprise)
       end.pluck(:id)
       visible[enterprise.id] = variants if variants.any?
     end
diff --git a/app/serializers/api/admin/subscription_serializer.rb b/app/serializers/api/admin/subscription_serializer.rb
index 1ffd4d3eed..10b2634772 100644
--- a/app/serializers/api/admin/subscription_serializer.rb
+++ b/app/serializers/api/admin/subscription_serializer.rb
@@ -1,7 +1,8 @@
 module Api
   module Admin
     class SubscriptionSerializer < ActiveModel::Serializer
-      attributes :id, :shop_id, :customer_id, :schedule_id, :payment_method_id, :shipping_method_id, :begins_at, :ends_at,
+      attributes :id, :shop_id, :customer_id, :schedule_id, :payment_method_id, :shipping_method_id,
+                 :begins_at, :ends_at,
                  :customer_email, :schedule_name, :edit_path, :canceled_at, :paused_at, :state,
                  :shipping_fee_estimate, :payment_fee_estimate
 
diff --git a/app/serializers/api/admin/tag_rule_serializer.rb b/app/serializers/api/admin/tag_rule_serializer.rb
index 61cbb7ad5f..7e07fa117c 100644
--- a/app/serializers/api/admin/tag_rule_serializer.rb
+++ b/app/serializers/api/admin/tag_rule_serializer.rb
@@ -16,7 +16,8 @@ module Api::Admin::TagRule
   end
 
   class FilterShippingMethodsSerializer < BaseSerializer
-    attributes :preferred_matched_shipping_methods_visibility, :preferred_shipping_method_tags, :shipping_method_tags
+    attributes :preferred_matched_shipping_methods_visibility, :preferred_shipping_method_tags,
+               :shipping_method_tags
 
     def shipping_method_tags
       object.preferred_shipping_method_tags.split(",")
@@ -24,7 +25,8 @@ module Api::Admin::TagRule
   end
 
   class FilterPaymentMethodsSerializer < BaseSerializer
-    attributes :preferred_matched_payment_methods_visibility, :preferred_payment_method_tags, :payment_method_tags
+    attributes :preferred_matched_payment_methods_visibility, :preferred_payment_method_tags,
+               :payment_method_tags
 
     def payment_method_tags
       object.preferred_payment_method_tags.split(",")
diff --git a/app/serializers/api/admin/variant_override_serializer.rb b/app/serializers/api/admin/variant_override_serializer.rb
index 683ad0101e..f21f751911 100644
--- a/app/serializers/api/admin/variant_override_serializer.rb
+++ b/app/serializers/api/admin/variant_override_serializer.rb
@@ -1,6 +1,6 @@
 class Api::Admin::VariantOverrideSerializer < ActiveModel::Serializer
-  attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand, :default_stock, :resettable,
-             :tag_list, :tags, :import_date
+  attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand, :default_stock,
+             :resettable, :tag_list, :tags, :import_date
 
   def tag_list
     object.tag_list.join(",")

From 78cf35807a61ce466b2ca47303fce5301550e2a8 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Mon, 23 Sep 2019 18:37:21 +0100
Subject: [PATCH 09/12] Improve preloading of order query to avoid N+1 queries

---
 app/controllers/api/orders_controller.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/controllers/api/orders_controller.rb b/app/controllers/api/orders_controller.rb
index 03d4e8caa4..9db18d88c4 100644
--- a/app/controllers/api/orders_controller.rb
+++ b/app/controllers/api/orders_controller.rb
@@ -26,7 +26,10 @@ module Api
     end
 
     def order
-      @order ||= Spree::Order.find_by_number!(params[:id])
+      @order ||= Spree::Order.
+        where(number: params[:id]).
+        includes(line_items: { variant: [:product, :stock_items, :default_price] }).
+        first!
     end
   end
 end

From 2f60a855934ec48aff1ec981b8d4c28d349c861f Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Mon, 23 Sep 2019 22:18:49 +0100
Subject: [PATCH 10/12] Improve spec/controllers/api/orders_controller_spec,
 make it more readable

---
 .../controllers/api/orders_controller_spec.rb | 34 +++++++++----------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb
index 7786536635..1412024770 100644
--- a/spec/controllers/api/orders_controller_spec.rb
+++ b/spec/controllers/api/orders_controller_spec.rb
@@ -221,18 +221,26 @@ module Api
         it "returns an order with all required fields" do
           get :show, id: order.number
           expect_order
-          expect_detailed_attributes_to_be_present(json_response)
+          expect(json_response.symbolize_keys.keys).to include(*order_detailed_attributes)
 
-          expect(json_response[:bill_address][:address1]).to eq order.bill_address.address1
-          expect(json_response[:bill_address][:lastname]).to eq order.bill_address.lastname
-          expect(json_response[:ship_address][:address1]).to eq order.ship_address.address1
-          expect(json_response[:ship_address][:lastname]).to eq order.ship_address.lastname
+          expect(json_response[:bill_address]).to include(
+            'address1' => order.bill_address.address1,
+            'lastname' => order.bill_address.lastname
+          )
+          expect(json_response[:ship_address]).to include(
+            'address1' => order.ship_address.address1,
+            'lastname' => order.ship_address.lastname
+          )
           expect(json_response[:shipping_method][:name]).to eq order.shipping_method.name
 
-          expect(json_response[:adjustments].first[:label]).to eq "Transaction fee"
-          expect(json_response[:adjustments].first[:amount]).to eq order.adjustments.payment_fee.first.amount.to_s
-          expect(json_response[:adjustments].second[:label]).to eq "Shipping"
-          expect(json_response[:adjustments].second[:amount]).to eq order.adjustments.shipping.first.amount.to_s
+          expect(json_response[:adjustments].first).to include(
+            'label' => "Transaction fee",
+            'amount' => order.adjustments.payment_fee.first.amount.to_s
+          )
+          expect(json_response[:adjustments].second).to include(
+            'label' => "Shipping",
+            'amount' => order.adjustments.shipping.first.amount.to_s
+          )
 
           expect(json_response[:payments].first[:amount]).to eq order.payments.first.amount.to_s
           expect(json_response[:line_items].size).to eq order.line_items.size
@@ -242,16 +250,8 @@ module Api
 
       def expect_order
         expect(response.status).to eq 200
-        expect_correct_order(json_response, order)
-      end
-
-      def expect_correct_order(json_response, order)
         expect(json_response[:number]).to eq order.number
       end
-
-      def expect_detailed_attributes_to_be_present(json_response)
-        expect(order_detailed_attributes.all?{ |attr| json_response.key? attr.to_s }).to eq true
-      end
     end
 
     private

From baa09b88f7580f2993cf7f1dab01de74ae32c0de Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Tue, 24 Sep 2019 16:57:48 +0100
Subject: [PATCH 11/12] Fix issue with nil current_order where shipping_method
 serializer requires a current_order to calculate the shipping fees

---
 app/controllers/api/orders_controller.rb       | 2 +-
 spec/controllers/api/orders_controller_spec.rb | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/controllers/api/orders_controller.rb b/app/controllers/api/orders_controller.rb
index 9db18d88c4..9b7bf9b1d6 100644
--- a/app/controllers/api/orders_controller.rb
+++ b/app/controllers/api/orders_controller.rb
@@ -2,7 +2,7 @@ module Api
   class OrdersController < BaseController
     def show
       authorize! :read, order
-      render json: order, serializer: Api::OrderDetailedSerializer
+      render json: order, serializer: Api::OrderDetailedSerializer, current_order: order
     end
 
     def index
diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb
index 1412024770..240a97fc94 100644
--- a/spec/controllers/api/orders_controller_spec.rb
+++ b/spec/controllers/api/orders_controller_spec.rb
@@ -218,8 +218,16 @@ module Api
           expect_order
         end
 
+        it "can view an order with weight calculator (this validates case where options[current_order] is nil on the shipping method serializer)" do
+          order.shipping_method.update_attribute(:calculator, create(:weight_calculator, calculable: order))
+          allow(controller).to receive(:current_order).and_return order
+          get :show, id: order.number
+          expect_order
+        end
+
         it "returns an order with all required fields" do
           get :show, id: order.number
+
           expect_order
           expect(json_response.symbolize_keys.keys).to include(*order_detailed_attributes)
 

From 50731e929e0db02e55870deff86bd8f050021187 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Wed, 25 Sep 2019 09:54:33 +0100
Subject: [PATCH 12/12] Remove some attributes from test as they are already
 verified subsequently

---
 spec/controllers/api/orders_controller_spec.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb
index 240a97fc94..05f9743303 100644
--- a/spec/controllers/api/orders_controller_spec.rb
+++ b/spec/controllers/api/orders_controller_spec.rb
@@ -291,8 +291,7 @@ module Api
     def order_detailed_attributes
       [
         :number, :item_total, :total, :state, :adjustment_total, :payment_total,
-        :completed_at, :shipment_state, :payment_state, :email, :special_instructions,
-        :adjustments, :payments, :bill_address, :ship_address, :line_items, :shipping_method
+        :completed_at, :shipment_state, :payment_state, :email, :special_instructions
       ]
     end
   end