From 24b9a1cd5bed22ddc25a4760eb7ee345ba93e871 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Mon, 16 Jul 2018 16:11:12 +0800
Subject: [PATCH 01/54] Add changelog and doc

---
 doc/api/epics.md                              | 77 +++++++++++++++----
 ...-milestone-dates-integrated-into-epics.yml |  5 ++
 2 files changed, 65 insertions(+), 17 deletions(-)
 create mode 100644 ee/changelogs/unreleased/6470-milestone-dates-integrated-into-epics.yml

diff --git a/doc/api/epics.md b/doc/api/epics.md
index 502e327a3bbb5e..96aa42a27a5b9c 100644
--- a/doc/api/epics.md
+++ b/doc/api/epics.md
@@ -10,6 +10,14 @@ If epics feature is not available a `403` status code will be returned.
 
 The [epic issues API](epic_issues.md) allows you to interact with issues associated with an epic.
 
+# Milestone dates integration
+
+> [Introduced][ee-6448] in GitLab 11.2.
+
+Since start date and due date can be dynamically sourced from related issue milestones, when user has edit permission, additional fields will be shown. These include two boolean fields `start_date_is_fixed` and `due_date_is_fixed`, and four date fields `start_date_fixed`, `start_date_from_milestones`, `due_date_fixed` and `due_date_from_milestones`.
+
+`end_date` has been deprecated in favor of `due_date`.
+
 ## List epics for a group
 
 Gets all epics of the requested group and its subgroups.
@@ -51,11 +59,18 @@ Example response:
     "avatar_url": "http://www.gravatar.com/avatar/018729e129a6f31c80a6327a30196823?s=80&d=identicon",
     "web_url": "http://localhost:3001/kam"
   },
-  "labels": [],
   "start_date": null,
-  "end_date": null,
-  "created_at": "2018-01-21T06:21:13.165Z",
-  "updated_at": "2018-01-22T12:41:41.166Z"
+  "start_date_is_fixed": false,
+  "start_date_fixed": null,
+  "start_date_from_milestones": null,
+  "end_date": "2018-07-31",
+  "due_date": "2018-07-31",
+  "due_date_is_fixed": false,
+  "due_date_fixed": null,
+  "due_date_from_milestones": "2018-07-31",
+  "created_at": "2018-07-17T13:36:22.770Z",
+  "updated_at": "2018-07-18T12:22:05.239Z",
+  "labels": []
   }
 ]
 ```
@@ -95,9 +110,17 @@ Example response:
     "web_url": "http://localhost:3001/arnita"
   },
   "start_date": null,
-  "end_date": null,
-  "created_at": "2018-01-21T06:21:13.165Z",
-  "updated_at": "2018-01-22T12:41:41.166Z"
+  "start_date_is_fixed": false,
+  "start_date_fixed": null,
+  "start_date_from_milestones": null,
+  "end_date": "2018-07-31",
+  "due_date": "2018-07-31",
+  "due_date_is_fixed": false,
+  "due_date_fixed": null,
+  "due_date_from_milestones": "2018-07-31",
+  "created_at": "2018-07-17T13:36:22.770Z",
+  "updated_at": "2018-07-18T12:22:05.239Z",
+  "labels": []
 }
 ```
 
@@ -139,11 +162,18 @@ Example response:
     "id" : 18,
     "username" : "eileen.lowe"
   },
-  "labels": [],
   "start_date": null,
-  "end_date": null,
-  "created_at": "2018-01-21T06:21:13.165Z",
-  "updated_at": "2018-01-22T12:41:41.166Z"
+  "start_date_is_fixed": false,
+  "start_date_fixed": null,
+  "start_date_from_milestones": null,
+  "end_date": "2018-07-31",
+  "due_date": "2018-07-31",
+  "due_date_is_fixed": false,
+  "due_date_fixed": null,
+  "due_date_from_milestones": "2018-07-31",
+  "created_at": "2018-07-17T13:36:22.770Z",
+  "updated_at": "2018-07-18T12:22:05.239Z",
+  "labels": []
 }
 ```
 
@@ -151,6 +181,8 @@ Example response:
 
 Updates an epic
 
+Note that after 11.2, `start_date` and `end_date` should no longer be updated directly, as they are now composite fields. User can configure the `_is_fixed` and `_fixed` fields instead.
+
 ```
 PUT /groups/:id/epics/:epic_iid
 ```
@@ -162,8 +194,10 @@ PUT /groups/:id/epics/:epic_iid
 | `title`             | string           | no         | The title of an epic |
 | `description`       | string           | no         | The description of an epic  |
 | `labels`            | string           | no         | The comma separated list of labels |
-| `start_date`        | string           | no         | The start date of an epic  |
-| `end_date`          | string.          | no         | The end date of an epic |
+| `start_date_is_fixed` | boolean        | no         | Whether start date should be sourced from `start_date_fixed` |
+| `start_date_fixed`  | string           | no         | The fixed start date of an epic  |
+| `due_date_is_fixed` | boolean          | no         | Whether due date should be sourced from `due_date_fixed` |
+| `due_date_fixed`    | string           | no         | The fixed due date of an epic |
 
 ```bash
 curl --header PUT "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/1/epics/5?title=New%20Title
@@ -186,11 +220,18 @@ Example response:
     "id" : 18,
     "username" : "eileen.lowe"
   },
-  "labels": [],
   "start_date": null,
-  "end_date": null,
-  "created_at": "2018-01-21T06:21:13.165Z",
-  "updated_at": "2018-01-22T12:41:41.166Z"
+  "start_date_is_fixed": false,
+  "start_date_fixed": null,
+  "start_date_from_milestones": null,
+  "end_date": "2018-07-31",
+  "due_date": "2018-07-31",
+  "due_date_is_fixed": false,
+  "due_date_fixed": null,
+  "due_date_from_milestones": "2018-07-31",
+  "created_at": "2018-07-17T13:36:22.770Z",
+  "updated_at": "2018-07-18T12:22:05.239Z",
+  "labels": []
 }
 ```
 
@@ -278,3 +319,5 @@ Example response:
   "created_at": "2016-07-01T11:09:13.992Z"
 }
 ```
+
+[ee-6448]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6448
diff --git a/ee/changelogs/unreleased/6470-milestone-dates-integrated-into-epics.yml b/ee/changelogs/unreleased/6470-milestone-dates-integrated-into-epics.yml
new file mode 100644
index 00000000000000..c7064e677692de
--- /dev/null
+++ b/ee/changelogs/unreleased/6470-milestone-dates-integrated-into-epics.yml
@@ -0,0 +1,5 @@
+---
+title: Allow epic start/due dates to be sourceable from issue milestones
+merge_request: 6470
+author:
+type: added
-- 
GitLab


From 23138484ccc8b9e2e1e25b780aa5277bb8275cc0 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 10 Jul 2018 18:12:21 +0800
Subject: [PATCH 02/54] Add methods to get start/due dates from epic issue's
 milestones

Earliest's start date and latests due date are returned.
---
 ee/app/models/ee/epic.rb    | 10 +++++++
 ee/spec/models/epic_spec.rb | 55 +++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index bc9f6686fcc668..711793daf21d9d 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -109,6 +109,16 @@ def elapsed_days
     # Needed to use EntityDateHelper#remaining_days_in_words
     alias_attribute(:due_date, :end_date)
 
+    # Earliest start date from issues' milestones
+    def start_date_from_milestones
+      epic_issues.joins(issue: :milestone).minimum('milestones.start_date')
+    end
+
+    # Latest end date from issues' milestones
+    def due_date_from_milestones
+      epic_issues.joins(issue: :milestone).maximum('milestones.due_date')
+    end
+
     def to_reference(from = nil, full: false)
       reference = "#{self.class.reference_prefix}#{iid}"
 
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index e33378aa1627c9..ddddf39acfe3e5 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -7,6 +7,7 @@
     it { is_expected.to belong_to(:author).class_name('User') }
     it { is_expected.to belong_to(:assignee).class_name('User') }
     it { is_expected.to belong_to(:group) }
+    it { is_expected.to have_many(:epic_issues) }
   end
 
   describe 'validations' do
@@ -84,6 +85,60 @@
     end
   end
 
+  describe '#start_date_from_milestones' do
+    subject { create(:epic) }
+    let(:date) { Date.new(2017, 3, 4) }
+
+    before do
+      milestone1 = create(
+        :milestone,
+        start_date: date,
+        due_date: date + 10.days
+      )
+      epic_issue1 = create(:epic_issue, epic: subject)
+      epic_issue1.issue.update(milestone: milestone1)
+
+      milestone2 = create(
+        :milestone,
+        start_date: date + 5.days,
+        due_date: date + 15.days
+      )
+      epic_issue2 = create(:epic_issue, epic: subject)
+      epic_issue2.issue.update(milestone: milestone2)
+    end
+
+    it 'returns earliest start date from issue milestones' do
+      expect(subject.start_date_from_milestones).to eq(date)
+    end
+  end
+
+  describe '#due_date_from_milestones' do
+    subject { create(:epic) }
+    let(:date) { Date.new(2017, 3, 4) }
+
+    before do
+      milestone1 = create(
+        :milestone,
+        start_date: date - 30.days,
+        due_date: date - 20.days
+      )
+      epic_issue1 = create(:epic_issue, epic: subject)
+      epic_issue1.issue.update(milestone: milestone1)
+
+      milestone2 = create(
+        :milestone,
+        start_date: date - 10.days,
+        due_date: date
+      )
+      epic_issue2 = create(:epic_issue, epic: subject)
+      epic_issue2.issue.update(milestone: milestone2)
+    end
+
+    it 'returns latest due date from issue milestones' do
+      expect(subject.due_date_from_milestones).to eq(date)
+    end
+  end
+
   describe '#issues' do
     let(:user) { create(:user) }
     let(:group) { create(:group, :private) }
-- 
GitLab


From 88601cbbc6c9c04bf2135cd906794f69c107fd8d Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 11 Jul 2018 15:47:40 +0800
Subject: [PATCH 03/54] Add date fixed/from_migrations columns to Epic

Indicate whether start and due dates should be sourced from Epic's issue's milestones.

Post migrate epics to copy milestone dates
---
 db/schema.rb                                  |  4 +++
 ...0180711014025_add_date_columns_to_epics.rb | 14 +++++++++++
 ...0711014026_update_date_columns_on_epics.rb | 25 +++++++++++++++++++
 ...71825_update_epic_dates_from_milestones.rb | 22 ++++++++++++++++
 4 files changed, 65 insertions(+)
 create mode 100644 ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
 create mode 100644 ee/db/migrate/20180711014026_update_date_columns_on_epics.rb
 create mode 100644 ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb

diff --git a/db/schema.rb b/db/schema.rb
index ad229d8717bdd6..d9ea50b9f33834 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -954,6 +954,10 @@
     t.string "title_html", null: false
     t.text "description"
     t.text "description_html"
+    t.boolean "start_date_is_fixed"
+    t.boolean "due_date_is_fixed"
+    t.date "start_date_fixed"
+    t.date "due_date_fixed"
   end
 
   add_index "epics", ["assignee_id"], name: "index_epics_on_assignee_id", using: :btree
diff --git a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
new file mode 100644
index 00000000000000..f80953f13e1c73
--- /dev/null
+++ b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
@@ -0,0 +1,14 @@
+class AddDateColumnsToEpics < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    change_table :epics do |t|
+      t.boolean :start_date_is_fixed
+      t.boolean :due_date_is_fixed
+      t.date :start_date_fixed
+      t.date :due_date_fixed
+    end
+  end
+end
diff --git a/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb b/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb
new file mode 100644
index 00000000000000..ccce5ab694112e
--- /dev/null
+++ b/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb
@@ -0,0 +1,25 @@
+class UpdateDateColumnsOnEpics < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  disable_ddl_transaction!
+
+  class Epic < ActiveRecord::Base
+    self.table_name = 'epics'
+    include EachBatch
+  end
+
+  def up
+    Epic.where.not(start_date: nil).each_batch do |batch|
+      batch.update_all('start_date_is_fixed = true, start_date_fixed = start_date')
+    end
+
+    Epic.where.not(end_date: nil).each_batch do |batch|
+      batch.update_all('due_date_is_fixed = true, due_date_fixed = end_date')
+    end
+  end
+
+  def down
+    # NOOP
+  end
+end
diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
new file mode 100644
index 00000000000000..041f97142ac470
--- /dev/null
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -0,0 +1,22 @@
+class UpdateEpicDatesFromMilestones < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Epic < ActiveRecord::Base
+    self.table_name = 'epics'
+    include EachBatch
+  end
+
+  def up
+    Epic.where(start_date: nil).find_each do |epic|
+      epic.update_dates
+    end
+  end
+
+  def down
+    # NOOP
+  end
+end
-- 
GitLab


From 81476a98d6907d2fbf8ae6fff8c5760d83f6cb7e Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 12 Jul 2018 15:11:14 +0800
Subject: [PATCH 04/54] Add method for updating Epic date fields

---
 ee/app/models/ee/epic.rb    | 16 +++++++++++++++
 ee/spec/factories/epics.rb  |  9 ++++++++
 ee/spec/models/epic_spec.rb | 41 +++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 711793daf21d9d..1f5749c826d026 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -109,6 +109,12 @@ def elapsed_days
     # Needed to use EntityDateHelper#remaining_days_in_words
     alias_attribute(:due_date, :end_date)
 
+    def update_dates
+      self.start_date = compute_start_date
+      self.due_date = compute_due_date
+      save if changed?
+    end
+
     # Earliest start date from issues' milestones
     def start_date_from_milestones
       epic_issues.joins(issue: :milestone).minimum('milestones.start_date')
@@ -155,5 +161,15 @@ def discussions_rendered_on_frontend?
     def banzai_render_context(field)
       super.merge(label_url_method: :group_epics_url)
     end
+
+    private
+
+    def compute_start_date
+      start_date_is_fixed? ? start_date_fixed : start_date_from_milestones
+    end
+
+    def compute_due_date
+      due_date_is_fixed? ? due_date_fixed : due_date_from_milestones
+    end
   end
 end
diff --git a/ee/spec/factories/epics.rb b/ee/spec/factories/epics.rb
index 022e1dc95e0229..eb5a62b1909d18 100644
--- a/ee/spec/factories/epics.rb
+++ b/ee/spec/factories/epics.rb
@@ -4,6 +4,15 @@
     group
     author
 
+    trait :use_fixed_dates do
+      start_date { Date.new(2010, 1, 1) }
+      start_date_fixed { Date.new(2010, 1, 1) }
+      start_date_is_fixed true
+      end_date { Date.new(2010, 1, 3) }
+      due_date_fixed { Date.new(2010, 1, 3) }
+      due_date_is_fixed true
+    end
+
     factory :labeled_epic do
       transient do
         labels []
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index ddddf39acfe3e5..6b9d2c9bfb496f 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -85,6 +85,18 @@
     end
   end
 
+  describe '#start_date' do
+    let(:date) { Date.new(2000, 1, 1) }
+
+    context 'is set' do
+      subject { build(:epic, :use_fixed_dates, start_date: date) }
+
+      it 'returns as is' do
+        expect(subject.start_date).to eq(date)
+      end
+    end
+  end
+
   describe '#start_date_from_milestones' do
     subject { create(:epic) }
     let(:date) { Date.new(2017, 3, 4) }
@@ -139,6 +151,35 @@
     end
   end
 
+  describe '#update_dates' do
+    context 'fixed date is set' do
+      subject { create(:epic, :use_fixed_dates, start_date: nil, end_date: nil) }
+
+      it 'updates to fixed date' do
+        subject.update_dates
+
+        expect(subject.start_date).to eq(subject.start_date_fixed)
+        expect(subject.due_date).to eq(subject.due_date_fixed)
+      end
+    end
+
+    context 'fixed date is not set' do
+      subject { create(:epic, start_date: nil, end_date: nil) }
+      let(:start_date) { Date.new(2001, 1, 1) }
+      let(:due_date) { Date.new(2002, 1, 1) }
+
+      it 'updates to milestone dates' do
+        expect(subject).to receive(:start_date_from_milestones).and_return(start_date)
+        expect(subject).to receive(:due_date_from_milestones).and_return(due_date)
+
+        subject.update_dates
+
+        expect(subject.start_date).to eq(start_date)
+        expect(subject.due_date).to eq(due_date)
+      end
+    end
+  end
+
   describe '#issues' do
     let(:user) { create(:user) }
     let(:group) { create(:group, :private) }
-- 
GitLab


From 695d71d481bb83a4c12d6065aa71a525705dd769 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 13 Jul 2018 15:29:29 +0800
Subject: [PATCH 05/54] Ignore `start_date` and `end_date` columns during
 update

Those are composite fields managed by the system so no direct updates should be allowed
---
 ee/app/services/epics/update_service.rb       |  4 ++++
 ee/spec/services/epics/update_service_spec.rb | 22 +++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/ee/app/services/epics/update_service.rb b/ee/app/services/epics/update_service.rb
index 6519da04febadb..dd869760de009e 100644
--- a/ee/app/services/epics/update_service.rb
+++ b/ee/app/services/epics/update_service.rb
@@ -1,6 +1,10 @@
 module Epics
   class UpdateService < Epics::BaseService
     def execute(epic)
+      # start_date and end_date columns are no longer writable by users because those
+      # are composite fields managed by the system.
+      params.except!(:start_date, :end_date)
+
       update(epic)
 
       epic
diff --git a/ee/spec/services/epics/update_service_spec.rb b/ee/spec/services/epics/update_service_spec.rb
index 58a9525d502e93..bdfe1e7ce61575 100644
--- a/ee/spec/services/epics/update_service_spec.rb
+++ b/ee/spec/services/epics/update_service_spec.rb
@@ -18,8 +18,10 @@ def update_epic(opts)
         {
           title: 'New title',
           description: 'New description',
-          start_date: '2017-01-09',
-          end_date: '2017-10-21'
+          start_date_fixed: '2017-01-09',
+          start_date_is_fixed: true,
+          due_date_fixed: '2017-10-21',
+          due_date_is_fixed: true
         }
       end
 
@@ -29,8 +31,10 @@ def update_epic(opts)
         expect(epic).to be_valid
         expect(epic.title).to eq(opts[:title])
         expect(epic.description).to eq(opts[:description])
-        expect(epic.start_date).to eq(Date.strptime(opts[:start_date]))
-        expect(epic.end_date).to eq(Date.strptime(opts[:end_date]))
+        expect(epic.start_date_fixed).to eq(Date.strptime(opts[:start_date_fixed]))
+        expect(epic.start_date_is_fixed).to eq(opts[:start_date_is_fixed])
+        expect(epic.due_date_fixed).to eq(Date.strptime(opts[:due_date_fixed]))
+        expect(epic.due_date_is_fixed).to eq(opts[:due_date_is_fixed])
       end
 
       it 'updates the last_edited_at value' do
@@ -115,5 +119,15 @@ def update_epic(opts)
         end
       end
     end
+
+    context 'filter out start_date and end_date' do
+      it 'ignores start_date and end_date' do
+        expect { update_epic(start_date: Date.today, end_date: Date.today) }.not_to change { Note.count }
+
+        expect(epic).to be_valid
+        expect(epic.start_date).to eq(nil)
+        expect(epic.due_date).to eq(nil)
+      end
+    end
   end
 end
-- 
GitLab


From 06bb783f9172dc12918af3b203e23bca2db146e2 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 12 Jul 2018 17:56:28 +0800
Subject: [PATCH 06/54] Update epic date fields when Epic/Issue/Milestone is
 updated

1. When milestone's dates changes
2. When issue changes milestone
3. When epic's issues change
4. When Epic changes date fields

Convert use of any_instance as it does not work with prepend
---
 app/services/issues/update_service.rb         |  1 +
 app/services/milestones/update_service.rb     |  2 ++
 ee/app/services/ee/issues/update_service.rb   | 18 ++++++++++
 .../services/ee/milestones/update_service.rb  | 20 +++++++++++
 ee/app/services/epic_issues/create_service.rb |  6 ++++
 .../services/epic_issues/destroy_service.rb   |  6 ++++
 ee/app/services/epics/update_service.rb       |  4 +++
 .../services/ee/issues/update_service_spec.rb | 36 +++++++++++++++++++
 .../epic_issues/create_service_spec.rb        |  7 ++++
 .../epic_issues/destroy_service_spec.rb       |  7 ++++
 ee/spec/services/epics/update_service_spec.rb | 16 +++++++++
 .../milestones/update_service_spec.rb         | 23 ++++++++++++
 .../milestones/update_service_spec.rb         | 20 +++++++++++
 spec/services/notes/create_service_spec.rb    |  4 ++-
 .../services/boards/issues_move_service.rb    |  4 ++-
 15 files changed, 172 insertions(+), 2 deletions(-)
 create mode 100644 ee/app/services/ee/issues/update_service.rb
 create mode 100644 ee/app/services/ee/milestones/update_service.rb
 create mode 100644 ee/spec/services/ee/issues/update_service_spec.rb
 create mode 100644 ee/spec/services/milestones/update_service_spec.rb
 create mode 100644 spec/services/milestones/update_service_spec.rb

diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index faa4c8a5a4f239..ec958e02e6b40c 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -2,6 +2,7 @@
 
 module Issues
   class UpdateService < Issues::BaseService
+    prepend EE::Issues::UpdateService
     include SpamCheckService
 
     def execute(issue)
diff --git a/app/services/milestones/update_service.rb b/app/services/milestones/update_service.rb
index 81b20943babac7..691081a4582ff1 100644
--- a/app/services/milestones/update_service.rb
+++ b/app/services/milestones/update_service.rb
@@ -2,6 +2,8 @@
 
 module Milestones
   class UpdateService < Milestones::BaseService
+    prepend EE::Milestones::UpdateService
+
     def execute(milestone)
       state = params[:state_event]
 
