Closed iloveitaly closed 10 years ago
BDQ
commented
10 years ago @iloveitaly hey, we were reviewing this issue and we're a little confused with your flow.
So, the order completes in Spree are you not creating a shipment in Spree? From reading the steps it sounds like you want NetSuite to create the shipment? and then that new shipment be created in Spree?
You can have spree_wombat push shipments as well, so when the order completes you'll have both records in Wombat.
iloveitaly
commented
10 years ago (I apologize in advance for the long comment here, trying to be as clear as possible about our business case)
So, the order completes in Spree are you not creating a shipment in Spree? From reading the steps it sounds like you want NetSuite to create the shipment? and then that new shipment be created in Spree?
Yes, that's correct: the shipment is not created in Spree first. It's created in NetSuite and then pushed to Spree.
In our business, fulfillment happens in NetSuite. Warehouse employees look at NS SaleOrders and fulfill them using the standard NS processes already in place. Employes don't know that Spree exists, they will treat these orders just like an order from any other channel. Once the SalesOrder is fulfilled in NetSuite, the NS endpoint pulls the shipment information from NetSuite and pushes that data to Spree.
In the current NS integration there is not a persistent connection between a ItemFulfillment object (NS equivalent of a Shipment object) on the NS side and a Shipment object on the Spree side. So, although a Shipment object in pending status exists by default in Spree there is no way (that I'm aware of) to associate the ItemFulfillment object with the Shipment object in Spree.
Here is our business logic with a bit more detail:
Washington suggested that we should be using the update_shipment handler instead of the add_shipment handler, since it would make sense that update_shipment would create additional InventoryItems. This makes total sense semantically, I'm just struggling to see how this would work in situations where fulfillment happens in an external system that doesn't have a association between shipment objects in the two systems.
If we use the update_shipment handler it pulls the ID from the JSON and assumes that it's the shipment order number. However, with how the NS endpoint is setup the ID in the shipment JSON is the internalId of the ItemFulfillment object, so it would cause an exception since Spree expects a Shipment number (ex: H58341025022).
If there was a persistent connection between the ItemFulfillment object and the Shipment object in Spree we could use the update_shipment handler. However, since the ItemFulfillment object is created by a warehouse employee at the time of shipment I'm not aware of any way to create a persistent connection between the two objects.
Another issue here is that sometimes an order is only partially fulfilled. So, semantically a add_shipment handler should be called, but we would want the partial fulfillment of the order to use the existing InventoryItems on the spree side so the order is marked as shipped on spree and the correct notifications are sent to the user. I don't have in depth knowledge of the InventoryItem/Shipment models in spree, so I might be missing something here, but I believe if we don't use the existing InventoryItems the order will not be marked as shipped correctly (because additional InventoryItems will be associated with that order) and it will mess up inventory levels on the spree side. Additionally, it seems as though the update_shipment handler expects the items in the shipping payload to match the spree shipment object exactly, so a partial fulfillment would break the system.
To handle the following two cases:
A couple possible solutions that came to mind:
Any thoughts on this? How do others that manage fulfillment in external systems and do partial fulfillments (when part of the order is a pre-order item, for example)?
BDQ
commented
10 years ago @iloveitaly apologies for the delay in getting back to you on this.
spree_wombat is only intended to be a starting point for connecting your store to Wombat, our experience with customers so far is that they all do shipment processing in a slightly different way and I don't think it's feasible to get spree_wombat to support all this different usage patterns.
We fully expect people to diverge from the default handlers, so thats why we've made it easy to override a given handler.
To address your specific situation, here's how I might tackle it.
Presumptions:
Workflow: 1) Add a custom attribute to the shipment model in Spree to hold a reference (item_fulfillment_id) to the NS ItemFulfillment object. 2) When polling the ItemFulfillment's from NS into Wombat include their id as the shipment's ID in Wombat. 3) Using a custom add_shipment handler (as the shipments will always be initially new coming from NS) you can: 3.a) check if a shipment already exists with the item_fulfillment_id that matches the current json id and update that to match the rest of the details. 3.b) if no shipment exists with the item_fulfillment_id: 3.b.i) look for any shipment with no item_fulfillment_id attribute populated and set it to the id present in the JSON and update the shipment to match. 3.b.ii) if no shipments remain (without a item_fulfillment_id) - create a shipment and set its item_fulfillment_id to match.
To move Spree::IventoryUnits between shipments, just need to change the shipment_id attribute.
And there's helper method to increase / decrease inventory units as needed:
https://github.com/spree/spree/blob/1-3-stable/core/app/models/spree/inventory_unit.rb#L48-L71
An alternative approach is to poll NS ItemFulfillments into Wombat as custom objects ('fulfillments') and just route those via a custom webhook + handler to your Spree store and have it follow the workflow laid out above. It just handles the conversion of ItemFulfillment to Shipment at the Spree store as opposed to converting them before they reach Wombat.
Either way, I hope this was helpful.
iloveitaly
commented
10 years ago Thanks for the response @BDQ. Some of the ideas were definitely helpful.
If anyone needs to accomplish this in the future, my implementation is below. Includes some custom stuff for our situation, but most of it is reusable:
Spree::Wombat::Handler::AddShipmentHandler.class_eval do
NETSUITE_TO_SPREE_SHIP_METHOD_MAPPING = {
"123" => "UPS Ground",
"123" => "UPS Standard",
"123" => "UPS Next Day Air",
"123" => "UPS Second Day Air",
"123" => "UPS Worldwide Expedited",
"123" => "UPS Saver",
"123" => "UPS Express",
"123" => "USPS Priority Mail",
"123" => "USPS Media Mail",
"123" => "USPS Priority Mail International"
}
# https://github.com/spree/spree_wombat/blob/1-3-stable/lib/spree/wombat/handler/add_shipment_handler.rb
def process
order_number = @shipment_payload.delete(:order_id)
order = fetch_order(order_number)
return response("Can't find order #{order_number} associated with this shipment", 500) unless order
external_id = @shipment_payload.delete(:id)
address_attributes = prepare_address_attributes
@shipment_payload[:address_attributes] = address_attributes
# some customization to deal with shipping names not being equal; using shipping method internal IDs to map
# https://github.com/spree/spree_wombat/blob/1-3-stable/lib/spree/wombat/handler/shipment_handler_base.rb#L76
shipping_method_name = NETSUITE_TO_SPREE_SHIP_METHOD_MAPPING[@shipment_payload[:shipping_method_id].to_s]
shipping_method = Spree::ShippingMethod.find_by_name(shipping_method_name)
return response("Can't find a ShippingMethod with ID #{@shipment_payload[:shipping_method_id]}!", 500) unless shipping_method
# clear out shipping method entries in the shipment payload
# this is done in the standard spree shipping hook implementation
@shipment_payload.delete(:shipping_method_id)
@shipment_payload.delete(:shipping_method)
inventory_units_hash = prepare_inventory_units(order)
missing_variants = inventory_units_hash[:missing_variants]
missing_line_items = inventory_units_hash[:missing_line_items]
missing_inventory_units = inventory_units_hash[:missing_inventory_units]
inventory_units = inventory_units_hash[:inventory_units]
return response("Can't find variants with the following skus: #{missing_variants.join(', ')}", 500) unless missing_variants.empty?
return response("Can't find line_items with the following skus: #{missing_line_items.join(', ')} in the order.", 500) unless missing_line_items.empty?
return response("Missing inventory units: #{missing_inventory_units.join(', ')}") unless missing_inventory_units.empty?
@shipment_payload[:state] = @shipment_payload.delete(:status)
# delete all unshipped objects; this is desigend to remove the default shipping object that spree creates
order.shipments.ready.delete_all
shipment = order.shipments.new(@shipment_payload)
# associate the shipment with an existing adjustment to prevent an additional adjustment from being created
shipment.adjustment = order.adjustments.shipping.detect { |s| s.source.blank? && s.amount.to_f == @shipment_payload[:cost].to_f }
shipment.shipping_method = shipping_method
shipment.save!
# reload to avoid state object error; reassociate existing inventory units with new shipment
inventory_units.each { |i| i.reload; i.update_attribute(:shipment_id, shipment.id) }
shipment.reload
# after_ship is normally called when a shipment object transitions state to shipped
# in our case we are setting the shipment state so it isn't fired; we need to fire it manually
shipment.send(:after_ship)
shipment.update!(order)
return response("Added shipment #{shipment.number} for order #{order.number}")
end
# https://github.com/spree/spree_wombat/blob/1-3-stable/lib/spree/wombat/handler/shipment_handler_base.rb
def prepare_inventory_units(order)
# build the inventory units
inventory_units = []
missing_inventory_units = []
missing_variants = []
missing_line_items = []
shipping_items = @shipment_payload.delete(:items)
shipping_items.each do |shipping_item|
# get variant
sku = shipping_item[:product_id]
variant = Spree::Variant.find_by_sku(sku)
unless variant.present?
missing_variants << sku
next
end
line_item_id = order.line_items.where(variant_id: variant.id).pluck(:id).first
unless line_item_id
missing_line_items << sku
next
end
# customization: pick from existing inventory items
# https://github.com/spree/spree_wombat/issues/6
quantity = shipping_item[:quantity]
quantity.times do
inventory_unit = order.inventory_units.where(variant_id: variant.id, order_id: order.id)
inventory_unit = inventory_unit.where('id NOT IN (?)', inventory_units.map(&:id)) if inventory_units.present?
inventory_unit = inventory_unit.first
if inventory_unit
inventory_units << inventory_unit
else
missing_inventory_units << variant.sku
end
end
end
{
missing_variants: missing_variants,
missing_line_items: missing_line_items,
missing_inventory_units: missing_inventory_units,
inventory_units: inventory_units,
}
end
end
MisinformedDNA
commented
9 years ago I need something similar, but I'm seeing various run-time errors. Do you have any updated code? I am planning on releasing on 2.4.
feigner
commented
9 years ago Thanks, @iloveitaly -- very helpful.
The add shipment handler doesn't use InventoryItems that are already associated with an order, it creates new inventory item objects associated with that order.
This causes a problem with our Wombat flow: