Validate store presence on orders

CHANGELOG.md

@@ -1,5 +1,11 @@
 ## Solidus 1.3.0 (unreleased)
 
+*   Made Spree::Order validate :store_id
+
+    All orders created since Spree v2.4 should have a store assigned. We want to build more
+    functionality onto that relation, so we need to make sure that every order has a store.
+    Please run `rake solidus:upgrade:one_point_three` to make sure your orders have a store id set.
+
 *   Removed Spree::Stock::Coordinator#packages from the public interface.
 
     This will allow us to refactor more easily.

api/spec/controllers/spree/api/line_items_controller_spec.rb

@@ -118,6 +118,7 @@ module Spree
       end
 
       context "order contents changed after shipments were created" do
+        let!(:store) { create(:store) }
         let!(:order) { Order.create }
         let!(:line_item) { order.contents.add(product.master) }
 

backend/spec/controllers/spree/admin/orders_controller_spec.rb

@@ -3,6 +3,7 @@
 require 'spree/testing_support/bar_ability'
 
 describe Spree::Admin::OrdersController, type: :controller do
+  let!(:store) { create(:store) }
   context "with authorization" do
     stub_authorization!
 

core/app/models/spree/order.rb

@@ -83,6 +83,7 @@ def states
     accepts_nested_attributes_for :shipments
 
     # Needs to happen before save_permalink is called
+    before_validation :associate_store
     before_validation :set_currency
     before_validation :generate_order_number, on: :create
     before_validation :assign_billing_to_shipping_address, if: :use_billing?
@@ -94,6 +95,7 @@ def states
     validates :email, presence: true, if: :require_email
     validates :email, email: true, if: :require_email, allow_blank: true
     validates :number, presence: true, uniqueness: { allow_blank: true }
+    validates :store_id, presence: true
 
     make_permalink field: :number
 
@@ -683,6 +685,10 @@ def display_store_credit_remaining_after_capture
 
     private
 
+    def associate_store
+      self.store ||= Spree::Store.default
+    end
+
     def link_by_email
       self.email = user.email if user
     end

core/lib/tasks/migrations/assure_store_on_orders.rake

@@ -0,0 +1,38 @@
+namespace :solidus do
+  namespace :migrations do
+    namespace :assure_store_on_orders do
+      desc "Makes sure every order in the system has a store attached"
+      task up: :environment do
+
+        attach_store_to_all_orders
+      end
+
+      def attach_store_to_all_orders
+        orders_without_store_count = Spree::Order.where(store_id: nil).count
+        if orders_without_store_count == 0
+          puts "Everything is good, all orders in your database have a store attached."
+          return
+        end
+
+        spree_store_count = Spree::Store.count
+        if spree_store_count == 0
+          abort "You do not have a store set up. Please create a store instance for your installation."
+        elsif spree_store_count > 1
+          abort(<<-TEXT.squish)
+            You have more than one store set up. We can not be sure which store to attach your
+            orders to. Please attach store ids to all your orders, and run this task again
+            when you're finished.
+          TEXT
+        end
+
+        default_store = Store.where(default: true).first
+        unless default_store
+          abort "Your store is not marked as default. Please mark your one store as the default store and run this task again."
+        end
+
+        Spree::Order.where(store_id: nil).update_all(store_id: Spree::Store.default.id)
+        puts "All orders updated with the default store."
+      end
+    end
+  end
+end

core/lib/tasks/upgrade.rake

@@ -0,0 +1,10 @@
+namespace :solidus do
+  namespace :upgrade do
+    desc "Upgrade Solidus to version 1.3"
+    task one_point_three: [
+        'solidus:migrations:assure_store_on_orders:up'
+      ] do
+      puts "Your Solidus install is ready for Solidus 1.3."
+    end
+  end
+end

core/spec/lib/spree/core/importer/order_spec.rb

@@ -3,6 +3,7 @@
 module Spree
   module Core
     describe Importer::Order do
+      let!(:store) { create(:store) }
       let!(:country) { create(:country) }
       let!(:state) { country.states.first || create(:state, country: country) }
       let!(:stock_location) { create(:stock_location, admin_name: 'Admin Name') }

core/spec/models/spree/adjustment_spec.rb

@@ -4,6 +4,7 @@
 require 'spec_helper'
 
 describe Spree::Adjustment, type: :model do
+  let!(:store) { create :store }
   let(:order) { Spree::Order.new }
   let(:line_item) { create :line_item, order: order }
 

core/spec/models/spree/gateway_spec.rb

@@ -47,8 +47,9 @@ def provider_class
   end
 
   context "fetching payment sources" do
+    let(:store) { create :store }
     let(:user) { create :user }
-    let(:order) { Spree::Order.create(user: user, completed_at: completed_at) }
+    let(:order) { Spree::Order.create(user: user, completed_at: completed_at, store: store) }
 
     let(:payment_method) { create(:credit_card_payment_method) }
 

