Skip to content

Commit

Permalink
Allow order object to be passed in. We have shipping_method.id which
Browse files Browse the repository at this point in the history 
is required.
  • Loading branch information
@rterbush
rterbush committed Jun 24, 2013
1 parent acfcb5f commit e537fa3
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions app/models/spree/calculator/shipping/active_shipping/base.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@ def self.service_name
end

def compute(object)
return nil unless object.is_a?(Spree::Shipment) || object.is_a?(Spree::Stock::Package)
return nil unless object.is_a?(Spree::Shipment) ||
object.is_a?(Spree::Stock::Package) ||
object.is_a?(Spree::Order)

order = object.order
if object.is_a?(Spree::Order)
order = object
else
order = object.order
end

if object.is_a?(Spree::Shipment)
@stock_location_id = object.stock_location_id
Expand Down

4 comments on commit e537fa3

@mrpollo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rterbush Hey Randy, I think we need to change line 33 when you do compute(Spree::Order) there is no order.stock_location.id you might need to do order.shipment.stock_location.id, ping me if you need more details, I can submit a PR as well

@rterbush
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrpollo not sure if you saw this PR I submitted recently: #96

I'm beginning to lose hope here. Seems just about anything in any condition is getting passed into these compute methods. I call again on @cmar @BDQ @LBRapid @radar

Is it really normal behavior to have all of these different object types, some with stock_location, some without, some with shipment, some without? Just doesn't feel right to me and leads me to think there is a bigger problem upstream.

@mrpollo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rterbush i created this issue so we can discuss there, maybe we can ping them the guys at IRC so we can get their input -> #97

@radar
Copy link
Contributor

@radar radar commented on e537fa3 Jul 16, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's absolutely mental to be passing in any kind of random object depending on who the developer is and what kind of day they're having. Just sheer insanity.

Can we please find where these objects are being passed from and normalize to using Spree::Stock::Package objects instead? That is the path to sanity.

Please sign in to comment.