Merge pull request #3101 from consul/backport-fix_milestone_validation

Fix milestone validation

app/models/milestone.rb

@@ -13,7 +13,9 @@ class Milestone < ActiveRecord::Base
 
   validates :milestoneable, presence: true
   validates :publication_date, presence: true
-  validate :description_or_status_present?
+
+  before_validation :assign_milestone_to_translations
+  validates_translation :description, presence: true, unless: -> { status_id.present? }
 
   scope :order_by_publication_date, -> { order(publication_date: :asc, created_at: :asc) }
   scope :published,                 -> { where("publication_date <= ?", Date.current) }
@@ -23,9 +25,9 @@ class Milestone < ActiveRecord::Base
     80
   end
 
-  def description_or_status_present?
-    unless description.present? || status_id.present?
-      errors.add(:description)
+  private
+
+    def assign_milestone_to_translations
+      translations.each { |translation| translation.globalized_model = self }
     end
-  end
 end

app/models/milestone/translation.rb

@@ -0,0 +1,3 @@
+class Milestone::Translation < Globalize::ActiveRecord::Translation
+  delegate :status_id, to: :globalized_model
+end

spec/models/milestone_spec.rb

@@ -40,33 +40,6 @@ describe Milestone do
       milestone.status_id = nil
       expect(milestone).to be_valid
     end
-
-    it "is valid without description if status is present" do
-      milestone.description = nil
-      expect(milestone).to be_valid
-    end
-  end
-
-  describe "#description_or_status_present?" do
-    let(:milestone) { build(:milestone) }
-
-    it "is not valid when status is removed and there's no description" do
-      milestone.update(description: nil)
-      expect(milestone.update(status_id: nil)).to be false
-    end
-
-    it "is not valid when description is removed and there's no status" do
-      milestone.update(status_id: nil)
-      expect(milestone.update(description: nil)).to be false
-    end
-
-    it "is valid when description is removed and there is a status" do
-      expect(milestone.update(description: nil)).to be true
-    end
-
-    it "is valid when status is removed and there is a description" do
-      expect(milestone.update(status_id: nil)).to be true
-    end
   end
 
   describe ".published" do

spec/shared/features/admin_milestoneable.rb

@@ -68,6 +68,18 @@ shared_examples "admin_milestoneable" do |factory_name, path_name|
           expect(page).to have_content 'New description milestone'
         end
       end
+
+      scenario "Show validation errors with no description nor status" do
+        visit path
+        click_link "Create new milestone"
+
+        fill_in "Date", with: Date.current
+        click_button "Create milestone"
+
+        within "#new_milestone" do
+          expect(page).to have_content "can't be blank", count: 1
+        end
+      end
     end
 
     context "Edit" do