core/spec/models/spree/order/checkout_spec.rb

@@ -2,7 +2,8 @@
 require 'spree/testing_support/order_walkthrough'
 
 describe Spree::Order, type: :model do
-  let(:order) { Spree::Order.new }
+  let!(:store) { create(:store) }
+  let(:order) { Spree::Order.new(store: store) }
 
   def assert_state_changed(order, from, to)
     state_change_exists = order.state_changes.where(previous_state: from, next_state: to).exists?
@@ -696,15 +697,13 @@ class SubclassedOrder < Spree::Order
 
     it "does not attempt to check shipping rates" do
       order.email = '[email protected]'
-      order.store = FactoryGirl.build(:store)
       expect(order).not_to receive(:ensure_available_shipping_rates)
       order.next!
       assert_state_changed(order, 'cart', 'complete')
     end
 
     it "does not attempt to process payments" do
       order.email = '[email protected]'
-      order.store = FactoryGirl.build(:store)
       allow(order).to receive(:ensure_promotions_eligible).and_return(true)
       allow(order).to receive(:ensure_line_item_variants_are_not_deleted).and_return(true)
       allow(order).to receive_message_chain(:line_items, :present?).and_return(true)

core/spec/models/spree/order/finalizing_spec.rb

@@ -4,8 +4,8 @@
   let(:order) { stub_model("Spree::Order") }
 
   context "#finalize!" do
+    let!(:store) { create(:store) }
     let(:order) { Spree::Order.create(email: '[email protected]', store: store) }
-    let(:store) { FactoryGirl.build(:store) }
 
     before do
       order.update_column :state, 'complete'

core/spec/models/spree/order_contents_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe Spree::OrderContents, type: :model do
+  let!(:store) { create :store }
   let(:order) { Spree::Order.create }
   let(:variant) { create(:variant) }
   let!(:stock_location) { variant.stock_locations.first }

core/spec/models/spree/order_spec.rb

@@ -7,13 +7,39 @@ def compute(_computable)
 end
 
 describe Spree::Order, type: :model do
+  let(:store) { build_stubbed(:store) }
   let(:user) { stub_model(Spree::LegacyUser, email: "[email protected]") }
-  let(:order) { stub_model(Spree::Order, user: user) }
+  let(:order) { stub_model(Spree::Order, user: user, store: store) }
 
   before do
     allow(Spree::LegacyUser).to receive_messages(current: mock_model(Spree::LegacyUser, id: 123))
   end
 
+  context '#store' do
+    it { is_expected.to respond_to(:store) }
+
+    context 'when there is no store assigned' do
+      subject { Spree::Order.new }
+
+      context 'when there is no default store' do
+        it "will not be valid" do
+          expect(subject).not_to be_valid
+        end
+      end
+
+      context "when there is a default store" do
+        let!(:store) { create(:store) }
+
+        it { is_expected.to be_valid }
+      end
+    end
+
+    context 'when a store is assigned' do
+      subject { Spree::Order.new(store: create(:store)) }
+      it { is_expected.to be_valid }
+    end
+  end
+
   context "#canceled_by" do
     let(:admin_user) { create :admin_user }
     let(:order) { create :order }
@@ -46,6 +72,7 @@ def compute(_computable)
   end
 
   context "#create" do
+    let!(:store) { create :store }
     let(:order) { Spree::Order.create }
 
     it "should assign an order number" do
@@ -254,6 +281,7 @@ def merge!(other_order, user = nil)
   end
 
   context "ensure shipments will be updated" do
+    subject(:order) { create :order }
     before do
       Spree::Shipment.create!(order: order)
     end
@@ -821,6 +849,7 @@ def merge!(other_order, user = nil)
   end
 
   describe "#create_proposed_shipments" do
+    subject(:order) { create(:order) }
     it "assigns the coordinator returned shipments to its shipments" do
       shipment = build(:shipment)
       allow_any_instance_of(Spree::Stock::Coordinator).to receive(:shipments).and_return([shipment])

core/spec/models/spree/order_updater_spec.rb

@@ -2,6 +2,7 @@
 
 module Spree
   describe OrderUpdater, type: :model do
+    let!(:store) { create :store }
     let(:order) { Spree::Order.create }
     let(:updater) { Spree::OrderUpdater.new(order) }
 

core/spec/models/spree/payment_spec.rb

@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe Spree::Payment, type: :model do
-  let(:order) { Spree::Order.create }
+  let(:store) { create :store }
+  let(:order) { Spree::Order.create(store: store) }
   let(:refund_reason) { create(:refund_reason) }
 
   let(:gateway) do

core/spec/models/spree/promotion_handler/coupon_spec.rb

@@ -251,6 +251,7 @@ def expect_adjustment_creation(adjustable:, promotion:, promotion_code:nil)
         end
 
         context "for an order with taxable line items" do