diff --git a/ee/app/services/ee/issues/update_service.rb b/ee/app/services/ee/issues/update_service.rb
new file mode 100644
index 00000000000000..97a6901f682ada
--- /dev/null
+++ b/ee/app/services/ee/issues/update_service.rb
@@ -0,0 +1,18 @@
+module EE
+  module Issues
+    module UpdateService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(issue)
+        result = super
+
+        if (params.include?(:milestone) || params.include?(:milestone_id)) && issue.epic
+          issue.epic.update_dates
+        end
+
+        result
+      end
+    end
+  end
+end
diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
new file mode 100644
index 00000000000000..4c6d8d7e6bd36e
--- /dev/null
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -0,0 +1,20 @@
+module EE
+  module Milestones
+    module UpdateService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(milestone)
+        super
+
+        milestone.issues.includes(epic: :group).each do |issue|
+          if issue.epic
+            issue.epic.update_dates
+          end
+        end
+
+        milestone
+      end
+    end
+  end
+end
diff --git a/ee/app/services/epic_issues/create_service.rb b/ee/app/services/epic_issues/create_service.rb
index 5a9129ee1c340d..fa5ff3e5c09748 100644
--- a/ee/app/services/epic_issues/create_service.rb
+++ b/ee/app/services/epic_issues/create_service.rb
@@ -1,5 +1,11 @@
 module EpicIssues
   class CreateService < IssuableLinks::CreateService
+    def execute
+      result = super
+      issuable.update_dates
+      result
+    end
+
     private
 
     def relate_issues(referenced_issue)
diff --git a/ee/app/services/epic_issues/destroy_service.rb b/ee/app/services/epic_issues/destroy_service.rb
index 5440b9505ecdd6..0b08f895d08e29 100644
--- a/ee/app/services/epic_issues/destroy_service.rb
+++ b/ee/app/services/epic_issues/destroy_service.rb
@@ -1,5 +1,11 @@
 module EpicIssues
   class DestroyService < IssuableLinks::DestroyService
+    def execute
+      result = super
+      link.epic.update_dates
+      result
+    end
+
     private
 
     def source
diff --git a/ee/app/services/epics/update_service.rb b/ee/app/services/epics/update_service.rb
index dd869760de009e..89fa715eeec828 100644
--- a/ee/app/services/epics/update_service.rb
+++ b/ee/app/services/epics/update_service.rb
@@ -7,6 +7,10 @@ def execute(epic)
 
       update(epic)
 
+      if (params.keys & [:start_date_fixed, :start_date_is_fixed, :due_date_fixed, :due_date_is_fixed]).present?
+        epic.update_dates
+      end
+
       epic
     end
 
diff --git a/ee/spec/services/ee/issues/update_service_spec.rb b/ee/spec/services/ee/issues/update_service_spec.rb
new file mode 100644
index 00000000000000..31a269dae8694f
--- /dev/null
+++ b/ee/spec/services/ee/issues/update_service_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe Issues::UpdateService do
+  let(:issue) { create(:issue) }
+  let(:user) { issue.author }
+  let(:project) { issue.project }
+
+  describe 'execute' do
+    def update_issue(opts)
+      described_class.new(project, user, opts).execute(issue)
+    end
+
+    context 'refresh epic dates' do
+      let(:epic) { create(:epic) }
+      let(:issue) { create(:issue, epic: epic) }
+
+      context 'updating milestone' do
+        let(:milestone) { create(:milestone) }
+
+        it 'calls epic#update_dates' do
+          expect(epic).to receive(:update_dates).twice
+
+          update_issue(milestone: milestone)
+          update_issue(milestone_id: nil)
+        end
+      end
+
+      context 'updating other fields' do
+        it 'does not call epic#update_dates' do
+          expect(epic).not_to receive(:update_dates)
+          update_issue(title: 'foo')
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/services/epic_issues/create_service_spec.rb b/ee/spec/services/epic_issues/create_service_spec.rb
index 3db29ab7034005..0c88d6c97dca3f 100644
--- a/ee/spec/services/epic_issues/create_service_spec.rb
+++ b/ee/spec/services/epic_issues/create_service_spec.rb
@@ -268,6 +268,13 @@ def assign_issue(references)
 
         include_examples 'returns an error'
       end
+
+      context 'refresh epic dates' do
+        it 'calls epic#update_dates' do
+          expect(epic).to receive(:update_dates)
+          assign_issue([valid_reference])
+        end
+      end
     end
   end
 end
diff --git a/ee/spec/services/epic_issues/destroy_service_spec.rb b/ee/spec/services/epic_issues/destroy_service_spec.rb
index 02e0594403cb6d..740736b1031b28 100644
--- a/ee/spec/services/epic_issues/destroy_service_spec.rb
+++ b/ee/spec/services/epic_issues/destroy_service_spec.rb
@@ -75,6 +75,13 @@
           is_expected.to eq(message: 'No Issue Link found', status: :error, http_status: 404)
         end
       end
+
+      context 'refresh epic dates' do
+        it 'calls epic#update_dates' do
+          expect(epic).to receive(:update_dates)
+          subject
+        end
+      end
     end
   end
 end
diff --git a/ee/spec/services/epics/update_service_spec.rb b/ee/spec/services/epics/update_service_spec.rb
index bdfe1e7ce61575..bde796f59290aa 100644
--- a/ee/spec/services/epics/update_service_spec.rb
+++ b/ee/spec/services/epics/update_service_spec.rb
@@ -129,5 +129,21 @@ def update_epic(opts)
         expect(epic.due_date).to eq(nil)
       end
     end
+
+    context 'refresh epic dates' do
+      context 'date fields are updated' do
+        it 'calls epic#update_dates' do
+          expect(epic).to receive(:update_dates)
+          update_epic(start_date_is_fixed: true, start_date_fixed: Date.today)
+        end
+      end
+
+      context 'date fields are not updated' do
+        it 'does not call epic#update_dates' do
+          expect(epic).not_to receive(:update_dates)
+          update_epic(title: 'foo')
+        end
+      end
+    end
   end
 end
diff --git a/ee/spec/services/milestones/update_service_spec.rb b/ee/spec/services/milestones/update_service_spec.rb
new file mode 100644
index 00000000000000..44240bbef713b6
--- /dev/null
+++ b/ee/spec/services/milestones/update_service_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Milestones::UpdateService do
+  let(:project) { create(:project) }
+  let(:user) { build(:user) }
+  let(:milestone) { create(:milestone, project: project) }
+
+  describe '#execute' do
+    context 'refresh related epic dates' do
+      let(:epic) { create(:epic) }
+      let!(:issue) { create(:issue, milestone: milestone, epic: epic) }
+      let(:due_date) { 3.days.from_now.to_date }
+
+      it 'calls epic#update_dates' do
+        described_class.new(project, user, { due_date: due_date }).execute(milestone)
+
+        epic.reload
+
+        expect(epic.due_date).to eq(due_date)
+      end
+    end
+  end
+end
diff --git a/spec/services/milestones/update_service_spec.rb b/spec/services/milestones/update_service_spec.rb
new file mode 100644
index 00000000000000..ac94c39c8a5016
--- /dev/null
+++ b/spec/services/milestones/update_service_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Milestones::UpdateService do
+  let(:project) { create(:project) }
+  let(:user) { build(:user) }
+  let(:milestone) { create(:milestone, project: project) }
+
+  describe '#execute' do
+    context "valid params" do
+      before do
+        project.add_maintainer(user)
+
+        @milestone = described_class.new(project, user, { title: 'new_title' }).execute(milestone)
+      end
+
+      it { expect(@milestone).to be_valid }
+      it { expect(@milestone.title).to eq('new_title') }
+    end
+  end
+end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 0fd37c95e42938..b1290fd0d47eaa 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -145,7 +145,9 @@
           let(:note_text) { %(HELLO\n/close\n/assign @#{user.username}\nWORLD) }
 
           it 'saves the note and does not alter the note text' do
-            expect_any_instance_of(Issues::UpdateService).to receive(:execute).and_call_original
+            service = double(:service)
+            allow(Issues::UpdateService).to receive(:new).and_return(service)
+            expect(service).to receive(:execute)
 
             note = described_class.new(project, user, opts.merge(note: note_text)).execute
 
diff --git a/spec/support/shared_examples/services/boards/issues_move_service.rb b/spec/support/shared_examples/services/boards/issues_move_service.rb
index 737863ea41126c..6d29a97c56db17 100644
--- a/spec/support/shared_examples/services/boards/issues_move_service.rb
+++ b/spec/support/shared_examples/services/boards/issues_move_service.rb
@@ -4,7 +4,9 @@
     let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } }
 
     it 'delegates the label changes to Issues::UpdateService' do
-      expect_any_instance_of(Issues::UpdateService).to receive(:execute).with(issue).once
+      service = double(:service)
+      expect(Issues::UpdateService).to receive(:new).and_return(service)
+      expect(service).to receive(:execute).with(issue).once
 
       described_class.new(parent, user, params).execute(issue)
     end
-- 
GitLab


From bad81180cd90a2646347d4eda9464eea2d5960cd Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 13 Jul 2018 14:35:27 +0800
Subject: [PATCH 07/54] Update API for epic dates to utilize milestones dates

Expose new date fields.
Translate start_date and end_date params as '_fixed' for backward compatibility.
---
 ee/lib/api/epics.rb                           | 14 ++++++----
 ee/lib/ee/api/entities.rb                     |  9 ++++++-
 .../api/schemas/public_api/v4/epic.json       |  7 +++++
 .../public_api/v4/epic_issue_link.json        |  7 +++++
 ee/spec/requests/api/epics_spec.rb            | 27 +++++++++++++++++++
 5 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/ee/lib/api/epics.rb b/ee/lib/api/epics.rb
index 4008688d60ccda..edc6942763134a 100644
--- a/ee/lib/api/epics.rb
+++ b/ee/lib/api/epics.rb
@@ -79,8 +79,10 @@ def find_epics(args = {})
       params do
         requires :title, type: String, desc: 'The title of an epic'
         optional :description, type: String, desc: 'The description of an epic'
-        optional :start_date, type: String, desc: 'The start date of an epic'
-        optional :end_date, type: String, desc: 'The end date of an epic'
+        optional :start_date, as: :start_date_fixed, type: String, desc: 'The start date of an epic'
+        optional :start_date_is_fixed, type: Boolean, desc: 'Indicates start date should be sourced from start_date_fixed field not the issue milestones'
+        optional :end_date, as: :due_date_fixed, type: String, desc: 'The due date of an epic'
+        optional :due_date_is_fixed, type: Boolean, desc: 'Indicates due date should be sourced from due_date_fixed field not the issue milestones'
         optional :labels, type: String, desc: 'Comma-separated list of label names'
       end
       post ':id/(-/)epics' do
@@ -101,10 +103,12 @@ def find_epics(args = {})
         requires :epic_iid, type: Integer, desc: 'The internal ID of an epic'
         optional :title, type: String, desc: 'The title of an epic'
         optional :description, type: String, desc: 'The description of an epic'
-        optional :start_date, type: String, desc: 'The start date of an epic'
-        optional :end_date, type: String, desc: 'The end date of an epic'
+        optional :start_date, as: :start_date_fixed, type: String, desc: 'The start date of an epic'
+        optional :start_date_is_fixed, type: Boolean, desc: 'Indicates start date should be sourced from start_date_fixed field not the issue milestones'
+        optional :end_date, as: :due_date_fixed, type: String, desc: 'The due date of an epic'
+        optional :due_date_is_fixed, type: Boolean, desc: 'Indicates due date should be sourced from due_date_fixed field not the issue milestones'
         optional :labels, type: String, desc: 'Comma-separated list of label names'
-        at_least_one_of :title, :description, :start_date, :end_date, :labels
+        at_least_one_of :title, :description, :start_date_fixed, :due_date_fixed, :labels
       end
       put ':id/(-/)epics/:epic_iid' do
         authorize_can_admin!
diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb
index 816e967ab63f10..e279c0b633f218 100644
--- a/ee/lib/ee/api/entities.rb
+++ b/ee/lib/ee/api/entities.rb
@@ -162,7 +162,14 @@ class Epic < Grape::Entity
         expose :description
         expose :author, using: ::API::Entities::UserBasic
         expose :start_date
-        expose :end_date
+        expose :start_date_is_fixed?, as: :start_date_is_fixed
+        expose :start_date_fixed
+        expose :start_date_from_milestones
+        expose :end_date # @deprecated
+        expose :end_date, as: :due_date
+        expose :due_date_is_fixed?, as: :due_date_is_fixed
+        expose :due_date_fixed
+        expose :due_date_from_milestones
         expose :created_at
         expose :updated_at
         expose :labels do |epic, options|
diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/epic.json b/ee/spec/fixtures/api/schemas/public_api/v4/epic.json
index 873f5c04ef4f24..3b2caee33f67c9 100644
--- a/ee/spec/fixtures/api/schemas/public_api/v4/epic.json
+++ b/ee/spec/fixtures/api/schemas/public_api/v4/epic.json
@@ -25,7 +25,14 @@
       }
     },
     "start_date": { "type": ["string", "null"] },
+    "start_date_fixed": { "type": ["string", "null"] },
+    "start_date_from_milestones": { "type": ["string", "null"] },
+    "start_date_is_fixed": { "type": ["boolean"] },
     "end_date": { "type": ["string", "null"] },
+    "due_date": { "type": ["string", "null"] },
+    "due_date_fixed": { "type": ["string", "null"] },
+    "due_date_from_milestones": { "type": ["string", "null"] },
+    "due_date_is_fixed": { "type": ["boolean"] },
     "created_at": { "type": ["string", "null"] },
     "updated_at": { "type": ["string", "null"] }
   },
diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json b/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json
index 74b102f95594a0..34445d6dbcc37c 100644
--- a/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json
+++ b/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json
@@ -19,7 +19,14 @@
         "description": { "type": ["string", "null"] },
         "author": { "type": ["object", "null"] },
         "start_date": { "type": ["string", "null"] },
+        "start_date_fixed": { "type": ["string", "null"] },
+        "start_date_from_milestones": { "type": ["string", "null"] },
+        "start_date_is_fixed": { "type": ["boolean"] },
         "end_date": { "type": ["string", "null"] },
+        "due_date": { "type": ["string", "null"] },
+        "due_date_fixed": { "type": ["string", "null"] },
+        "due_date_from_milestones": { "type": ["string", "null"] },
+        "due_date_is_fixed": { "type": ["boolean"] },
         "created_at": { "type": ["string", "null"] },
         "updated_at": { "type": ["string", "null"] },
         "labels": {
diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb
index bdd99dac134b26..cca9ef3bffb6ae 100644
--- a/ee/spec/requests/api/epics_spec.rb
+++ b/ee/spec/requests/api/epics_spec.rb
@@ -206,6 +206,19 @@ def expect_array_response(expected)
           expect(epic.description).to eq('epic description')
           expect(epic.labels.first.title).to eq('label1')
         end
+
+        context 'when deprecated start_date and end_date params are present' do
+          let(:start_date) { Date.new(2001, 1, 1) }
+          let(:due_date) { Date.new(2001, 1, 2) }
+          let(:params) { { title: 'new epic', start_date: start_date, end_date: due_date } }
+
+          it 'updates start_date_fixed and due_date_fixed' do
+            result = Epic.last
+
+            expect(result.start_date_fixed).to eq(start_date)
+            expect(result.due_date_fixed).to eq(due_date)
+          end
+        end
       end
     end
   end
@@ -261,6 +274,20 @@ def expect_array_response(expected)
           expect(result.description).to eq('new description')
           expect(result.labels.first.title).to eq('label2')
         end
+
+        context 'when deprecated start_date and end_date params are present' do
+          let(:epic) { create(:epic, :use_fixed_dates, group: group) }
+          let(:new_start_date) { epic.start_date + 1.day }
+          let(:new_due_date) { epic.end_date + 1.day }
+          let!(:params) { { start_date: new_start_date, end_date: new_due_date } }
+
+          it 'updates start_date_fixed and due_date_fixed' do
+            result = epic.reload
+
+            expect(result.start_date_fixed).to eq(new_start_date)
+            expect(result.due_date_fixed).to eq(new_due_date)
+          end
+        end
       end
     end
   end
-- 
GitLab


From 6d1fca0e6bb3572759a285fbbdef40eae55f9c68 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 18 Jul 2018 13:33:15 +0800
Subject: [PATCH 08/54] Update page epic init data to include new date fields

---
 ee/app/helpers/epics_helper.rb       | 12 +++++++++++
 ee/spec/helpers/epics_helper_spec.rb | 30 +++++++++++++++++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/ee/app/helpers/epics_helper.rb b/ee/app/helpers/epics_helper.rb
index 46992fc18b1792..045793b84ea93a 100644
--- a/ee/app/helpers/epics_helper.rb
+++ b/ee/app/helpers/epics_helper.rb
@@ -16,11 +16,23 @@ def epic_show_app_data(epic, opts)
       todo_exists: todo.present?,
       todo_path: group_todos_path(group),
       start_date: epic.start_date,
+      due_date: epic.due_date,
       end_date: epic.end_date
     }
 
     epic_meta[:todo_delete_path] = dashboard_todo_path(todo) if todo.present?
 
+    if Ability.allowed?(current_user, :update_epic, epic.group)
+      epic_meta.merge!(
+        start_date_fixed: epic.start_date_fixed,
+        start_date_is_fixed: epic.start_date_is_fixed?,
+        start_date_from_milestones: epic.start_date_from_milestones,
+        due_date_fixed: epic.due_date_fixed,
+        due_date_is_fixed: epic.due_date_is_fixed?,
+        due_date_from_milestones: epic.due_date_from_milestones
+      )
+    end
+
     participants = UserSerializer.new.represent(epic.participants)
     initial = opts[:initial].merge(labels: epic.labels,
                                    participants: participants,
diff --git a/ee/spec/helpers/epics_helper_spec.rb b/ee/spec/helpers/epics_helper_spec.rb
index 45c57dcc91cc62..6ec212cdfba990 100644
--- a/ee/spec/helpers/epics_helper_spec.rb
+++ b/ee/spec/helpers/epics_helper_spec.rb
@@ -4,18 +4,21 @@
   include ApplicationHelper
 
   describe '#epic_show_app_data' do
-    it 'returns the correct json' do
-      user = create(:user)
-      @epic = create(:epic, author: user)
+    let(:user) { create(:user) }
+    let!(:epic) { create(:epic, author: user) }
 
+    before do
       allow(helper).to receive(:current_user).and_return(user)
+      stub_licensed_features(epics: true)
+    end
 
-      data = helper.epic_show_app_data(@epic, initial: {}, author_icon: 'icon_path')
+    it 'returns the correct json' do
+      data = helper.epic_show_app_data(epic, initial: {}, author_icon: 'icon_path')
       meta_data = JSON.parse(data[:meta])
 
       expected_keys = %i(initial meta namespace labels_path toggle_subscription_path labels_web_url epics_web_url)
       expect(data.keys).to match_array(expected_keys)
-      expect(meta_data.keys).to match_array(%w[created author start_date end_date epic_id todo_exists todo_path])
+      expect(meta_data.keys).to match_array(%w[created author start_date end_date due_date epic_id todo_exists todo_path])
       expect(meta_data['author']).to eq({
         'name' => user.name,
         'url' => "/#{user.username}",
@@ -23,6 +26,23 @@
         'src' => 'icon_path'
       })
     end
+
+    context 'when user has edit permission' do
+      before do
+        epic.group.add_developer(user)
+      end
+
+      it 'returns extra date fields if user can edit' do
+        data = helper.epic_show_app_data(epic, initial: {}, author_icon: 'icon_path')
+        meta_data = JSON.parse(data[:meta])
+
+        expect(meta_data.keys).to match_array(%w[
+          created author
+          start_date start_date_fixed start_date_is_fixed start_date_from_milestones
+          end_date due_date due_date_fixed due_date_is_fixed due_date_from_milestones
+        ])
+      end
+    end
   end
 
   describe '#epic_endpoint_query_params' do
-- 
GitLab


From 646a37be556d5a03e9fbb15753198bc7d4747f46 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 18 Jul 2018 18:09:41 +0800
Subject: [PATCH 09/54] Hide date fields related to edit if user does not have
 permission

Rationale: match closer to web UI where users without epic edit access can not see those fields.
---
 ee/lib/api/epics.rb                |  8 +++----
 ee/lib/ee/api/entities.rb          | 12 +++++-----
 ee/spec/requests/api/epics_spec.rb | 36 ++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/ee/lib/api/epics.rb b/ee/lib/api/epics.rb
index edc6942763134a..8625c087edd93e 100644
--- a/ee/lib/api/epics.rb
+++ b/ee/lib/api/epics.rb
@@ -58,7 +58,7 @@ def find_epics(args = {})
         optional :labels, type: String, desc: 'Comma-separated list of label names'
       end
       get ':id/(-/)epics' do
-        present find_epics(group_id: user_group.id), with: EE::API::Entities::Epic
+        present find_epics(group_id: user_group.id), with: EE::API::Entities::Epic, user: current_user
       end
 
       desc 'Get details of an epic' do
@@ -70,7 +70,7 @@ def find_epics(args = {})
       get ':id/(-/)epics/:epic_iid' do
         authorize_can_read!
 
-        present epic, with: EE::API::Entities::Epic
+        present epic, with: EE::API::Entities::Epic, user: current_user
       end
 
       desc 'Create a new epic' do
@@ -90,7 +90,7 @@ def find_epics(args = {})
 
         epic = ::Epics::CreateService.new(user_group, current_user, declared_params(include_missing: false)).execute
         if epic.valid?
-          present epic, with: EE::API::Entities::Epic
+          present epic, with: EE::API::Entities::Epic, user: current_user
         else
           render_validation_error!(epic)
         end
@@ -118,7 +118,7 @@ def find_epics(args = {})
         result = ::Epics::UpdateService.new(user_group, current_user, update_params).execute(epic)
 
         if result.valid?
-          present result, with: EE::API::Entities::Epic
+          present result, with: EE::API::Entities::Epic, user: current_user
         else
           render_validation_error!(result)
         end
diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb
index e279c0b633f218..810f515728ec3c 100644
--- a/ee/lib/ee/api/entities.rb
+++ b/ee/lib/ee/api/entities.rb
@@ -155,6 +155,8 @@ class RelatedIssue < ::API::Entities::Issue
       end
 
       class Epic < Grape::Entity
+        allowed_to_admin = ->(epic, opts) { Ability.allowed?(opts[:user], :admin_epic, epic) }
+
         expose :id
         expose :iid
         expose :group_id
@@ -162,14 +164,12 @@ class Epic < Grape::Entity
         expose :description
         expose :author, using: ::API::Entities::UserBasic
         expose :start_date
-        expose :start_date_is_fixed?, as: :start_date_is_fixed
-        expose :start_date_fixed
-        expose :start_date_from_milestones
+        expose :start_date_is_fixed?, as: :start_date_is_fixed, if: allowed_to_admin
+        expose :start_date_fixed, :start_date_from_milestones, if: allowed_to_admin
         expose :end_date # @deprecated
         expose :end_date, as: :due_date
-        expose :due_date_is_fixed?, as: :due_date_is_fixed
-        expose :due_date_fixed
-        expose :due_date_from_milestones
+        expose :due_date_is_fixed?, as: :due_date_is_fixed, if: allowed_to_admin
+        expose :due_date_fixed, :due_date_from_milestones, if: allowed_to_admin
         expose :created_at
         expose :updated_at
         expose :labels do |epic, options|
diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb
index cca9ef3bffb6ae..403c6d3160287f 100644
--- a/ee/spec/requests/api/epics_spec.rb
+++ b/ee/spec/requests/api/epics_spec.rb
@@ -40,6 +40,32 @@
     end
   end
 
+  shared_examples 'admin_epic permission' do
+    let(:extra_date_fields) { %w[start_date_is_fixed start_date_fixed due_date_is_fixed due_date_fixed] }
+
+    context 'when permission is absent' do
+      RSpec::Matchers.define_negated_matcher :exclude, :include
+
+      it 'returns epic with extra date fields' do
+        get api(url, user), params
+
+        expect(Array.wrap(JSON.parse(response.body))).to all(exclude(*extra_date_fields))
+      end
+    end
+
+    context 'when permission is present' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'returns epic with extra date fields' do
+        get api(url, user), params
+
+        expect(Array.wrap(JSON.parse(response.body))).to all(include(*extra_date_fields))
+      end
+    end
+  end
+
   describe 'GET /groups/:id/epics' do
     let(:url) { "/groups/#{group.path}/epics" }
 
@@ -138,6 +164,8 @@ def expect_array_response(expected)
 
         expect_array_response([epic2.id])
       end
+
+      it_behaves_like 'admin_epic permission'
     end
   end
 
@@ -149,17 +177,21 @@ def expect_array_response(expected)
     context 'when the request is correct' do
       before do
         stub_licensed_features(epics: true)
-
-        get api(url, user)
       end
 
       it 'returns 200 status' do
+        get api(url, user)
+
         expect(response).to have_gitlab_http_status(200)
       end
 
       it 'matches the response schema' do
+        get api(url, user)
+
         expect(response).to match_response_schema('public_api/v4/epic', dir: 'ee')
       end
+
+      it_behaves_like 'admin_epic permission'
     end
   end
 
-- 
GitLab


From 8d403ef618cd638a2800805ca170778117e37a82 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 25 Jul 2018 11:45:49 +0800
Subject: [PATCH 10/54] Add _date_from_milestone_id column

Tracking which milestone the milestone dates are sourced from
---
 db/schema.rb                                              | 4 +++-
 ee/db/migrate/20180711014025_add_date_columns_to_epics.rb | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/db/schema.rb b/db/schema.rb
index d9ea50b9f33834..62b0e8a2d8246a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -955,9 +955,11 @@
     t.text "description"
     t.text "description_html"
     t.boolean "start_date_is_fixed"
-    t.boolean "due_date_is_fixed"
     t.date "start_date_fixed"
+    t.integer "start_date_sourcing_milestone_id"
+    t.boolean "due_date_is_fixed"
     t.date "due_date_fixed"
+    t.integer "due_date_sourcing_milestone_id"
   end
 
   add_index "epics", ["assignee_id"], name: "index_epics_on_assignee_id", using: :btree
diff --git a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
index f80953f13e1c73..4b761deb54bce5 100644
--- a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
+++ b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
@@ -6,9 +6,11 @@ class AddDateColumnsToEpics < ActiveRecord::Migration
   def change
     change_table :epics do |t|
       t.boolean :start_date_is_fixed
-      t.boolean :due_date_is_fixed
       t.date :start_date_fixed
+      t.references :start_date_sourcing_milestone
+      t.boolean :due_date_is_fixed
       t.date :due_date_fixed
+      t.references :due_date_sourcing_milestone
     end
   end
 end
-- 
GitLab


From 03d1f887674d70b67034bbbe552a613b33152333 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 25 Jul 2018 14:07:49 +0800
Subject: [PATCH 11/54] Add convenience through association

---
 ee/app/models/ee/epic.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 1f5749c826d026..ebe35300681142 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -17,6 +17,7 @@ module Epic
       has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.epics&.maximum(:iid) }
 
       has_many :epic_issues
