Connect DistributorShippingMethods to OrderCycles instead of Spree::ShippingMethods

Before if a shipping method was shared between multiple distributors it could only be disabled/enabled on that order cycle for all the distributors which have that shipping method e.g. you couldn't select that shipping method for one distributor but disable it for another.

app/models/order_cycle.rb

@@ -24,9 +24,9 @@ class OrderCycle < ApplicationRecord
   has_many :distributors, -> { distinct }, source: :receiver, through: :cached_outgoing_exchanges
   has_many :order_cycle_schedules
   has_many :schedules, through: :order_cycle_schedules
-  has_and_belongs_to_many :selected_shipping_methods,
-                          class_name: 'Spree::ShippingMethod',
-                          join_table: 'order_cycles_shipping_methods'
+  has_and_belongs_to_many :selected_distributor_shipping_methods,
+                          class_name: 'DistributorShippingMethod',
+                          join_table: 'order_cycles_distributor_shipping_methods'
   has_paper_trail meta: { custom_data: proc { |order_cycle| order_cycle.schedule_ids.to_s } }
 
   attr_accessor :incoming_exchanges, :outgoing_exchanges
@@ -160,11 +160,10 @@ class OrderCycle < ApplicationRecord
       distinct
   end
 
-  def attachable_shipping_methods
-    Spree::ShippingMethod.frontend.
-      joins(:distributor_shipping_methods).
-      where("distributor_id IN (?)", distributor_ids).
-      distinct
+  def attachable_distributor_shipping_methods
+    DistributorShippingMethod.joins(:shipping_method).
+      merge(Spree::ShippingMethod.frontend).
+      where("distributor_id IN (?)", distributor_ids)
   end
 
   def clone!
@@ -178,8 +177,9 @@ class OrderCycle < ApplicationRecord
     oc.schedule_ids = schedule_ids
     oc.save!
     exchanges.each { |e| e.clone!(oc) }
-    oc.selected_shipping_method_ids = attachable_shipping_methods.map(&:id) &
-                                      selected_shipping_method_ids
+    oc.selected_distributor_shipping_method_ids = (
+      attachable_distributor_shipping_methods.map(&:id) & selected_distributor_shipping_method_ids
+    )
     sync_subscriptions
     oc.reload
   end
@@ -293,11 +293,11 @@ class OrderCycle < ApplicationRecord
     items.each { |li| scoper.scope(li.variant) }
   end
 
-  def shipping_methods
-    if simple? || selected_shipping_methods.none?
-      attachable_shipping_methods
+  def distributor_shipping_methods
+    if simple? || selected_distributor_shipping_methods.none?
+      attachable_distributor_shipping_methods
     else
-      selected_shipping_methods
+      selected_distributor_shipping_methods
     end
   end
 

app/services/order_available_shipping_methods.rb

@@ -29,7 +29,9 @@ class OrderAvailableShippingMethods
     if order_cycle.nil? || order_cycle.simple?
       distributor.shipping_methods
     else
-      distributor.shipping_methods.where(id: order_cycle.shipping_methods.select(:id))
+      distributor.shipping_methods.where(
+        id: order_cycle.distributor_shipping_methods.select(:shipping_method_id)
+      )
     end.frontend.to_a
   end
 end

app/services/order_cycle_form.rb

@@ -11,7 +11,9 @@ class OrderCycleForm
     @user = user
     @permissions = OpenFoodNetwork::Permissions.new(user)
     @schedule_ids = order_cycle_params.delete(:schedule_ids)
-    @selected_shipping_method_ids = order_cycle_params.delete(:selected_shipping_method_ids)
+    @selected_distributor_shipping_method_ids = order_cycle_params.delete(
+      :selected_distributor_shipping_method_ids
+    )
   end
 
   def save
@@ -24,7 +26,7 @@ class OrderCycleForm
       order_cycle.schedule_ids = schedule_ids
       order_cycle.save!
       apply_exchange_changes
-      attach_selected_shipping_methods
+      attach_selected_distributor_shipping_methods
       sync_subscriptions
       true
     end
