fix: add DB unique index to prevent duplicate ArticlesSales & show pr…

…etty error in form

app/models/articles_sale.rb

@@ -6,15 +6,8 @@ class ArticlesSale < ApplicationRecord
 
   validates :quantity, presence: true, numericality: { only_integer: true, greater_than: 0 }
 
-  before_create :consolidate_duplication
-
-  private
-
-  def consolidate_duplication
-    duplicate = ArticlesSale.where(article_id: article_id, sale_id: sale_id).where.not(id: id).first
-    return unless duplicate
-
-    errors.add(:base, 'Please merge the quantities of the same articles !')
-    throw :abort
-  end
+  validates :article_id, uniqueness: {
+    scope: :sale_id,
+    message: 'can only be added once to a sale, please merge the quantities of the same articles'
+  }
 end

app/models/sale.rb

@@ -22,6 +22,7 @@ class Sale < ApplicationRecord
   validates :duration, numericality: { greater_than_or_equal_to: 0 }
   validate :not_empty_sale, unless: ->(sale) { sale.errors.include?(:duration) }
   validate :exhaustive_subscription_offers, unless: ->(sale) { sale.errors.include?(:duration) }
+  validate :no_duplicate_article_sale
 
   def verify
     self.verified_at = Time.zone.now if verified_at.nil?
@@ -57,6 +58,21 @@ def exhaustive_subscription_offers
     errors.add(:base, 'Subscription offers are not exhaustive!')
   end
 
+  def no_duplicate_article_sale
+    # The unique constraint on ArticlesSales prevents us from duplicating articles in a single sale,
+    # but Rails doesn't rescue from hard DB integrity errors, so we need to validate the association
+    # in-memory before to show pretty errors.
+    # See https://github.com/rails/rails/issues/20676 that seems very related
+    duplicate_article_ids = articles_sales.map(&:article_id).tally.filter { |_, v| v > 1 }
+    return if duplicate_article_ids.empty?
+
+    articles_sales
+      .filter { |article_sale| article_sale.article_id.in? duplicate_article_ids }
+      .each { |article_sale| article_sale.errors.add(:article_id, :taken) }
+
+    errors.add(:base, 'Please merge the quantities of the same articles')
+  end
+
   class << self
     private
 

db/migrate/20241102103028_add_unique_index_to_articles_sales.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddUniqueIndexToArticlesSales < ActiveRecord::Migration[7.1]
+  def change
+    add_index :articles_sales, [:article_id, :sale_id], unique: true
+  end
+end