From 8a010bc0947fe80f6e5f3666e4bf14ca0c9f7e2f Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 13 Nov 2024 14:01:56 +1100
Subject: [PATCH] Fix unique validator for vouche code

Paranoia doesn't support unique validation including deleted records:
  https://github.com/rubysherpas/paranoia/pull/333
We use a custom validator, ScopedUniquenessValidator to avoid the issue
---
 app/models/vouchers/flat_rate.rb              |  2 +-
 app/models/vouchers/percentage_rate.rb        |  2 +-
 .../vouchers/scoped_uniqueness_validator.rb   | 25 ++++++++++++
 spec/models/vouchers/flat_rate_spec.rb        |  2 +-
 spec/models/vouchers/percentage_rate_spec.rb  |  2 +-
 spec/support/voucher_uniqueness_helper.rb     | 39 +++++++++++++++++++
 6 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100644 app/validators/vouchers/scoped_uniqueness_validator.rb
 create mode 100644 spec/support/voucher_uniqueness_helper.rb

diff --git a/app/models/vouchers/flat_rate.rb b/app/models/vouchers/flat_rate.rb
index 45b85e26abf..b09013d11e8 100644
--- a/app/models/vouchers/flat_rate.rb
+++ b/app/models/vouchers/flat_rate.rb
@@ -4,6 +4,6 @@ module Vouchers
   class FlatRate < Voucher
     include FlatRatable
 
-    validates :code, uniqueness: { scope: :enterprise_id }
+    validates_with ScopedUniquenessValidator
   end
 end
diff --git a/app/models/vouchers/percentage_rate.rb b/app/models/vouchers/percentage_rate.rb
index 06d0263280d..833aeaaa353 100644
--- a/app/models/vouchers/percentage_rate.rb
+++ b/app/models/vouchers/percentage_rate.rb
@@ -5,7 +5,7 @@ class PercentageRate < Voucher
     validates :amount,
               presence: true,
               numericality: { greater_than: 0, less_than_or_equal_to: 100 }
-    validates :code, uniqueness: { scope: :enterprise_id }
+    validates_with ScopedUniquenessValidator
 
     def display_value
       ActionController::Base.helpers.number_to_percentage(amount, precision: 2)
diff --git a/app/validators/vouchers/scoped_uniqueness_validator.rb b/app/validators/vouchers/scoped_uniqueness_validator.rb
new file mode 100644
index 00000000000..c537f0a1942
--- /dev/null
+++ b/app/validators/vouchers/scoped_uniqueness_validator.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: false
+
+# paranoia doesn't support unique validation including deleted records:
+#  https://github.com/rubysherpas/paranoia/pull/333
+# We use a custom validator to fix the issue, so we don't need to fork/patch the gem
+module Vouchers
+  class ScopedUniquenessValidator < ActiveModel::Validator
+    def validate(record)
+      @record = record
+
+      return unless unique_voucher_code_per_enterprise?
+
+      record.errors.add :code, :taken, value: @record.code
+    end
+
+    private
+
+    def unique_voucher_code_per_enterprise?
+      query = Voucher.with_deleted.where(code: @record.code, enterprise_id: @record.enterprise_id)
+      query = query.where.not(id: @record.id) unless @record.id.nil?
+
+      query.present?
+    end
+  end
+end
diff --git a/spec/models/vouchers/flat_rate_spec.rb b/spec/models/vouchers/flat_rate_spec.rb
index 10c0f56a467..b9e85d9fbb0 100644
--- a/spec/models/vouchers/flat_rate_spec.rb
+++ b/spec/models/vouchers/flat_rate_spec.rb
@@ -8,7 +8,7 @@
 
     it { is_expected.to validate_presence_of(:amount) }
     it { is_expected.to validate_numericality_of(:amount).is_greater_than(0) }
-    it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) }
+    it_behaves_like 'has a unique code per enterprise', "voucher_flat_rate"
   end
 
   describe '#compute_amount' do
diff --git a/spec/models/vouchers/percentage_rate_spec.rb b/spec/models/vouchers/percentage_rate_spec.rb
index 116751d1267..89c4834da3f 100644
--- a/spec/models/vouchers/percentage_rate_spec.rb
+++ b/spec/models/vouchers/percentage_rate_spec.rb
@@ -12,7 +12,7 @@
         .is_greater_than(0)
         .is_less_than_or_equal_to(100)
     end
-    it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) }
+    it_behaves_like 'has a unique code per enterprise', "voucher_percentage_rate"
   end
 
   describe '#compute_amount' do
diff --git a/spec/support/voucher_uniqueness_helper.rb b/spec/support/voucher_uniqueness_helper.rb
new file mode 100644
index 00000000000..7e8ddd1cf3f
--- /dev/null
+++ b/spec/support/voucher_uniqueness_helper.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+shared_examples_for 'has a unique code per enterprise' do |voucher_type|
+  describe "code" do
+    let(:code) { "super_code" }
+    let(:enterprise) { create(:enterprise) }
+
+    it "is unique per enterprise" do
+      voucher = create(voucher_type, code:, enterprise:)
+      expect(voucher).to be_valid
+
+      expect_voucher_with_same_enterprise_to_be_invalid(voucher_type)
+
+      expect_voucher_with_other_enterprise_to_be_valid(voucher_type)
+    end
+
+    context "with deleted voucher" do
+      it "is unique per enterprise" do
+        create(voucher_type, code:, enterprise:).destroy!
+
+        expect_voucher_with_same_enterprise_to_be_invalid(voucher_type)
+
+        expect_voucher_with_other_enterprise_to_be_valid(voucher_type)
+      end
+    end
+  end
+
+  def expect_voucher_with_same_enterprise_to_be_invalid(voucher_type)
+    new_voucher = build(voucher_type, code:, enterprise: )
+
+    expect(new_voucher).not_to be_valid
+    expect(new_voucher.errors.full_messages).to include("Code has already been taken")
+  end
+
+  def expect_voucher_with_other_enterprise_to_be_valid(voucher_type)
+    other_voucher = build(voucher_type, code:, enterprise: create(:enterprise) )
+    expect(other_voucher).to be_valid
+  end
+end