+      has_many :issues, through: :epic_issues
 
       validates :group, presence: true
 
-- 
GitLab


From ceb0924c70220b4c73bccc5699ab5f2fd14de9d2 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 25 Jul 2018 14:08:16 +0800
Subject: [PATCH 12/54] Refactor query to loop through epics

---
 ee/app/services/ee/milestones/update_service.rb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
index 4c6d8d7e6bd36e..26c3ae9410d8de 100644
--- a/ee/app/services/ee/milestones/update_service.rb
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -7,10 +7,8 @@ module UpdateService
       def execute(milestone)
         super
 
-        milestone.issues.includes(epic: :group).each do |issue|
-          if issue.epic
-            issue.epic.update_dates
-          end
+        ::Epic.joins(:issues).where(issues: { milestone_id: milestone.id }).each do |epic|
+          epic.update_dates
         end
 
         milestone
-- 
GitLab


From dbb08f2a845aac80dd4ee58da2b68d3a9e5f6597 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 25 Jul 2018 15:39:18 +0800
Subject: [PATCH 13/54] Rename existing Epic#issues to Epic#issues_readable_by

The name is reserved for more common association methods.
---
 ee/app/models/ee/epic.rb                    | 2 +-
 ee/app/services/epic_issues/list_service.rb | 2 +-
 ee/lib/api/epic_issues.rb                   | 4 ++--
 ee/spec/models/epic_spec.rb                 | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index ebe35300681142..639da374389cd0 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -142,7 +142,7 @@ def cross_reference?(from)
     def update_project_counter_caches
     end
 
-    def issues(current_user)
+    def issues_readable_by(current_user)
       related_issues = ::Issue.select('issues.*, epic_issues.id as epic_issue_id, epic_issues.relative_position')
         .joins(:epic_issue)
         .where("epic_issues.epic_id = #{id}")
diff --git a/ee/app/services/epic_issues/list_service.rb b/ee/app/services/epic_issues/list_service.rb
index 45535bec9e7f86..90628c82b888c4 100644
--- a/ee/app/services/epic_issues/list_service.rb
+++ b/ee/app/services/epic_issues/list_service.rb
@@ -5,7 +5,7 @@ class ListService < IssuableLinks::ListService
     def issues
       return [] unless issuable&.group&.feature_available?(:epics)
 
-      issuable.issues(current_user)
+      issuable.issues_readable_by(current_user)
     end
 
     def relation_path(issue)
diff --git a/ee/lib/api/epic_issues.rb b/ee/lib/api/epic_issues.rb
index 2c01dd6e1dd555..7db1540aa738a0 100644
--- a/ee/lib/api/epic_issues.rb
+++ b/ee/lib/api/epic_issues.rb
@@ -53,7 +53,7 @@ def link
         # For now we return empty body
         # The issues list in the correct order in body will be returned as part of #4250
         if result
-          present epic.issues(current_user),
+          present epic.issues_readable_by(current_user),
             with: EE::API::Entities::EpicIssue,
             current_user: current_user
         else
@@ -70,7 +70,7 @@ def link
       get ':id/(-/)epics/:epic_iid/issues' do
         authorize_can_read!
 
-        present epic.issues(current_user),
+        present epic.issues_readable_by(current_user),
           with: EE::API::Entities::EpicIssue,
           current_user: current_user
       end
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index 6b9d2c9bfb496f..ca35593144643b 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -180,7 +180,7 @@
     end
   end
 
-  describe '#issues' do
+  describe '#issues_readable_by' do
     let(:user) { create(:user) }
     let(:group) { create(:group, :private) }
     let(:project) { create(:project, group: group) }
@@ -197,7 +197,7 @@
       ]
     end
 
-    let(:result) { epic.issues(user) }
+    let(:result) { epic.issues_readable_by(user) }
 
     it 'returns all issues if a user has access to them' do
       group.add_developer(user)
-- 
GitLab


From 93b05616c9172777a4ea103a2fc5c87fda453668 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 25 Jul 2018 16:08:54 +0800
Subject: [PATCH 14/54] Reduce query for update_dates to 1 select

---
 ee/app/models/ee/epic.rb    | 11 +++++++++--
 ee/spec/models/epic_spec.rb | 30 +++++++++++++++++++++++-------
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 639da374389cd0..bdb517b3f9b989 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -111,8 +111,15 @@ def elapsed_days
     alias_attribute(:due_date, :end_date)
 
     def update_dates
-      self.start_date = compute_start_date
-      self.due_date = compute_due_date
+      if !start_date_is_fixed? || !due_date_is_fixed?
+        milestone_dates = issues
+          .joins(:milestone)
+          .pluck('MIN(milestones.start_date)', 'MAX(milestones.due_date)')
+          .first
+      end
+
+      self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_dates[0]
+      self.due_date = due_date_is_fixed? ? due_date_fixed : milestone_dates[1]
       save if changed?
     end
 
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index ca35593144643b..e1917926b01664 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -165,17 +165,33 @@
 
     context 'fixed date is not set' do
       subject { create(:epic, start_date: nil, end_date: nil) }
-      let(:start_date) { Date.new(2001, 1, 1) }
-      let(:due_date) { Date.new(2002, 1, 1) }
+      let(:milestone1) {
+        create(
+          :milestone,
+          start_date: Date.new(2000, 1, 1),
+          due_date: Date.new(2000, 1, 10)
+        )
+      }
+      let(:milestone2) {
+        create(
+          :milestone,
+          start_date: Date.new(2000, 1, 3),
+          due_date: Date.new(2000, 1, 20)
+        )
+      }
+
+      before do
+        epic_issue1 = create(:epic_issue, epic: subject)
+        epic_issue1.issue.update(milestone: milestone1)
+        epic_issue2 = create(:epic_issue, epic: subject)
+        epic_issue2.issue.update(milestone: milestone2)
+      end
 
       it 'updates to milestone dates' do
-        expect(subject).to receive(:start_date_from_milestones).and_return(start_date)
-        expect(subject).to receive(:due_date_from_milestones).and_return(due_date)
-
         subject.update_dates
 
-        expect(subject.start_date).to eq(start_date)
-        expect(subject.due_date).to eq(due_date)
+        expect(subject.start_date).to eq(milestone1.start_date)
+        expect(subject.due_date).to eq(milestone2.due_date)
       end
     end
   end
-- 
GitLab


From 5bec56da685c80bc33304d0672a62ca637563b3c Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 10:50:26 +0800
Subject: [PATCH 15/54] Allow persisting start/due dates and milestone ids

---
 ee/app/models/ee/epic.rb    |  44 +++++++++---
 ee/spec/models/epic_spec.rb | 136 +++++++++++++++++++++++++++++++++---
 2 files changed, 163 insertions(+), 17 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index bdb517b3f9b989..b4b4c15a759b1f 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -111,15 +111,13 @@ def elapsed_days
     alias_attribute(:due_date, :end_date)
 
     def update_dates
-      if !start_date_is_fixed? || !due_date_is_fixed?
-        milestone_dates = issues
-          .joins(:milestone)
-          .pluck('MIN(milestones.start_date)', 'MAX(milestones.due_date)')
-          .first
-      end
+      milestone_data = fetch_milestone_date_data
+
+      self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_data[:start_date]
+      self.start_date_sourcing_milestone_id = milestone_data[:start_date_sourcing_milestone_id]
+      self.due_date = due_date_is_fixed? ? due_date_fixed : milestone_data[:due_date]
+      self.due_date_sourcing_milestone_id = milestone_data[:due_date_sourcing_milestone_id]
 
-      self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_dates[0]
-      self.due_date = due_date_is_fixed? ? due_date_fixed : milestone_dates[1]
       save if changed?
     end
 
@@ -179,5 +177,35 @@ def compute_start_date
     def compute_due_date
       due_date_is_fixed? ? due_date_fixed : due_date_from_milestones
     end
