Add association between stores and shipping #2557
Conversation
Like payment methods, shipping methods are typically store specific. Solidus sites running multiple stores are likely to want the ability to scope shipping methods to specific stores in a similar manner. This only adds the join table between the two models.
When estimating rates, we only want shipping methods available to the same store that the order was placed in to be used.
This adds a select field allowing us to easily control the association between shipping methods and stores.
| validates :name, presence: true | ||
|
|
||
| validate :at_least_one_shipping_category | ||
|
|
||
| scope :available_to_store, ->(store) do | ||
| raise ArgumentError, "You must provide a store" if store.nil? | ||
| store.shipping_methods.empty? ? all : where(id: store.shipping_method_ids) |
There was a problem hiding this comment.
I think this is the opposite of the direction that we should have for the "none means all" rule.
I'd rather have "shipping method with no store is available to all stores" not "a store with no shipping methods can use all shpiping methods"
There was a problem hiding this comment.
Apparently payment methods do the same
I'm not really a fan of that either
There was a problem hiding this comment.
This is the direction we use for most of these types of associations. (As you found with payment methods.)
I agree that it's confusing and I'm also not a fan of the "none means all" pattern we've adopted, but it's probably best to at least keep them consistent.
| class CreateStoreShippingMethods < ActiveRecord::Migration[5.1] | ||
| def change | ||
| create_table :spree_store_shipping_methods do |t| | ||
| t.references :store, foreign_key: { to_table: "spree_stores" } |
There was a problem hiding this comment.
I'm not sure this syntax is valid. It fails under MySQL with
Mysql2::Error: Can't create table `solidus_core_test`.`spree_store_shipping_methods` (errno: 150 "Foreign key constraint is incorrectly formed")
There was a problem hiding this comment.
Reading more, to_table does seem like a valid argument (I had thought it was not), but for some reason this isn't working.
There was a problem hiding this comment.
Fails after
(60.4ms) CREATE TABLE `spree_store_shipping_methods` (`id` bigint NOT NULL AUTO_INCREMENT PRIMARY KEY, `store_id` bigint, `shipping_method_id` bigint, `created_at` datetime NOT NULL, `updated_at` datetime NOT NULL, INDEX `index_spree_store_shipping_methods_on_store_id` (`store_id`), INDEX `index_spree_store_shipping_methods_on_shipping_method_id` (`shipping_method_id`), CONSTRAINT `fk_rails_1ec4e4430e`
FOREIGN KEY (`store_id`)
REFERENCES `spree_stores` (`id`)
, CONSTRAINT `fk_rails_24cc4a3cd1`
FOREIGN KEY (`shipping_method_id`)
REFERENCES `spree_shipping_methods` (`id`)
) ENGINE=InnoDB
There was a problem hiding this comment.
The issue is that this is creating an FK from a bigint to a normal size int.
Removed the FK entirely in in #2596
Like payment methods, shipping methods are typically store specific.