Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

0-70-stable branch #17

Closed
wants to merge 4 commits into from

3 participants

@tavon

I fixed a couple issues:

1) The gem was not loading any decorators or overrides and didn't include an engine name. This lead to inconsistent behavior depending on whether you were in development/production env.
2) The admin/shipping_methods new/edit form was not working b/c the select tag was generating the wrong values for each options. Instead of the "Calculator::Ups::Ground" format, it was generating "Ups/Ground" which would error out in an uninitialized constant error.

@BDQ BDQ commented on the diff
lib/spree_active_shipping.rb
((10 lines not shown))
def self.activate
- Dir.glob(File.join(File.dirname(__FILE__), "../app/models/calculator/**/base.rb")) do |c|
- Rails.env.production? ? require(c) : load(c)
+ Dir.glob(File.join(File.dirname(__FILE__), "../app/**/*_decorator*.rb")) do |c|
+ Rails.application.config.cache_classes ? require(c) : load(c)
+ end
+
+ Dir.glob(File.join(File.dirname(__FILE__), "../app/overrides/**/*.rb")) do |c|
@BDQ Owner
BDQ added a note

You don't need to manually load overrides anymore, Deface will find them inside an engine automatically.

@tavon
tavon added a note

That's great! The issues is more of the decorator not being loaded. I'm not sure what the best way to modify this pull request to not include the overrides...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rterbush

I'm closing this given the age of the PR and the age of the Spree version it is targeting. Feel free to reopen if this is still an issue.

@rterbush rterbush closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
6 app/controllers/admin/shipping_methods_controller_decorator.rb
@@ -4,10 +4,6 @@
# overriding to return an array of Calculator objects instead of a Set
def load_data
@available_zones = Zone.find :all, :order => :name
- @calculators = []
- ShippingMethod.calculators.each {|calc|
- @calculators << eval(calc.name).new
- }
- @calculators = @calculators.sort_by(&:description)
+ @calculators = ShippingMethod.calculators.sort_by(&:description)
end
end
View
27 lib/spree_active_shipping.rb
@@ -1,23 +1,33 @@
require 'spree_core'
require 'active_shipping'
-module ActiveShippingExtension
- class Engine < Rails::Engine
+module SpreeActiveShipping
+ class Engine < Rails::Engine
+ engine_name 'spree_active_shipping'
+
def self.activate
- Dir.glob(File.join(File.dirname(__FILE__), "../app/models/calculator/**/base.rb")) do |c|
- Rails.env.production? ? require(c) : load(c)
+ Dir.glob(File.join(File.dirname(__FILE__), "../app/**/*_decorator*.rb")) do |c|
+ Rails.application.config.cache_classes ? require(c) : load(c)
+ end
+
+ Dir.glob(File.join(File.dirname(__FILE__), "../app/overrides/**/*.rb")) do |c|
@BDQ Owner
BDQ added a note

You don't need to manually load overrides anymore, Deface will find them inside an engine automatically.

@tavon
tavon added a note

That's great! The issues is more of the decorator not being loaded. I'm not sure what the best way to modify this pull request to not include the overrides...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ Rails.application.config.cache_classes ? require(c) : load(c)
+ end
+
+ Dir[File.join(File.dirname(__FILE__), "../app/models/calculator/**/*.rb")].sort.each do |c|
+ Rails.application.config.cache_classes ? require(c) : load(c)
end
#Only required until following active_shipping commit is merged (add negotiated rates).
#http://github.com/BDQ/active_shipping/commit/2f2560d53aa7264383e5a35deb7264db60eb405a
- ActiveMerchant::Shipping::UPS.send(:include, Spree::ActiveShipping::UpsOverride)
+ ActiveMerchant::Shipping::UPS.send(:include, Spree::ActiveShipping::UpsOverride)
end
-
+
config.autoload_paths += %W(#{config.root}/lib)
config.to_prepare &method(:activate).to_proc
-
+
initializer "spree_active_shipping.register.calculators" do |app|
- Dir.glob(File.join(File.dirname(__FILE__), "../app/models/calculator/**/*.rb")) do |c|
+ Dir[File.join(File.dirname(__FILE__), "../app/models/calculator/**/*.rb")].sort.each do |c|
Rails.env.production? ? require(c) : load(c)
end
@@ -27,6 +37,7 @@ def self.activate
Calculator::Usps::Base.descendants
)
end
+
end
end
Something went wrong with that request. Please try again.