+
+    def fetch_milestone_date_data
+      sql = <<~SQL
+        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones 
+        INNER JOIN issues ON issues.milestone_id = milestones.id
+        INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+        INNER JOIN (
+          SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
+          FROM milestones 
+          INNER JOIN issues ON issues.milestone_id = milestones.id
+          INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+          WHERE epic_issues.epic_id = #{id}
+        ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
+        WHERE epic_issues.epic_id = #{id}
+        ORDER BY milestones.start_date, milestones.due_date;
+      SQL
+
+      db_results = ActiveRecord::Base.connection.select_all(sql).to_a
+
+      results = {}
+      db_results.find { |row| row['start_date'] }&.tap do |row|
+        results[:start_date] = row['start_date']
+        results[:start_date_sourcing_milestone_id] = row['id']
+      end
+      db_results.reverse.find { |row| row['due_date'] }&.tap do |row|
+        results[:due_date] = row['due_date']
+        results[:due_date_sourcing_milestone_id] = row['id']
+      end
+      results
+    end
   end
 end
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index e1917926b01664..f69364d583755d 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -165,6 +165,7 @@
 
     context 'fixed date is not set' do
       subject { create(:epic, start_date: nil, end_date: nil) }
+
       let(:milestone1) {
         create(
           :milestone,
@@ -180,18 +181,135 @@
         )
       }
 
-      before do
-        epic_issue1 = create(:epic_issue, epic: subject)
-        epic_issue1.issue.update(milestone: milestone1)
-        epic_issue2 = create(:epic_issue, epic: subject)
-        epic_issue2.issue.update(milestone: milestone2)
+      context 'multiple milestones' do
+        before do
+          epic_issue1 = create(:epic_issue, epic: subject)
+          epic_issue1.issue.update(milestone: milestone1)
+          epic_issue2 = create(:epic_issue, epic: subject)
+          epic_issue2.issue.update(milestone: milestone2)
+        end
+
+        context 'complete start and due dates' do
+          it 'updates to milestone dates' do
+            subject.update_dates
+
+            expect(subject.start_date).to eq(milestone1.start_date)
+            expect(subject.due_date).to eq(milestone2.due_date)
+          end
+        end
+
+        context 'without due date' do
+          let(:milestone1) {
+            create(
+              :milestone,
+              start_date: Date.new(2000, 1, 1),
+              due_date: nil
+            )
+          }
+          let(:milestone2) {
+            create(
+              :milestone,
+              start_date: Date.new(2000, 1, 3),
+              due_date: nil
+            )
+          }
+
+          it 'updates to milestone dates' do
+            subject.update_dates
+
+            expect(subject.start_date).to eq(milestone1.start_date)
+            expect(subject.due_date).to eq(nil)
+          end
+        end
+
+        context 'without any dates' do
+          let(:milestone1) {
+            create(
+              :milestone,
+              start_date: nil,
+              due_date: nil
+            )
+          }
+          let(:milestone2) {
+            create(
+              :milestone,
+              start_date: nil,
+              due_date: nil
+            )
+          }
+
+          it 'updates to milestone dates' do
+            subject.update_dates
+
+            expect(subject.start_date).to eq(nil)
+            expect(subject.due_date).to eq(nil)
+          end
+        end
       end
 
-      it 'updates to milestone dates' do
-        subject.update_dates
+      context 'without milestone' do
+        before do
+          create(:epic_issue, epic: subject)
+        end
+
+        it 'updates to milestone dates' do
+          subject.update_dates
+
+          expect(subject.start_date).to eq(nil)
+          expect(subject.start_date_sourcing_milestone_id).to eq(nil)
+          expect(subject.due_date).to eq(nil)
+          expect(subject.due_date_sourcing_milestone_id).to eq(nil)
+        end
+      end
 
-        expect(subject.start_date).to eq(milestone1.start_date)
-        expect(subject.due_date).to eq(milestone2.due_date)
+      context 'single milestone' do
+        before do
+          epic_issue1 = create(:epic_issue, epic: subject)
+          epic_issue1.issue.update(milestone: milestone1)
+        end
+
+        context 'complete start and due dates' do
+          it 'updates to milestone dates' do
+            subject.update_dates
+
+            expect(subject.start_date).to eq(milestone1.start_date)
+            expect(subject.due_date).to eq(milestone1.due_date)
+          end
+        end
+
+        context 'without due date' do
+          let(:milestone1) {
+            create(
+              :milestone,
+              start_date: Date.new(2000, 1, 1),
+              due_date: nil
+            )
+          }
+
+          it 'updates to milestone dates' do
+            subject.update_dates
+
+            expect(subject.start_date).to eq(milestone1.start_date)
+            expect(subject.due_date).to eq(nil)
+          end
+        end
+
+        context 'without any dates' do
+          let(:milestone1) {
+            create(
+              :milestone,
+              start_date: nil,
+              due_date: nil
+            )
+          }
+
+          it 'updates to milestone dates' do
+            subject.update_dates
+
+            expect(subject.start_date).to eq(nil)
+            expect(subject.due_date).to eq(nil)
+          end
+        end
       end
     end
   end
-- 
GitLab


From 175d2048499bd77bfe7fac4152d3d2b58f75a507 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 14:20:31 +0800
Subject: [PATCH 16/54] Prevent query of calculating milestone dates when fixed
 dates are not used

---
 ee/app/models/ee/epic.rb    |   4 +-
 ee/spec/models/epic_spec.rb | 102 ++++++++++++++++++++++--------------
 2 files changed, 64 insertions(+), 42 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index b4b4c15a759b1f..bb8b8e9ec50569 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -123,12 +123,12 @@ def update_dates
 
     # Earliest start date from issues' milestones
     def start_date_from_milestones
-      epic_issues.joins(issue: :milestone).minimum('milestones.start_date')
+      start_date_is_fixed? ? epic_issues.joins(issue: :milestone).minimum('milestones.start_date') : start_date
     end
 
     # Latest end date from issues' milestones
     def due_date_from_milestones
-      epic_issues.joins(issue: :milestone).maximum('milestones.due_date')
+      due_date_is_fixed? ? epic_issues.joins(issue: :milestone).maximum('milestones.due_date') : due_date
     end
 
     def to_reference(from = nil, full: false)
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index f69364d583755d..aedab3e0b665fa 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -98,56 +98,78 @@
   end
 
   describe '#start_date_from_milestones' do
-    subject { create(:epic) }
-    let(:date) { Date.new(2017, 3, 4) }
+    context 'fixed date' do
+      subject { create(:epic, :use_fixed_dates) }
+      let(:date) { Date.new(2017, 3, 4) }
 
-    before do
-      milestone1 = create(
-        :milestone,
-        start_date: date,
-        due_date: date + 10.days
-      )
-      epic_issue1 = create(:epic_issue, epic: subject)
-      epic_issue1.issue.update(milestone: milestone1)
-
-      milestone2 = create(
-        :milestone,
-        start_date: date + 5.days,
-        due_date: date + 15.days
-      )
-      epic_issue2 = create(:epic_issue, epic: subject)
-      epic_issue2.issue.update(milestone: milestone2)
+      before do
+        milestone1 = create(
+          :milestone,
+          start_date: date,
+          due_date: date + 10.days
+        )
+        epic_issue1 = create(:epic_issue, epic: subject)
+        epic_issue1.issue.update(milestone: milestone1)
+
+        milestone2 = create(
+          :milestone,
+          start_date: date + 5.days,
+          due_date: date + 15.days
+        )
+        epic_issue2 = create(:epic_issue, epic: subject)
+        epic_issue2.issue.update(milestone: milestone2)
+      end
+
+      it 'returns earliest start date from issue milestones' do
+        expect(subject.start_date_from_milestones).to eq(date)
+      end
     end
 
-    it 'returns earliest start date from issue milestones' do
-      expect(subject.start_date_from_milestones).to eq(date)
+    context 'milestone date' do
+      subject { create(:epic, start_date: date) }
+      let(:date) { Date.new(2017, 3, 4) }
+
+      it 'returns start_date' do
+        expect(subject.start_date_from_milestones).to eq(date)
+      end
     end
   end
 
   describe '#due_date_from_milestones' do
-    subject { create(:epic) }
-    let(:date) { Date.new(2017, 3, 4) }
+    context 'fixed date' do
+      subject { create(:epic, :use_fixed_dates) }
+      let(:date) { Date.new(2017, 3, 4) }
 
-    before do
-      milestone1 = create(
-        :milestone,
-        start_date: date - 30.days,
-        due_date: date - 20.days
-      )
-      epic_issue1 = create(:epic_issue, epic: subject)
-      epic_issue1.issue.update(milestone: milestone1)
-
-      milestone2 = create(
-        :milestone,
-        start_date: date - 10.days,
-        due_date: date
-      )
-      epic_issue2 = create(:epic_issue, epic: subject)
-      epic_issue2.issue.update(milestone: milestone2)
+      before do
+        milestone1 = create(
+          :milestone,
+          start_date: date - 30.days,
+          due_date: date - 20.days
+        )
+        epic_issue1 = create(:epic_issue, epic: subject)
+        epic_issue1.issue.update(milestone: milestone1)
+
+        milestone2 = create(
+          :milestone,
+          start_date: date - 10.days,
+          due_date: date
+        )
+        epic_issue2 = create(:epic_issue, epic: subject)
+        epic_issue2.issue.update(milestone: milestone2)
+      end
+
+      it 'returns latest due date from issue milestones' do
+        expect(subject.due_date_from_milestones).to eq(date)
+      end
     end
 
-    it 'returns latest due date from issue milestones' do
-      expect(subject.due_date_from_milestones).to eq(date)
+    context 'milestone date' do
+      subject { create(:epic, due_date: date) }
+      let(:date) { Date.new(2017, 3, 4) }
+
+      it 'returns due_date' do
+        expect(subject.due_date_from_milestones).to eq(date)
+      end
     end
   end
 
-- 
GitLab


From b535e62b5b2a20a075b9b12310d9c629b8b43016 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 14:48:56 +0800
Subject: [PATCH 17/54] Remove unused

---
 ee/app/models/ee/epic.rb | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index bb8b8e9ec50569..c86e79efbcfaec 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -170,14 +170,6 @@ def banzai_render_context(field)
 
     private
 
-    def compute_start_date
-      start_date_is_fixed? ? start_date_fixed : start_date_from_milestones
-    end
-
-    def compute_due_date
-      due_date_is_fixed? ? due_date_fixed : due_date_from_milestones
-    end
-
     def fetch_milestone_date_data
       sql = <<~SQL
         SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones 
-- 
GitLab


From b7006ffe6de587082f258891b7d82e1ab9547066 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 11:09:27 +0800
Subject: [PATCH 18/54] Fix migration calling Model methods

---
 ...71825_update_epic_dates_from_milestones.rb | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 041f97142ac470..ea6608827fe5f5 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -8,6 +8,49 @@ class UpdateEpicDatesFromMilestones < ActiveRecord::Migration
   class Epic < ActiveRecord::Base
     self.table_name = 'epics'
     include EachBatch
+
+    def update_dates
+      milestone_data = fetch_milestone_date_data
+
+      self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_data[:start_date]
+      self.start_date_sourcing_milestone_id = milestone_data[:start_date_sourcing_milestone_id]
+      self.due_date = due_date_is_fixed? ? due_date_fixed : milestone_data[:due_date]
+      self.due_date_sourcing_milestone_id = milestone_data[:due_date_sourcing_milestone_id]
+
+      save if changed?
+    end
+
+    private
+
+    def fetch_milestone_date_data
+      sql = <<~SQL
+        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones 
+        INNER JOIN issues ON issues.milestone_id = milestones.id
+        INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+        INNER JOIN (
+          SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
+          FROM milestones 
+          INNER JOIN issues ON issues.milestone_id = milestones.id
+          INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+          WHERE epic_issues.epic_id = #{id}
+        ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
+        WHERE epic_issues.epic_id = #{id}
+        ORDER BY milestones.start_date, milestones.due_date;
+      SQL
+
+      db_results = ActiveRecord::Base.connection.select_all(sql).to_a
+
+      results = {}
+      db_results.find { |row| row['start_date'] }&.tap do |row|
+        results[:start_date] = row['start_date']
+        results[:start_date_sourcing_milestone_id] = row['id']
+      end
+      db_results.reverse.find { |row| row['due_date'] }&.tap do |row|
+        results[:due_date] = row['due_date']
+        results[:due_date_sourcing_milestone_id] = row['id']
+      end
+      results
+    end
   end
 
   def up
-- 
GitLab


From 816f21d3138bedcab26274679e571c581add4066 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 19:18:03 +0800
Subject: [PATCH 19/54] fix sql

---
 ee/app/models/ee/epic.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index c86e79efbcfaec..d17e128599cbe0 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -172,12 +172,12 @@ def banzai_render_context(field)
 
     def fetch_milestone_date_data
       sql = <<~SQL
-        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones 
+        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
         INNER JOIN issues ON issues.milestone_id = milestones.id
         INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
         INNER JOIN (
           SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
-          FROM milestones 
+          FROM milestones
           INNER JOIN issues ON issues.milestone_id = milestones.id
           INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
           WHERE epic_issues.epic_id = #{id}
-- 
GitLab


From 5b20b100b1a1955fa3c257886dc5cad6642c458f Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 19:18:28 +0800
Subject: [PATCH 20/54] fix spec

---
 ee/spec/models/epic_spec.rb | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index aedab3e0b665fa..a950cc6de7f745 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -188,20 +188,20 @@
     context 'fixed date is not set' do
       subject { create(:epic, start_date: nil, end_date: nil) }
 
-      let(:milestone1) {
+      let(:milestone1) do
         create(
           :milestone,
           start_date: Date.new(2000, 1, 1),
           due_date: Date.new(2000, 1, 10)
         )
-      }
-      let(:milestone2) {
+      end
+      let(:milestone2) do
         create(
           :milestone,
           start_date: Date.new(2000, 1, 3),
           due_date: Date.new(2000, 1, 20)
         )
-      }
+      end
 
       context 'multiple milestones' do
         before do
@@ -221,20 +221,20 @@
         end
 
         context 'without due date' do
-          let(:milestone1) {
+          let(:milestone1) do
             create(
               :milestone,
               start_date: Date.new(2000, 1, 1),
               due_date: nil
             )
-          }
-          let(:milestone2) {
+          end
+          let(:milestone2) do
             create(
               :milestone,
               start_date: Date.new(2000, 1, 3),
               due_date: nil
             )
-          }
+          end
 
           it 'updates to milestone dates' do
             subject.update_dates
@@ -245,20 +245,20 @@
         end
 
         context 'without any dates' do
-          let(:milestone1) {
+          let(:milestone1) do
             create(
               :milestone,
               start_date: nil,
               due_date: nil
             )
-          }
-          let(:milestone2) {
+          end
+          let(:milestone2) do
             create(
               :milestone,
               start_date: nil,
               due_date: nil
             )
-          }
+          end
 
           it 'updates to milestone dates' do
             subject.update_dates
@@ -300,13 +300,13 @@
         end
 
         context 'without due date' do
-          let(:milestone1) {
+          let(:milestone1) do
             create(
               :milestone,
               start_date: Date.new(2000, 1, 1),
               due_date: nil
             )
-          }
+          end
 
           it 'updates to milestone dates' do
             subject.update_dates
@@ -317,13 +317,13 @@
         end
 
         context 'without any dates' do
-          let(:milestone1) {
+          let(:milestone1) do
             create(
               :milestone,
               start_date: nil,
               due_date: nil
             )
-          }
+          end
 
           it 'updates to milestone dates' do
             subject.update_dates
-- 
GitLab


From 30010f18e7b7fd1ee4a7e742e903e3b9e0e1f7c3 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 26 Jul 2018 19:18:54 +0800
Subject: [PATCH 21/54] fix migration

---
 .../20180713171825_update_epic_dates_from_milestones.rb       | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index ea6608827fe5f5..b2827b84616013 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -24,12 +24,12 @@ def update_dates
 
     def fetch_milestone_date_data
       sql = <<~SQL
-        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones 
+        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
         INNER JOIN issues ON issues.milestone_id = milestones.id
         INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
         INNER JOIN (
           SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
-          FROM milestones 
+          FROM milestones
           INNER JOIN issues ON issues.milestone_id = milestones.id
           INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
           WHERE epic_issues.epic_id = #{id}
-- 
GitLab


From dfb77a17743af55608ad2276451d06b8d89da640 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 27 Jul 2018 13:22:30 +0800
Subject: [PATCH 22/54] Add frozen_string_literal

---
 ee/app/services/ee/issues/update_service.rb                     | 2 ++
 ee/app/services/ee/milestones/update_service.rb                 | 2 ++
 ee/db/migrate/20180711014025_add_date_columns_to_epics.rb       | 2 ++
 ee/db/migrate/20180711014026_update_date_columns_on_epics.rb    | 2 ++
 .../20180713171825_update_epic_dates_from_milestones.rb         | 2 ++
 ee/spec/services/ee/issues/update_service_spec.rb               | 1 +
 ee/spec/services/milestones/update_service_spec.rb              | 1 +
 spec/services/milestones/update_service_spec.rb                 | 1 +
 8 files changed, 13 insertions(+)

diff --git a/ee/app/services/ee/issues/update_service.rb b/ee/app/services/ee/issues/update_service.rb
index 97a6901f682ada..03d90eb7a7aec1 100644
--- a/ee/app/services/ee/issues/update_service.rb
+++ b/ee/app/services/ee/issues/update_service.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module EE
   module Issues
     module UpdateService
diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
index 26c3ae9410d8de..964321e0cb772f 100644
--- a/ee/app/services/ee/milestones/update_service.rb
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module EE
   module Milestones
     module UpdateService
diff --git a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
index 4b761deb54bce5..c3b8c5fd2ad1d7 100644
--- a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
+++ b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class AddDateColumnsToEpics < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
diff --git a/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb b/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb
index ccce5ab694112e..c39531940c1380 100644
--- a/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb
+++ b/ee/db/migrate/20180711014026_update_date_columns_on_epics.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class UpdateDateColumnsOnEpics < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index b2827b84616013..56c69952675c3e 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class UpdateEpicDatesFromMilestones < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
diff --git a/ee/spec/services/ee/issues/update_service_spec.rb b/ee/spec/services/ee/issues/update_service_spec.rb
index 31a269dae8694f..87f47eb2dcf7e6 100644
--- a/ee/spec/services/ee/issues/update_service_spec.rb
+++ b/ee/spec/services/ee/issues/update_service_spec.rb
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 require 'spec_helper'
 
 describe Issues::UpdateService do
diff --git a/ee/spec/services/milestones/update_service_spec.rb b/ee/spec/services/milestones/update_service_spec.rb
index 44240bbef713b6..1d140b42466c00 100644
--- a/ee/spec/services/milestones/update_service_spec.rb
+++ b/ee/spec/services/milestones/update_service_spec.rb
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 require 'spec_helper'
 
 describe Milestones::UpdateService do
diff --git a/spec/services/milestones/update_service_spec.rb b/spec/services/milestones/update_service_spec.rb
index ac94c39c8a5016..33ec02e17aaa3a 100644
--- a/spec/services/milestones/update_service_spec.rb
+++ b/spec/services/milestones/update_service_spec.rb
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 require 'spec_helper'
 
 describe Milestones::UpdateService do
-- 
GitLab


From fe7a76111f8c432d8105d16f6c53d6b80db14a7b Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 31 Jul 2018 17:09:44 +0800
Subject: [PATCH 23/54] Add date sourcing milestone titles in meta data for
 rendering

---
 ee/app/helpers/epics_helper.rb       |  4 +++-
 ee/app/models/ee/epic.rb             |  2 ++
 ee/spec/helpers/epics_helper_spec.rb | 21 +++++++++++++++++++--
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/ee/app/helpers/epics_helper.rb b/ee/app/helpers/epics_helper.rb
index 045793b84ea93a..97789c3c7380ad 100644
--- a/ee/app/helpers/epics_helper.rb
+++ b/ee/app/helpers/epics_helper.rb
@@ -27,9 +27,11 @@ def epic_show_app_data(epic, opts)
         start_date_fixed: epic.start_date_fixed,
         start_date_is_fixed: epic.start_date_is_fixed?,
         start_date_from_milestones: epic.start_date_from_milestones,
+        start_date_sourcing_milestone_title: epic.start_date_sourcing_milestone&.title,
         due_date_fixed: epic.due_date_fixed,
         due_date_is_fixed: epic.due_date_is_fixed?,
-        due_date_from_milestones: epic.due_date_from_milestones
+        due_date_from_milestones: epic.due_date_from_milestones,
+        due_date_sourcing_milestone_title: epic.due_date_sourcing_milestone&.title
       )
     end
 
diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index d17e128599cbe0..30dc66b677fb8a 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -13,6 +13,8 @@ module Epic
 
       belongs_to :assignee, class_name: "User"
       belongs_to :group
+      belongs_to :start_date_sourcing_milestone, class_name: 'Milestone'
+      belongs_to :due_date_sourcing_milestone, class_name: 'Milestone'
 
       has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.epics&.maximum(:iid) }
 
diff --git a/ee/spec/helpers/epics_helper_spec.rb b/ee/spec/helpers/epics_helper_spec.rb
index 6ec212cdfba990..d0e03996deb259 100644
--- a/ee/spec/helpers/epics_helper_spec.rb
+++ b/ee/spec/helpers/epics_helper_spec.rb
@@ -28,6 +28,19 @@
     end
 
     context 'when user has edit permission' do
+      let(:milestone) { create(:milestone, title: 'make me a sandwich') }
+
+      let!(:epic) do
+        create(
+          :epic,
+          author: user,
+          start_date_sourcing_milestone: milestone,
+          start_date: Date.new(2000, 1, 1),
+          due_date_sourcing_milestone: milestone,
+          due_date: Date.new(2000, 1, 2)
+        )
+      end
+
       before do
         epic.group.add_developer(user)
       end
@@ -38,9 +51,13 @@
 
         expect(meta_data.keys).to match_array(%w[
           created author
-          start_date start_date_fixed start_date_is_fixed start_date_from_milestones
-          end_date due_date due_date_fixed due_date_is_fixed due_date_from_milestones
+          start_date start_date_fixed start_date_is_fixed start_date_from_milestones start_date_sourcing_milestone_title
+          end_date due_date due_date_fixed due_date_is_fixed due_date_from_milestones due_date_sourcing_milestone_title
         ])
+        expect(meta_data['start_date']).to eq('2000-01-01')
+        expect(meta_data['start_date_sourcing_milestone_title']).to eq(milestone.title)
+        expect(meta_data['due_date']).to eq('2000-01-02')
+        expect(meta_data['due_date_sourcing_milestone_title']).to eq(milestone.title)
       end
     end
   end
-- 
GitLab


From 5afd84816fc2b6fc22828488911d45f111e0a51f Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 31 Jul 2018 17:26:56 +0800
Subject: [PATCH 24/54] Add test for dates update

---
 ee/spec/requests/api/epics_spec.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb
index 403c6d3160287f..eabbc8b9878ac5 100644
--- a/ee/spec/requests/api/epics_spec.rb
+++ b/ee/spec/requests/api/epics_spec.rb
@@ -257,7 +257,7 @@ def expect_array_response(expected)
 
   describe 'PUT /groups/:id/epics/:epic_iid' do
     let(:url) { "/groups/#{group.path}/epics/#{epic.iid}" }
-    let(:params) { { title: 'new title', description: 'new description', labels: 'label2' } }
+    let(:params) { { title: 'new title', description: 'new description', labels: 'label2', start_date_fixed: "2018-07-17", start_date_is_fixed: true } }
 
     it_behaves_like 'error requests'
 
@@ -305,6 +305,9 @@ def expect_array_response(expected)
           expect(result.title).to eq('new title')
           expect(result.description).to eq('new description')
           expect(result.labels.first.title).to eq('label2')
+          expect(result.start_date).to eq(Date.new(2018, 7, 17))
+          expect(result.start_date_fixed).to eq(Date.new(2018, 7, 17))
+          expect(result.start_date_is_fixed).to eq(true)
         end
 
         context 'when deprecated start_date and end_date params are present' do
-- 
GitLab


From db9629b37e7c4f88e687e382f780908b9ad31b5e Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 31 Jul 2018 20:13:10 +0800
Subject: [PATCH 25/54] Allow update dates at web controller

---
 ee/app/controllers/groups/epics_controller.rb       | 6 ++++--
 ee/app/serializers/epic_entity.rb                   | 8 +++++++-
 ee/spec/controllers/groups/epics_controller_spec.rb | 4 +++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/ee/app/controllers/groups/epics_controller.rb b/ee/app/controllers/groups/epics_controller.rb
index 6d13b87dd05ed9..dbf81689f36dea 100644
--- a/ee/app/controllers/groups/epics_controller.rb
+++ b/ee/app/controllers/groups/epics_controller.rb
@@ -66,8 +66,10 @@ def epic_params_attributes
     [
       :title,
       :description,
-      :start_date,
-      :end_date,
+      :start_date_fixed,
+      :start_date_is_fixed,
+      :due_date_fixed,
+      :due_date_is_fixed,
       label_ids: []
     ]
   end
diff --git a/ee/app/serializers/epic_entity.rb b/ee/app/serializers/epic_entity.rb
index 031e5a618f7751..0f3f24cc270d3f 100644
--- a/ee/app/serializers/epic_entity.rb
+++ b/ee/app/serializers/epic_entity.rb
@@ -6,8 +6,14 @@ class EpicEntity < IssuableEntity
   expose :group_full_name do |epic|
     epic.group.full_name
   end
+
   expose :start_date
-  expose :end_date
+  expose :start_date_is_fixed?, as: :start_date_is_fixed
+  expose :start_date_fixed, :start_date_from_milestones
+  expose :end_date, as: :due_date
+  expose :due_date_is_fixed?, as: :due_date_is_fixed
+  expose :due_date_fixed, :due_date_from_milestones
+
   expose :web_url do |epic|
     group_epic_path(epic.group, epic)
   end
diff --git a/ee/spec/controllers/groups/epics_controller_spec.rb b/ee/spec/controllers/groups/epics_controller_spec.rb
index f29d1217b5a1f8..9352a053622f3a 100644
--- a/ee/spec/controllers/groups/epics_controller_spec.rb
+++ b/ee/spec/controllers/groups/epics_controller_spec.rb
@@ -183,7 +183,7 @@ def show_epic(format = :html)
     describe 'PUT #update' do
       before do
         group.add_developer(user)
-        put :update, group_id: group, id: epic.to_param, epic: { title: 'New title', label_ids: [label.id] }, format: :json
+        put :update, group_id: group, id: epic.to_param, epic: { title: 'New title', label_ids: [label.id], start_date_fixed: '2002-01-01', start_date_is_fixed: true }, format: :json
       end
 
       it 'returns status 200' do
@@ -195,6 +195,8 @@ def show_epic(format = :html)
 
         expect(epic.title).to eq('New title')
         expect(epic.labels).to eq([label])
+        expect(epic.start_date_fixed).to eq(Date.new(2002, 1, 1))
+        expect(epic.start_date_is_fixed).to eq(true)
       end
     end
 
-- 
GitLab


From e23ee67a8a1b8d9808f757778a011c80126e1f18 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 31 Jul 2018 20:34:26 +0800
Subject: [PATCH 26/54] Fix bug where start_date is not updated at web

---
 ee/app/controllers/groups/epics_controller.rb       | 2 +-
 ee/app/services/epics/update_service.rb             | 2 +-
 ee/spec/controllers/groups/epics_controller_spec.rb | 5 ++++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/ee/app/controllers/groups/epics_controller.rb b/ee/app/controllers/groups/epics_controller.rb
index dbf81689f36dea..cdc2a51fc45339 100644
--- a/ee/app/controllers/groups/epics_controller.rb
+++ b/ee/app/controllers/groups/epics_controller.rb
@@ -83,7 +83,7 @@ def discussion_serializer
   end
 
   def update_service
-    ::Epics::UpdateService.new(@group, current_user, epic_params)
+    ::Epics::UpdateService.new(@group, current_user, epic_params.stringify_keys)
   end
 
   def finder_type
diff --git a/ee/app/services/epics/update_service.rb b/ee/app/services/epics/update_service.rb
index 89fa715eeec828..8cf09e0c5d5825 100644
--- a/ee/app/services/epics/update_service.rb
+++ b/ee/app/services/epics/update_service.rb
@@ -7,7 +7,7 @@ def execute(epic)
 
       update(epic)
 
-      if (params.keys & [:start_date_fixed, :start_date_is_fixed, :due_date_fixed, :due_date_is_fixed]).present?
+      if (params.keys.map(&:to_sym) & [:start_date_fixed, :start_date_is_fixed, :due_date_fixed, :due_date_is_fixed]).present?
         epic.update_dates
       end
 
diff --git a/ee/spec/controllers/groups/epics_controller_spec.rb b/ee/spec/controllers/groups/epics_controller_spec.rb
index 9352a053622f3a..102ea787e0a587 100644
--- a/ee/spec/controllers/groups/epics_controller_spec.rb
+++ b/ee/spec/controllers/groups/epics_controller_spec.rb
@@ -181,6 +181,8 @@ def show_epic(format = :html)
     end
 
     describe 'PUT #update' do
+      let(:date) { Date.new(2002, 1, 1)}
+
       before do
         group.add_developer(user)
         put :update, group_id: group, id: epic.to_param, epic: { title: 'New title', label_ids: [label.id], start_date_fixed: '2002-01-01', start_date_is_fixed: true }, format: :json
@@ -195,7 +197,8 @@ def show_epic(format = :html)
 
         expect(epic.title).to eq('New title')
         expect(epic.labels).to eq([label])
-        expect(epic.start_date_fixed).to eq(Date.new(2002, 1, 1))
+        expect(epic.start_date_fixed).to eq(date)
+        expect(epic.start_date).to eq(date)
         expect(epic.start_date_is_fixed).to eq(true)
       end
     end
-- 
GitLab


From 8fbc88b788a7bce3f630e50ce872adb74381f587 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 31 Jul 2018 21:48:05 +0800
Subject: [PATCH 27/54] Fix migration to work with existing end_date column

---
 .../20180713171825_update_epic_dates_from_milestones.rb         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 56c69952675c3e..5532dfd36bc647 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -16,7 +16,7 @@ def update_dates
 
       self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_data[:start_date]
       self.start_date_sourcing_milestone_id = milestone_data[:start_date_sourcing_milestone_id]
-      self.due_date = due_date_is_fixed? ? due_date_fixed : milestone_data[:due_date]
+      self.end_date = due_date_is_fixed? ? due_date_fixed : milestone_data[:due_date]
       self.due_date_sourcing_milestone_id = milestone_data[:due_date_sourcing_milestone_id]
 
       save if changed?
-- 
GitLab


From 141d7d11aa140552dfcf5faad22374381e5aff7b Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 1 Aug 2018 10:01:23 +0800
Subject: [PATCH 28/54] Fix spec by matching entity with schema

---
 ee/app/serializers/epic_entity.rb               | 1 +
 ee/spec/fixtures/api/schemas/entities/epic.json | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/ee/app/serializers/epic_entity.rb b/ee/app/serializers/epic_entity.rb
index 0f3f24cc270d3f..68fa5e82be29b9 100644
--- a/ee/app/serializers/epic_entity.rb
+++ b/ee/app/serializers/epic_entity.rb
@@ -10,6 +10,7 @@ class EpicEntity < IssuableEntity
   expose :start_date
   expose :start_date_is_fixed?, as: :start_date_is_fixed
   expose :start_date_fixed, :start_date_from_milestones
+  expose :end_date # @deprecated
   expose :end_date, as: :due_date
   expose :due_date_is_fixed?, as: :due_date_is_fixed
   expose :due_date_fixed, :due_date_from_milestones
diff --git a/ee/spec/fixtures/api/schemas/entities/epic.json b/ee/spec/fixtures/api/schemas/entities/epic.json
index 8d4615816150ce..6ab2d6ae653fa9 100644
--- a/ee/spec/fixtures/api/schemas/entities/epic.json
+++ b/ee/spec/fixtures/api/schemas/entities/epic.json
@@ -12,7 +12,14 @@
     "milestone_id": { "type": ["string", "null"] },
     "state": { "type": "string" },
     "start_date": { "type": ["date", "null"] },
+    "start_date_fixed": { "type": ["date", "null"] },
+    "start_date_is_fixed": { "type": "boolean" },
+    "start_date_from_milestones": { "type": ["date", "null"] },
     "end_date": { "type": ["date", "null"] },
+    "due_date": { "type": ["date", "null"] },
+    "due_date_fixed": { "type": ["date", "null"] },
+    "due_date_from_milestones": { "type": ["date", "null"] },
+    "due_date_is_fixed": { "type": "boolean" },
     "updated_by_id": { "type": ["string", "null"] },
     "created_at": { "type": "string" },
     "updated_at": { "type": "string" },
-- 
GitLab


From 13fac290b421410afd84139a25bfcaa90bb23c6e Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 1 Aug 2018 10:18:09 +0800
Subject: [PATCH 29/54] Refactor epic schema to use date as type

---
 .../api/schemas/public_api/v4/epic.json        | 18 +++++++++---------
 .../schemas/public_api/v4/epic_issue_link.json | 18 +++++++++---------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/epic.json b/ee/spec/fixtures/api/schemas/public_api/v4/epic.json
index 3b2caee33f67c9..a171a7f658339b 100644
--- a/ee/spec/fixtures/api/schemas/public_api/v4/epic.json
+++ b/ee/spec/fixtures/api/schemas/public_api/v4/epic.json
@@ -24,15 +24,15 @@
         "type": "string"
       }
     },