@@ -48,16 +50,16 @@ class OrderCycleForm
     OpenFoodNetwork::OrderCycleFormApplicator.new(order_cycle, user).go!
   end
 
-  def attach_selected_shipping_methods
-    return if @selected_shipping_method_ids.nil?
+  def attach_selected_distributor_shipping_methods
+    return if @selected_distributor_shipping_method_ids.nil?
 
     order_cycle.reload # so outgoing exchanges are up-to-date for shipping method validations
-    order_cycle.selected_shipping_method_ids = selected_shipping_method_ids
+    order_cycle.selected_distributor_shipping_method_ids = selected_distributor_shipping_method_ids
     order_cycle.save!
   end
 
-  def attachable_shipping_method_ids
-    @attachable_shipping_method_ids ||= order_cycle.attachable_shipping_methods.map(&:id)
+  def attachable_distributor_shipping_method_ids
+    @attachable_distributor_shipping_method_ids ||= order_cycle.attachable_distributor_shipping_methods.map(&:id)
   end
 
   def exchanges_unchanged?
@@ -66,15 +68,17 @@ class OrderCycleForm
     end
   end
 
-  def selected_shipping_method_ids
-    @selected_shipping_method_ids = attachable_shipping_method_ids &
-                                    @selected_shipping_method_ids.reject(&:blank?).map(&:to_i)
+  def selected_distributor_shipping_method_ids
+    @selected_distributor_shipping_method_ids = (
+      attachable_distributor_shipping_method_ids &
+      @selected_distributor_shipping_method_ids.reject(&:blank?).map(&:to_i)
+    )
 
-    if attachable_shipping_method_ids.sort == @selected_shipping_method_ids.sort
-      @selected_shipping_method_ids = []
+    if attachable_distributor_shipping_method_ids.sort == @selected_distributor_shipping_method_ids.sort
+      @selected_distributor_shipping_method_ids = []
     end
 
-    @selected_shipping_method_ids
+    @selected_distributor_shipping_method_ids
   end
 
   def schedule_ids?

app/services/permitted_attributes/order_cycle.rb

@@ -17,7 +17,7 @@ module PermittedAttributes
         :name, :orders_open_at, :orders_close_at, :coordinator_id,
         :preferred_product_selection_from_coordinator_inventory_only,
         :automatic_notifications,
-        { schedule_ids: [], selected_shipping_method_ids: [], coordinator_fee_ids: [] }
+        { schedule_ids: [], selected_distributor_shipping_method_ids: [], coordinator_fee_ids: [] }
       ]
     end
 

app/views/admin/order_cycles/checkout_options.html.haml

@@ -10,7 +10,7 @@
   %fieldset.no-border-bottom
     %legend{ align: 'center'}= t('.checkout_options')
 
-  = hidden_field_tag "order_cycle[selected_shipping_method_ids][]", ""
+  = hidden_field_tag "order_cycle[selected_distributor_shipping_method_ids][]", ""
 
   .row
     .three.columns
@@ -21,18 +21,18 @@
           %tr
             %th{ colspan: 2 }= t('.shipping_methods')
           - @order_cycle.distributors.each do |distributor|
-            - shipping_methods = @order_cycle.attachable_shipping_methods.where("distributor_id = ?", distributor.id)
+            - distributor_shipping_methods = @order_cycle.attachable_distributor_shipping_methods.where("distributor_id = ?", distributor.id).includes(:shipping_method)
             %tr
               %td
               %td
                 %em= distributor.name
-                - shipping_methods.each do |shipping_method|
+                - distributor_shipping_methods.each do |distributor_shipping_method|
                   %p
