Add a spec for #transfer_to_location to verify the attributes of the new shipment

[hectoregm]

When a shipment split is done the new shipment created by #transfer_to_location doesn't associate
the order shipping address to this new shipment so all new shipments have nil for address.

[houndci-bot]

Prefer map over collect.

[hectoregm]

The main problem is that splitting a completed order doesn't fill the address and the shipping method
for the new shipment, this later raises an error when doing a transition to shipped:

ActiveRecord::RecordInvalid: Validation of Address can't be blank, Shipping method can't be blank.

https://github.com/solidusio/solidus/blob/master/core/app/models/spree/order_shipping.rb#L51

when creating a Spree::Carton called on the after_transition hook for shipped.

[mamhoff]

Thanks, this makes sense to me. @forkata Any idea why this was omitted in the first place?

Btw really nice specs.

[forkata]

@mamhoff It appears that the address association on shipment isn't used consistently and this was probably omitted by mistake. I looked through the history of this code and as far back as I could trace it, it seems this would have been an issue b252e9a.

Related to this there is a proposed change to completely remove the address association on shipment here #1138 and always use the Order#ship_address instead. If we decide to go down that direction, then this change may no longer be needed.

[hectoregm]

@forkata yeah I read the proposal to delegate to Order#ship_address but I think we can use this spec
for #trasnfer_for_location right ?, I could remove address expectation and the injection of address.

[forkata]

@hectoregm We can definitely use the spec, thank you for adding that. I would keep an eye on what the decision is regarding the address association on Shipment and make any changes based on that if needed.

[forkata]

It would be nice if we can wrap all these expectations in an aggregate_failures block so we can see multiple failures in a single run.

http://www.rubydoc.info/github/rspec/rspec-expectations/RSpec%2FMatchers%3Aaggregate_failures

[hectoregm]

@forkata made the changes we mentioned and added aggregate_failures block

[jhawthorn]

👍