Fix with_ids scope bug, and cleanup spec_helper.

Fixes #5

.gitignore

@@ -8,3 +8,4 @@ tmp
 nbproject
 *.swp
 spec/dummy
+Gemfile.lock

Gemfile

@@ -1,10 +1,5 @@
 source 'http://rubygems.org'
 
-group :test do
-  gem 'ffaker'
-  gem 'shoulda-matchers'
-end
-
 if RUBY_VERSION < "1.9"
   gem "ruby-debug"
 else

Versionfile

@@ -7,4 +7,5 @@
 # "0.60.x" => { :branch => "0-60-stable" }
 # "0.40.x" => { :tag => "v1.0.0", :version => "1.0.0" }
 
-"1.1.x" => { :branch => "master" }
+"1.2.x" => { :branch => "master" }
+"1.1.x" => { :branch => "master" }

app/models/spree/product_scope.rb

@@ -20,7 +20,12 @@ class ProductScope < ActiveRecord::Base
     # Get all products with this scope
     def products
       if Product.respond_to?(name)
-        Product.send(name, *arguments)
+        # Since IDs are comma seperated in the first argument we need to correctly pass the ID array to the with_ids scope.
+        if name == 'with_ids'
+          Product.with_ids(arguments.first.split(','))
+        else
+          Product.send(name, *arguments)
+        end
       end
     end
 
@@ -32,7 +37,12 @@ def apply_on(another_scope)
                       if Product.method(self.name.intern).arity == 0
                         Product.send(self.name.intern)
                       else
-                        Product.send(self.name.intern, array.try(:first))
+                        # Since IDs are comma seperated in the first argument we need to correctly pass the ID array to the with_ids scope.
+                        if self.name == 'with_ids'
+                          Product.with_ids(array.first.split(','))
+                        else
+                          Product.send(self.name.intern, array.try(:first))
+                        end
                       end
                     else
                         Product.send(self.name.intern, *array)

spec/models/product_group_spec.rb

@@ -10,6 +10,25 @@
     it { should have_valid_factory(:product_group) }
   end
 
+  describe '#dynamic_products' do
+
+    context 'with a scope named with_ids' do
+      let!(:product_1) { Factory(:product) }
+      let!(:product_2) { Factory(:product) }
+      let!(:product_3) { Factory(:product) }
+      let!(:product_group) do
+        product_group = Factory(:product_group, :name => "With IDs")
+        product_group.product_scopes.create!(:name => "with_ids", :arguments => ["#{product_1.id},#{product_2.id}"])
+        product_group
+      end
+
+      it 'should return proper products' do
+        product_group.dynamic_products.to_a.should eql([product_1, product_2])
+      end
+    end
+
+  end
+
   describe '#from_route' do
     context "wth valid scopes" do
       before do
@@ -77,4 +96,5 @@
     end
 
   end
+
 end

spec/models/product_scope_spec.rb

@@ -3,7 +3,7 @@
 describe Spree::ProductScope do
 
   context "validations" do
-    #it { should have_valid_factory(:product_scope) }
+    it { should have_valid_factory(:product_scope) }
   end
 
   # FIXME use factory in the following test
@@ -35,6 +35,19 @@
         subject.products.should be_nil
       end
     end
+
+    context 'when scope name is with_ids' do
+      let!(:product_1) { Factory(:product) }
+      let!(:product_2) { Factory(:product) }
+      let!(:product_3) { Factory(:product) }
+
+      it 'should properly pass ids to scope' do
+        subject.name = 'with_ids'
+        subject.arguments = ["#{product_1.id},#{product_2.id}"]
+        subject.products.to_a.should eql([product_1, product_2])
+      end
+    end
+
   end
 
 end

spec/requests/admin/products/product_groups_spec.rb

@@ -13,8 +13,8 @@
       Factory(:product_group, :name => 'casual')
 
       click_link "Product Groups"