-    "start_date": { "type": ["string", "null"] },
-    "start_date_fixed": { "type": ["string", "null"] },
-    "start_date_from_milestones": { "type": ["string", "null"] },
-    "start_date_is_fixed": { "type": ["boolean"] },
-    "end_date": { "type": ["string", "null"] },
-    "due_date": { "type": ["string", "null"] },
-    "due_date_fixed": { "type": ["string", "null"] },
-    "due_date_from_milestones": { "type": ["string", "null"] },
-    "due_date_is_fixed": { "type": ["boolean"] },
+    "start_date": { "type": ["date", "null"] },
+    "start_date_fixed": { "type": ["date", "null"] },
+    "start_date_from_milestones": { "type": ["date", "null"] },
+    "start_date_is_fixed": { "type": "boolean" },
+    "end_date": { "type": ["date", "null"] },
+    "due_date": { "type": ["date", "null"] },
+    "due_date_fixed": { "type": ["date", "null"] },
+    "due_date_from_milestones": { "type": ["date", "null"] },
+    "due_date_is_fixed": { "type": "boolean" },
     "created_at": { "type": ["string", "null"] },
     "updated_at": { "type": ["string", "null"] }
   },
diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json b/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json
index 34445d6dbcc37c..63c89cc791c97c 100644
--- a/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json
+++ b/ee/spec/fixtures/api/schemas/public_api/v4/epic_issue_link.json
@@ -18,15 +18,15 @@
         "group_id": { "type": "integer" },
         "description": { "type": ["string", "null"] },
         "author": { "type": ["object", "null"] },
-        "start_date": { "type": ["string", "null"] },
-        "start_date_fixed": { "type": ["string", "null"] },
-        "start_date_from_milestones": { "type": ["string", "null"] },
-        "start_date_is_fixed": { "type": ["boolean"] },
-        "end_date": { "type": ["string", "null"] },
-        "due_date": { "type": ["string", "null"] },
-        "due_date_fixed": { "type": ["string", "null"] },
-        "due_date_from_milestones": { "type": ["string", "null"] },
-        "due_date_is_fixed": { "type": ["boolean"] },
+        "start_date": { "type": ["date", "null"] },
+        "start_date_fixed": { "type": ["date", "null"] },
+        "start_date_from_milestones": { "type": ["date", "null"] },
+        "start_date_is_fixed": { "type": "boolean" },
+        "end_date": { "type": ["date", "null"] },
+        "due_date": { "type": ["date", "null"] },
+        "due_date_fixed": { "type": ["date", "null"] },
+        "due_date_from_milestones": { "type": ["date", "null"] },
+        "due_date_is_fixed": { "type": "boolean" },
         "created_at": { "type": ["string", "null"] },
         "updated_at": { "type": ["string", "null"] },
         "labels": {
-- 
GitLab


From dfe844e9e694d47328087a9eb1ce3cd58330de93 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 1 Aug 2018 10:52:18 +0800
Subject: [PATCH 30/54] Fix migration ordering

---
 db/schema.rb                                              | 8 ++++----
 ee/db/migrate/20180711014025_add_date_columns_to_epics.rb | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/db/schema.rb b/db/schema.rb
index 62b0e8a2d8246a..27717e0fb65fac 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -954,12 +954,12 @@
     t.string "title_html", null: false
     t.text "description"
     t.text "description_html"
-    t.boolean "start_date_is_fixed"
-    t.date "start_date_fixed"
     t.integer "start_date_sourcing_milestone_id"
-    t.boolean "due_date_is_fixed"
-    t.date "due_date_fixed"
     t.integer "due_date_sourcing_milestone_id"
+    t.date "start_date_fixed"
+    t.date "due_date_fixed"
+    t.boolean "start_date_is_fixed"
+    t.boolean "due_date_is_fixed"
   end
 
   add_index "epics", ["assignee_id"], name: "index_epics_on_assignee_id", using: :btree
diff --git a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
index c3b8c5fd2ad1d7..ae4e0ac31606cb 100644
--- a/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
+++ b/ee/db/migrate/20180711014025_add_date_columns_to_epics.rb
@@ -7,12 +7,12 @@ class AddDateColumnsToEpics < ActiveRecord::Migration
 
   def change
     change_table :epics do |t|
-      t.boolean :start_date_is_fixed
-      t.date :start_date_fixed
       t.references :start_date_sourcing_milestone
-      t.boolean :due_date_is_fixed
-      t.date :due_date_fixed
       t.references :due_date_sourcing_milestone
+      t.date :start_date_fixed
+      t.date :due_date_fixed
+      t.boolean :start_date_is_fixed
+      t.boolean :due_date_is_fixed
     end
   end
 end
-- 
GitLab


From fa2273cfd0c146725974e422322212eb6827ee9c Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 1 Aug 2018 11:58:06 +0800
Subject: [PATCH 31/54] Fix migration to cover end_date nil cases

---
 .../20180713171825_update_epic_dates_from_milestones.rb         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 5532dfd36bc647..0732e042ee48d5 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -56,7 +56,7 @@ def fetch_milestone_date_data
   end
 
   def up
-    Epic.where(start_date: nil).find_each do |epic|
+    Epic.where('start_date IS NULL OR end_date IS NULL').find_each do |epic|
       epic.update_dates
     end
   end
-- 
GitLab


From 46cde392218345bd2068226cec66de39b512a319 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Wed, 1 Aug 2018 14:00:12 +0800
Subject: [PATCH 32/54] Update epic as long as it is related to milestone

in order to fill _milestone_id columns
---
 .../20180713171825_update_epic_dates_from_milestones.rb      | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 0732e042ee48d5..b49a5f45085d6b 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -11,6 +11,9 @@ class Epic < ActiveRecord::Base
     self.table_name = 'epics'
     include EachBatch
 
+    has_many :epic_issues
+    has_many :issues, through: :epic_issues
+
     def update_dates
       milestone_data = fetch_milestone_date_data
 
@@ -56,7 +59,7 @@ def fetch_milestone_date_data
   end
 
   def up
-    Epic.where('start_date IS NULL OR end_date IS NULL').find_each do |epic|
+    Epic.joins(:issues).where('issues.milestone_id IS NOT NULL').find_each do |epic|
       epic.update_dates
     end
   end
-- 
GitLab


From ed0c9424465066c390df85625e2fa7cb05da3f29 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 2 Aug 2018 10:33:11 +0800
Subject: [PATCH 33/54] revert change

---
 ee/app/controllers/groups/epics_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/app/controllers/groups/epics_controller.rb b/ee/app/controllers/groups/epics_controller.rb
index cdc2a51fc45339..dbf81689f36dea 100644
--- a/ee/app/controllers/groups/epics_controller.rb
+++ b/ee/app/controllers/groups/epics_controller.rb
@@ -83,7 +83,7 @@ def discussion_serializer
   end
 
   def update_service
-    ::Epics::UpdateService.new(@group, current_user, epic_params.stringify_keys)
+    ::Epics::UpdateService.new(@group, current_user, epic_params)
   end
 
   def finder_type
-- 
GitLab


From be4b01a8bcb5a474606347786b1d895fb7859f78 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 2 Aug 2018 13:24:19 +0800
Subject: [PATCH 34/54] Bulk update epics when milestone date changes

---
 ee/app/models/ee/epic.rb                      |  2 --
 .../services/ee/milestones/update_service.rb  | 34 +++++++++++++++++--
 .../milestones/update_service_spec.rb         |  3 ++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 30dc66b677fb8a..6b0a605acf48b7 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -170,8 +170,6 @@ def banzai_render_context(field)
       super.merge(label_url_method: :group_epics_url)
     end
 
-    private
-
     def fetch_milestone_date_data
       sql = <<~SQL
         SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
index 964321e0cb772f..c14a5ce45fa47e 100644
--- a/ee/app/services/ee/milestones/update_service.rb
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -9,12 +9,42 @@ module UpdateService
       def execute(milestone)
         super
 
-        ::Epic.joins(:issues).where(issues: { milestone_id: milestone.id }).each do |epic|
-          epic.update_dates
+        if milestone.previous_changes.key?(:start_date) || milestone.previous_changes.key?(:due_date)
+          update_epics(milestone)
         end
 
         milestone
       end
+
+      private
+
+      def update_epics(milestone)
+        groups = ::Epic.joins(:issues).includes(:issues).where(issues: { milestone_id: milestone.id }).group_by do |epic|
+          milestone_ids = epic.issues.map(&:milestone_id)
+          milestone_ids.compact!
+          milestone_ids.uniq!
+          milestone_ids
+        end
+
+        groups.each do |milestone_ids, epics|
+          data = epics.first.fetch_milestone_date_data
+
+          ::Epic.where(id: epics.map(&:id)).update_all(
+            [
+              %{
+                start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
+                start_date_sourcing_milestone_id = ?,
+                end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
+                due_date_sourcing_milestone_id = ?
+              },
+              data[:start_date],
+              data[:start_date_sourcing_milestone_id],
+              data[:due_date],
+              data[:due_date_sourcing_milestone_id]
+            ]
+          )
+        end
+      end
     end
   end
 end
diff --git a/ee/spec/services/milestones/update_service_spec.rb b/ee/spec/services/milestones/update_service_spec.rb
index 1d140b42466c00..53380b52e4d142 100644
--- a/ee/spec/services/milestones/update_service_spec.rb
+++ b/ee/spec/services/milestones/update_service_spec.rb
@@ -17,7 +17,10 @@
 
         epic.reload
 
+        expect(epic.start_date).to eq(nil)
+        expect(epic.start_date_sourcing_milestone).to eq(nil)
         expect(epic.due_date).to eq(due_date)
+        expect(epic.due_date_sourcing_milestone).to eq(milestone)
       end
     end
   end
-- 
GitLab


From e65b1ea328b2892eab2ec9b57e29cfdc9d5e4fbb Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 2 Aug 2018 16:00:43 +0800
Subject: [PATCH 35/54] Refactor bulk update by moving it to Epic

---
 ee/app/models/ee/epic.rb                      | 30 ++++++++
 .../services/ee/milestones/update_service.rb  | 34 +-------
 ee/spec/models/epic_spec.rb                   | 77 +++++++++++++++++++
 3 files changed, 110 insertions(+), 31 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 6b0a605acf48b7..bd979489599b02 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -81,6 +81,36 @@ def order_by(method)
       def parent_class
         ::Group
       end
+
+      def update_dates(epics)
+        groups = epics.includes(:issues).group_by do |epic|
+          milestone_ids = epic.issues.map(&:milestone_id)
+          milestone_ids.compact!
+          milestone_ids.uniq!
+          milestone_ids
+        end
+
+        groups.each do |milestone_ids, epics|
+          next if milestone_ids.empty?
+
+          data = epics.first.fetch_milestone_date_data
+
+          self.where(id: epics.map(&:id)).update_all(
+            [
+              %{
+                start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
+                start_date_sourcing_milestone_id = ?,
+                end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
+                due_date_sourcing_milestone_id = ?
+              },
+              data[:start_date],
+              data[:start_date_sourcing_milestone_id],
+              data[:due_date],
+              data[:due_date_sourcing_milestone_id]
+            ]
+          )
+        end
+      end
     end
 
     def assignees
diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
index c14a5ce45fa47e..ceb687612e49bf 100644
--- a/ee/app/services/ee/milestones/update_service.rb
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -10,41 +10,13 @@ def execute(milestone)
         super
 
         if milestone.previous_changes.key?(:start_date) || milestone.previous_changes.key?(:due_date)
-          update_epics(milestone)
+          ::Epic.update_dates(
+            ::Epic.joins(:issues).where(issues: { milestone_id: milestone.id })
+          )
         end
 
         milestone
       end
-
-      private
-
-      def update_epics(milestone)
-        groups = ::Epic.joins(:issues).includes(:issues).where(issues: { milestone_id: milestone.id }).group_by do |epic|
-          milestone_ids = epic.issues.map(&:milestone_id)
-          milestone_ids.compact!
-          milestone_ids.uniq!
-          milestone_ids
-        end
-
-        groups.each do |milestone_ids, epics|
-          data = epics.first.fetch_milestone_date_data
-
-          ::Epic.where(id: epics.map(&:id)).update_all(
-            [
-              %{
-                start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
-                start_date_sourcing_milestone_id = ?,
-                end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
-                due_date_sourcing_milestone_id = ?
-              },
-              data[:start_date],
-              data[:start_date_sourcing_milestone_id],
-              data[:due_date],
-              data[:due_date_sourcing_milestone_id]
-            ]
-          )
-        end
-      end
     end
   end
 end
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index a950cc6de7f745..d152c3373e4ca8 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -336,6 +336,83 @@
     end
   end
 
+  describe '.update_dates' do
+    def link_epic_to_milestone(epic, milestone)
+      create(:issue, epic: epic, milestone: milestone)
+    end
+
+    it 'updates in bulk' do
+      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
+      milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30))
+
+      epics = [
+        create(:epic),
+        create(:epic),
+        create(:epic, :use_fixed_dates)
+      ]
+      old_attributes = epics.map(&:attributes)
+
+      link_epic_to_milestone(epics[0], milestone1)
+      link_epic_to_milestone(epics[0], milestone2)
+      link_epic_to_milestone(epics[1], milestone2)
+      link_epic_to_milestone(epics[2], milestone1)
+      link_epic_to_milestone(epics[2], milestone2)
+
+      described_class.update_dates(described_class.where(id: epics.map(&:id)))
+
+      epics.each(&:reload)
+
+      expect(epics[0].start_date).to eq(milestone1.start_date)
+      expect(epics[0].start_date_sourcing_milestone).to eq(milestone1)
+      expect(epics[0].due_date).to eq(milestone2.due_date)
+      expect(epics[0].due_date_sourcing_milestone).to eq(milestone2)
+
+      expect(epics[1].start_date).to eq(nil)
+      expect(epics[1].start_date_sourcing_milestone).to eq(nil)
+      expect(epics[1].due_date).to eq(milestone2.due_date)
+      expect(epics[1].due_date_sourcing_milestone).to eq(milestone2)
+
+      expect(epics[2].start_date).to eq(old_attributes[2]['start_date'])
+      expect(epics[2].start_date_sourcing_milestone).to eq(milestone1)
+      expect(epics[2].due_date).to eq(old_attributes[2]['end_date'])
+      expect(epics[2].due_date_sourcing_milestone).to eq(milestone2)
+    end
+
+    context 'query count check' do
+      let(:milestone) { create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10)) }
+      let!(:epics) { [ create(:epic) ] }
+
+      def setup_control_group
+        link_epic_to_milestone(epics[0], milestone)
+
+        ActiveRecord::QueryRecorder.new do
+          described_class.update_dates(described_class.where(id: epics.map(&:id)))
+        end.count
+      end
+
+      it 'does not increase query count when adding epics without milestones' do
+        control_count = setup_control_group
+
+        epics << create(:epic)
+
+        expect do
+          described_class.update_dates(described_class.where(id: epics.map(&:id)))
+        end.not_to exceed_query_limit(control_count)
+      end
+
+      it 'does not increase query count when adding epics belongs to same milestones' do
+        control_count = setup_control_group
+
+        epics << create(:epic)
+        link_epic_to_milestone(epics[1], milestone)
+
+        expect do
+          described_class.update_dates(described_class.where(id: epics.map(&:id)))
+        end.not_to exceed_query_limit(control_count)
+      end
+    end
+  end
+
   describe '#issues_readable_by' do
     let(:user) { create(:user) }
     let(:group) { create(:group, :private) }
-- 
GitLab


From 1c94320d84cbb5071e9df797a32fea9158933da8 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 2 Aug 2018 16:10:50 +0800
Subject: [PATCH 36/54] Batch update in migration

---
 ...71825_update_epic_dates_from_milestones.rb | 39 +++++++++++++------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index b49a5f45085d6b..9593588e004851 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -14,18 +14,35 @@ class Epic < ActiveRecord::Base
     has_many :epic_issues
     has_many :issues, through: :epic_issues
 
-    def update_dates
-      milestone_data = fetch_milestone_date_data
+    def self.update_dates(epics)
+      groups = epics.includes(:issues).group_by do |epic|
+        milestone_ids = epic.issues.map(&:milestone_id)
+        milestone_ids.compact!
+        milestone_ids.uniq!
+        milestone_ids
+      end
 
-      self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_data[:start_date]
-      self.start_date_sourcing_milestone_id = milestone_data[:start_date_sourcing_milestone_id]
-      self.end_date = due_date_is_fixed? ? due_date_fixed : milestone_data[:due_date]
-      self.due_date_sourcing_milestone_id = milestone_data[:due_date_sourcing_milestone_id]
+      groups.each do |milestone_ids, epics|
+        next if milestone_ids.empty?
 
-      save if changed?
-    end
+        data = epics.first.fetch_milestone_date_data
 
-    private
+        self.where(id: epics.map(&:id)).update_all(
+          [
+            %{
+                start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
+                start_date_sourcing_milestone_id = ?,
+                end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
+                due_date_sourcing_milestone_id = ?
+              },
+            data[:start_date],
+            data[:start_date_sourcing_milestone_id],
+            data[:due_date],
+            data[:due_date_sourcing_milestone_id]
+          ]
+        )
+      end
+    end
 
     def fetch_milestone_date_data
       sql = <<~SQL
@@ -59,8 +76,8 @@ def fetch_milestone_date_data
   end
 
   def up
-    Epic.joins(:issues).where('issues.milestone_id IS NOT NULL').find_each do |epic|
-      epic.update_dates
+    Epic.joins(:issues).where('issues.milestone_id IS NOT NULL').each_batch do |epics|
+      Epic.update_dates(epics)
     end
   end
 
-- 
GitLab


From 280dab0d8b4b3f42ffae2c816704e8243a0289e2 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Thu, 2 Aug 2018 19:57:05 +0800
Subject: [PATCH 37/54] Fix MySQL order null value to the front

---
 ee/app/models/ee/epic.rb                                 | 9 ++++++---
 .../20180713171825_update_epic_dates_from_milestones.rb  | 9 ++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index bd979489599b02..d330354be164c2 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -213,17 +213,20 @@ def fetch_milestone_date_data
           WHERE epic_issues.epic_id = #{id}
         ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
         WHERE epic_issues.epic_id = #{id}
-        ORDER BY milestones.start_date, milestones.due_date;
       SQL
 
       db_results = ActiveRecord::Base.connection.select_all(sql).to_a
 
       results = {}
-      db_results.find { |row| row['start_date'] }&.tap do |row|
+      db_results
+        .reject { |row| row['start_date'].nil? }
+        .min_by { |row| row['start_date'] }&.tap do |row|
         results[:start_date] = row['start_date']
         results[:start_date_sourcing_milestone_id] = row['id']
       end
-      db_results.reverse.find { |row| row['due_date'] }&.tap do |row|
+      db_results
+        .reject { |row| row['due_date'].nil? }
+        .max_by { |row| row['due_date'] }&.tap do |row|
         results[:due_date] = row['due_date']
         results[:due_date_sourcing_milestone_id] = row['id']
       end
diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 9593588e004851..053a97409e5d9c 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -57,17 +57,20 @@ def fetch_milestone_date_data
           WHERE epic_issues.epic_id = #{id}
         ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
         WHERE epic_issues.epic_id = #{id}
-        ORDER BY milestones.start_date, milestones.due_date;
       SQL
 
       db_results = ActiveRecord::Base.connection.select_all(sql).to_a
 
       results = {}
-      db_results.find { |row| row['start_date'] }&.tap do |row|
+      db_results
+        .reject { |row| row['start_date'].nil? }
+        .min_by { |row| row['start_date'] }&.tap do |row|
         results[:start_date] = row['start_date']
         results[:start_date_sourcing_milestone_id] = row['id']
       end
-      db_results.reverse.find { |row| row['due_date'] }&.tap do |row|
+      db_results
+        .reject { |row| row['due_date'].nil? }
+        .max_by { |row| row['due_date'] }&.tap do |row|
         results[:due_date] = row['due_date']
         results[:due_date_sourcing_milestone_id] = row['id']
       end
-- 
GitLab


From bbe2bc71df551a9c418524cd05194a5cb0283f12 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 13:42:23 +0800
Subject: [PATCH 38/54] Use previous_changes instead

---
 ee/app/services/ee/issues/update_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/app/services/ee/issues/update_service.rb b/ee/app/services/ee/issues/update_service.rb
index 03d90eb7a7aec1..e5affbc4d6943c 100644
--- a/ee/app/services/ee/issues/update_service.rb
+++ b/ee/app/services/ee/issues/update_service.rb
@@ -9,7 +9,7 @@ module UpdateService
       def execute(issue)
         result = super
 
