Merge branch 'add-constraint-for-milestone-dates' into 'master'

Limit milestones to before year 9999

Closes #58576

See merge request gitlab-org/gitlab-ce!28742

app/models/milestone.rb

@@ -58,6 +58,7 @@ class Milestone < ApplicationRecord
   validate :uniqueness_of_title, if: :title_changed?
   validate :milestone_type_check
   validate :start_date_should_be_less_than_due_date, if: proc { |m| m.start_date.present? && m.due_date.present? }
+  validate :dates_within_4_digits
 
   strip_attributes :title
 
@@ -326,6 +327,16 @@ class Milestone < ApplicationRecord
     end
   end
 
+  def dates_within_4_digits
+    if start_date && start_date > Date.new(9999, 12, 31)
+      errors.add(:start_date, _("date must not be after 9999-12-31"))
+    end
+
+    if due_date && due_date > Date.new(9999, 12, 31)
+      errors.add(:due_date, _("date must not be after 9999-12-31"))
+    end
+  end
+
   def issues_finder_params
     { project_id: project_id }
   end

changelogs/unreleased/add-constraint-for-milestone-dates.yml

+---
+title: Limit milestone dates to before year 9999
+merge_request: 28742
+author: Luke Picciau
+type: fixed

db/migrate/20190523112344_limit_milestone_date_years_to_4_digits.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class LimitMilestoneDateYearsTo4Digits < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    Milestone.where("start_date > '9999-12-31'").update_all(
+      "start_date = '9999-12-31'"
+    )
+    Milestone.where("due_date > '9999-12-31'").update_all(
+      "due_date = '9999-12-31'"
+    )
+  end
+end

locale/gitlab.pot

@@ -11991,6 +11991,9 @@ msgstr ""
 msgid "customize"
 msgstr ""
 
+msgid "date must not be after 9999-12-31"
+msgstr ""
+
 msgid "day"
 msgid_plural "days"
 msgstr[0] ""

spec/models/milestone_spec.rb

@@ -31,12 +31,28 @@ describe Milestone do
     end
 
     describe 'start_date' do
-      it 'adds an error when start_date is greated then due_date' do
+      it 'adds an error when start_date is greater then due_date' do
         milestone = build(:milestone, start_date: Date.tomorrow, due_date: Date.yesterday)
 
         expect(milestone).not_to be_valid
         expect(milestone.errors[:due_date]).to include("must be greater than start date")
       end
+
+      it 'adds an error when start_date is greater than 9999-12-31' do
+        milestone = build(:milestone, start_date: Date.new(10000, 1, 1))
+
+        expect(milestone).not_to be_valid
+        expect(milestone.errors[:start_date]).to include("date must not be after 9999-12-31")
+      end
+    end
+
+    describe 'due_date' do
+      it 'adds an error when due_date is greater than 9999-12-31' do
+        milestone = build(:milestone, due_date: Date.new(10000, 1, 1))
+
+        expect(milestone).not_to be_valid
+        expect(milestone.errors[:due_date]).to include("date must not be after 9999-12-31")
+      end
     end
   end
 
@@ -381,21 +397,6 @@ describe Milestone do
         expect(milestone_ids).to be_empty
       end
     end
-
-    context 'when there is a milestone with a date after 294276 AD', :postgresql do
-      before do
-        past_milestone_project_1.update!(due_date: Date.new(294277, 1, 1))
-      end
-
-      it 'returns the next upcoming open milestone ID for each project and group' do
-        expect(milestone_ids).to contain_exactly(
-          current_milestone_project_1.id,
-          current_milestone_project_2.id,
-          current_milestone_group_1.id,
-          current_milestone_group_2.id
-        )
-      end
-    end
   end
 
   describe '#to_reference' do