[NUOPEN-435] Fix estimate detail price policy assign/validation

app/models/estimate.rb

@@ -33,6 +33,7 @@ def duplicate(created_by_user)
     details_to_copy = estimate_details.map(&:dup)
 
     details_to_copy.each do |detail|
+      detail.recalculate = true
       duplicated_estimate.estimate_details << detail
     end
 
@@ -45,7 +46,7 @@ def recalculate
     success = true
 
     transaction do
-      if estimate_details.all?(&:assign_price_policy_and_cost)
+      if estimate_details.all?(&:set_price_policy)
         save!
       else
         success = false

app/models/estimate_detail.rb

@@ -7,14 +7,13 @@ class EstimateDetail < ApplicationRecord
   belongs_to :product
   belongs_to :price_policy
 
+  after_validation :set_price_policy, if: -> { recalculate || price_policy_id.nil? }
+
   before_save :clear_duration_fields
-  before_create :assign_price_policy_and_cost
-  before_update :assign_price_policy_and_cost, if: :recalculate
 
   validates :quantity, presence: true, numericality: { greater_than: 0 }
   validates :duration, numericality: { greater_than: 0 }, allow_nil: true
   validates :duration_unit, inclusion: { in: TIME_UNITS }, allow_nil: true
-  validate :price_policy_exists
 
   delegate :user, to: :estimate
 
@@ -29,20 +28,21 @@ def price_groups
     end
   end
 
-  def assign_price_policy_and_cost
-    pp = product.cheapest_price_policy(self, Time.current)
+  def set_price_policy
+    return if errors.present?
 
-    if pp.blank?
-      errors.add(:base, I18n.t("activerecord.errors.models.estimate_detail.no_price_policy"))
-      return false
-    end
+    price_policy = product.cheapest_price_policy(self, Time.current)
 
-    cost = pp.estimate_cost_from_estimate_detail(self)
+    if price_policy.blank?
+      errors.add(:base, :no_price_policy)
 
-    self.price_policy = pp
-    self.cost = cost
+      false
+    else
+      self.price_policy = price_policy
+      self.cost = price_policy.estimate_cost_from_estimate_detail(self)
 
-    true
+      true
+    end
   end
 
   private
@@ -53,14 +53,4 @@ def clear_duration_fields
       self.duration_unit = nil
     end
   end
-
-  def price_policy_exists
-    return if product.blank? || user.blank?
-    return if marked_for_destruction?
-
-    pp = product.cheapest_price_policy(self, Time.current)
-    if pp.blank?
-      errors.add(:base, I18n.t("activerecord.errors.models.estimate_detail.no_price_policy"))
-    end
-  end
 end

spec/models/estimates_spec.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Estimate do
+  describe "price policy validation" do
+    let(:facility) { create(:setup_facility) }
+    let(:product) { create(:item, facility:) }
+    let(:price_group) { create(:price_group) }
+    let(:estimate_details_attributes) do
+      [{ product_id: product.id, quantity: 1 }]
+    end
+    let(:subject) do
+      build(
+        :estimate,
+        facility:,
+        price_group:,
+        user: nil,
+        custom_name: "Some name",
+        estimate_details_attributes:,
+      )
+    end
+
+    context "when product has a price for the price group" do
+      before do
+        create(:item_price_policy, product:, price_group:)
+      end
+
+      it { is_expected.to be_valid }
+    end
+
+    context "when product does not have a price" do
+      it { is_expected.not_to be_valid }
+
+      it "adds correct error" do
+        subject.valid?
+
+        expect(subject.errors).to(
+          be_added("estimate_details.base", :no_price_policy)
+        )
+      end
+    end
+  end
+end