-        if (params.include?(:milestone) || params.include?(:milestone_id)) && issue.epic
+        if issue.previous_changes.include?(:milestone_id) && issue.epic
           issue.epic.update_dates
         end
 
-- 
GitLab


From 821b14bee169b96da0c4cba016992b2b7aede69c Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 13:42:34 +0800
Subject: [PATCH 39/54] Improve naming

---
 ee/lib/ee/api/entities.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb
index 810f515728ec3c..75bdcb3fad8e0a 100644
--- a/ee/lib/ee/api/entities.rb
+++ b/ee/lib/ee/api/entities.rb
@@ -155,7 +155,7 @@ class RelatedIssue < ::API::Entities::Issue
       end
 
       class Epic < Grape::Entity
-        allowed_to_admin = ->(epic, opts) { Ability.allowed?(opts[:user], :admin_epic, epic) }
+        can_admin_epic = ->(epic, opts) { Ability.allowed?(opts[:user], :admin_epic, epic) }
 
         expose :id
         expose :iid
@@ -164,12 +164,12 @@ class Epic < Grape::Entity
         expose :description
         expose :author, using: ::API::Entities::UserBasic
         expose :start_date
-        expose :start_date_is_fixed?, as: :start_date_is_fixed, if: allowed_to_admin
-        expose :start_date_fixed, :start_date_from_milestones, if: allowed_to_admin
+        expose :start_date_is_fixed?, as: :start_date_is_fixed, if: can_admin_epic
+        expose :start_date_fixed, :start_date_from_milestones, if: can_admin_epic
         expose :end_date # @deprecated
         expose :end_date, as: :due_date
-        expose :due_date_is_fixed?, as: :due_date_is_fixed, if: allowed_to_admin
-        expose :due_date_fixed, :due_date_from_milestones, if: allowed_to_admin
+        expose :due_date_is_fixed?, as: :due_date_is_fixed, if: can_admin_epic
+        expose :due_date_fixed, :due_date_from_milestones, if: can_admin_epic
         expose :created_at
         expose :updated_at
         expose :labels do |epic, options|
-- 
GitLab


From 1d5f62641a76423bc1a968c6a8a430773ca43be2 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 16:49:31 +0800
Subject: [PATCH 40/54] Use stored milestone references to obtain milestone
 dates

---
 ee/app/models/ee/epic.rb    |  4 +--
 ee/spec/models/epic_spec.rb | 68 +++++++++----------------------------
 2 files changed, 18 insertions(+), 54 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index d330354be164c2..2df3ce5c1b6d00 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -155,12 +155,12 @@ def update_dates
 
     # Earliest start date from issues' milestones
     def start_date_from_milestones
-      start_date_is_fixed? ? epic_issues.joins(issue: :milestone).minimum('milestones.start_date') : start_date
+      start_date_is_fixed? ? start_date_sourcing_milestone.start_date : start_date
     end
 
     # Latest end date from issues' milestones
     def due_date_from_milestones
-      due_date_is_fixed? ? epic_issues.joins(issue: :milestone).maximum('milestones.due_date') : due_date
+      due_date_is_fixed? ? due_date_sourcing_milestone.due_date : due_date
     end
 
     def to_reference(from = nil, full: false)
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index d152c3373e4ca8..7b838205a7d743 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -99,76 +99,40 @@
 
   describe '#start_date_from_milestones' do
     context 'fixed date' do
-      subject { create(:epic, :use_fixed_dates) }
-      let(:date) { Date.new(2017, 3, 4) }
+      it 'returns start date from start date sourcing milestone' do
+        subject = create(:epic, :use_fixed_dates)
+        milestone = create(:milestone, :with_dates)
+        subject.start_date_sourcing_milestone = milestone
 
-      before do
-        milestone1 = create(
-          :milestone,
-          start_date: date,
-          due_date: date + 10.days
-        )
-        epic_issue1 = create(:epic_issue, epic: subject)
-        epic_issue1.issue.update(milestone: milestone1)
-
-        milestone2 = create(
-          :milestone,
-          start_date: date + 5.days,
-          due_date: date + 15.days
-        )
-        epic_issue2 = create(:epic_issue, epic: subject)
-        epic_issue2.issue.update(milestone: milestone2)
-      end
-
-      it 'returns earliest start date from issue milestones' do
-        expect(subject.start_date_from_milestones).to eq(date)
+        expect(subject.start_date_from_milestones).to eq(milestone.start_date)
       end
     end
 
     context 'milestone date' do
-      subject { create(:epic, start_date: date) }
-      let(:date) { Date.new(2017, 3, 4) }
-
       it 'returns start_date' do
-        expect(subject.start_date_from_milestones).to eq(date)
+        subject = create(:epic, start_date: Date.new(2017, 3, 4))
+
+        expect(subject.start_date_from_milestones).to eq(subject.start_date)
       end
     end
   end
 
   describe '#due_date_from_milestones' do
     context 'fixed date' do
-      subject { create(:epic, :use_fixed_dates) }
-      let(:date) { Date.new(2017, 3, 4) }
+      it 'returns due date from due date sourcing milestone' do
+        subject = create(:epic, :use_fixed_dates)
+        milestone = create(:milestone, :with_dates)
+        subject.due_date_sourcing_milestone = milestone
 
-      before do
-        milestone1 = create(
-          :milestone,
-          start_date: date - 30.days,
-          due_date: date - 20.days
-        )
-        epic_issue1 = create(:epic_issue, epic: subject)
-        epic_issue1.issue.update(milestone: milestone1)
-
-        milestone2 = create(
-          :milestone,
-          start_date: date - 10.days,
-          due_date: date
-        )
-        epic_issue2 = create(:epic_issue, epic: subject)
-        epic_issue2.issue.update(milestone: milestone2)
-      end
-
-      it 'returns latest due date from issue milestones' do
-        expect(subject.due_date_from_milestones).to eq(date)
+        expect(subject.due_date_from_milestones).to eq(milestone.due_date)
       end
     end
 
     context 'milestone date' do
-      subject { create(:epic, due_date: date) }
-      let(:date) { Date.new(2017, 3, 4) }
-
       it 'returns due_date' do
-        expect(subject.due_date_from_milestones).to eq(date)
+        subject = create(:epic, due_date: Date.new(2017, 3, 4))
+
+        expect(subject.due_date_from_milestones).to eq(subject.due_date)
       end
     end
   end
-- 
GitLab


From f68eda2befddd3402dddf6280426b2f632ec45cb Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 21:38:15 +0800
Subject: [PATCH 41/54] Extract milestone date calculation into own query class

---
 .../epics/date_sourcing_milestones_finder.rb  | 73 +++++++++++++++++++
 ee/app/models/ee/epic.rb                      | 53 +++-----------
 .../date_sourcing_milestones_finder_spec.rb   | 59 +++++++++++++++
 3 files changed, 142 insertions(+), 43 deletions(-)
 create mode 100644 ee/app/finders/epics/date_sourcing_milestones_finder.rb
 create mode 100644 ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb

diff --git a/ee/app/finders/epics/date_sourcing_milestones_finder.rb b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
new file mode 100644
index 00000000000000..50a561d6dbed59
--- /dev/null
+++ b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
@@ -0,0 +1,73 @@
+module Epics
+  class DateSourcingMilestonesFinder
+    def self.execute(epic_id)
+      sql = <<~SQL
+        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
+        INNER JOIN issues ON issues.milestone_id = milestones.id
+        INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+        INNER JOIN (
+          SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
+          FROM milestones
+          INNER JOIN issues ON issues.milestone_id = milestones.id
+          INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+          WHERE epic_issues.epic_id = #{epic_id}
+        ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
+        WHERE epic_issues.epic_id = #{epic_id}
+      SQL
+
+      new(ActiveRecord::Base.connection.select_all(sql).to_a)
+    end
+
+    def initialize(results)
+      @results = results
+    end
+
+    def start_date
+      cast_as_date(start_date_sourcing_milestone&.fetch('start_date', nil))
+    end
+
+    def start_date_sourcing_milestone_id
+      cast_as_id(start_date_sourcing_milestone&.fetch('id', nil))
+    end
+
+    def due_date
+      cast_as_date(due_date_sourcing_milestone&.fetch('due_date', nil))
+    end
+
+    def due_date_sourcing_milestone_id
+      cast_as_id(due_date_sourcing_milestone&.fetch('id', nil))
+    end
+
+    private
+
+    attr_reader :results
+
+    def start_date_sourcing_milestone
+      @start_date_sourcing_milestone ||= results
+        .reject { |row| row['start_date'].nil? }
+        .min_by { |row| row['start_date'] }
+    end
+
+    def due_date_sourcing_milestone
+      @due_date_sourcing_milestone ||= results
+        .reject { |row| row['due_date'].nil? }
+        .max_by { |row| row['due_date'] }
+    end
+
+    def cast_as_date(result)
+      if result
+        Date.strptime(result, '%Y-%m-%d')
+      else
+        result
+      end
+    end
+
+    def cast_as_id(result)
+      if result
+        result.to_i
+      else
+        result
+      end
+    end
+  end
+end
diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 2df3ce5c1b6d00..2ac878f99eef73 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -93,7 +93,7 @@ def update_dates(epics)
         groups.each do |milestone_ids, epics|
           next if milestone_ids.empty?
 
-          data = epics.first.fetch_milestone_date_data
+          results = Epics::DateSourcingMilestonesFinder.execute(epics.first.id)
 
           self.where(id: epics.map(&:id)).update_all(
             [
@@ -103,10 +103,10 @@ def update_dates(epics)
                 end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
                 due_date_sourcing_milestone_id = ?
               },
-              data[:start_date],
-              data[:start_date_sourcing_milestone_id],
-              data[:due_date],
-              data[:due_date_sourcing_milestone_id]
+              results.start_date,
+              results.start_date_sourcing_milestone_id,
+              results.due_date,
+              results.due_date_sourcing_milestone_id
             ]
           )
         end
@@ -143,12 +143,12 @@ def elapsed_days
     alias_attribute(:due_date, :end_date)
 
     def update_dates
-      milestone_data = fetch_milestone_date_data
+      results = Epics::DateSourcingMilestonesFinder.execute(id)
 
-      self.start_date = start_date_is_fixed? ? start_date_fixed : milestone_data[:start_date]
-      self.start_date_sourcing_milestone_id = milestone_data[:start_date_sourcing_milestone_id]
-      self.due_date = due_date_is_fixed? ? due_date_fixed : milestone_data[:due_date]
-      self.due_date_sourcing_milestone_id = milestone_data[:due_date_sourcing_milestone_id]
+      self.start_date = start_date_is_fixed? ? start_date_fixed : results.start_date
+      self.start_date_sourcing_milestone_id = results.start_date_sourcing_milestone_id
+      self.due_date = due_date_is_fixed? ? due_date_fixed : results.due_date
+      self.due_date_sourcing_milestone_id = results.due_date_sourcing_milestone_id
 
       save if changed?
     end
@@ -199,38 +199,5 @@ def discussions_rendered_on_frontend?
     def banzai_render_context(field)
       super.merge(label_url_method: :group_epics_url)
     end
-
-    def fetch_milestone_date_data
-      sql = <<~SQL
-        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
-        INNER JOIN issues ON issues.milestone_id = milestones.id
-        INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-        INNER JOIN (
-          SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
-          FROM milestones
-          INNER JOIN issues ON issues.milestone_id = milestones.id
-          INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-          WHERE epic_issues.epic_id = #{id}
-        ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
-        WHERE epic_issues.epic_id = #{id}
-      SQL
-
-      db_results = ActiveRecord::Base.connection.select_all(sql).to_a
-
-      results = {}
-      db_results
-        .reject { |row| row['start_date'].nil? }
-        .min_by { |row| row['start_date'] }&.tap do |row|
-        results[:start_date] = row['start_date']
-        results[:start_date_sourcing_milestone_id] = row['id']
-      end
-      db_results
-        .reject { |row| row['due_date'].nil? }
-        .max_by { |row| row['due_date'] }&.tap do |row|
-        results[:due_date] = row['due_date']
-        results[:due_date_sourcing_milestone_id] = row['id']
-      end
-      results
-    end
   end
 end
diff --git a/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb b/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
new file mode 100644
index 00000000000000..0fe1ac428f3e50
--- /dev/null
+++ b/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
@@ -0,0 +1,59 @@
+require 'spec_helper'
+
+describe Epics::DateSourcingMilestonesFinder do
+  describe '#execute' do
+    it 'returns date and id from milestones' do
+      epic = create(:epic)
+      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
+      milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30))
+      milestone3 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 20))
+      create(:issue, epic: epic, milestone: milestone1)
+      create(:issue, epic: epic, milestone: milestone2)
+      create(:issue, epic: epic, milestone: milestone3)
+
+      results = described_class.execute(epic.id)
+
+      expect(results.start_date).to eq(milestone1.start_date)
+      expect(results.start_date_sourcing_milestone_id).to eq(milestone1.id)
+      expect(results.due_date).to eq(milestone2.due_date)
+      expect(results.due_date_sourcing_milestone_id).to eq(milestone2.id)
+    end
+
+    it 'returns date and id from single milestone' do
+      epic = create(:epic)
+      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
+      create(:issue, epic: epic, milestone: milestone1)
+
+      results = described_class.execute(epic.id)
+
+      expect(results.start_date).to eq(milestone1.start_date)
+      expect(results.start_date_sourcing_milestone_id).to eq(milestone1.id)
+      expect(results.due_date).to eq(milestone1.due_date)
+      expect(results.due_date_sourcing_milestone_id).to eq(milestone1.id)
+    end
+
+    it 'returns date and id from milestone without date' do
+      epic = create(:epic)
+      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1))
+      create(:issue, epic: epic, milestone: milestone1)
+
+      results = described_class.execute(epic.id)
+
+      expect(results.start_date).to eq(milestone1.start_date)
+      expect(results.start_date_sourcing_milestone_id).to eq(milestone1.id)
+      expect(results.due_date).to eq(nil)
+      expect(results.due_date_sourcing_milestone_id).to eq(nil)
+    end
+
+    it 'handles epics without milestone' do
+      epic = create(:epic)
+
+      results = described_class.execute(epic.id)
+
+      expect(results.start_date).to eq(nil)
+      expect(results.start_date_sourcing_milestone_id).to eq(nil)
+      expect(results.due_date).to eq(nil)
+      expect(results.due_date_sourcing_milestone_id).to eq(nil)
+    end
+  end
+end
-- 
GitLab


From c0c0cd257ef1c56900b36004e36bcafe776fc6e8 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 21:38:45 +0800
Subject: [PATCH 42/54] Extract condition as method for clarity

---
 ee/app/services/ee/milestones/update_service.rb | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
index ceb687612e49bf..21437fd2a5fb08 100644
--- a/ee/app/services/ee/milestones/update_service.rb
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -9,7 +9,7 @@ module UpdateService
       def execute(milestone)
         super
 
-        if milestone.previous_changes.key?(:start_date) || milestone.previous_changes.key?(:due_date)
+        if dates_changed?(milestone)
           ::Epic.update_dates(
             ::Epic.joins(:issues).where(issues: { milestone_id: milestone.id })
           )
@@ -17,6 +17,13 @@ def execute(milestone)
 
         milestone
       end
+
+      private
+
+      def dates_changed?(milestone)
+        changes = milestone.previous_changes
+        changes.include?(:start_date) || changes.include?(:due_date)
+      end
     end
   end
 end
-- 
GitLab


From dfc1a7c362ce6cffbf81f9eae3bea1dbabd807cd Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 21:38:56 +0800
Subject: [PATCH 43/54] spec styling

---
 ee/spec/models/epic_spec.rb                   |  2 +-
 ee/spec/requests/api/epics_spec.rb            |  6 ++---
 .../epic_issues/create_service_spec.rb        |  1 +
 .../epic_issues/destroy_service_spec.rb       |  1 +
 ee/spec/services/epics/update_service_spec.rb | 16 ++++++------
 .../milestones/update_service_spec.rb         | 25 +++++++++---------
 spec/factories/milestones.rb                  |  5 ++++
 .../milestones/update_service_spec.rb         | 26 ++++++++++++++++---
 8 files changed, 55 insertions(+), 27 deletions(-)

diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index 7b838205a7d743..b1f6135e82c2e2 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -344,7 +344,7 @@ def link_epic_to_milestone(epic, milestone)
 
     context 'query count check' do
       let(:milestone) { create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10)) }
-      let!(:epics) { [ create(:epic) ] }
+      let!(:epics) { [create(:epic)] }
 
       def setup_control_group
         link_epic_to_milestone(epics[0], milestone)
diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb
index eabbc8b9878ac5..7dcbca40a971ec 100644
--- a/ee/spec/requests/api/epics_spec.rb
+++ b/ee/spec/requests/api/epics_spec.rb
@@ -40,7 +40,7 @@
     end
   end
 
-  shared_examples 'admin_epic permission' do
+  shared_examples 'can admin epics' do
     let(:extra_date_fields) { %w[start_date_is_fixed start_date_fixed due_date_is_fixed due_date_fixed] }
 
     context 'when permission is absent' do
@@ -165,7 +165,7 @@ def expect_array_response(expected)
         expect_array_response([epic2.id])
       end
 
-      it_behaves_like 'admin_epic permission'
+      it_behaves_like 'can admin epics'
     end
   end
 
@@ -191,7 +191,7 @@ def expect_array_response(expected)
         expect(response).to match_response_schema('public_api/v4/epic', dir: 'ee')
       end
 
-      it_behaves_like 'admin_epic permission'
+      it_behaves_like 'can admin epics'
     end
   end
 
diff --git a/ee/spec/services/epic_issues/create_service_spec.rb b/ee/spec/services/epic_issues/create_service_spec.rb
index 0c88d6c97dca3f..c2efcf38bc4955 100644
--- a/ee/spec/services/epic_issues/create_service_spec.rb
+++ b/ee/spec/services/epic_issues/create_service_spec.rb
@@ -272,6 +272,7 @@ def assign_issue(references)
       context 'refresh epic dates' do
         it 'calls epic#update_dates' do
           expect(epic).to receive(:update_dates)
+
           assign_issue([valid_reference])
         end
       end
diff --git a/ee/spec/services/epic_issues/destroy_service_spec.rb b/ee/spec/services/epic_issues/destroy_service_spec.rb
index 740736b1031b28..39dac47e3212a8 100644
--- a/ee/spec/services/epic_issues/destroy_service_spec.rb
+++ b/ee/spec/services/epic_issues/destroy_service_spec.rb
@@ -79,6 +79,7 @@
       context 'refresh epic dates' do
         it 'calls epic#update_dates' do
           expect(epic).to receive(:update_dates)
+
           subject
         end
       end
diff --git a/ee/spec/services/epics/update_service_spec.rb b/ee/spec/services/epics/update_service_spec.rb
index bde796f59290aa..fb70b008fe5dd1 100644
--- a/ee/spec/services/epics/update_service_spec.rb
+++ b/ee/spec/services/epics/update_service_spec.rb
@@ -29,12 +29,11 @@ def update_epic(opts)
         update_epic(opts)
 
         expect(epic).to be_valid
-        expect(epic.title).to eq(opts[:title])
-        expect(epic.description).to eq(opts[:description])
-        expect(epic.start_date_fixed).to eq(Date.strptime(opts[:start_date_fixed]))
-        expect(epic.start_date_is_fixed).to eq(opts[:start_date_is_fixed])
-        expect(epic.due_date_fixed).to eq(Date.strptime(opts[:due_date_fixed]))
-        expect(epic.due_date_is_fixed).to eq(opts[:due_date_is_fixed])
+        expect(epic).to have_attributes(opts.except(:due_date_fixed, :start_date_fixed))
+        expect(epic).to have_attributes(
+          start_date_fixed: Date.strptime(opts[:start_date_fixed]),
+          due_date_fixed: Date.strptime(opts[:due_date_fixed])
+        )
       end
 
       it 'updates the last_edited_at value' do
@@ -125,8 +124,7 @@ def update_epic(opts)
         expect { update_epic(start_date: Date.today, end_date: Date.today) }.not_to change { Note.count }
 
         expect(epic).to be_valid
-        expect(epic.start_date).to eq(nil)
-        expect(epic.due_date).to eq(nil)
+        expect(epic).to have_attributes(start_date: nil, due_date: nil)
       end
     end
 
@@ -134,6 +132,7 @@ def update_epic(opts)
       context 'date fields are updated' do
         it 'calls epic#update_dates' do
           expect(epic).to receive(:update_dates)
+
           update_epic(start_date_is_fixed: true, start_date_fixed: Date.today)
         end
       end
@@ -141,6 +140,7 @@ def update_epic(opts)
       context 'date fields are not updated' do
         it 'does not call epic#update_dates' do
           expect(epic).not_to receive(:update_dates)
+
           update_epic(title: 'foo')
         end
       end
diff --git a/ee/spec/services/milestones/update_service_spec.rb b/ee/spec/services/milestones/update_service_spec.rb
index 53380b52e4d142..e25dab794727c1 100644
--- a/ee/spec/services/milestones/update_service_spec.rb
+++ b/ee/spec/services/milestones/update_service_spec.rb
@@ -2,25 +2,26 @@
 require 'spec_helper'
 
 describe Milestones::UpdateService do
-  let(:project) { create(:project) }
-  let(:user) { build(:user) }
-  let(:milestone) { create(:milestone, project: project) }
-
   describe '#execute' do
     context 'refresh related epic dates' do
-      let(:epic) { create(:epic) }
-      let!(:issue) { create(:issue, milestone: milestone, epic: epic) }
-      let(:due_date) { 3.days.from_now.to_date }
-
       it 'calls epic#update_dates' do
+        project = create(:project)
+        user = build(:user)
+        milestone = create(:milestone, project: project)
+        epic = create(:epic)
+        create(:issue, milestone: milestone, epic: epic)
+        due_date = 3.days.from_now.to_date
+
         described_class.new(project, user, { due_date: due_date }).execute(milestone)
 
         epic.reload
 
-        expect(epic.start_date).to eq(nil)
-        expect(epic.start_date_sourcing_milestone).to eq(nil)
-        expect(epic.due_date).to eq(due_date)
-        expect(epic.due_date_sourcing_milestone).to eq(milestone)
+        expect(epic.reload).to have_attributes(
+          start_date: nil,
+          start_date_sourcing_milestone: nil,
+          due_date: due_date,
+          due_date_sourcing_milestone: milestone
+        )
       end
     end
   end