-                    %label
-                      = check_box_tag "order_cycle[selected_shipping_method_ids][]",
-                          shipping_method.id, @order_cycle.shipping_methods.include?(shipping_method),
-                          id: "order_cycle_selected_shipping_method_ids_#{shipping_method.id}"
-                      = shipping_method.name
+                    %label{ class: ("disabled" unless distributor_shipping_method.shipping_method.frontend?) }
+                      = check_box_tag "order_cycle[selected_distributor_shipping_method_ids][]",
+                          distributor_shipping_method.id, @order_cycle.distributor_shipping_methods.include?(distributor_shipping_method),
+                          id: "order_cycle_selected_distributor_shipping_method_ids_#{distributor_shipping_method.id}"
+                      = distributor_shipping_method.shipping_method.name
                 - distributor.shipping_methods.backend.each do |shipping_method|
                   %label.disabled
                     = check_box_tag nil, nil, false, disabled: true

spec/models/order_cycle_spec.rb

@@ -408,18 +408,24 @@ describe OrderCycle do
              e.g. shipping method is backoffice only" do
       it "only attaches the valid ones to the clone" do
         distributor = create(:distributor_enterprise)
-        shipping_method_i = create(:shipping_method, distributors: [distributor])
-        shipping_method_ii = create(
+        distributor_shipping_method_i = create(
+          :shipping_method,
+          distributors: [distributor]
+        ).distributor_shipping_methods.first
+        distributor_shipping_method_ii = create(
           :shipping_method,
           distributors: [distributor],
           display_on: Spree::ShippingMethod::DISPLAY_ON_OPTIONS[:back_end]
-        )
+        ).distributor_shipping_methods.first
         order_cycle = create(:distributor_order_cycle, distributors: [distributor])
-        order_cycle.selected_shipping_methods = [shipping_method_i, shipping_method_ii]
+        order_cycle.selected_distributor_shipping_methods = [
+          distributor_shipping_method_i,
+          distributor_shipping_method_ii
+        ]
 
         cloned_order_cycle = order_cycle.clone!
 
-        expect(cloned_order_cycle.shipping_methods).to eq [shipping_method_i]
+        expect(cloned_order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method_i]
       end
     end
   end
@@ -631,52 +637,65 @@ describe OrderCycle do
     end
   end
 
-  describe "#attachable_shipping_methods" do
-    it "includes shipping methods from the distributors on the order cycle" do
+  describe "#attachable_distributor_shipping_methods" do
+    it "includes distributor shipping methods from the distributors on the order cycle" do
       shipping_method = create(:shipping_method)
-      enterprise = create(:enterprise, shipping_methods: [shipping_method])
-      oc = create(:simple_order_cycle, distributors: [enterprise])
+      oc = create(:simple_order_cycle, distributors: [shipping_method.distributors.first])
+      distributor_shipping_method = shipping_method.distributor_shipping_methods.first
 
-      expect(oc.attachable_shipping_methods).to eq([shipping_method])
+      expect(oc.attachable_distributor_shipping_methods).to eq([distributor_shipping_method])
     end
 
-    it "does not include backoffice only shipping methods" do
+    it "does not include backoffice only distributor shipping methods" do
       shipping_method = create(:shipping_method, display_on: "back_end")
       enterprise = create(:enterprise, shipping_methods: [shipping_method])
       oc = create(:simple_order_cycle, distributors: [enterprise])
 
-      expect(oc.attachable_shipping_methods).to be_empty
+      expect(oc.attachable_distributor_shipping_methods).to be_empty
     end
   end
 
-  describe "#shipping_methods" do
+  describe "#distributor_shipping_methods" do
     let(:distributor) { create(:distributor_enterprise) }
 
-    it "returns all attachable shipping methods if the order cycle is simple" do
+    it "returns all attachable distributor shipping methods if the order cycle is simple" do
       oc = create(:sells_own_order_cycle, distributors: [distributor])
 
-      shipping_method = create(:shipping_method, distributors: [distributor])
+      distributor_shipping_method = create(
+        :shipping_method,
+        distributors: [distributor]
+      ).distributor_shipping_methods.first
 
-      expect(oc.shipping_methods).to eq [shipping_method]
+      expect(oc.distributor_shipping_methods).to eq [distributor_shipping_method]
     end
 
     context "distributor order cycle i.e. non-simple" do
       let(:oc) { create(:distributor_order_cycle, distributors: [distributor]) }
 
