Validate store presence on orders

[mamhoff]

I need to make some behaviour dependent on the store an order is
associated to. Under normal circumstances, the order helper takes care
of that in 100% of cases. However, there's no presence validation on
the store, and for the feature I'm envisioning (Differing default taxation
depending on which store you access) I need every order to have a store.

This PR should be a point of discussion as to wether we want to actually validate
this.

[mamhoff]

A-ha! If I just make sure the store is somehow set in a before_validation filter, no tests fail. This is now proper PR and not an RfC anymore 👯

[mamhoff]

Let me rebase this, the commit comment is still a bit off.

[cbrunsdon]

I'm good by this too, as going forward it would be nice to count on an order having a store. The original migration set up the store_id on orders set them so we should be good to count on this usually being there: 15df7e7

👍

[mamhoff]

Oh no. So validating :store does essentially nothing, because Spree::Store.default defaults to an empty, invalid store. If I validate :order_id instead, hell ensues.

[mamhoff]

Take away your thumbs @jhawthorn @cbrunsdon, this needs to be reevaluated.

[jhawthorn]

👍

[mamhoff]

@tvdeyen Thanks for the raise vs abort comment - I've amended the last commit.

[gmacdougall]

@mamhoff This looks good to go. Could you please rebase so we can merge this in?

Thanks!

[mamhoff]

Rebased!

[gmacdougall]

Github is showing conflicts again. Could you give it another rebase please?

[mamhoff]

Rebased

[adammathys]

👍 From me. Also, very much appreciate the Rake task.

[tvdeyen]

Could you please try to use required: true on belongs_to :store, instead if validating the store_id presence?

In Rails 5 this will be default and should work here. Not shure about the before_validation running before that, but we should give it a try.

Either way 👍

[mamhoff]

Nope, that doesn't work as it generates a validates :store, presence: true, which in turn breaks with the before_validation hook.

I could look into Spree::Store.default, where a || new causes all this trouble, and I haven't quite found out why it is there. There is a possibility that that clause was inserted in order to prevent the kind of trouble I'm trying to solve with the validation. Thoughts, anyone?