diff --git a/spec/factories/milestones.rb b/spec/factories/milestones.rb
index f95632e7187f5b..019e44202122dd 100644
--- a/spec/factories/milestones.rb
+++ b/spec/factories/milestones.rb
@@ -18,6 +18,11 @@
       state "closed"
     end
 
+    trait :with_dates do
+      start_date { Date.new(2000, 1, 1) }
+      due_date { Date.new(2000, 1, 30) }
+    end
+
     after(:build, :stub) do |milestone, evaluator|
       if evaluator.group
         milestone.group = evaluator.group
diff --git a/spec/services/milestones/update_service_spec.rb b/spec/services/milestones/update_service_spec.rb
index 33ec02e17aaa3a..3b91442c0ba084 100644
--- a/spec/services/milestones/update_service_spec.rb
+++ b/spec/services/milestones/update_service_spec.rb
@@ -8,14 +8,34 @@
 
   describe '#execute' do
     context "valid params" do
+      let(:inner_service) { double(:service) }
+
       before do
         project.add_maintainer(user)
+      end
+
+      subject { described_class.new(project, user, { title: 'new_title' }).execute(milestone) }
+
+      it { expect(subject).to be_valid }
+      it { expect(subject.title).to eq('new_title') }
 
-        @milestone = described_class.new(project, user, { title: 'new_title' }).execute(milestone)
+      context 'state_event is activate' do
+        it 'calls ReopenService' do
+          expect(Milestones::ReopenService).to receive(:new).with(project, user, {}).and_return(inner_service)
+          expect(inner_service).to receive(:execute).with(milestone)
+
+          described_class.new(project, user, { state_event: 'activate' }).execute(milestone)
+        end
       end
 
-      it { expect(@milestone).to be_valid }
-      it { expect(@milestone.title).to eq('new_title') }
+      context 'state_event is close' do
+        it 'calls ReopenService' do
+          expect(Milestones::CloseService).to receive(:new).with(project, user, {}).and_return(inner_service)
+          expect(inner_service).to receive(:execute).with(milestone)
+
+          described_class.new(project, user, { state_event: 'close' }).execute(milestone)
+        end
+      end
     end
   end
 end
-- 
GitLab


From 23fee68b07301d8449d4627137a2c923258a3da7 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 22:58:59 +0800
Subject: [PATCH 44/54] Rename to update_start_and_due_dates for clarity

---
 ee/app/models/ee/epic.rb                      |  4 +--
 ee/app/services/ee/issues/update_service.rb   |  2 +-
 .../services/ee/milestones/update_service.rb  |  2 +-
 ee/app/services/epic_issues/create_service.rb |  2 +-
 .../services/epic_issues/destroy_service.rb   |  2 +-
 ee/app/services/epics/update_service.rb       |  2 +-
 ee/spec/models/epic_spec.rb                   | 28 +++++++++----------
 .../services/ee/issues/update_service_spec.rb |  8 +++---
 .../epic_issues/create_service_spec.rb        |  4 +--
 .../epic_issues/destroy_service_spec.rb       |  4 +--
 ee/spec/services/epics/update_service_spec.rb |  8 +++---
 .../milestones/update_service_spec.rb         |  4 +--
 12 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 2ac878f99eef73..e0b75056d09c57 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -82,7 +82,7 @@ def parent_class
         ::Group
       end
 
-      def update_dates(epics)
+      def update_start_and_due_dates(epics)
         groups = epics.includes(:issues).group_by do |epic|
           milestone_ids = epic.issues.map(&:milestone_id)
           milestone_ids.compact!
@@ -142,7 +142,7 @@ def elapsed_days
     # Needed to use EntityDateHelper#remaining_days_in_words
     alias_attribute(:due_date, :end_date)
 
-    def update_dates
+    def update_start_and_due_dates
       results = Epics::DateSourcingMilestonesFinder.execute(id)
 
       self.start_date = start_date_is_fixed? ? start_date_fixed : results.start_date
diff --git a/ee/app/services/ee/issues/update_service.rb b/ee/app/services/ee/issues/update_service.rb
index e5affbc4d6943c..5887b349d1faf9 100644
--- a/ee/app/services/ee/issues/update_service.rb
+++ b/ee/app/services/ee/issues/update_service.rb
@@ -10,7 +10,7 @@ def execute(issue)
         result = super
 
         if issue.previous_changes.include?(:milestone_id) && issue.epic
-          issue.epic.update_dates
+          issue.epic.update_start_and_due_dates
         end
 
         result
diff --git a/ee/app/services/ee/milestones/update_service.rb b/ee/app/services/ee/milestones/update_service.rb
index 21437fd2a5fb08..4b8152caf55a00 100644
--- a/ee/app/services/ee/milestones/update_service.rb
+++ b/ee/app/services/ee/milestones/update_service.rb
@@ -10,7 +10,7 @@ def execute(milestone)
         super
 
         if dates_changed?(milestone)
