Merge quantity of units when serializing shipment.items

[huoxito]

Currently when receiving that shipment back via a webhook from wombat
the extention would complain about a diff in the items.

e.g. The received shipment items do not match with the shipment, diff: [{:sku=>"123", :quantity=>1}, {:sku=>"123", :quantity=>1}]

Before:

    items: [{ product_id: 1, quantity: 1 }, { product_id: 1, quantity: 1 }]

Now:

    items: [{ product_id: 1, quantity: 2 }]

// @JDutil @peterberkenbosch could you please review?

I think this is ok given the shipment.items key would be a representation of what we want in wombat and not what we have in spree exactly (inventory units don't have a quantity in spree).

[athal7]

fyi this is going to conflict with 918f215

[athal7]

sometimes an inventory unit's variant doesn't match a line item's variant. wouldn't we want to use the inventory unit's variant here?

[JDutil]

When doesn't a IU variant match a LI variant?

[athal7]

exchange shipments and product assembly line items

[JDutil]

The new exchanges?

[huoxito]

aaah right thanks for reminding that @athal7

I had started grouping by variants but later changed because it was easier to group by line items. I think I changed it because I couldn't see how to fetch the price reliably. I will give another try.

[athal7]

yea the new exchanges. they have an inventory unit for the new variant, but don't add a new line item because that would add price to the order. they associate to the old line item so that if the new inventory unit gets returned we know how much money to give back

[JDutil]

This looks okay to me, but I had merged PRs before that changed shipments to use the IU serializer instead of the LI serializer which this feels like it's reverting back.

I don't remember the exact reason for the change off hand, but the commits referenced by @athal7 seem to be the ones.

[huoxito]

just pushed an updated version, would you guys mind showing how does this idea conflict with 918f215 I dont get it. Perhaps we're missing a spec?

I don't see the handler specs using the same serializers here on the examples.

[athal7]

That handler checks whether the shipment items is valid by building back up its own representation of the shipment items (maybe should use the serializer?) and comparing against the payload. It builds the items with the quantity of the inventory unit if that field exists, but for now a quantity of 1. Therefore if shipment items have a quantity of greater than 1, the handler will think that payload is invalid since it doesn't match a subset of the shipment items representation it builds.

[huoxito]

thanks @athal7

I'm very confused about having a quantity key but this quantity key can't be greater than 1? Why is that? Sounds like we should also fix the handler code then?

We need to figure this. Again the motivation for this fix started from this error:

The received shipment items do not match with the shipment, diff: [{:sku=>"123", :quantity=>1}, {:sku=>"123", :quantity=>1}]

[athal7]

That diff may just mean that those items exist in the payload but are unexpected as far as the shipment is concerned

[athal7]

Quantity key is there to support the upcoming addition of inventory item quantity as well as to maintain consistency with the line item payload that existed prior to the change to use inventory units

[frahugo]

Any news on this issue? we have to merge JSON every morning in Wombat for shipments that got some line items exploded.

[huoxito]

@frahugo yes I'm double checking the shipment handler code today and should be resolved soon along with this.

[peterberkenbosch]

@huoxito the handler code will make inventory_units based on the quantity. https://github.com/spree/spree_wombat/blob/master/lib/spree/wombat/handler/add_shipment_handler.rb#L73

[huoxito]

Thats the add to shipment handler @peterberkenbosch, we need to fix it on the update handler here, many other services can return shipment.items with quantity > 1 and too bad the handler cant deal with it.