-      it "returns all attachable shipping methods if no preferred shipping methods have been chosen" do
-        shipping_method = create(:shipping_method, distributors: [distributor])
+      it "returns all attachable distributor shipping methods if no preferred shipping methods have
+          been chosen" do
+        distributor_shipping_method = create(
+          :shipping_method,
+          distributors: [distributor]
+        ).distributor_shipping_methods.first
 
-        expect(oc.selected_shipping_methods).to be_empty
-        expect(oc.shipping_methods).to eq [shipping_method]
+        expect(oc.selected_distributor_shipping_methods).to be_empty
+        expect(oc.distributor_shipping_methods).to eq [distributor_shipping_method]
       end
 
-      it "returns preferred shipping methods if they have been specified" do
-        shipping_method_i = create(:shipping_method, distributors: [distributor])
-        shipping_method_ii = create(:shipping_method, distributors: [distributor])
+      it "returns selected distributor shipping methods if they have been specified" do
+        distributor_shipping_method_i = create(
+          :shipping_method,
+          distributors: [distributor]
+        ).distributor_shipping_methods.first
+        distributor_shipping_method_ii = create(
+          :shipping_method,
+          distributors: [distributor]
+        ).distributor_shipping_methods.first
 
-        oc.selected_shipping_methods << shipping_method_ii
+        oc.selected_distributor_shipping_methods << distributor_shipping_method_ii
 
-        expect(oc.shipping_methods).to eq [shipping_method_ii]
+        expect(oc.distributor_shipping_methods).to eq [distributor_shipping_method_ii]
       end
     end
   end

spec/services/order_available_shipping_methods_spec.rb

@@ -47,15 +47,18 @@ describe OrderAvailableShippingMethods do
     context "distributor order cycle i.e. not simple" do
       it "only returns the shipping methods which are available on the order cycle
           and belong to the order distributor" do
-        distributor_i = create(:distributor_enterprise)
-        distributor_ii = create(:distributor_enterprise)
+        distributor_i = create(:distributor_enterprise, shipping_methods: [])
+        distributor_ii = create(:distributor_enterprise, shipping_methods: [])
         shipping_method_i = create(:shipping_method, distributors: [distributor_i])
         shipping_method_ii = create(:shipping_method, distributors: [distributor_i])
         shipping_method_iii = create(:shipping_method, distributors: [distributor_ii])
         shipping_method_iv = create(:shipping_method, distributors: [distributor_ii])
         order_cycle = create(:distributor_order_cycle,
                              distributors: [distributor_i, distributor_ii])
-        order_cycle.selected_shipping_methods << [shipping_method_i, shipping_method_iii]
+        order_cycle.selected_distributor_shipping_methods << [
+          distributor_i.distributor_shipping_methods.first,
+          distributor_ii.distributor_shipping_methods.first,
+        ]
         order = build(:order, distributor: distributor_i, order_cycle: order_cycle)
 
         available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a

spec/services/order_cycle_form_spec.rb

@@ -125,6 +125,7 @@ describe OrderCycleForm do
     let(:supplier) { create(:supplier_enterprise) }
     let(:user) { distributor.owner }
     let(:shipping_method) { create(:shipping_method, distributors: [distributor]) }
+    let(:distributor_shipping_method) { shipping_method.distributor_shipping_methods.first }
     let(:variant) { create(:variant, product: create(:product, supplier: supplier)) }
     let(:params) { { name: 'Some new name' } }
     let(:form) { OrderCycleForm.new(order_cycle, params, user) }
@@ -159,7 +160,7 @@ describe OrderCycleForm do
             enterprise_fee_ids: []
           }],
           outgoing_exchanges: [outgoing_exchange_params],
-          selected_shipping_method_ids: [shipping_method.id]
+          selected_distributor_shipping_method_ids: [distributor_shipping_method.id]
         )
       end
 
@@ -168,27 +169,30 @@ describe OrderCycleForm do
         expect(order_cycle.name).to eq 'Some new name'
         expect(order_cycle.cached_incoming_exchanges.count).to eq 1
         expect(order_cycle.cached_outgoing_exchanges.count).to eq 1