-          ::Epic.update_dates(
+          ::Epic.update_start_and_due_dates(
             ::Epic.joins(:issues).where(issues: { milestone_id: milestone.id })
           )
         end
diff --git a/ee/app/services/epic_issues/create_service.rb b/ee/app/services/epic_issues/create_service.rb
index fa5ff3e5c09748..1bfe62e6f6633a 100644
--- a/ee/app/services/epic_issues/create_service.rb
+++ b/ee/app/services/epic_issues/create_service.rb
@@ -2,7 +2,7 @@ module EpicIssues
   class CreateService < IssuableLinks::CreateService
     def execute
       result = super
-      issuable.update_dates
+      issuable.update_start_and_due_dates
       result
     end
 
diff --git a/ee/app/services/epic_issues/destroy_service.rb b/ee/app/services/epic_issues/destroy_service.rb
index 0b08f895d08e29..ea6a441ddb6f50 100644
--- a/ee/app/services/epic_issues/destroy_service.rb
+++ b/ee/app/services/epic_issues/destroy_service.rb
@@ -2,7 +2,7 @@ module EpicIssues
   class DestroyService < IssuableLinks::DestroyService
     def execute
       result = super
-      link.epic.update_dates
+      link.epic.update_start_and_due_dates
       result
     end
 
diff --git a/ee/app/services/epics/update_service.rb b/ee/app/services/epics/update_service.rb
index 8cf09e0c5d5825..93bfa72f732d02 100644
--- a/ee/app/services/epics/update_service.rb
+++ b/ee/app/services/epics/update_service.rb
@@ -8,7 +8,7 @@ def execute(epic)
       update(epic)
 
       if (params.keys.map(&:to_sym) & [:start_date_fixed, :start_date_is_fixed, :due_date_fixed, :due_date_is_fixed]).present?
-        epic.update_dates
+        epic.update_start_and_due_dates
       end
 
       epic
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index b1f6135e82c2e2..d7fc5bd2c96aa7 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -137,12 +137,12 @@
     end
   end
 
-  describe '#update_dates' do
+  describe '#update_start_and_due_dates' do
     context 'fixed date is set' do
       subject { create(:epic, :use_fixed_dates, start_date: nil, end_date: nil) }
 
       it 'updates to fixed date' do
-        subject.update_dates
+        subject.update_start_and_due_dates
 
         expect(subject.start_date).to eq(subject.start_date_fixed)
         expect(subject.due_date).to eq(subject.due_date_fixed)
@@ -177,7 +177,7 @@
 
         context 'complete start and due dates' do
           it 'updates to milestone dates' do
-            subject.update_dates
+            subject.update_start_and_due_dates
 
             expect(subject.start_date).to eq(milestone1.start_date)
             expect(subject.due_date).to eq(milestone2.due_date)
@@ -201,7 +201,7 @@
           end
 
           it 'updates to milestone dates' do
-            subject.update_dates
+            subject.update_start_and_due_dates
 
             expect(subject.start_date).to eq(milestone1.start_date)
             expect(subject.due_date).to eq(nil)
@@ -225,7 +225,7 @@
           end
 
           it 'updates to milestone dates' do
-            subject.update_dates
+            subject.update_start_and_due_dates
 
             expect(subject.start_date).to eq(nil)
             expect(subject.due_date).to eq(nil)
@@ -239,7 +239,7 @@
         end
 
         it 'updates to milestone dates' do
-          subject.update_dates
+          subject.update_start_and_due_dates
 
           expect(subject.start_date).to eq(nil)
           expect(subject.start_date_sourcing_milestone_id).to eq(nil)
@@ -256,7 +256,7 @@
 
         context 'complete start and due dates' do
           it 'updates to milestone dates' do
-            subject.update_dates
+            subject.update_start_and_due_dates
 
             expect(subject.start_date).to eq(milestone1.start_date)
             expect(subject.due_date).to eq(milestone1.due_date)
@@ -273,7 +273,7 @@
           end
 
           it 'updates to milestone dates' do
-            subject.update_dates
+            subject.update_start_and_due_dates
 
             expect(subject.start_date).to eq(milestone1.start_date)
             expect(subject.due_date).to eq(nil)
@@ -290,7 +290,7 @@
           end
 
           it 'updates to milestone dates' do
-            subject.update_dates
+            subject.update_start_and_due_dates
 
             expect(subject.start_date).to eq(nil)
             expect(subject.due_date).to eq(nil)
@@ -300,7 +300,7 @@
     end
   end
 
-  describe '.update_dates' do
+  describe '.update_start_and_due_dates' do
     def link_epic_to_milestone(epic, milestone)
       create(:issue, epic: epic, milestone: milestone)
     end
@@ -322,7 +322,7 @@ def link_epic_to_milestone(epic, milestone)
       link_epic_to_milestone(epics[2], milestone1)
       link_epic_to_milestone(epics[2], milestone2)
 
-      described_class.update_dates(described_class.where(id: epics.map(&:id)))
+      described_class.update_start_and_due_dates(described_class.where(id: epics.map(&:id)))
 
       epics.each(&:reload)
 
@@ -350,7 +350,7 @@ def setup_control_group
         link_epic_to_milestone(epics[0], milestone)
 
         ActiveRecord::QueryRecorder.new do
-          described_class.update_dates(described_class.where(id: epics.map(&:id)))
+          described_class.update_start_and_due_dates(described_class.where(id: epics.map(&:id)))
         end.count
       end
 
@@ -360,7 +360,7 @@ def setup_control_group
         epics << create(:epic)
 
         expect do
-          described_class.update_dates(described_class.where(id: epics.map(&:id)))
+          described_class.update_start_and_due_dates(described_class.where(id: epics.map(&:id)))
         end.not_to exceed_query_limit(control_count)
       end
 
@@ -371,7 +371,7 @@ def setup_control_group
         link_epic_to_milestone(epics[1], milestone)
 
         expect do
-          described_class.update_dates(described_class.where(id: epics.map(&:id)))
+          described_class.update_start_and_due_dates(described_class.where(id: epics.map(&:id)))
         end.not_to exceed_query_limit(control_count)
       end
     end
diff --git a/ee/spec/services/ee/issues/update_service_spec.rb b/ee/spec/services/ee/issues/update_service_spec.rb
index 87f47eb2dcf7e6..90f82d5b57ff7a 100644
--- a/ee/spec/services/ee/issues/update_service_spec.rb
+++ b/ee/spec/services/ee/issues/update_service_spec.rb
@@ -18,8 +18,8 @@ def update_issue(opts)
       context 'updating milestone' do
         let(:milestone) { create(:milestone) }
 
-        it 'calls epic#update_dates' do
-          expect(epic).to receive(:update_dates).twice
+        it 'calls epic#update_start_and_due_dates' do
+          expect(epic).to receive(:update_start_and_due_dates).twice
 
           update_issue(milestone: milestone)
           update_issue(milestone_id: nil)
@@ -27,8 +27,8 @@ def update_issue(opts)
       end
 
       context 'updating other fields' do
-        it 'does not call epic#update_dates' do
-          expect(epic).not_to receive(:update_dates)
+        it 'does not call epic#update_start_and_due_dates' do
+          expect(epic).not_to receive(:update_start_and_due_dates)
           update_issue(title: 'foo')
         end
       end
diff --git a/ee/spec/services/epic_issues/create_service_spec.rb b/ee/spec/services/epic_issues/create_service_spec.rb
index c2efcf38bc4955..8034a04045e4d6 100644
--- a/ee/spec/services/epic_issues/create_service_spec.rb
+++ b/ee/spec/services/epic_issues/create_service_spec.rb
@@ -270,8 +270,8 @@ def assign_issue(references)
       end
 
       context 'refresh epic dates' do
-        it 'calls epic#update_dates' do
-          expect(epic).to receive(:update_dates)
+        it 'calls epic#update_start_and_due_dates' do
+          expect(epic).to receive(:update_start_and_due_dates)
 
           assign_issue([valid_reference])
         end
diff --git a/ee/spec/services/epic_issues/destroy_service_spec.rb b/ee/spec/services/epic_issues/destroy_service_spec.rb
index 39dac47e3212a8..91751fc4c0295b 100644
--- a/ee/spec/services/epic_issues/destroy_service_spec.rb
+++ b/ee/spec/services/epic_issues/destroy_service_spec.rb
@@ -77,8 +77,8 @@
       end
 
       context 'refresh epic dates' do
-        it 'calls epic#update_dates' do
-          expect(epic).to receive(:update_dates)
+        it 'calls epic#update_start_and_due_dates' do
+          expect(epic).to receive(:update_start_and_due_dates)
 
           subject
         end
diff --git a/ee/spec/services/epics/update_service_spec.rb b/ee/spec/services/epics/update_service_spec.rb
index fb70b008fe5dd1..052e3c52c9dbfc 100644
--- a/ee/spec/services/epics/update_service_spec.rb
+++ b/ee/spec/services/epics/update_service_spec.rb
@@ -130,16 +130,16 @@ def update_epic(opts)
 
     context 'refresh epic dates' do
       context 'date fields are updated' do
-        it 'calls epic#update_dates' do
-          expect(epic).to receive(:update_dates)
+        it 'calls epic#update_start_and_due_dates' do
+          expect(epic).to receive(:update_start_and_due_dates)
 
           update_epic(start_date_is_fixed: true, start_date_fixed: Date.today)
         end
       end
 
       context 'date fields are not updated' do
-        it 'does not call epic#update_dates' do
-          expect(epic).not_to receive(:update_dates)
+        it 'does not call epic#update_start_and_due_dates' do
+          expect(epic).not_to receive(:update_start_and_due_dates)
 
           update_epic(title: 'foo')
         end
diff --git a/ee/spec/services/milestones/update_service_spec.rb b/ee/spec/services/milestones/update_service_spec.rb
index e25dab794727c1..bdf13abc89b6ec 100644
--- a/ee/spec/services/milestones/update_service_spec.rb
+++ b/ee/spec/services/milestones/update_service_spec.rb
@@ -4,7 +4,7 @@
 describe Milestones::UpdateService do
   describe '#execute' do
     context 'refresh related epic dates' do
-      it 'calls epic#update_dates' do
+      it 'updates milestone sourced dates' do
         project = create(:project)
         user = build(:user)
         milestone = create(:milestone, project: project)
@@ -14,8 +14,6 @@
 
         described_class.new(project, user, { due_date: due_date }).execute(milestone)
 
-        epic.reload
-
         expect(epic.reload).to have_attributes(
           start_date: nil,
           start_date_sourcing_milestone: nil,
-- 
GitLab


From df8d472a7f86f3086c965097169592dac9941829 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Fri, 3 Aug 2018 23:08:37 +0800
Subject: [PATCH 45/54] Only update when fields actually changed.

---
 ee/app/services/epics/update_service.rb | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/ee/app/services/epics/update_service.rb b/ee/app/services/epics/update_service.rb
index 93bfa72f732d02..485b468e0f279d 100644
--- a/ee/app/services/epics/update_service.rb
+++ b/ee/app/services/epics/update_service.rb
@@ -1,5 +1,12 @@
 module Epics
   class UpdateService < Epics::BaseService
+    EPIC_DATE_FIELDS = %I[
+      start_date_fixed
+      start_date_is_fixed
+      due_date_fixed
+      due_date_is_fixed
+    ].freeze
+
     def execute(epic)
       # start_date and end_date columns are no longer writable by users because those
       # are composite fields managed by the system.
@@ -7,9 +14,7 @@ def execute(epic)
 
       update(epic)
 
-      if (params.keys.map(&:to_sym) & [:start_date_fixed, :start_date_is_fixed, :due_date_fixed, :due_date_is_fixed]).present?
-        epic.update_start_and_due_dates
-      end
+      epic.update_start_and_due_dates if have_epic_dates_changed?(epic)
 
       epic
     end
@@ -25,5 +30,11 @@ def handle_changes(epic, options)
 
       todo_service.update_epic(epic, current_user, old_mentioned_users)
     end
+
+    private
+
+    def have_epic_dates_changed?(epic)
+      (epic.previous_changes.keys.map(&:to_sym) & EPIC_DATE_FIELDS).present?
+    end
   end
 end
-- 
GitLab


From 84314df7ce16add8d15de29aea64ef06496ead34 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Sun, 5 Aug 2018 20:45:02 +0800
Subject: [PATCH 46/54] remove comment

---
 ee/app/models/ee/epic.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index e0b75056d09c57..1b374c54ecdb78 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -153,12 +153,10 @@ def update_start_and_due_dates
       save if changed?
     end
 
-    # Earliest start date from issues' milestones
     def start_date_from_milestones
       start_date_is_fixed? ? start_date_sourcing_milestone.start_date : start_date
     end
 
-    # Latest end date from issues' milestones
     def due_date_from_milestones
       due_date_is_fixed? ? due_date_sourcing_milestone.due_date : due_date
     end
-- 
GitLab


From 38b34c87989eaefc69c720d0665885cb8bb91903 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Sun, 5 Aug 2018 21:04:55 +0800
Subject: [PATCH 47/54] use more robust instance_double

---
 spec/services/notes/create_service_spec.rb                  | 4 +++-
 .../shared_examples/services/boards/issues_move_service.rb  | 6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index b1290fd0d47eaa..72c5662e866afb 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -145,8 +145,10 @@
           let(:note_text) { %(HELLO\n/close\n/assign @#{user.username}\nWORLD) }
 
           it 'saves the note and does not alter the note text' do
-            service = double(:service)
+            service = instance_double('Issues::UpdateService', :service)
+
             allow(Issues::UpdateService).to receive(:new).and_return(service)
+
             expect(service).to receive(:execute)
 
             note = described_class.new(project, user, opts.merge(note: note_text)).execute
diff --git a/spec/support/shared_examples/services/boards/issues_move_service.rb b/spec/support/shared_examples/services/boards/issues_move_service.rb
index 6d29a97c56db17..17d1fda9410454 100644
--- a/spec/support/shared_examples/services/boards/issues_move_service.rb
+++ b/spec/support/shared_examples/services/boards/issues_move_service.rb
@@ -4,8 +4,10 @@
     let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } }
 
     it 'delegates the label changes to Issues::UpdateService' do
-      service = double(:service)
-      expect(Issues::UpdateService).to receive(:new).and_return(service)
+      service = instance_double('Issues::UpdateService', :service)
+
+      allow(Issues::UpdateService).to receive(:new).and_return(service)
+
       expect(service).to receive(:execute).with(issue).once
 
       described_class.new(parent, user, params).execute(issue)
-- 
GitLab


From 74bdc9a37a2f39f96a8dafe9e155ceb8420d127d Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Mon, 6 Aug 2018 12:59:27 +0800
Subject: [PATCH 48/54] Use DateSourcingMilestonesFinder in migration

---
 ...71825_update_epic_dates_from_milestones.rb | 57 ++++-----------
 .../update_epic_dates_from_milestones_spec.rb | 72 +++++++++++++++++++
 2 files changed, 84 insertions(+), 45 deletions(-)
 create mode 100644 ee/spec/migrations/update_epic_dates_from_milestones_spec.rb

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 053a97409e5d9c..7954b8a953b195 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -14,7 +14,7 @@ class Epic < ActiveRecord::Base
     has_many :epic_issues
     has_many :issues, through: :epic_issues
 
-    def self.update_dates(epics)
+    def self.update_start_and_due_dates(epics)
       groups = epics.includes(:issues).group_by do |epic|
         milestone_ids = epic.issues.map(&:milestone_id)
         milestone_ids.compact!
@@ -25,62 +25,29 @@ def self.update_dates(epics)
       groups.each do |milestone_ids, epics|
         next if milestone_ids.empty?
 
-        data = epics.first.fetch_milestone_date_data
+        data = ::Epics::DateSourcingMilestonesFinder.execute(epics.first.id)
 
         self.where(id: epics.map(&:id)).update_all(
           [
             %{
-                start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
-                start_date_sourcing_milestone_id = ?,
-                end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
-                due_date_sourcing_milestone_id = ?
-              },
-            data[:start_date],
-            data[:start_date_sourcing_milestone_id],
-            data[:due_date],
-            data[:due_date_sourcing_milestone_id]
+              start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
+              start_date_sourcing_milestone_id = ?,
+              end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
+              due_date_sourcing_milestone_id = ?
+            },
+            data.start_date,
+            data.start_date_sourcing_milestone_id,
+            data.due_date,
+            data.due_date_sourcing_milestone_id
           ]
         )
       end
     end
-
-    def fetch_milestone_date_data
-      sql = <<~SQL
-        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
-        INNER JOIN issues ON issues.milestone_id = milestones.id
-        INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-        INNER JOIN (
-          SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
-          FROM milestones
-          INNER JOIN issues ON issues.milestone_id = milestones.id
-          INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-          WHERE epic_issues.epic_id = #{id}
-        ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
-        WHERE epic_issues.epic_id = #{id}
-      SQL
-
-      db_results = ActiveRecord::Base.connection.select_all(sql).to_a
-
-      results = {}
-      db_results
-        .reject { |row| row['start_date'].nil? }
-        .min_by { |row| row['start_date'] }&.tap do |row|
-        results[:start_date] = row['start_date']
-        results[:start_date_sourcing_milestone_id] = row['id']
-      end
-      db_results
-        .reject { |row| row['due_date'].nil? }
-        .max_by { |row| row['due_date'] }&.tap do |row|
-        results[:due_date] = row['due_date']
-        results[:due_date_sourcing_milestone_id] = row['id']
-      end
-      results
-    end
   end
 
   def up
     Epic.joins(:issues).where('issues.milestone_id IS NOT NULL').each_batch do |epics|
-      Epic.update_dates(epics)
+      Epic.update_start_and_due_dates(epics)
     end
   end
 
diff --git a/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb b/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb
new file mode 100644
index 00000000000000..13d474dd01a919
--- /dev/null
+++ b/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper'
+require Rails.root.join('ee', 'db', 'post_migrate', '20180713171825_update_epic_dates_from_milestones.rb')
+
+describe UpdateEpicDatesFromMilestones, :migration do
+  let(:migration) { described_class.new }
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:epics) { table(:epics) }
+  let(:projects) { table(:projects) }
+  let(:issues) { table(:issues) }
+  let(:epic_issues) { table(:epic_issues) }
+  let(:milestones) { table(:milestones) }
+
+  describe '#up' do
+    before do
+      user = users.create!(email: 'test@example.com', projects_limit: 100, username: 'test')
+      namespaces.create(id: 1, name: 'gitlab-org', path: 'gitlab-org')
+      projects.create!(id: 1, namespace_id: 1, name: 'gitlab1', path: 'gitlab1')
+
+      epics.create!(id: 1, iid: 1, group_id: 1, title: 'epic with start and due dates', title_html: '', author_id: user.id)
+      epics.create!(id: 2, iid: 2, group_id: 1, title: 'epic with only due date', title_html: '', author_id: user.id)
+      epics.create!(id: 3, iid: 3, group_id: 1, title: 'epic without milestone', title_html: '', author_id: user.id)
+
+      milestones.create!(
+        id: 1,
+        iid: 1,
+        project_id: 1,
+        title: 'milestone-1',
+        start_date: Date.new(2000, 1, 1),
+        due_date: Date.new(2000, 1, 10)
+      )
+      milestones.create!(
+        id: 2,
+        iid: 2,
+        project_id: 1,
+        title: 'milestone-2',
+        due_date: Date.new(2000, 1, 30)
+      )
+
+      issues.create!(id: 1, iid: 1, project_id: 1, milestone_id: 1, title: 'issue-1')
+      issues.create!(id: 2, iid: 2, project_id: 1, milestone_id: 2, title: 'issue-2')
+      issues.create!(id: 3, iid: 3, project_id: 1, milestone_id: 2, title: 'issue-2')
+
+      epic_issues.create!(epic_id: 1, issue_id: 1)
+      epic_issues.create!(epic_id: 1, issue_id: 2)
+      epic_issues.create!(epic_id: 2, issue_id: 3)
+    end
+
+    it 'updates dates milestone ids' do
+      migration.up
+
+      expect(Epic.find(1)).to have_attributes(
+        start_date: Date.new(2000, 1, 1),
+        start_date_sourcing_milestone_id: 1,
+        due_date: Date.new(2000, 1, 30),
+        due_date_sourcing_milestone_id: 2
+      )
+      expect(Epic.find(2)).to have_attributes(
+        start_date: nil,
+        start_date_sourcing_milestone_id: nil,
+        due_date: Date.new(2000, 1, 30),
+        due_date_sourcing_milestone_id: 2
+      )
+      expect(Epic.find(3)).to have_attributes(
+        start_date: nil,
+        start_date_sourcing_milestone_id: nil,
+        due_date: nil,
+        due_date_sourcing_milestone_id: nil
+      )
+    end
+  end
+end
-- 
GitLab


From af65c04d44a5319c013ffda8862da63bf7cdd690 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Mon, 6 Aug 2018 15:09:19 +0800
Subject: [PATCH 49/54] Add frozen string literal header

---
 .../epics/date_sourcing_milestones_finder.rb  |  2 +
 .../date_sourcing_milestones_finder_spec.rb   | 42 ++++++++++++-------
 .../update_epic_dates_from_milestones_spec.rb |  2 +
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/ee/app/finders/epics/date_sourcing_milestones_finder.rb b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
index 50a561d6dbed59..332d94c3297aed 100644
--- a/ee/app/finders/epics/date_sourcing_milestones_finder.rb
+++ b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Epics
   class DateSourcingMilestonesFinder
     def self.execute(epic_id)
diff --git a/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb b/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
index 0fe1ac428f3e50..b3a57830e02bbc 100644
--- a/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
+++ b/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Epics::DateSourcingMilestonesFinder do
@@ -13,10 +15,12 @@
 
       results = described_class.execute(epic.id)
 
-      expect(results.start_date).to eq(milestone1.start_date)
-      expect(results.start_date_sourcing_milestone_id).to eq(milestone1.id)
-      expect(results.due_date).to eq(milestone2.due_date)
-      expect(results.due_date_sourcing_milestone_id).to eq(milestone2.id)
+      expect(results).to have_attributes(
+        start_date: milestone1.start_date,
+        start_date_sourcing_milestone_id: milestone1.id,
+        due_date: milestone2.due_date,
+        due_date_sourcing_milestone_id: milestone2.id
+      )
     end
 
     it 'returns date and id from single milestone' do
@@ -26,10 +30,12 @@
 
       results = described_class.execute(epic.id)
 
-      expect(results.start_date).to eq(milestone1.start_date)
-      expect(results.start_date_sourcing_milestone_id).to eq(milestone1.id)
-      expect(results.due_date).to eq(milestone1.due_date)
-      expect(results.due_date_sourcing_milestone_id).to eq(milestone1.id)
+      expect(results).to have_attributes(
+        start_date: milestone1.start_date,
+        start_date_sourcing_milestone_id: milestone1.id,
+        due_date: milestone1.due_date,
+        due_date_sourcing_milestone_id: milestone1.id
+      )
     end
 
     it 'returns date and id from milestone without date' do
@@ -39,10 +45,12 @@
 
       results = described_class.execute(epic.id)
 
-      expect(results.start_date).to eq(milestone1.start_date)
-      expect(results.start_date_sourcing_milestone_id).to eq(milestone1.id)
-      expect(results.due_date).to eq(nil)
-      expect(results.due_date_sourcing_milestone_id).to eq(nil)
+      expect(results).to have_attributes(
+        start_date: milestone1.start_date,
+        start_date_sourcing_milestone_id: milestone1.id,
+        due_date: nil,
+        due_date_sourcing_milestone_id: nil
+      )
     end
 
     it 'handles epics without milestone' do
@@ -50,10 +58,12 @@
 
       results = described_class.execute(epic.id)
 
-      expect(results.start_date).to eq(nil)
-      expect(results.start_date_sourcing_milestone_id).to eq(nil)
-      expect(results.due_date).to eq(nil)
-      expect(results.due_date_sourcing_milestone_id).to eq(nil)
+      expect(results).to have_attributes(
+        start_date: nil,
+        start_date_sourcing_milestone_id: nil,
+        due_date: nil,
+        due_date_sourcing_milestone_id: nil
+      )
     end
   end
 end
diff --git a/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb b/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb
index 13d474dd01a919..42dd8e6b58fdbb 100644
--- a/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb
+++ b/ee/spec/migrations/update_epic_dates_from_milestones_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 require Rails.root.join('ee', 'db', 'post_migrate', '20180713171825_update_epic_dates_from_milestones.rb')
 
-- 
GitLab


From 5703c9fb30b760fdfc2bbfd9988ccb5abc20ab0f Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Mon, 6 Aug 2018 16:46:10 +0800
Subject: [PATCH 50/54] Silently ignore case when date_sourcing_milestone is
 not setup

---
 ee/app/models/ee/epic.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 1b374c54ecdb78..925cb265279ac9 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -154,11 +154,11 @@ def update_start_and_due_dates
     end
 
     def start_date_from_milestones
-      start_date_is_fixed? ? start_date_sourcing_milestone.start_date : start_date
+      start_date_is_fixed? ? start_date_sourcing_milestone&.start_date : start_date
     end
 
     def due_date_from_milestones
-      due_date_is_fixed? ? due_date_sourcing_milestone.due_date : due_date
+      due_date_is_fixed? ? due_date_sourcing_milestone&.due_date : due_date
     end
 
     def to_reference(from = nil, full: false)
-- 
GitLab


From 63a2a54a73283a97028dfbe661b08c2475a12351 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Mon, 6 Aug 2018 23:32:08 +0800
Subject: [PATCH 51/54] Query using ActiveRecord so columns are casted in
 correct types

---
 .../epics/date_sourcing_milestones_finder.rb  | 63 ++++++++-----------
 1 file changed, 25 insertions(+), 38 deletions(-)

diff --git a/ee/app/finders/epics/date_sourcing_milestones_finder.rb b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
index 332d94c3297aed..634f80db5ab086 100644
--- a/ee/app/finders/epics/date_sourcing_milestones_finder.rb
+++ b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
@@ -2,22 +2,25 @@
 
 module Epics
   class DateSourcingMilestonesFinder
+    FIELDS = [:id, :start_date, :due_date].freeze
+    ID_INDEX = FIELDS.index(:id)
+    START_DATE_INDEX = FIELDS.index(:start_date)
+    DUE_DATE_INDEX = FIELDS.index(:due_date)
+
     def self.execute(epic_id)
-      sql = <<~SQL
-        SELECT milestones.id, milestones.start_date, milestones.due_date FROM milestones
-        INNER JOIN issues ON issues.milestone_id = milestones.id
-        INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-        INNER JOIN (
-          SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
-          FROM milestones
-          INNER JOIN issues ON issues.milestone_id = milestones.id
-          INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-          WHERE epic_issues.epic_id = #{epic_id}
-        ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
-        WHERE epic_issues.epic_id = #{epic_id}
-      SQL
+      results = Milestone.joins(issues: :epic_issue).where(epic_issues: { epic_id: epic_id }).joins(
+        <<~SQL
+          INNER JOIN (
+            SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
+            FROM milestones
+            INNER JOIN issues ON issues.milestone_id = milestones.id
+            INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+            WHERE epic_issues.epic_id = #{epic_id}
+          ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
+        SQL
+      ).pluck(*FIELDS)
 
-      new(ActiveRecord::Base.connection.select_all(sql).to_a)
+      new(results)
     end
 
     def initialize(results)
@@ -25,19 +28,19 @@ def initialize(results)
     end
 
     def start_date
-      cast_as_date(start_date_sourcing_milestone&.fetch('start_date', nil))
+      start_date_sourcing_milestone&.slice(START_DATE_INDEX)
     end
 
     def start_date_sourcing_milestone_id
-      cast_as_id(start_date_sourcing_milestone&.fetch('id', nil))
+      start_date_sourcing_milestone&.slice(ID_INDEX)
     end
 
     def due_date
-      cast_as_date(due_date_sourcing_milestone&.fetch('due_date', nil))
+      due_date_sourcing_milestone&.slice(DUE_DATE_INDEX)
     end
 
     def due_date_sourcing_milestone_id
-      cast_as_id(due_date_sourcing_milestone&.fetch('id', nil))
+      due_date_sourcing_milestone&.slice(ID_INDEX)
     end
 
     private
@@ -46,30 +49,14 @@ def due_date_sourcing_milestone_id
 
     def start_date_sourcing_milestone
       @start_date_sourcing_milestone ||= results
-        .reject { |row| row['start_date'].nil? }
-        .min_by { |row| row['start_date'] }
+        .reject { |row| row[START_DATE_INDEX].nil? }
+        .min_by { |row| row[START_DATE_INDEX] }
     end
 
     def due_date_sourcing_milestone
       @due_date_sourcing_milestone ||= results
-        .reject { |row| row['due_date'].nil? }
-        .max_by { |row| row['due_date'] }
-    end
-
-    def cast_as_date(result)
-      if result
-        Date.strptime(result, '%Y-%m-%d')
-      else
-        result
-      end
-    end
-
-    def cast_as_id(result)
-      if result
-        result.to_i
-      else
-        result
-      end
+        .reject { |row| row[DUE_DATE_INDEX].nil? }
+        .max_by { |row| row[DUE_DATE_INDEX] }
     end
   end
 end
-- 
GitLab


From 681dd0da8d0765bd993b981d11bc0a303282f319 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 7 Aug 2018 00:22:53 +0800
Subject: [PATCH 52/54] Update fixed start_date for records changed between
 regular migration and completion of deployment.

---
 .../20180713171825_update_epic_dates_from_milestones.rb  | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index 7954b8a953b195..eab4557b88d0a7 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -46,6 +46,15 @@ def self.update_start_and_due_dates(epics)
   end
 
   def up
+    # Fill fixed date columns for remaining eligible records touched after regular migration is run
+    # (20180711014026_update_date_columns_on_epics) but before new app code takes effect.
+    Epic.where(start_date_is_fixed: nil).where.not(start_date: nil).each_batch do |batch|
+      batch.update_all('start_date_is_fixed = true, start_date_fixed = start_date')
+    end
+    Epic.where(due_date_is_fixed: nil).where.not(end_date: nil).each_batch do |batch|
+      batch.update_all('due_date_is_fixed = true, due_date_fixed = end_date')
+    end
+
     Epic.joins(:issues).where('issues.milestone_id IS NOT NULL').each_batch do |epics|
       Epic.update_start_and_due_dates(epics)
     end
-- 
GitLab


From 657712a98b4a1fe2686131ce6a15134971a33fae Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 7 Aug 2018 09:49:15 +0800
Subject: [PATCH 53/54] Change DateSourcingMilestonesFinder to be similar to
 other finderd

Embed DateSourcingMilestoneFinder into migration
---
 .../epics/date_sourcing_milestones_finder.rb  | 40 ++++++------
 ee/app/models/ee/epic.rb                      |  4 +-
 ...71825_update_epic_dates_from_milestones.rb | 65 ++++++++++++++++++-
 .../date_sourcing_milestones_finder_spec.rb   |  8 +--
 4 files changed, 91 insertions(+), 26 deletions(-)

diff --git a/ee/app/finders/epics/date_sourcing_milestones_finder.rb b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
index 634f80db5ab086..5ff63dbd5576bc 100644
--- a/ee/app/finders/epics/date_sourcing_milestones_finder.rb
+++ b/ee/app/finders/epics/date_sourcing_milestones_finder.rb
@@ -2,29 +2,31 @@
 
 module Epics
   class DateSourcingMilestonesFinder
+    include Gitlab::Utils::StrongMemoize
+
     FIELDS = [:id, :start_date, :due_date].freeze
     ID_INDEX = FIELDS.index(:id)
     START_DATE_INDEX = FIELDS.index(:start_date)
     DUE_DATE_INDEX = FIELDS.index(:due_date)
 
-    def self.execute(epic_id)
-      results = Milestone.joins(issues: :epic_issue).where(epic_issues: { epic_id: epic_id }).joins(
-        <<~SQL
-          INNER JOIN (
-            SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
-            FROM milestones
-            INNER JOIN issues ON issues.milestone_id = milestones.id
-            INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
-            WHERE epic_issues.epic_id = #{epic_id}
-          ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
-        SQL
-      ).pluck(*FIELDS)
-
-      new(results)
+    def initialize(epic_id)
+      @epic_id = epic_id
     end
 
-    def initialize(results)
-      @results = results
+    def execute
+      strong_memoize(:execute) do
+        Milestone.joins(issues: :epic_issue).where(epic_issues: { epic_id: epic_id }).joins(
+          <<~SQL
+            INNER JOIN (
+              SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
+              FROM milestones
+              INNER JOIN issues ON issues.milestone_id = milestones.id
+              INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+              WHERE epic_issues.epic_id = #{epic_id}
+            ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
+          SQL
+        ).pluck(*FIELDS)
+      end
     end
 
     def start_date
@@ -45,16 +47,16 @@ def due_date_sourcing_milestone_id
 
     private
 
-    attr_reader :results
+    attr_reader :epic_id
 
     def start_date_sourcing_milestone
-      @start_date_sourcing_milestone ||= results
+      @start_date_sourcing_milestone ||= execute
         .reject { |row| row[START_DATE_INDEX].nil? }
         .min_by { |row| row[START_DATE_INDEX] }
     end
 
     def due_date_sourcing_milestone
-      @due_date_sourcing_milestone ||= results
+      @due_date_sourcing_milestone ||= execute
         .reject { |row| row[DUE_DATE_INDEX].nil? }
         .max_by { |row| row[DUE_DATE_INDEX] }
     end
diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 925cb265279ac9..65a63316c6e57f 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -93,7 +93,7 @@ def update_start_and_due_dates(epics)
         groups.each do |milestone_ids, epics|
           next if milestone_ids.empty?
 
-          results = Epics::DateSourcingMilestonesFinder.execute(epics.first.id)
+          results = Epics::DateSourcingMilestonesFinder.new(epics.first.id)
 
           self.where(id: epics.map(&:id)).update_all(
             [
@@ -143,7 +143,7 @@ def elapsed_days
     alias_attribute(:due_date, :end_date)
 
     def update_start_and_due_dates
-      results = Epics::DateSourcingMilestonesFinder.execute(id)
+      results = Epics::DateSourcingMilestonesFinder.new(id)
 
       self.start_date = start_date_is_fixed? ? start_date_fixed : results.start_date
       self.start_date_sourcing_milestone_id = results.start_date_sourcing_milestone_id
diff --git a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
index eab4557b88d0a7..d71840b3d7bed0 100644
--- a/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
+++ b/ee/db/post_migrate/20180713171825_update_epic_dates_from_milestones.rb
@@ -25,7 +25,7 @@ def self.update_start_and_due_dates(epics)
       groups.each do |milestone_ids, epics|
         next if milestone_ids.empty?
 
-        data = ::Epics::DateSourcingMilestonesFinder.execute(epics.first.id)
+        data = ::UpdateEpicDatesFromMilestones::Epics::DateSourcingMilestonesFinder.new(epics.first.id)
 
         self.where(id: epics.map(&:id)).update_all(
           [
@@ -45,6 +45,69 @@ def self.update_start_and_due_dates(epics)
     end
   end
 
+  module Epics
+    class DateSourcingMilestonesFinder
+      include Gitlab::Utils::StrongMemoize
+
+      FIELDS = [:id, :start_date, :due_date].freeze
+      ID_INDEX = FIELDS.index(:id)
+      START_DATE_INDEX = FIELDS.index(:start_date)
+      DUE_DATE_INDEX = FIELDS.index(:due_date)
+
+      def initialize(epic_id)
+        @epic_id = epic_id
+      end
+
+      def execute
+        strong_memoize(:execute) do
+          Milestone.joins(issues: :epic_issue).where(epic_issues: { epic_id: epic_id }).joins(
+            <<~SQL
+            INNER JOIN (
+              SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
+              FROM milestones
+              INNER JOIN issues ON issues.milestone_id = milestones.id
+              INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
+              WHERE epic_issues.epic_id = #{epic_id}
+            ) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
+          SQL
+          ).pluck(*FIELDS)
+        end
+      end
+
+      def start_date
+        start_date_sourcing_milestone&.slice(START_DATE_INDEX)
+      end
+
+      def start_date_sourcing_milestone_id
+        start_date_sourcing_milestone&.slice(ID_INDEX)
+      end
+
+      def due_date
+        due_date_sourcing_milestone&.slice(DUE_DATE_INDEX)
+      end
+
+      def due_date_sourcing_milestone_id
+        due_date_sourcing_milestone&.slice(ID_INDEX)
+      end
+
+      private
+
+      attr_reader :epic_id
+
+      def start_date_sourcing_milestone
+        @start_date_sourcing_milestone ||= execute
+          .reject { |row| row[START_DATE_INDEX].nil? }
+          .min_by { |row| row[START_DATE_INDEX] }
+      end
+
+      def due_date_sourcing_milestone
+        @due_date_sourcing_milestone ||= execute
+          .reject { |row| row[DUE_DATE_INDEX].nil? }
+          .max_by { |row| row[DUE_DATE_INDEX] }
+      end
+    end
+  end
+
   def up
     # Fill fixed date columns for remaining eligible records touched after regular migration is run
     # (20180711014026_update_date_columns_on_epics) but before new app code takes effect.
diff --git a/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb b/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
index b3a57830e02bbc..1bdb88dbb87644 100644
--- a/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
+++ b/ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb
@@ -13,7 +13,7 @@
       create(:issue, epic: epic, milestone: milestone2)
       create(:issue, epic: epic, milestone: milestone3)
 
-      results = described_class.execute(epic.id)
+      results = described_class.new(epic.id)
 
       expect(results).to have_attributes(
         start_date: milestone1.start_date,
@@ -28,7 +28,7 @@
       milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
       create(:issue, epic: epic, milestone: milestone1)
 
-      results = described_class.execute(epic.id)
+      results = described_class.new(epic.id)
 
       expect(results).to have_attributes(
         start_date: milestone1.start_date,
@@ -43,7 +43,7 @@
       milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1))
       create(:issue, epic: epic, milestone: milestone1)
 
-      results = described_class.execute(epic.id)
+      results = described_class.new(epic.id)
 
       expect(results).to have_attributes(
         start_date: milestone1.start_date,
@@ -56,7 +56,7 @@
     it 'handles epics without milestone' do
       epic = create(:epic)
 
-      results = described_class.execute(epic.id)
+      results = described_class.new(epic.id)
 
       expect(results).to have_attributes(
         start_date: nil,
-- 
GitLab


From 1e950bac78216de24b1aa33eafe135a5c4244e4e Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 7 Aug 2018 23:05:12 +0800
Subject: [PATCH 54/54] Fix spec: add new fields introduced in the new TODO
 feature

---
 ee/spec/helpers/epics_helper_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/helpers/epics_helper_spec.rb b/ee/spec/helpers/epics_helper_spec.rb
index d0e03996deb259..02e53942953933 100644
--- a/ee/spec/helpers/epics_helper_spec.rb
+++ b/ee/spec/helpers/epics_helper_spec.rb
@@ -50,7 +50,7 @@
         meta_data = JSON.parse(data[:meta])
 
         expect(meta_data.keys).to match_array(%w[
-          created author
+          created author epic_id todo_exists todo_path
           start_date start_date_fixed start_date_is_fixed start_date_from_milestones start_date_sourcing_milestone_title
           end_date due_date due_date_fixed due_date_is_fixed due_date_from_milestones due_date_sourcing_milestone_title
         ])
-- 
GitLab