From 5e8184cc6601bfc690f533b48993904dfe3833eb Mon Sep 17 00:00:00 2001 From: Luke Picciau Date: Fri, 31 May 2019 10:31:47 +0000 Subject: Change query to work on mysql as well. Also set entire date because setting only the year can trip 'start_date_should_be_less_than_due_date' --- app/models/milestone.rb | 11 +++++++ .../add-constraint-for-milestone-dates.yml | 5 +++ ...12344_limit_milestone_date_years_to_4_digits.rb | 38 ++++++++++++++++++++++ locale/gitlab.pot | 3 ++ spec/models/milestone_spec.rb | 33 ++++++++++--------- 5 files changed, 74 insertions(+), 16 deletions(-) create mode 100644 changelogs/unreleased/add-constraint-for-milestone-dates.yml create mode 100644 db/migrate/20190523112344_limit_milestone_date_years_to_4_digits.rb diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 787600569fa..37c129e843a 100644 --- a/app/models/milestone.rb +++ b/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 diff --git a/changelogs/unreleased/add-constraint-for-milestone-dates.yml b/changelogs/unreleased/add-constraint-for-milestone-dates.yml new file mode 100644 index 00000000000..485149cf62e --- /dev/null +++ b/changelogs/unreleased/add-constraint-for-milestone-dates.yml @@ -0,0 +1,5 @@ +--- +title: Limit milestone dates to before year 9999 +merge_request: 28742 +author: Luke Picciau +type: fixed diff --git a/db/migrate/20190523112344_limit_milestone_date_years_to_4_digits.rb b/db/migrate/20190523112344_limit_milestone_date_years_to_4_digits.rb new file mode 100644 index 00000000000..86fe09d7573 --- /dev/null +++ b/db/migrate/20190523112344_limit_milestone_date_years_to_4_digits.rb @@ -0,0 +1,38 @@ +# 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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f9bf9936552..9627e922dfd 100644 --- a/locale/gitlab.pot +++ b/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] "" diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index b82368318f2..752a7965704 100644 --- a/spec/models/milestone_spec.rb +++ b/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 -- cgit v1.2.1