-        expect(order_cycle.shipping_methods).to eq [shipping_method]
+        expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method]
       end
     end
 
     context "updating outgoing exchanges and shipping methods simultaneously but the shipping
              method doesn't belong to the new or any existing order cycle distributor" do
       let(:other_distributor_shipping_method) do
-        create(:shipping_method, distributors: [create(:distributor_enterprise)])
+        create(
+          :shipping_method,
+          distributors: [create(:distributor_enterprise)]
+        ).distributor_shipping_methods.first
       end
 
       before do
         params.merge!(
           outgoing_exchanges: [outgoing_exchange_params],
-          selected_shipping_method_ids: [other_distributor_shipping_method.id]
+          selected_distributor_shipping_method_ids: [other_distributor_shipping_method.id]
         )
       end
 
       it "saves the outgoing exchange but ignores the shipping method" do
         expect(form.save).to be true
         expect(order_cycle.distributors).to eq [distributor]
-        expect(order_cycle.shipping_methods).to be_empty
+        expect(order_cycle.distributor_shipping_methods).to be_empty
       end
     end
 
@@ -196,15 +200,20 @@ describe OrderCycleForm do
       context "and it's valid" do
         it "saves the changes" do
           distributor = create(:distributor_enterprise)
-          shipping_method = create(:shipping_method, distributors: [distributor])
+          distributor_shipping_method = create(
+            :shipping_method,
+            distributors: [distributor]
+          ).distributor_shipping_methods.first
           order_cycle = create(:distributor_order_cycle, distributors: [distributor])
 
-          form = OrderCycleForm.new(order_cycle,
-                                    { selected_shipping_method_ids: [shipping_method.id] },
-                                    order_cycle.coordinator)
+          form = OrderCycleForm.new(
+            order_cycle,
+            { selected_distributor_shipping_method_ids: [distributor_shipping_method.id] },
+            order_cycle.coordinator
+          )
 
           expect(form.save).to be true
-          expect(order_cycle.shipping_methods).to eq [shipping_method]
+          expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method]
         end
       end
 
@@ -212,17 +221,25 @@ describe OrderCycleForm do
         it "ignores it" do
           distributor_i = create(:distributor_enterprise)
           distributor_ii = create(:distributor_enterprise)
-          shipping_method_i = create(:shipping_method, distributors: [distributor_i])
-          shipping_method_ii = create(:shipping_method, distributors: [distributor_ii])
+          distributor_shipping_method_i = create(
+            :shipping_method,
+            distributors: [distributor_i]
+          ).distributor_shipping_methods.first
+          distributor_shipping_method_ii = create(
+            :shipping_method,
+            distributors: [distributor_ii]
+          ).distributor_shipping_methods.first
           order_cycle = create(:distributor_order_cycle,
                                distributors: [distributor_i])
 
-          form = OrderCycleForm.new(order_cycle,
-                                    { selected_shipping_method_ids: [shipping_method_ii.id] },
-                                    order_cycle.coordinator)
+          form = OrderCycleForm.new(
+            order_cycle,
+            { selected_distributor_shipping_method_ids: [distributor_shipping_method_ii.id] },
+            order_cycle.coordinator
+          )
 
           expect(form.save).to be true
-          expect(order_cycle.shipping_methods).to eq [shipping_method_i]
+          expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method_i]
         end
       end
     end

spec/system/admin/order_cycles/complex_creating_specific_time_spec.rb

@@ -66,7 +66,7 @@ describe '
     ## UPDATE
     add_supplier_with_fees
     add_distributor_with_fees
-    select_shipping_methods
+    select_distributor_shipping_methods
 
     expect_all_data_saved
   end
@@ -160,7 +160,7 @@ describe '
     click_button 'Save and Next'
   end
 
-  def select_shipping_methods
+  def select_distributor_shipping_methods
     expect(page).to have_checked_field "Select all"
 
     expect(page).to have_checked_field "Pickup - always available"