Isolate each component of Spree

This commit contains a whole bunch of changes, from pull request spree#1296
which will fix spree#1292.

To understand the gist of the problem, please see the comment threads in

The main bunch of work that was done here was done in two parts: remove all current_user
references from Core and to isolate each component during its testing so
that it is not loading all of Spree for each component, just the
bare-bones components that are needed.

For instance, Core now will only load Core. API will now load API, Auth
and Core. Dash will load Auth and Core. Promo will load Auth and Core.

.gitignore

@@ -23,7 +23,6 @@ db/*.sqlite3*
 db/schema.rb
 doc/**/*
 Gemfile.lock
-*/Gemfile
 */Gemfile.lock
 lib/products_index_profiler.rb
 log/*.log

.travis.yml

@@ -5,7 +5,15 @@ env:
   - DB=sqlite
   - DB=mysql
   - DB=postgres
-script: "DISPLAY=:99.0 bundle exec rake"
+script:
+  - "export DISPLAY=:99.0"
+  - "alias set_gemfile='export BUNDLE_GEMFILE=\"`pwd`/Gemfile\"'"
+  - "cd api; set_gemfile; bundle install --quiet; bundle exec rspec spec"
+  - "cd ../auth; set_gemfile; bundle install --quiet; bundle exec rspec spec"
+  - "cd ../core; set_gemfile; bundle install --quiet; bundle exec rspec spec"
+  - "cd ../dash; set_gemfile; bundle install --quiet; bundle exec rspec spec"
+  - "cd ../promo; set_gemfile; bundle install --quiet; bundle exec rspec spec"
+
 notifications:
   email:
     - [email protected]
@@ -14,6 +22,7 @@ branches:
   only:
     - 1-0-stable
     - master
+    - isolation
 rvm:
   - 1.8.7
   - 1.9.3

Gemfile

@@ -1,48 +1,3 @@
-source 'http://rubygems.org'
-
-gem 'json'
-gem 'sqlite3'
-gem 'mysql2'
-gem 'pg'
-
-# Gems used only for assets and not required
-# in production environments by default.
-group :assets do
-  gem 'sass-rails', "~> 3.2"
-  gem 'coffee-rails', "~> 3.2"
-end
-
-group :test do
-  gem 'guard'
-  gem 'guard-rspec', '~> 0.5.0'
-  gem 'rspec-rails', '~> 2.8.0'
-  gem 'factory_girl_rails', '~> 1.7.0'
-  gem 'email_spec', '~> 1.2.1'
-
-  platform :ruby_18 do
-    gem 'rcov'
-  end
-
-  platform :ruby_19 do
-    gem 'simplecov'
-  end
-
-  gem 'ffaker'
-  gem 'shoulda-matchers', '~> 1.0.0'
-  gem 'capybara'
-  gem 'selenium-webdriver', '2.16.0'
-  gem 'database_cleaner', '0.7.1'
-  gem 'launchy'
-end
-
-# platform :ruby_18 do
-#   gem "ruby-debug"
-# end
-
-# platform :ruby_19 do
-#   gem "ruby-debug19"
-# end
+eval(File.read(File.dirname(__FILE__) + '/common_spree_dependencies.rb'))
 
 gemspec
-
-

Rakefile

@@ -20,14 +20,6 @@ end
 
 task :default => :all_tests
 
-desc "Run all tests for sqlite3 only"
-task :all_tests do
-  %w(api auth core dash promo).each do |gem_name|
-    puts "########################### #{gem_name} (spec) ###########################"
-    sh "cd #{gem_name} && #{$0} spec"
-  end
-end
-
 desc "Run all tests for all supported databases"
 task :ci do
   cmd = "bundle update"; puts cmd; system cmd;

api/Gemfile

@@ -0,0 +1,6 @@
+eval(File.read(File.dirname(__FILE__) + '/../common_spree_dependencies.rb'))
+
+gem 'spree_core', :path => '../core'
+gem 'spree_auth', :path => '../auth'
+
+gemspec

auth/Gemfile

@@ -0,0 +1,5 @@
+eval(File.read(File.dirname(__FILE__) + '/../common_spree_dependencies.rb'))
+
+gem 'spree_core', :path => '../core'
+
+gemspec

auth/app/controllers/spree/admin/admin_users_controller_decorator.rb

@@ -1,5 +1,21 @@
 require File.expand_path('../../base_controller_decorator', __FILE__)
 Spree::Admin::UsersController.class_eval do
   rescue_from Spree::User::DestroyWithOrdersError, :with => :user_destroy_with_orders_error
+
+  update.after :sign_in_if_change_own_password
+
+  before_filter :load_roles, :only => [:edit, :new, :update, :create]
+
+  private
+
+    def sign_in_if_change_own_password
+      if current_user == @user && @user.password.present?
+        sign_in(@user, :event => :authentication, :bypass => true)
+      end
+    end
+
+    def load_roles
+      @roles = Spree::Role.scoped
+    end
 end
 

auth/app/overrides/admin_payment_methods_index.rb

@@ -0,0 +1,6 @@
+Deface::Override.new(:virtual_path => "spree/admin/payment_methods/index",
+                     :name => "gateway_banner",
+                     :insert_after => "#listing_payment_methods",
+                     :partial => "spree/admin/banners/gateway")
+
+

auth/spec/requests/admin/users_spec.rb

@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe 'Users' do
+  before do
+    user = Factory(:admin_user, :email => "[email protected]")
+    sign_in_as!(user)
+    visit spree.admin_users_path
+  end
+
+  context "editing own user" do
+    it "should let me edit own password" do
+      click_link("[email protected]")
+      click_link("Edit")
+      fill_in "user_password", :with => "welcome"
+      fill_in "user_password_confirmation", :with => "welcome"
+      click_button "Update"
+
+      page.should have_content("successfully updated!")
+    end
+  end
+end

common_spree_dependencies.rb

@@ -0,0 +1,51 @@
+# By placing all of Spree's shared dependencies in this file and then loading 
+# it for each component's Gemfile, we can be sure that we're only testing just
+# the one component of Spree.
+source 'http://rubygems.org'
+
+gem 'json'
+gem 'sqlite3'
+gem 'mysql2'
+gem 'pg'
+
+# Gems used only for assets and not required
+# in production environments by default.
+group :assets do
+  gem 'sass-rails', "~> 3.2"
+  gem 'coffee-rails', "~> 3.2"
+end
+
+group :test do
+  gem 'guard'
+  gem 'guard-rspec', '~> 0.5.0'
+  gem 'rspec-rails', '~> 2.8.0'
+  gem 'factory_girl_rails', '~> 1.7.0'
+  gem 'email_spec', '~> 1.2.1'
+
+  platform :ruby_18 do
+    gem 'rcov'
+  end
+
+  platform :ruby_19 do
+    gem 'simplecov'
+  end
+
+  gem 'ffaker'
+  gem 'shoulda-matchers', '~> 1.0.0'
+  gem 'capybara'
+  gem 'selenium-webdriver', '2.16.0'
+  gem 'database_cleaner', '0.7.1'
+  gem 'launchy'
+end
+
+# platform :ruby_18 do
+#   gem "ruby-debug"
+# end
+
+# platform :ruby_19 do
+#   gem "ruby-debug19"
+# end
+
+gemspec
+
+

core/Gemfile

@@ -0,0 +1,3 @@
+eval(File.read(File.dirname(__FILE__) + '/../common_spree_dependencies.rb'))
+
+gemspec

core/app/controllers/spree/admin/mail_methods_controller.rb

@@ -5,7 +5,7 @@ class MailMethodsController < ResourceController
 
       def testmail
         @mail_method = Spree::MailMethod.find(params[:id])
-        if TestMailer.test_email(@mail_method, current_user).deliver
+        if TestMailer.test_email(@mail_method, respond_to?(:current_user) ? current_user : nil).deliver
           flash.notice = t('admin.mail_methods.testmail.delivery_success')
         else
           flash[:error] = t('admin.mail_methods.testmail.delivery_error')

core/app/controllers/spree/admin/users_controller.rb

@@ -4,9 +4,6 @@ class UsersController < ResourceController
 
       # http://spreecommerce.com/blog/2010/11/02/json-hijacking-vulnerability/
       before_filter :check_json_authenticity, :only => :index
-      before_filter :load_roles, :only => [:edit, :new, :update, :create]
-
-      update.after :sign_in_if_change_own_password
 
       def index
         respond_with(@collection) do |format|
@@ -66,15 +63,6 @@ def json_data
           end
         end
 
-        def load_roles
-          @roles = Role.all
-        end
-
-        def sign_in_if_change_own_password
-          if current_user == @user && @user.password.present?
-            sign_in(@user, :event => :authentication, :bypass => true)
-          end
-        end
     end
   end
 end

core/app/models/spree/state_change.rb

@@ -9,9 +9,6 @@ def <=>(other)
     end
 
     def assign_user
-      # if Session.activated? && current_user_session = Session.find
-      #   self.user_id ||= current_user_session.user.id
-      # end
       true   # don't stop the filters
     end
   end

core/app/views/spree/admin/payment_methods/index.html.erb

@@ -37,5 +37,3 @@
     <% end %>
   </tbody>
 </table>
-
-<%= render :partial => 'spree/admin/banners/gateway' %>

core/app/views/spree/orders/show.html.erb

@@ -13,10 +13,10 @@
   <p data-hook="links">
     <%= link_to t(:back_to_store), spree.root_path, :class => "button" %>
     <% unless params.has_key? :checkout_complete %>
-      <% if current_user %>
+      <% if respond_to?(:current_user) && current_user %>
         <%= link_to t(:my_account), spree.account_path, :class => "button" %>
       <% end %>
     <% end %>
   </p>
 </div>
-</fieldset>
+</fieldset>

core/lib/spree/core/testing_support/factories/product_factory.rb

@@ -1,21 +1,21 @@
 FactoryGirl.define do
   sequence(:product_sequence) { |n| "Product ##{n} - #{rand(9999)}" }
 
-  factory :product, :class => Spree::Product do
+  factory :simple_product, :class => Spree::Product do
     name { FactoryGirl.generate :product_sequence }
     description { Faker::Lorem.paragraphs(1 + Kernel.rand(5)).join("\n") }
-
-    # associations:
-    tax_category { |r| Spree::TaxCategory.find(:first) || r.association(:tax_category) }
-    shipping_category { |r| Spree::ShippingCategory.find(:first) || r.association(:shipping_category) }
-
     price 19.99
     cost_price 17.00
     sku 'ABC'
     available_on 1.year.ago
     deleted_at nil
   end
 
+  factory :product, :parent => :simple_product do
+    tax_category { |r| Spree::TaxCategory.find(:first) || r.association(:tax_category) }
+    shipping_category { |r| Spree::ShippingCategory.find(:first) || r.association(:shipping_category) }
+  end
+
   factory :product_with_option_types, :parent => :product do
     after_create { |product| Factory(:product_option_type, :product => product) }
   end

core/spec/controllers/controller_helpers_spec.rb

@@ -6,7 +6,6 @@
 describe Spree::ProductsController do
 
   before do
-    controller.stub(:set_current_user)
     I18n.stub(:available_locales => [:en, :de])
     Spree::Config[:default_locale] = nil
     Rails.application.config.i18n.default_locale = :de

core/spec/controllers/spree/admin/base_controller_spec.rb

@@ -11,10 +11,6 @@ def index
     end
   end
 
-  before do
-    controller.stub :current_user => Factory(:admin_user)
-  end
-
   describe "check alerts" do
     it "checks alerts with before_filter" do
       controller.should_receive :check_alerts

core/spec/controllers/spree/admin/image_settings_controller_spec.rb

@@ -1,9 +1,6 @@
 require 'spec_helper'
 
 describe Spree::Admin::ImageSettingsController do
-  before do
-    controller.stub :current_user => Factory(:admin_user)
-  end
 
   context "updating image settings" do
     it "should be able to update paperclip settings" do

core/spec/controllers/spree/admin/mail_methods_controller_spec.rb

@@ -5,9 +5,9 @@
   let(:mail_method) { mock_model(Spree::MailMethod).as_null_object }
 
   before do
-    controller.stub :current_user => Factory(:admin_user)
     Spree::Order.stub :find => order
     Spree::MailMethod.stub :find => mail_method
+    request.env["HTTP_REFERER"] = "/"
   end
 
   context "#create" do
@@ -23,4 +23,9 @@
       put :update, {:order_id => "123", :id => "456", :mail_method_parmas => {:environment => "foo"}}
     end
   end
+
+  it "can trigger testmail without current_user" do
+    post :testmail, :id => Factory(:mail_method).id
+    flash[:error].should_not include("undefined local variable or method `current_user'")
+  end
 end

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

@@ -5,7 +5,6 @@
   let(:order) { mock_model(Spree::Order, :complete? => true, :total => 100) }
 
   before do
-    controller.stub :current_user => Factory(:admin_user)
     Spree::Order.stub :find_by_number => order
     request.env["HTTP_REFERER"] = "http://localhost:3000"
   end

core/spec/controllers/spree/admin/products_controller_spec.rb

@@ -1,10 +1,6 @@
 require 'spec_helper'
 
 describe Spree::Admin::ProductsController do
-  before do
-    controller.stub :current_user => Factory(:admin_user)
-  end
-
   context "#index" do
     it "should not allow JSON request without a valid token" do
       controller.should_receive(:protect_against_forgery?).at_least(:once).and_return(true)
@@ -65,12 +61,7 @@
       get :new
       response.should render_template("admin/products/new")
     end
-
-    it "should create product" do
-      post :create, :product => product_attributes      
-      response.should redirect_to(spree.edit_admin_product_path(Spree::Product.last))
-    end
-
+
     it "should create product from prototype" do
       post :create, :product => product_attributes.merge(:prototype_id => prototype.id)
       product = Spree::Product.last