+          let(:store) { create(:store) }
           before(:each) do
             @country = create(:country)
             @zone = create(:zone, name: "Country Zone", default_tax: true, zone_members: [])
@@ -263,7 +264,7 @@ def expect_adjustment_creation(adjustable:, promotion:, promotion_code:nil)
               zone: @zone
             )
 
-            @order = Spree::Order.create!
+            @order = Spree::Order.create!(store: store)
             allow(@order).to receive_messages coupon_code: "10off"
           end
           context "and the product price is less than promo discount" do

core/spec/models/spree/shipment_spec.rb

@@ -532,9 +532,14 @@
   end
 
   context "changes shipping rate via general update" do
+    let(:store) { create :store }
     let(:order) do
       Spree::Order.create(
-        payment_total: 100, payment_state: 'paid', total: 100, item_total: 100
+        payment_total: 100,
+        payment_state: 'paid',
+        total: 100,
+        item_total: 100,
+        store: store
       )
     end
 

core/spec/models/spree/shipping_manifest_spec.rb

@@ -2,6 +2,7 @@
 
 module Spree
   describe ShippingManifest, type: :model do
+    let!(:store) { create :store }
     let(:order) { Order.create! }
     let(:variant) { create :variant }
     let!(:shipment) { create(:shipment, state: 'pending', order: order) }

frontend/spec/controllers/spree/current_order_tracking_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe 'current order tracking', type: :controller do
+  let!(:store) { create(:store) }
   let(:user) { create(:user) }
 
   controller(Spree::StoreController) do

frontend/spec/controllers/spree/orders_controller_ability_spec.rb

@@ -4,6 +4,7 @@ module Spree
   describe OrdersController, type: :controller do
     ORDER_TOKEN = 'ORDER_TOKEN'
 
+    let!(:store) { create(:store) }
     let(:user) { create(:user) }
     let(:guest_user) { create(:user) }
     let(:order) { Spree::Order.create }

frontend/spec/controllers/spree/orders_controller_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe Spree::OrdersController, type: :controller do
+  let!(:store) { create(:store) }
   let(:user) { create(:user) }
 
   context "Order model mock" do
@@ -116,7 +117,7 @@
   end
 
   context "line items quantity is 0" do
-    let(:order) { Spree::Order.create }
+    let(:order) { Spree::Order.create(store: store) }
     let(:variant) { create(:variant) }
     let!(:line_item) { order.contents.add(variant, 1) }
 

frontend/spec/features/automatic_promotion_adjustments_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe "Automatic promotions", type: :feature, js: true do
+  let!(:store) { create(:store) }
   let!(:country) { create(:country, name: "United States of America", states_required: true) }
   let!(:state) { create(:state, name: "Alabama", country: country) }
   let!(:zone) { create(:zone) }

frontend/spec/features/cart_spec.rb

@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe "Cart", type: :feature, inaccessible: true do
+  before { create(:store) }
+
   it "shows cart icon on non-cart pages" do
     visit spree.root_path
     expect(page).to have_selector("li#link-to-cart a", visible: true)

frontend/spec/features/coupon_code_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe "Coupon code promotions", type: :feature, js: true do
+  let!(:store) { create(:store) }
   let!(:country) { create(:country, name: "United States of America", states_required: true) }
   let!(:state) { create(:state, name: "Alabama", country: country) }
   let!(:zone) { create(:zone) }

frontend/spec/features/currency_spec.rb

@@ -2,6 +2,7 @@
 
 describe "Switching currencies in backend", type: :feature do
   before do
+    create(:store)
     create(:base_product, name: "RoR Mug")
   end
 

frontend/spec/features/free_shipping_promotions_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe "Free shipping promotions", type: :feature, js: true do
+  let!(:store) { create(:store) }
   let!(:country) { create(:country, name: "United States of America", states_required: true) }
   let!(:state) { create(:state, name: "Alabama", country: country) }
   let!(:zone) { create(:zone) }

frontend/spec/features/locale_spec.rb

@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe 'setting locale', type: :feature do
+  let!(:store) { create(:store) }
   def with_locale(locale)
     I18n.locale = locale
     Spree::Frontend::Config[:locale] = locale

frontend/spec/features/promotion_code_invalidation_spec.rb

@@ -11,6 +11,7 @@
   end
 
   background do
+    create(:store)
     FactoryGirl.create(:product, name: "DL-44")
     FactoryGirl.create(:product, name: "E-11")
 

frontend/spec/features/quantity_promotions_spec.rb

@@ -12,6 +12,7 @@
   given(:calculator) { FactoryGirl.create(:calculator, preferred_amount: 5) }
 
   background do
+    create(:store)
     FactoryGirl.create(:product, name: "DL-44")
     FactoryGirl.create(:product, name: "E-11")
     promotion.actions << action