Move Vine voucher to Vouchers::Vine

A Vine voucher is really a specific type of FlatRate voucher but because
a Vine voucher can be used by mutiple enterprise, it can be considered
different enough to warrant it's own class.
It still share a lot of the behaviour of a FlatRate voucher, so to avoid
duplication, all the shared functionality have been moved to a
Vouchers::FlatRatable concern.

app/models/concerns/vouchers/flat_ratable.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require "active_support/concern"
+
+module Vouchers
+  module FlatRatable
+    extend ActiveSupport::Concern
+
+    included do
+      validates :amount,
+                presence: true,
+                numericality: { greater_than: 0 }
+    end
+
+    def display_value
+      Spree::Money.new(amount)
+    end
+
+    # We limit adjustment to the maximum amount needed to cover the order, ie if the voucher
+    # covers more than the order.total we only need to create an adjustment covering the order.total
+    def compute_amount(order)
+      -amount.clamp(0, order.pre_discount_total)
+    end
+
+    def rate(order)
+      amount = compute_amount(order)
+
+      amount / order.pre_discount_total
+    end
+  end
+end

app/models/voucher.rb

@@ -1,8 +1,6 @@
 # frozen_string_literal: false
 
 class Voucher < ApplicationRecord
-  VINE_TYPE = "VINE".freeze
-
   self.belongs_to_required_by_default = false
 
   acts_as_paranoid
@@ -16,13 +14,10 @@ class Voucher < ApplicationRecord
            class_name: 'Spree::Adjustment',
            dependent: nil
 
-  validates :code, presence: true, uniqueness: { scope: :enterprise_id }
+  validates :code, presence: true
 
   TYPES = ["Vouchers::FlatRate", "Vouchers::PercentageRate"].freeze
 
-  scope :vine, -> { where(voucher_type: VINE_TYPE) }
-  scope :local, -> { where("voucher_type IS DISTINCT FROM ?", VINE_TYPE) }
-
   def code=(value)
     super(value.to_s.strip)
   end
@@ -47,10 +42,6 @@ class Voucher < ApplicationRecord
     order.adjustments.create(adjustment_attributes)
   end
 
-  def vine?
-    voucher_type == VINE_TYPE
-  end
-
   # The following method must be overriden in a concrete voucher.
   def display_value
     raise NotImplementedError, 'please use concrete voucher'

app/models/vouchers/flat_rate.rb

@@ -2,24 +2,8 @@
 
 module Vouchers
   class FlatRate < Voucher
-    validates :amount,
-              presence: true,
-              numericality: { greater_than: 0 }
+    include FlatRatable
 
-    def display_value
-      Spree::Money.new(amount)
-    end
-
-    # We limit adjustment to the maximum amount needed to cover the order, ie if the voucher
-    # covers more than the order.total we only need to create an adjustment covering the order.total
-    def compute_amount(order)
-      -amount.clamp(0, order.pre_discount_total)
-    end
-
-    def rate(order)
-      amount = compute_amount(order)
-
-      amount / order.pre_discount_total
-    end
+    validates :code, uniqueness: { scope: :enterprise_id }
   end
 end

app/models/vouchers/percentage_rate.rb

@@ -5,6 +5,7 @@ module Vouchers
     validates :amount,
               presence: true,
               numericality: { greater_than: 0, less_than_or_equal_to: 100 }
+    validates :code, uniqueness: { scope: :enterprise_id }
 
     def display_value
       ActionController::Base.helpers.number_to_percentage(amount, precision: 2)

app/models/vouchers/vine.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: false
+
+module Vouchers
+  class Vine < Voucher
+    include FlatRatable
+
+    # a VINE voucher :
+    #  - can potentially be associated with mutiple enterprise
+    #  - code ( "short code" in VINE ) can be recycled, but they shouldn't be linked to the same
+    #    voucher_id
+    validates :code, uniqueness: { scope: [:enterprise_id, :external_voucher_id] }
+  end
+end

db/migrate/20241112230401_update_index_and_remove_voucher_type_from_voucher.rb

@@ -0,0 +1,9 @@
+class UpdateIndexAndRemoveVoucherTypeFromVoucher < ActiveRecord::Migration[7.0]
+  def change
+    remove_column :vouchers, :voucher_type
+    remove_index :vouchers, [:code, :enterprise_id], unique: true
+
+    add_index :vouchers, [:code, :enterprise_id]
+    add_index :vouchers, [:code, :enterprise_id, :external_voucher_id], name: "index_vouchers_on_code_and_enterprise_id_and_ext_voucher_id"
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_10_30_033956) do
+ActiveRecord::Schema[7.0].define(version: 2024_11_12_230401) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
   enable_extension "plpgsql"
@@ -1112,8 +1112,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_10_30_033956) do
     t.string "type", limit: 255, default: "Vouchers::FlatRate", null: false
     t.uuid "external_voucher_id"
     t.uuid "external_voucher_set_id"
