Merge pull request #10467 from abdellani/fix_hub_supplier_can_control_shipping_payment_method

limit users who can update shipping/payment method of an order cycle
2026-01-24 20:36:49 +00:00 · 2023-03-03 09:27:37 +01:00
parent 6371f7eff6 25d8ce1737
commit 0e13e5408f
2 changed files with 249 additions and 36 deletions
--- a/app/services/order_cycle_form.rb
+++ b/app/services/order_cycle_form.rb
@@ -30,8 +30,10 @@ class OrderCycleForm
      order_cycle.schedule_ids = schedule_ids if parameter_specified?(:schedule_ids)
      order_cycle.save!
      apply_exchange_changes
-      attach_selected_distributor_payment_methods
-      attach_selected_distributor_shipping_methods
+      if can_update_selected_payment_or_shipping_methods?
+        attach_selected_distributor_payment_methods
+        attach_selected_distributor_shipping_methods
+      end
      sync_subscriptions
      true
    end
@@ -61,14 +63,33 @@ class OrderCycleForm
  def attach_selected_distributor_payment_methods
    return if @selected_distributor_payment_method_ids.nil?

-    order_cycle.selected_distributor_payment_method_ids = selected_distributor_payment_method_ids
+    if distributor_only?
+      payment_method_ids = order_cycle.selected_distributor_payment_method_ids
+      payment_method_ids -= user_distributor_payment_method_ids
+      payment_method_ids += user_only_selected_distributor_payment_method_ids
+      order_cycle.selected_distributor_payment_method_ids = payment_method_ids
+    else
+      order_cycle.selected_distributor_payment_method_ids = selected_distributor_payment_method_ids
+    end
    order_cycle.save!
  end

  def attach_selected_distributor_shipping_methods
    return if @selected_distributor_shipping_method_ids.nil?

-    order_cycle.selected_distributor_shipping_method_ids = selected_distributor_shipping_method_ids
+    if distributor_only?
+      # A distributor can only update methods associated with their own
+      # enterprise, so we load all previously selected methods, and replace
+      # only the distributor's methods with their selection (not touching other
+      # distributor's methods).
+      shipping_method_ids = order_cycle.selected_distributor_shipping_method_ids
+      shipping_method_ids -= user_distributor_shipping_method_ids
+      shipping_method_ids += user_only_selected_distributor_shipping_method_ids
+      order_cycle.selected_distributor_shipping_method_ids = shipping_method_ids
+    else
+      order_cycle.selected_distributor_shipping_method_ids = selected_distributor_shipping_method_ids
+    end
+
    order_cycle.save!
  end

@@ -99,6 +120,10 @@ class OrderCycleForm
    @selected_distributor_payment_method_ids
  end

