Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Specs, Documentation, Refactor method missing #9

Closed
wants to merge 13 commits into from

5 participants

@jstirk

Hi all,

There is a fair bit in this pull request. It won't apply cleanly against edge, as it was built intended for a 0.60.1 application.

It includes an existing pull request from craigmcnamara spree-contrib/spree_related_products#6 , as that was required to get it working on 0.60.1.

In addition to this :

1) It gives the project some specs to test the core Product methods;
2) Specs can be easily run with no external projects;
3) Documentation for the Project methods;
4) Prevents the extension from failing with an exception if any of the Products have available_on = nil;
5) Refactors the method_missing so as that it just dispatches the call and nothing else. This makes it easier to override the functionality (which was my original use case).

Please let me know if I can offer any assistance with getting this pulled. I have already started merging it into master locally, but I don't have an edge Spree environment set up to test the 3.1-dependant functionality.

Cheers,
Jason

@jstirk

Just realised this patch will probably break all you gem-related rake tasks :(

@BDQ
Owner
BDQ commented

@jstirk - There's some great stuff in here and we should definitely get it merged in. Thanks for the contribution.

@ajjahn - Can you try and get this merged. Ping me on #spree if you need a hand.

@ajjahn

@bdq - Yeah I'll merge it, and let you know if I need any help. As @jstirk mentioned, it will break the gem-related tasks. Is there any need to preserve the gemcutter tasks since we are using github and the versionfile?

@BDQ
Owner
BDQ commented

@ajjahn - No I think you can rip the gemcutter stuff

@JDutil
Owner

I just updated master with the namespace changes, but didn't think to pull this in first. @ajjahn if you hadn't started on this I could work on updating it to work with the namespace changes. It would be nice to get the specs going.

@JDutil JDutil closed this in 0d43d9f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 10, 2011
  1. @craigmcnamara
  2. @craigmcnamara
Commits on Jul 7, 2011
  1. Remove blank tests

    Jason Stirk authored
  2. Add development dependencies

    Jason Stirk authored
  3. Updated rakefile

    Jason Stirk authored
  4. Update .gitignore

    Jason Stirk authored
  5. Create blank spec to get everything running

    Jason Stirk authored
  6. Don't let the test_app generator destroy our Gemfile

    Jason Stirk authored
  7. Initial specs for new Product functionality

    Jason Stirk authored
  8. Update .gitignore

    Jason Stirk authored
  9. Update README with development notes

    Jason Stirk authored
  10. Refactor logic out of method_missing

    Jason Stirk authored
    Moves the bulk of the logic out of method_missing and into a collection
    of methods that are easy to overload.
This page is out of date. Refresh to see the latest.
View
4 .gitignore
@@ -1,4 +1,6 @@
.DS_Store
*.swp
pkg
-
+.rvmrc
+spec/test_app
+.rspec
View
3  Gemfile
@@ -0,0 +1,3 @@
+source 'http://rubygems.org'
+
+gemspec
View
174 Gemfile.lock
@@ -0,0 +1,174 @@
+PATH
+ remote: .
+ specs:
+ spree_related_products (3.0.2)
+ spree_core (>= 0.30.0)
+
+GEM
+ remote: http://rubygems.org/
+ specs:
+ abstract (1.0.0)
+ actionmailer (3.0.9)
+ actionpack (= 3.0.9)
+ mail (~> 2.2.19)
+ actionpack (3.0.9)
+ activemodel (= 3.0.9)
+ activesupport (= 3.0.9)
+ builder (~> 2.1.2)
+ erubis (~> 2.6.6)
+ i18n (~> 0.5.0)
+ rack (~> 1.2.1)
+ rack-mount (~> 0.6.14)
+ rack-test (~> 0.5.7)
+ tzinfo (~> 0.3.23)
+ activemerchant (1.15.0)
+ activesupport (>= 2.3.8)
+ braintree (>= 2.0.0)
+ builder (>= 2.0.0)
+ activemodel (3.0.9)
+ activesupport (= 3.0.9)
+ builder (~> 2.1.2)
+ i18n (~> 0.5.0)
+ activerecord (3.0.9)
+ activemodel (= 3.0.9)
+ activesupport (= 3.0.9)
+ arel (~> 2.0.10)
+ tzinfo (~> 0.3.23)
+ activeresource (3.0.9)
+ activemodel (= 3.0.9)
+ activesupport (= 3.0.9)
+ activesupport (3.0.9)
+ acts_as_list (0.1.2)
+ arel (2.0.10)
+ bcrypt-ruby (2.1.4)
+ braintree (2.10.1)
+ builder (>= 2.0.0)
+ builder (2.1.2)
+ cancan (1.6.4)
+ devise (1.3.3)
+ bcrypt-ruby (~> 2.1.2)
+ orm_adapter (~> 0.0.3)
+ warden (~> 1.0.3)
+ diff-lcs (1.1.2)
+ erubis (2.6.6)
+ abstract (>= 1.0.0)
+ faker (0.9.5)
+ i18n (~> 0.4)
+ highline (1.5.1)
+ i18n (0.5.0)
+ jquery-rails (0.2.6)
+ rails (~> 3.0)
+ thor (~> 0.14.4)
+ mail (2.2.19)
+ activesupport (>= 2.3.6)
+ i18n (>= 0.4.0)
+ mime-types (~> 1.16)
+ treetop (~> 1.4.8)
+ meta_search (1.0.5)
+ actionpack (~> 3.0.2)
+ activerecord (~> 3.0.2)
+ activesupport (~> 3.0.2)
+ arel (~> 2.0.2)
+ mime-types (1.16)
+ nested_set (1.6.6)
+ activerecord (>= 3.0.0)
+ railties (>= 3.0.0)
+ orm_adapter (0.0.5)
+ paperclip (2.3.11)
+ activerecord (>= 2.3.0)
+ activesupport (>= 2.3.2)
+ polyglot (0.3.1)
+ rack (1.2.3)
+ rack-mount (0.6.14)
+ rack (>= 1.0.0)
+ rack-test (0.5.7)
+ rack (>= 1.0)
+ rails (3.0.9)
+ actionmailer (= 3.0.9)
+ actionpack (= 3.0.9)
+ activerecord (= 3.0.9)
+ activeresource (= 3.0.9)
+ activesupport (= 3.0.9)
+ bundler (~> 1.0)
+ railties (= 3.0.9)
+ railties (3.0.9)
+ actionpack (= 3.0.9)
+ activesupport (= 3.0.9)
+ rake (>= 0.8.7)
+ rdoc (~> 3.4)
+ thor (~> 0.14.4)
+ rake (0.9.2)
+ rd_find_by_param (0.1.1)
+ activerecord (~> 3.0)
+ activesupport (~> 3.0)
+ rd_resource_controller (1.0.1)
+ rd_unobtrusive_date_picker (0.1.0)
+ rdoc (3.8)
+ rspec (2.6.0)
+ rspec-core (~> 2.6.0)
+ rspec-expectations (~> 2.6.0)
+ rspec-mocks (~> 2.6.0)
+ rspec-core (2.6.4)
+ rspec-expectations (2.6.0)
+ diff-lcs (~> 1.1.2)
+ rspec-mocks (2.6.0)
+ rspec-rails (2.6.1)
+ actionpack (~> 3.0)
+ activesupport (~> 3.0)
+ railties (~> 3.0)
+ rspec (~> 2.6.0)
+ spree (0.60.1)
+ spree_api (= 0.60.1)
+ spree_auth (= 0.60.1)
+ spree_core (= 0.60.1)
+ spree_dash (= 0.60.1)
+ spree_promo (= 0.60.1)
+ spree_sample (= 0.60.1)
+ spree_api (0.60.1)
+ spree_auth (= 0.60.1)
+ spree_core (= 0.60.1)
+ spree_auth (0.60.1)
+ cancan (= 1.6.4)
+ devise (= 1.3.3)
+ spree_core (= 0.60.1)
+ spree_core (0.60.1)
+ activemerchant (= 1.15.0)
+ acts_as_list (= 0.1.2)
+ faker (= 0.9.5)
+ highline (= 1.5.1)
+ jquery-rails (= 0.2.6)
+ meta_search (= 1.0.5)
+ nested_set (= 1.6.6)
+ paperclip (= 2.3.11)
+ rails (= 3.0.9)
+ rd_find_by_param (= 0.1.1)
+ rd_resource_controller
+ rd_unobtrusive_date_picker (= 0.1.0)
+ state_machine (= 0.9.4)
+ stringex (= 1.0.3)
+ will_paginate (= 3.0.pre2)
+ spree_dash (0.60.1)
+ spree_core (= 0.60.1)
+ spree_promo (0.60.1)
+ spree_core (= 0.60.1)
+ spree_sample (0.60.1)
+ spree_core (= 0.60.1)
+ sqlite3 (1.3.3)
+ state_machine (0.9.4)
+ stringex (1.0.3)
+ thor (0.14.6)
+ treetop (1.4.9)
+ polyglot (>= 0.3.1)
+ tzinfo (0.3.29)
+ warden (1.0.4)
+ rack (>= 1.0)
+ will_paginate (3.0.pre2)
+
+PLATFORMS
+ ruby
+
+DEPENDENCIES
+ rspec-rails
+ spree
+ spree_related_products!
+ sqlite3
View
7 README.md
@@ -43,3 +43,10 @@ Discounts
---------
If you install the spree-automatic-coupons extension you can also specify a discount amount to be applied if a customer purchases both products. Note: In order for the coupon to be automatically applied, you must create a coupon of type: __RelatedProductDiscount__ and leave the __code__ value empty (blank codes are required for coupons to be automatically applied).
+
+Development
+-----------
+
+Run `rake test_app` to create the test application in `spec/test_app`.
+
+`rake` will run the specs.
View
67 Rakefile
@@ -1,18 +1,61 @@
require 'rubygems'
require 'rake'
-require 'rake/testtask'
-require 'rake/packagetask'
-require 'rake/gempackagetask'
-spec = eval(File.read('spree_related_products.gemspec'))
+require 'bundler'
+Bundler::GemHelper.install_tasks
+Bundler.setup
-Rake::GemPackageTask.new(spec) do |p|
- p.gem_spec = spec
+require 'rspec'
+require 'rspec/core/rake_task'
+RSpec::Core::RakeTask.new
+
+desc "Default Task"
+task :default => [:spec]
+
+desc "Default Task"
+task :default => [ :spec ]
+
+desc "Regenerates a rails 3 app for testing"
+task :test_app do
+ require 'spree'
+ require 'generators/spree/test_app_generator'
+
+ # The generator wrecks our Gemfile and Gemfile.lock. Back them up and restore them when we're done
+ cp('Gemfile', 'Gemfile.real')
+ cp('Gemfile.lock', 'Gemfile.lock.real')
+
+ class StoreTestAppGenerator < Spree::Generators::TestAppGenerator
+
+ def install_gems
+ inside "test_app" do
+ run 'rake spree_core:install'
+ run 'rake spree_related_products:install'
+ end
+ end
+
+ def migrate_db
+ run_migrations
+ end
+
+ protected
+ def full_path_for_local_gems
+ <<-gems
+gem 'spree_core'
+gem 'spree_related_products', :path => \'#{File.dirname(__FILE__)}\'
+ gems
+ end
+
+ end
+ StoreTestAppGenerator.start
+ cp('Gemfile.real', 'Gemfile')
+ cp('Gemfile.lock.real', 'Gemfile.lock')
+ rm('Gemfile.real')
+ rm('Gemfile.lock.real')
end
-desc "Release to gemcutter"
-task :release => :package do
- require 'rake/gemcutter'
- Rake::Gemcutter::Tasks.new(spec).define
- Rake::Task['gem:push'].invoke
-end
+namespace :test_app do
+ desc 'Rebuild test and cucumber databases'
+ task :rebuild_dbs do
+ system("cd spec/test_app && rake db:drop db:migrate RAILS_ENV=test")
+ end
+end
View
5 app/controllers/admin/products_controller_decorator.rb
@@ -0,0 +1,5 @@
+Admin::ProductsController.class_eval do
+ def related
+ @relation_types = Product.relation_types
+ end
+end
View
76 app/models/products_decorator.rb
@@ -0,0 +1,76 @@
+module SpreeRelatedProducts
+ module ProductDecorator
+ def self.included(target)
+ target.class_eval do
+ has_many :relations, :as => :relatable
+
+ # Returns all the RelationTypes which apply_to this class.
+ def self.relation_types
+ RelationType.find_all_by_applies_to(self.to_s, :order => :name)
+ end
+
+ # The AREL Relations that will be used to filter the resultant items.
+ #
+ # By default this will remove any items which are deleted, or not yet available.
+ #
+ # You can override this method to fine tune the filter. For example,
+ # to only return Products with more than 2 items in stock, you could
+ # do the following:
+ #
+ # def self.relation_filter
+ # set = super
+ # set.where('products.count_on_hand >= 2')
+ # end
+ #
+ # This could also feasibly be overridden to sort the result in a
+ # particular order, or restrict the number of items returned.
+ def self.relation_filter
+ where('products.deleted_at' => nil).where('products.available_on IS NOT NULL').where('products.available_on <= ?', Time.now)
+ end
+
+ # Decides if there is a relevant RelationType related to this class
+ # which should be returned for this method.
+ #
+ # If so, it calls relations_for_relation_type. Otherwise it passes
+ # it up the inheritance chain.
+ def method_missing(method, *args)
+ relation_type = self.class.relation_types.detect { |rt| rt.name.downcase.gsub(" ", "_").pluralize == method.to_s.downcase }
+
+ if relation_type.nil?
+ super
+ else
+ relations_for_relation_type(relation_type)
+ end
+ end
+
+ end
+ end
+
+private
+ # Returns all the Products that are related to this record for the given RelationType.
+ #
+ # Uses the Relations to find all the related items, and then filters
+ # them using +Product.relation_filter+ to remove unwanted items.
+ def relations_for_relation_type(relation_type)
+ # Find all the relations that belong to us for this RelationType
+ related_ids = relations.where(:relation_type_id => relation_type.id).select(:related_to_id).collect(&:related_to_id)
+
+ # Construct a query for all these records
+ result = self.class.where(:id => related_ids)
+
+ # Merge in the relation_filter if it's available
+ result = result.merge(self.class.relation_filter.scoped) if relation_filter
+
+ result
+ end
+
+ # Simple accessor for the class-level relation_filter.
+ # Could feasibly be overloaded to filter results relative to this
+ # record (eg. only higher priced items)
+ def relation_filter
+ self.class.relation_filter
+ end
+ end
+end
+
+Product.send(:include, SpreeRelatedProducts::ProductDecorator)
View
35 lib/spree_related_products.rb
@@ -4,39 +4,16 @@
module SpreeRelatedProducts
class Engine < Rails::Engine
- def self.activate
+ config.autoload_paths += %W(#{config.root}/lib)
+ def self.activate
+ Dir.glob(File.join(File.dirname(__FILE__), "../app/**/*_decorator*.rb")) do |c|
+ Rails.env.production? ? require(c) : load(c)
+ end
# Calculator::RelatedProductDiscount.register
- Product.class_eval do
- has_many :relations, :as => :relatable
-
- def self.relation_types
- RelationType.find_all_by_applies_to(self.to_s, :order => :name)
- end
-
- def method_missing(method, *args)
- relation_type = self.class.relation_types.detect { |rt| rt.name.downcase.gsub(" ", "_").pluralize == method.to_s.downcase }
-
- if relation_type.nil?
- super
- else
- relations.find_all_by_relation_type_id(relation_type.id).map(&:related_to).select {|product| product.deleted_at.nil? && product.available_on <= Time.now()}
- end
-
- end
- end
-
- Admin::ProductsController.class_eval do
- def related
- load_object
- @relation_types = Product.relation_types
- end
- end
-
end
-
- config.autoload_paths += %W(#{config.root}/lib #{config.root}/app/models/calculator)
+
config.to_prepare &method(:activate).to_proc
end
View
123 spec/models/product_spec.rb
@@ -0,0 +1,123 @@
+require 'spec_helper'
+
+describe Product do
+
+ context "class" do
+ describe ".relation_types" do
+ it "should return all the RelationTypes in use for this Product" do
+ relation_type = RelationType.create!(:name => "Related Products", :applies_to => "Product")
+ Product.relation_types.should include(relation_type)
+ end
+ end
+ end
+
+ context "instance" do
+ before(:each) do
+ @product = valid_product(:name => 'Product')
+ @relation_type = RelationType.create(:name => "Related Products", :applies_to => "Product")
+ end
+
+ describe ".relations" do
+ it "has many relations" do
+ @product.save!
+ other1 = valid_product!
+ other2 = valid_product!
+
+ relation1 = Relation.create!(:relatable => @product, :related_to => other1, :relation_type => @relation_type)
+ relation2 = Relation.create!(:relatable => @product, :related_to => other2, :relation_type => @relation_type)
+
+ @product.reload
+ @product.relations.should include(relation1)
+ @product.relations.should include(relation2)
+ end
+
+ it "has many relations for different RelationTypes" do
+ @product.save!
+ other = valid_product!
+
+ other_relation_type = RelationType.new(:name => "Recommended Products")
+
+ relation1 = Relation.create!(:relatable => @product, :related_to => other, :relation_type => @relation_type)
+ relation2 = Relation.create!(:relatable => @product, :related_to => other, :relation_type => other_relation_type)
+
+ @product.reload
+ @product.relations.should include(relation1)
+ @product.relations.should include(relation2)
+ end
+ end
+
+ describe "RelationType finders" do
+ before(:each) do
+ @product.save!
+ @other = valid_product!
+ @relation = Relation.create!(:relatable => @product, :related_to => @other, :relation_type => @relation_type)
+ @product.reload
+ end
+
+ it "should return the relevant relations" do
+ @product.related_products.should include(@other)
+ end
+
+ it "should be the pluralised form of the RelationType name" do
+ @relation_type.update_attributes(:name => 'Related Product')
+ @product.related_products.should include(@other)
+ end
+
+ it "should not return relations for another RelationType" do
+ @product.save!
+ other2 = valid_product!
+
+ other_relation_type = RelationType.new(:name => "Recommended Products")
+
+ relation1 = Relation.create!(:relatable => @product, :related_to => @other, :relation_type => @relation_type)
+ relation2 = Relation.create!(:relatable => @product, :related_to => other2, :relation_type => other_relation_type)
+
+ @product.reload
+ @product.related_products.should include(@other)
+ @product.related_products.should_not include(other2)
+ end
+
+ it "should not return Products that are deleted" do
+ @other.update_attributes(:deleted_at => Time.now)
+
+ @product.related_products.should be_blank
+ end
+
+ it "should not return Products that are not yet available" do
+ @other.update_attributes(:available_on => Time.now + 1.hour)
+
+ @product.related_products.should be_blank
+ end
+
+ it "should not return Products where available_on are blank" do
+ @other.update_attributes(:available_on => nil)
+
+ @product.related_products.should be_blank
+ end
+
+ it "should return all results if .relation_filter is nil" do
+ Product.should_receive(:relation_filter).and_return(nil)
+ @other.update_attributes(:available_on => Time.now + 1.hour)
+
+ @product.related_products.should include(@other)
+ end
+
+ context "with an enhanced Product.relation_filter" do
+ it "should restrict the filter" do
+ relation_filter = Product.relation_filter
+ Product.should_receive(:relation_filter).at_least(:once).and_return(relation_filter.includes(:master).where('variants.count_on_hand > 2'))
+
+ @other.master.update_attributes(:count_on_hand => 1)
+
+ other2 = valid_product!
+ other2.master.update_attributes(:count_on_hand => 3)
+ relation = Relation.create!(:relatable => @product, :related_to => other2, :relation_type => @relation_type)
+
+ results = @product.related_products
+ results.should_not include(@other)
+ results.should include(other2)
+ end
+ end
+ end
+ end
+end
View
30 spec/spec_helper.rb
@@ -0,0 +1,30 @@
+# This file is copied to ~/spec when you run 'ruby script/generate rspec'
+# from the project root directory.
+ENV["RAILS_ENV"] ||= 'test'
+require File.expand_path("../test_app/config/environment", __FILE__)
+require 'rspec/rails'
+
+# Requires supporting files with custom matchers and macros, etc,
+# in ./support/ and its subdirectories.
+Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].each {|f| require f}
+
+RSpec.configure do |config|
+ # == Mock Framework
+ #
+ # If you prefer to use mocha, flexmock or RR, uncomment the appropriate line:
+ #
+ # config.mock_with :mocha
+ # config.mock_with :flexmock
+ # config.mock_with :rr
+ config.mock_with :rspec
+
+ config.fixture_path = "#{::Rails.root}/spec/fixtures"
+
+ #config.include Devise::TestHelpers, :type => :controller
+ # If you're not using ActiveRecord, or you'd prefer not to run each of your
+ # examples within a transaction, comment the following line or assign false
+ # instead of true.
+ config.use_transactional_fixtures = true
+end
+
+@configuration ||= AppConfiguration.find_or_create_by_name("Default configuration")
View
14 spec/support/product_templates.rb
@@ -0,0 +1,14 @@
+def valid_product(options={})
+ defaults = { :name => "New Product",
+ :price => 19.95,
+ :available_on => 1.hour.ago
+ }
+
+ Product.new(defaults.merge(options))
+end
+
+def valid_product!(options={})
+ p = valid_product(options)
+ p.save!
+ p
+end
View
3  spree_related_products.gemspec
@@ -18,4 +18,7 @@ Gem::Specification.new do |s|
s.has_rdoc = true
s.add_dependency('spree_core', '>= 0.30.0')
+ s.add_development_dependency('rspec-rails')
+ s.add_development_dependency('spree')
+ s.add_development_dependency('sqlite3')
end
View
12 test/functional/admin/relations_controller_test.rb
@@ -1,12 +0,0 @@
-require File.dirname(__FILE__) + '/../../test_helper'
-
-# Re-raise errors caught by the controller.
-Admin::RelationsController.class_eval { def rescue_action(e) raise e end }
-
-class Admin::RelationsControllerTest < ActionController::TestCase
-
- # Replace this with your real tests.
- def test_truth
- assert true
- end
-end
View
0  test/unit/helpers/admin/relations_helper_test.rb
No changes.
Something went wrong with that request. Please try again.