-    t.string "voucher_type"
-    t.index ["code", "enterprise_id"], name: "index_vouchers_on_code_and_enterprise_id", unique: true
+    t.index ["code", "enterprise_id", "external_voucher_id"], name: "index_vouchers_on_code_and_enterprise_id_and_ext_voucher_id"
+    t.index ["code", "enterprise_id"], name: "index_vouchers_on_code_and_enterprise_id"
     t.index ["deleted_at"], name: "index_vouchers_on_deleted_at"
     t.index ["enterprise_id"], name: "index_vouchers_on_enterprise_id"
   end

spec/factories/voucher_factory.rb

@@ -15,8 +15,8 @@ FactoryBot.define do
     amount { rand(1..100) }
   end
 
-  factory :vine_voucher, parent: :voucher_flat_rate do
-    voucher_type { Voucher::VINE_TYPE }
+  factory :vine_voucher, parent: :voucher, class: Vouchers::Vine do
+    amount { 20 }
     external_voucher_id { SecureRandom.uuid }
     external_voucher_set_id { SecureRandom.uuid }
   end

spec/models/voucher_spec.rb

@@ -15,31 +15,6 @@ RSpec.describe Voucher do
     it { is_expected.to have_many(:adjustments).dependent(nil) }
   end
 
-  describe "scope" do
-    let!(:vine_voucher) { create(:vine_voucher) }
-
-    describe ".vine" do
-      it "returns only vine vouchers" do
-        create(:voucher)
-        expect(Voucher.vine.count).to eq(1)
-      end
-    end
-
-    describe ".local" do
-      it "returns only local vouchers" do
-        create(:voucher)
-        expect(Voucher.local.count).to eq(1)
-      end
-
-      context "with voucher type other than VINE" do
-        it "returns only local vouchers" do
-          create(:voucher, voucher_type: "other")
-          expect(Voucher.local.count).to eq(1)
-        end
-      end
-    end
-  end
-
   describe '#code=' do
     it "removes leading and trailing whitespace" do
       voucher = build(:voucher, code: "\r\n\t new_code \r\n\t")
@@ -52,7 +27,6 @@ RSpec.describe Voucher do
     subject { build(:voucher_flat_rate, code: 'new_code', enterprise:) }
 
     it { is_expected.to validate_presence_of(:code) }
-    it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) }
   end
 
   describe '#display_value' do

spec/models/vouchers/flat_rate_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Vouchers::FlatRate do
 
     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) }
   end
 
   describe '#compute_amount' do

spec/models/vouchers/percentage_rate_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe Vouchers::PercentageRate do
         .is_greater_than(0)
         .is_less_than_or_equal_to(100)
     end
+    it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) }
   end
 
   describe '#compute_amount' do

spec/models/vouchers/vine_spec.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vouchers::Vine do
+  describe 'validations' do
+    subject { build(:vine_voucher) }
+
+    it { is_expected.to validate_presence_of(:amount) }
+    it { is_expected.to validate_numericality_of(:amount).is_greater_than(0) }
+
+    describe "#code" do
+      subject { build(:vine_voucher, code: 'vine_code', enterprise:, external_voucher_id: ) }
+
+      let(:external_voucher_id) { SecureRandom.uuid }
+      let(:enterprise) { create(:enterprise) }
+
+      it {
+        is_expected.to validate_uniqueness_of(:code).scoped_to(
+          [:enterprise_id, :external_voucher_id]
+        )
+      }
+
+      it "can be reused within the same enterprise" do
+        subject.save!
+        # Voucher with the same code but different external_voucher_id, it is mapped to a
+        # different voucher in VINE
+        voucher = build(:vine_voucher, code: 'vine_code', enterprise: )
+        expect(voucher.valid?).to be(true)
+      end
+
+      it "can be used by mutiple enterprises" do
+        subject.save!
+        # Voucher with the same code and external_voucher_id, ie exiting VINE voucher used by
+        # another enterprise
+        voucher = build(:vine_voucher, code: 'vine_code', enterprise: build(:enterprise),
+                                       external_voucher_id: )
+        expect(voucher.valid?).to be(true)
+      end
+    end
+  end
+
+  describe '#compute_amount' do
+    let(:order) { create(:order_with_totals) }
+
+    before do
+      order.update_columns(item_total: 15)
+    end
+
+    context 'when order total is more than the voucher' do
+      subject { create(:vine_voucher, amount: 5) }
+
+      it 'uses the voucher total' do
+        expect(subject.compute_amount(order).to_f).to eq(-5)
+      end
+    end
+
+    context 'when order total is less than the voucher' do
+      subject { create(:vine_voucher, amount: 20) }
+
+      it 'matches the order total' do
+        expect(subject.compute_amount(order).to_f).to eq(-15)
+      end
+    end
+  end
+
+  describe "#rate" do
+    subject do
+      create(:vine_voucher, code: 'new_code', amount: 5)
+    end
+    let(:order) { create(:order_with_totals) }
+
+    before do
+      order.update_columns(item_total: 10)
+    end
+
+    it "returns the voucher rate" do
+      # rate = -voucher_amount / order.pre_discount_total
+      # -5 / 10 = -0.5
+      expect(subject.rate(order).to_f).to eq(-0.5)
+    end
+  end
+end