+  def user_only_selected_distributor_payment_method_ids
+    user_distributor_payment_method_ids.intersection(selected_distributor_payment_method_ids)
+  end
+
  def selected_distributor_shipping_method_ids
    @selected_distributor_shipping_method_ids = (
      attachable_distributor_shipping_method_ids &
@@ -112,6 +137,10 @@ class OrderCycleForm
    @selected_distributor_shipping_method_ids
  end

+  def user_only_selected_distributor_shipping_method_ids
+    user_distributor_shipping_method_ids.intersection(selected_distributor_shipping_method_ids)
+  end
+
  def build_schedule_ids
    return unless parameter_specified?(:schedule_ids)

@@ -160,4 +189,37 @@ class OrderCycleForm
  def new_schedule_ids
    @order_cycle.schedule_ids - existing_schedule_ids
  end
+
+  def can_update_selected_payment_or_shipping_methods?
+    @user.admin? || coordinator? || distributor?
+  end
+
+  def coordinator?
+    @user.enterprises.include?(@order_cycle.coordinator)
+  end
+
+  def distributor?
+    !user_distributors_ids.empty?
+  end
+
+  def distributor_only?
+    distributor? && !@user.admin? && !coordinator?
+  end
+
+  def user_distributors_ids
+    @user_distributors_ids ||= @user.enterprises.pluck(:id)
+      .intersection(@order_cycle.distributors.pluck(:id))
+  end
+
+  def user_distributor_payment_method_ids
+    @user_distributor_payment_method_ids ||=
+      DistributorPaymentMethod.where(distributor_id: user_distributors_ids)
+        .pluck(:id)
+  end
+
+  def user_distributor_shipping_method_ids
+    @user_distributor_shipping_method_ids ||=
+      DistributorShippingMethod.where(distributor_id: user_distributors_ids)
+        .pluck(:id)
+  end
 end
--- a/spec/services/order_cycle_form_spec.rb
+++ b/spec/services/order_cycle_form_spec.rb
@@ -223,22 +223,93 @@ describe OrderCycleForm do

    context "updating payment methods" do
      context "and it's valid" do
-        it "saves the changes" do
-          distributor = create(:distributor_enterprise)
-          distributor_payment_method = create(
-            :payment_method,
-            distributors: [distributor]
-          ).distributor_payment_methods.first
-          order_cycle = create(:distributor_order_cycle, distributors: [distributor])
+        let!(:distributor){ create(:distributor_enterprise) }
+        let!(:payment_method){ create(:payment_method, distributors: [distributor]) }
+        let!(:payment_method2){ create(:payment_method, distributors: [distributor]) }
+        let!(:distributor_payment_method){ distributor.distributor_payment_methods.first.id }
+        let!(:distributor_payment_method2){ distributor.distributor_payment_methods.second.id }
+        let!(:supplier){ create(:supplier_enterprise) }

-          form = OrderCycleForm.new(
-            order_cycle,
-            { selected_distributor_payment_method_ids: [distributor_payment_method.id] },
-            order_cycle.coordinator
-          )
+        context "the submitter is a coordinator" do
+          it "saves the changes" do
+            order_cycle = create(:distributor_order_cycle, distributors: [distributor])

-          expect(form.save).to be true
-          expect(order_cycle.distributor_payment_methods).to eq [distributor_payment_method]
+            form = OrderCycleForm.new(
+              order_cycle,
+              { selected_distributor_payment_method_ids: [distributor_payment_method] },
+              order_cycle.coordinator.users.first
+            )
+
+            expect{ form.save }.to change{ order_cycle.distributor_payment_methods.pluck(:id) }
+              .from([distributor_payment_method, distributor_payment_method2])
+              .to([distributor_payment_method])
+          end
+        end
+
+        context "submitter is a supplier" do
+          it "doesn't save the changes" do
+            order_cycle = create(:distributor_order_cycle, distributors: [distributor],
+                                                           suppliers: [supplier])
+
+            form = OrderCycleForm.new(
+              order_cycle,
+              { selected_distributor_payment_method_ids: [distributor_payment_method] },
+              supplier.users.first
+            )
+
+            expect{ form.save }.to_not change{ order_cycle.distributor_payment_methods.pluck(:id) }
+          end
+        end
+
+        context "submitter is an admin" do
+          it "saves the changes" do
+            order_cycle = create(:distributor_order_cycle, distributors: [distributor])
+
+            form = OrderCycleForm.new(
+              order_cycle,
+              { selected_distributor_payment_method_ids: [distributor_payment_method2] },
+              create(:admin_user)
+            )
+
+            expect{ form.save }.to change{ order_cycle.distributor_payment_methods.pluck(:id) }
+              .from([distributor_payment_method, distributor_payment_method2])
+              .to([distributor_payment_method2])
+          end
+        end
+
+        context "submitter is a distributor" do
+          context "can update his own payment methods" do
+            it "saves the changes" do
+              order_cycle = create(:distributor_order_cycle, distributors: [distributor])
+
+              form = OrderCycleForm.new(
+                order_cycle,
+                { selected_distributor_payment_method_ids: [distributor_payment_method] },
+                distributor.users.first
+              )
+
+              expect{ form.save }.to change{ order_cycle.distributor_payment_methods.pluck(:id) }
+                .from([distributor_payment_method, distributor_payment_method2])
+                .to([distributor_payment_method])
+            end
+          end
+          context "can't update other distributors' payment methods" do
+            let(:distributor2){ create(:distributor_enterprise) }
+            it "doesn't save the changes" do
+              order_cycle = create(:distributor_order_cycle,
+                                   distributors: [distributor, distributor2])
+
+              form = OrderCycleForm.new(
+                order_cycle,
+                { selected_distributor_payment_method_ids: [distributor_payment_method] },
+                distributor2.users.first
+              )
+
+              expect{ form.save }.to_not change{
+                                           order_cycle.distributor_payment_methods.pluck(:id)
+                                         }
+            end
+          end
        end
      end

@@ -260,33 +331,113 @@ describe OrderCycleForm do
          form = OrderCycleForm.new(
            order_cycle,
            { selected_distributor_payment_method_ids: [distributor_payment_method_ii.id] },
-            order_cycle.coordinator
+            order_cycle.coordinator.users.first
          )

-          expect(form.save).to be true
-          expect(order_cycle.distributor_payment_methods).to eq [distributor_payment_method_i]
+          expect{ form.save }.not_to change{
+            order_cycle.distributor_payment_methods.pluck(:id)
+          }.from([distributor_payment_method_i.id])
        end
      end
    end

    context "updating shipping methods" do
      context "and it's valid" do
-        it "saves the changes" do
-          distributor = create(:distributor_enterprise)
-          distributor_shipping_method = create(
-            :shipping_method,
-            distributors: [distributor]
-          ).distributor_shipping_methods.first
-          order_cycle = create(:distributor_order_cycle, distributors: [distributor])
+        let!(:distributor){ create(:distributor_enterprise) }
+        let!(:shipping_method){ create(:shipping_method, distributors: [distributor]) }
+        let!(:shipping_method2){ create(:shipping_method, distributors: [distributor]) }
+        let!(:distributor_shipping_method){ distributor.distributor_shipping_methods.first.id }
+        let!(:distributor_shipping_method2){ distributor.distributor_shipping_methods.second.id }

-          form = OrderCycleForm.new(
-            order_cycle,
-            { selected_distributor_shipping_method_ids: [distributor_shipping_method.id] },
-            order_cycle.coordinator
-          )
+        let(:supplier){ create(:supplier_enterprise) }
+        context "the submitter is a coordinator" do
+          it "saves the changes" do
+            order_cycle = create(:distributor_order_cycle, distributors: [distributor])

-          expect(form.save).to be true
-          expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method]
+            form = OrderCycleForm.new(
+              order_cycle,
+              { selected_distributor_shipping_method_ids: [distributor_shipping_method] },
+              order_cycle.coordinator.users.first
+            )
+
+            expect{ form.save }.to change{ order_cycle.distributor_shipping_methods.pluck(:id) }
+              .from([distributor_shipping_method, distributor_shipping_method2])
+              .to([distributor_shipping_method])
+          end
+        end
+        context "submitter is a supplier" do
+          it "doesn't save the changes" do
+            order_cycle = create(:distributor_order_cycle, distributors: [distributor],
+                                                           suppliers: [supplier])
+
+            form = OrderCycleForm.new(
+              order_cycle,
+              { selected_distributor_shipping_method_ids: [distributor_shipping_method] },
+              supplier.users.first
+            )
+
+            expect{ form.save }.not_to change{
+              order_cycle.distributor_shipping_methods.pluck(:id)
+            }.from([distributor_shipping_method, distributor_shipping_method2])
+          end
+        end
+        context "submitter is an admin" do
+          it "saves the changes" do
+            order_cycle = create(:distributor_order_cycle, distributors: [distributor])
+
+            form = OrderCycleForm.new(
+              order_cycle,
+              { selected_distributor_shipping_method_ids: [distributor_shipping_method] },
+              create(:admin_user)
+            )
+
+            expect{ form.save }.to change{ order_cycle.distributor_shipping_methods.pluck(:id) }
+              .from([distributor_shipping_method, distributor_shipping_method2])
+              .to([distributor_shipping_method])
+          end
+        end
+        context "submitter is a distributor" do
+          context "can update his own shipping methods" do
+            it "saves the changes" do
+              order_cycle = create(:distributor_order_cycle, distributors: [distributor])
+
+              form = OrderCycleForm.new(
+                order_cycle,
+                { selected_distributor_shipping_method_ids: [distributor_shipping_method] },
+                distributor.users.first
+              )
+
+              expect{ form.save }.to change{
+                order_cycle.distributor_shipping_methods.pluck(:id)
+              }.from([
+                       distributor_shipping_method, distributor_shipping_method2
+                     ]).to([distributor_shipping_method])
+            end
+          end
+          context "can't update other distributors' shipping methods" do
+            let!(:distributor2){ create(:distributor_enterprise) }
+            let!(:shipping_method3){ create(:shipping_method, distributors: [distributor2]) }
+            let!(:distributor_shipping_method3){
+              distributor2.distributor_shipping_methods.first.id
+            }
+            it "doesn't save the changes" do
+              order_cycle = create(:distributor_order_cycle,
+                                   distributors: [distributor, distributor2])
+
+              form = OrderCycleForm.new(
+                order_cycle,
+                { selected_distributor_shipping_method_ids: [distributor_shipping_method] },
+                distributor2.users.first
+              )
+
+              expect{ form.save }.not_to change{
+                order_cycle.distributor_shipping_methods.pluck(:id)
+              }.from [
+                distributor_shipping_method, distributor_shipping_method2,
+                distributor_shipping_method3
+              ]
+            end
+          end
        end
      end

@@ -308,7 +459,7 @@ describe OrderCycleForm do
          form = OrderCycleForm.new(
            order_cycle,
            { selected_distributor_shipping_method_ids: [distributor_shipping_method_ii.id] },
-            order_cycle.coordinator
+            order_cycle.coordinator.users.first
          )

          expect(form.save).to be true