-      find('table#listing_product_groups tbody tr:nth-child(1) td:nth-child(1)').text.should == 'casual'
-      find('table#listing_product_groups tbody tr:nth-child(2) td:nth-child(1)').text.should == 'sports'
+      find('table#listing_product_groups tbody tr:nth-child(1) td:nth-child(1)').text.should == 'sports'
+      find('table#listing_product_groups tbody tr:nth-child(2) td:nth-child(1)').text.should == 'casual'
     end
   end
 

spec/spec_helper.rb

@@ -3,15 +3,17 @@
 
 require File.expand_path("../dummy/config/environment.rb",  __FILE__)
 
+require 'ffaker'
 require 'rspec/rails'
-require 'spree/url_helpers'
+require 'shoulda-matchers'
 
 # Requires supporting ruby files with custom matchers and macros, etc,
 # in spec/support/ and its subdirectories.
 Dir[File.join(File.dirname(__FILE__), "support/**/*.rb")].each {|f| require f }
 
 # Requires factories defined in spree_core
 require 'spree/core/testing_support/factories'
+require 'spree/core/url_helpers'
 
 Dir["#{File.dirname(__FILE__)}/factories/**"].each do |f|
   fp =  File.expand_path(f)
@@ -36,28 +38,6 @@
   # instead of true.
   config.use_transactional_fixtures = true
 
-  config.include Spree::UrlHelpers
+  config.include AuthenticationHelpers, :type => :request
+  config.include Spree::Core::UrlHelpers
 end
-
-# valid factory tests
-RSpec::Matchers.define :have_valid_factory do |factory_name|
-  match do |model|
-    Factory(factory_name).new_record?.should be_false
-  end
-end
-
-# copied from spree/core for request specs
-module AuthenticationHelpers
-  def sign_in_as!(user)
-    visit '/login'
-    fill_in 'Email', :with => user.email
-    fill_in 'Password', :with => 'secret'
-    click_button 'Login'
-  end
-
-end
-
-RSpec.configure do |c|
-  c.include AuthenticationHelpers, :type => :request
-end
-

spec/support/authentication_helpers.rb

@@ -0,0 +1,9 @@
+# copied from spree/core for request specs
+module AuthenticationHelpers
+  def sign_in_as!(user)
+    visit '/login'
+    fill_in 'Email', :with => user.email
+    fill_in 'Password', :with => 'secret'
+    click_button 'Login'
+  end
+end

spec/support/have_valid_factory.rb

@@ -0,0 +1,6 @@
+# valid factory tests
+RSpec::Matchers.define :have_valid_factory do |factory_name|
+  match do |model|
+    Factory(factory_name).new_record?.should be_false
+  end
+end

spree_product_groups.gemspec

@@ -4,7 +4,7 @@ Gem::Specification.new do |s|
   s.name        = 'spree_product_groups'
   s.version     = '1.0.0'
   s.summary     = 'Product Group Extension for Spree'
-  s.description = 'Spree extension for product group functionality that was removed from Spree 1.0.1'
+  s.description = 'Spree extension for product group functionality that was removed from Spree 1.1.0'
   s.required_ruby_version = '>= 1.8.7'
 
   s.authors           = ['Chris Mar']
@@ -16,10 +16,11 @@ Gem::Specification.new do |s|
   s.require_path = 'lib'
   s.requirements << 'none'
 
-  s.add_dependency 'spree', '~> 1.1.0.beta'
+  s.add_dependency 'spree', '>= 1.1.0', '<= 1.3.0'
   s.add_development_dependency 'capybara', '1.0.1'
-  s.add_development_dependency 'factory_girl'
+  s.add_development_dependency 'factory_girl', '~> 2.6'
   s.add_development_dependency 'ffaker'
   s.add_development_dependency 'rspec-rails', '~> 2.7'
+  s.add_development_dependency 'shoulda-matchers'
   s.add_development_dependency 'sqlite3'
 end