Inherit {start,end}_date from child epics or milestones

Epic would now inherit start_date or end_date from its
related issues milestone dates or from child epic
depending on which one gives the wider time spread.

Uses a single update statement to update start_date, due_date,
start_date_sourcing_milestone_id, start_date_sourcing_epic_id,
due_date_sourcing_milestone_id, due_date_sourcing_epic_id

Epic inherited dates update for multiple epics moved to
an async worker

Update epic dates in batches

Add foreign key constraints and indexes on
start_date_sourcing_epic_id and due_date_sourcing_epic_id

https://gitlab.com/gitlab-org/gitlab-ee/issues/7332

config/sidekiq_queues.yml

@@ -120,3 +120,4 @@
   - [update_external_pull_requests, 3]
   - [refresh_license_compliance_checks, 2]
   - [design_management_new_version, 1]
+  - [epics, 2]

db/migrate/20190703171157_add_sourcing_epic_dates.rb

+# frozen_string_literal: true
+
+class AddSourcingEpicDates < ActiveRecord::Migration[5.1]
+  DOWNTIME = false
+
+  def change
+    add_column :epics, :start_date_sourcing_epic_id, :integer
+    add_column :epics, :due_date_sourcing_epic_id, :integer
+  end
+end

db/migrate/20190703171555_add_sourcing_epic_dates_fks.rb

+# frozen_string_literal: true
+
+class AddSourcingEpicDatesFks < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :epics, :start_date_sourcing_epic_id, where: 'start_date_sourcing_epic_id is not null'
+    add_concurrent_index :epics, :due_date_sourcing_epic_id, where: 'due_date_sourcing_epic_id is not null'
+
+    add_concurrent_foreign_key :epics, :epics, column: :start_date_sourcing_epic_id, on_delete: :nullify
+    add_concurrent_foreign_key :epics, :epics, column: :due_date_sourcing_epic_id, on_delete: :nullify
+  end
+
+  def down
+    remove_foreign_key_if_exists :epics, column: :start_date_sourcing_epic_id
+    remove_foreign_key_if_exists :epics, column: :due_date_sourcing_epic_id
+
+    remove_concurrent_index :epics, :start_date_sourcing_epic_id
+    remove_concurrent_index :epics, :due_date_sourcing_epic_id
+  end
+end

db/schema.rb

@@ -1423,15 +1423,19 @@ ActiveRecord::Schema.define(version: 2019_10_17_045817) do
     t.integer "parent_id"
     t.integer "relative_position"
     t.integer "state_id", limit: 2, default: 1, null: false
+    t.integer "start_date_sourcing_epic_id"
+    t.integer "due_date_sourcing_epic_id"
     t.index ["assignee_id"], name: "index_epics_on_assignee_id"
     t.index ["author_id"], name: "index_epics_on_author_id"
     t.index ["closed_by_id"], name: "index_epics_on_closed_by_id"
+    t.index ["due_date_sourcing_epic_id"], name: "index_epics_on_due_date_sourcing_epic_id", where: "(due_date_sourcing_epic_id IS NOT NULL)"
     t.index ["end_date"], name: "index_epics_on_end_date"
     t.index ["group_id"], name: "index_epics_on_group_id"
     t.index ["iid"], name: "index_epics_on_iid"
     t.index ["milestone_id"], name: "index_milestone"
     t.index ["parent_id"], name: "index_epics_on_parent_id"
     t.index ["start_date"], name: "index_epics_on_start_date"
+    t.index ["start_date_sourcing_epic_id"], name: "index_epics_on_start_date_sourcing_epic_id", where: "(start_date_sourcing_epic_id IS NOT NULL)"
   end
 
   create_table "events", id: :serial, force: :cascade do |t|
@@ -4158,7 +4162,9 @@ ActiveRecord::Schema.define(version: 2019_10_17_045817) do
   add_foreign_key "epic_issues", "epics", on_delete: :cascade
   add_foreign_key "epic_issues", "issues", on_delete: :cascade
   add_foreign_key "epic_metrics", "epics", on_delete: :cascade
+  add_foreign_key "epics", "epics", column: "due_date_sourcing_epic_id", name: "fk_013c9f36ca", on_delete: :nullify
   add_foreign_key "epics", "epics", column: "parent_id", name: "fk_25b99c1be3", on_delete: :cascade
+  add_foreign_key "epics", "epics", column: "start_date_sourcing_epic_id", name: "fk_9d480c64b2", on_delete: :nullify
   add_foreign_key "epics", "milestones", on_delete: :nullify
   add_foreign_key "epics", "namespaces", column: "group_id", name: "fk_f081aa4489", on_delete: :cascade
   add_foreign_key "epics", "users", column: "assignee_id", name: "fk_dccd3f98fc", on_delete: :nullify

doc/api/epic_links.md

@@ -50,12 +50,14 @@ Example response:
     "start_date": null,
     "start_date_is_fixed": false,
     "start_date_fixed": null,
-    "start_date_from_milestones": null,
-    "end_date": "2018-07-31",
+    "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+    "start_date_from_inherited_source": null,
+    "end_date": "2018-07-31",                 //deprecated in favor of due_date
     "due_date": "2018-07-31",
     "due_date_is_fixed": false,
     "due_date_fixed": null,
-    "due_date_from_milestones": "2018-07-31",
+    "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+    "due_date_from_inherited_source": "2018-07-31",
     "created_at": "2018-07-17T13:36:22.770Z",
     "updated_at": "2018-07-18T12:22:05.239Z",
     "labels": []
@@ -102,12 +104,14 @@ Example response:
   "start_date": null,
   "start_date_is_fixed": false,
   "start_date_fixed": null,
-  "start_date_from_milestones": null,
-  "end_date": "2018-07-31",
+  "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+  "start_date_from_inherited_source": null,
+  "end_date": "2018-07-31",                 //deprecated in favor of due_date
   "due_date": "2018-07-31",
   "due_date_is_fixed": false,
   "due_date_fixed": null,
-  "due_date_from_milestones": "2018-07-31",
+  "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+  "due_date_from_inherited_source": "2018-07-31",
   "created_at": "2018-07-17T13:36:22.770Z",
   "updated_at": "2018-07-18T12:22:05.239Z",
   "labels": []
@@ -189,12 +193,14 @@ Example response:
     "start_date": null,
     "start_date_is_fixed": false,
     "start_date_fixed": null,
-    "start_date_from_milestones": null,
-    "end_date": "2018-07-31",
+    "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+    "start_date_from_inherited_source": null,
+    "end_date": "2018-07-31",                 //deprecated in favor of due_date
     "due_date": "2018-07-31",
     "due_date_is_fixed": false,
     "due_date_fixed": null,
-    "due_date_from_milestones": "2018-07-31",
+    "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+    "due_date_from_inherited_source": "2018-07-31",
     "created_at": "2018-07-17T13:36:22.770Z",
     "updated_at": "2018-07-18T12:22:05.239Z",
     "labels": []
@@ -241,12 +247,14 @@ Example response:
   "start_date": null,
   "start_date_is_fixed": false,
   "start_date_fixed": null,
-  "start_date_from_milestones": null,
-  "end_date": "2018-07-31",
+  "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+  "start_date_from_inherited_source": null,
+  "end_date": "2018-07-31",                 //deprecated in favor of due_date
   "due_date": "2018-07-31",
   "due_date_is_fixed": false,
   "due_date_fixed": null,
-  "due_date_from_milestones": "2018-07-31",
+  "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+  "due_date_from_inherited_source": "2018-07-31",
   "created_at": "2018-07-17T13:36:22.770Z",
   "updated_at": "2018-07-18T12:22:05.239Z",
   "labels": []

doc/api/epics.md

@@ -14,9 +14,13 @@ The [epic issues API](epic_issues.md) allows you to interact with issues associa
 
 > [Introduced][ee-6448] in GitLab 11.3.
 
-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`.
+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_inherited_source`, `due_date_fixed` and `due_date_from_inherited_source`.
 
-`end_date` has been deprecated in favor of `due_date`.
+- `end_date` has been deprecated in favor of `due_date`.
+- `start_date_from_milestones` has been deprecated in favor of `start_date_from_inherited_source`
+- `due_date_from_milestones` has been deprecated in favor of `due_date_from_inherited_source`
 
 ## Epics pagination
 
@@ -80,12 +84,14 @@ Example response:
   "start_date": null,
   "start_date_is_fixed": false,
   "start_date_fixed": null,
-  "start_date_from_milestones": null,
-  "end_date": "2018-07-31",
+  "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+  "start_date_from_inherited_source": null,
+  "end_date": "2018-07-31",                 //deprecated in favor of due_date
   "due_date": "2018-07-31",
   "due_date_is_fixed": false,
   "due_date_fixed": null,
-  "due_date_from_milestones": "2018-07-31",
+  "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+  "due_date_from_inherited_source": "2018-07-31",
   "created_at": "2018-07-17T13:36:22.770Z",
   "updated_at": "2018-07-18T12:22:05.239Z",
   "closed_at": "2018-08-18T12:22:05.239Z",
@@ -136,12 +142,14 @@ Example response:
   "start_date": null,
   "start_date_is_fixed": false,
   "start_date_fixed": null,
-  "start_date_from_milestones": null,
-  "end_date": "2018-07-31",
+  "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+  "start_date_from_inherited_source": null,
+  "end_date": "2018-07-31",                 //deprecated in favor of due_date
   "due_date": "2018-07-31",
   "due_date_is_fixed": false,
   "due_date_fixed": null,
-  "due_date_from_milestones": "2018-07-31",
+  "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+  "due_date_from_inherited_source": "2018-07-31",
   "created_at": "2018-07-17T13:36:22.770Z",
   "updated_at": "2018-07-18T12:22:05.239Z",
   "closed_at": "2018-08-18T12:22:05.239Z",
@@ -204,12 +212,14 @@ Example response:
   "start_date": null,
   "start_date_is_fixed": false,
   "start_date_fixed": null,
-  "start_date_from_milestones": null,
-  "end_date": "2018-07-31",
+  "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+  "start_date_from_inherited_source": null,
+  "end_date": "2018-07-31",                 //deprecated in favor of due_date
   "due_date": "2018-07-31",
   "due_date_is_fixed": false,
   "due_date_fixed": null,
-  "due_date_from_milestones": "2018-07-31",
+  "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+  "due_date_from_inherited_source": "2018-07-31",
   "created_at": "2018-07-17T13:36:22.770Z",
   "updated_at": "2018-07-18T12:22:05.239Z",
   "closed_at": "2018-08-18T12:22:05.239Z",
@@ -272,12 +282,14 @@ Example response:
   "start_date": null,
   "start_date_is_fixed": false,
   "start_date_fixed": null,
-  "start_date_from_milestones": null,
-  "end_date": "2018-07-31",
+  "start_date_from_milestones": null,       //deprecated in favor of start_date_from_inherited_source
+  "start_date_from_inherited_source": null,
+  "end_date": "2018-07-31",                 //deprecated in favor of due_date
   "due_date": "2018-07-31",
   "due_date_is_fixed": false,
   "due_date_fixed": null,
-  "due_date_from_milestones": "2018-07-31",
+  "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source
+  "due_date_from_inherited_source": "2018-07-31",
   "created_at": "2018-07-17T13:36:22.770Z",
   "updated_at": "2018-07-18T12:22:05.239Z",
   "closed_at": "2018-08-18T12:22:05.239Z",

doc/user/group/epics/index.md

@@ -92,24 +92,44 @@ To remove a child epic from a parent epic:
 
 ## Start date and due date
 
-To set a **Start date** and **Due date** for an epic, you can choose either of the following:
+To set a **Start date** and **Due date** for an epic, select one of the following:
 
 - **Fixed**: Enter a fixed value.
-- **From milestones:** Inherit a dynamic value from the issues added to the epic.
+- **From milestones**: Inherit a dynamic value from the issues added to the epic.
+- **Inherited**: Inherit a dynamic value from the issues added to the epic. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/7332) in GitLab 12.5 to replace **From milestones**).
 
-If you select **From milestones** for the start date, GitLab will automatically set the
-date to be earliest start date across all milestones that are currently assigned
-to the issues that are added to the epic. Similarly, if you select "From milestones"
-for the due date, GitLab will set it to be the latest due date across all
-milestones that are currently assigned to those issues.
+### Milestones
 
-These are dynamic dates which are recalculated immediately if any of the following occur:
+If you select **From milestones** for the start date, GitLab will automatically set the date to be earliest
+start date across all milestones that are currently assigned to the issues that are added to the epic.
+Similarly, if you select **From milestones** for the due date, GitLab will set it to be the latest due date across
+all milestones that are currently assigned to those issues.
+
+These are dynamic dates which are recalculated if any of the following occur:
 
 - Milestones are re-assigned to the issues.
 - Milestone dates change.
 - Issues are added or removed from the epic.
 
-## Roadmap
+### Inherited
+
+If you select **Inherited** for the start date, GitLab will scan all child epics and issues assigned to the epic,
+and will set the start date to match the earliest found start date or milestone. Similarly, if you select
+**Inherited** for the due date, GitLab will set the due date to match the latest due date or milestone
+found among its child epics and issues.
+
+These are dynamic dates and recalculated if any of the following occur:
+
+- A child epic's dates change.
+- Milestones are reassigned to an issue.
+- A milestone's dates change.
+- Issues are added to, or removed from, the epic.
+
+Because the epic's dates can inherit dates from its children, the start date and due date propagate from the bottom to the top.
+If the start date of a child epic on the lowest level changes, that becomes the earliest possible start date for its parent epic,
+then the parent epic's start date will reflect the change and this will propagate upwards to the top epic.
+
+## Roadmap in epics
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/7327) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.10.
 

doc/user/group/roadmap/index.md

@@ -26,7 +26,7 @@ Epics in the view can be sorted by:
 Each option contains a button that toggles the sort order between **ascending** and **descending**. The sort option and order will be persisted when browsing Epics,
 including the [epics list view](../epics/index.md).
 
-Roadmaps can also be [visualized inside an epic](../epics/index.md#roadmap).
+Roadmaps can also be [visualized inside an epic](../epics/index.md#roadmap-in-epics).
 
 ## Timeline duration
 

ee/app/assets/javascripts/epic/components/sidebar_items/sidebar_date_picker.vue

@@ -282,7 +282,7 @@ export default {
           type="radio"
           @click="toggleDateType(false)"
         />
-        <span class="prepend-left-5">{{ __('From milestones:') }}</span>
+        <span class="prepend-left-5">{{ __('Inherited:') }}</span>
         <span class="value-content prepend-left-2">{{ dateFromMilestonesWords }}</span>
         <icon
           v-if="isDateInvalid && !selectedDateIsFixed"

ee/app/models/ee/epic.rb

@@ -14,6 +14,7 @@ module EE
       include LabelEventable
       include RelativePositioning
       include UsageStatistics
+      include FromUnion
 
       enum state_id: {
         opened: ::Epic.available_states[:opened],
@@ -40,6 +41,8 @@ module EE
       belongs_to :group
       belongs_to :start_date_sourcing_milestone, class_name: 'Milestone'
       belongs_to :due_date_sourcing_milestone, class_name: 'Milestone'
+      belongs_to :start_date_sourcing_epic, class_name: 'Epic'
+      belongs_to :due_date_sourcing_epic, class_name: 'Epic'
       belongs_to :parent, class_name: "Epic"
       has_many :children, class_name: "Epic", foreign_key: :parent_id
       has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
@@ -55,8 +58,12 @@ module EE
       alias_attribute :parent_ids, :parent_id
       alias_method :issuing_parent, :group
 
+      scope :for_ids, -> (ids) { where(id: ids) }
       scope :in_parents, -> (parent_ids) { where(parent_id: parent_ids) }
       scope :inc_group, -> { includes(:group) }
+      scope :in_milestone, -> (milestone_id) { joins(:issues).where(issues: { milestone_id: milestone_id }) }
+      scope :in_issues, -> (issues) { joins(:epic_issues).where(epic_issues: { issue_id: issues }).distinct }
+      scope :has_parent, -> { where.not(parent_id: nil) }
 
       scope :order_start_or_end_date_asc, -> do
         reorder("COALESCE(start_date, end_date) ASC NULLS FIRST")
@@ -83,12 +90,31 @@ module EE
       end
 
       scope :with_api_entity_associations, -> { preload(:author, :labels, group: :route) }
+      scope :start_date_inherited, -> { where(start_date_is_fixed: [nil, false]) }
+      scope :due_date_inherited, -> { where(due_date_is_fixed: [nil, false]) }
 
       MAX_HIERARCHY_DEPTH = 5
 
       def etag_caching_enabled?
         true
       end
+
+      before_save :set_fixed_start_date, if: :start_date_is_fixed?
+      before_save :set_fixed_due_date, if: :due_date_is_fixed?
+
+      private
+
+      def set_fixed_start_date
+        self.start_date = start_date_fixed
+        self.start_date_sourcing_milestone = nil
+        self.due_date_sourcing_epic = nil
+      end
+
+      def set_fixed_due_date
+        self.end_date = due_date_fixed
+        self.due_date_sourcing_milestone = nil
+        self.due_date_sourcing_epic = nil
+      end
     end
 
     class_methods do
@@ -173,45 +199,6 @@ module EE
       def deepest_relationship_level
         ::Gitlab::ObjectHierarchy.new(self.where(parent_id: nil)).max_descendants_depth
       end
-
-      def update_start_and_due_dates(epics)
-        self.where(id: epics).update_all(
-          [
-            %{
-              start_date = CASE WHEN start_date_is_fixed = true THEN start_date_fixed ELSE (?) END,
-              start_date_sourcing_milestone_id = (?),
-              end_date = CASE WHEN due_date_is_fixed = true THEN due_date_fixed ELSE (?) END,
-              due_date_sourcing_milestone_id = (?)
-            },
-            start_date_milestone_query.select(:start_date),
-            start_date_milestone_query.select(:id),
-            due_date_milestone_query.select(:due_date),
-            due_date_milestone_query.select(:id)
-          ]
-        )
-      end
-
-      private
-
-      def start_date_milestone_query
-        source_milestones_query
-          .where.not(start_date: nil)
-          .order(:start_date, :id)
-          .limit(1)
-      end
-
-      def due_date_milestone_query
-        source_milestones_query
-          .where.not(due_date: nil)
-          .order(due_date: :desc, id: :desc)
-          .limit(1)
-      end
-
-      def source_milestones_query
-        ::Milestone
-          .joins(issues: :epic_issue)
-          .where('epic_issues.epic_id = epics.id')
-      end
     end
 
     def resource_parent
@@ -247,10 +234,6 @@ module EE
     # Needed to use EntityDateHelper#remaining_days_in_words
     alias_attribute(:due_date, :end_date)
 
-    def update_start_and_due_dates
-      self.class.update_start_and_due_dates([self])
-    end
-
     def start_date_from_milestones
       start_date_is_fixed? ? start_date_sourcing_milestone&.start_date : start_date
     end
@@ -259,6 +242,22 @@ module EE
       due_date_is_fixed? ? due_date_sourcing_milestone&.due_date : due_date
     end
 
+    def start_date_from_inherited_source
+      start_date_sourcing_milestone&.start_date || start_date_sourcing_epic&.start_date
+    end
+
+    def due_date_from_inherited_source
+      due_date_sourcing_milestone&.due_date || due_date_sourcing_epic&.end_date
+    end
+
+    def start_date_from_inherited_source_title
+      start_date_sourcing_milestone&.title || start_date_sourcing_epic&.title
+    end
+
+    def due_date_from_inherited_source_title
+      due_date_sourcing_milestone&.title || due_date_sourcing_epic&.title
+    end
+
     def to_reference(from = nil, full: false)
       reference = "#{self.class.reference_prefix}#{iid}"
 

ee/app/presenters/epic_presenter.rb

@@ -87,30 +87,36 @@ class EpicPresenter < Gitlab::View::Presenter::Delegated
     paths
   end
 
+  # todo:
+  #
+  # rename the hash keys to something more like inherited_source rather than milestone
+  # as now source can be noth milestone and child epic, but it does require a bunch of renaming on frontend as well
   def start_dates
     {
       start_date: epic.start_date,
       start_date_is_fixed: epic.start_date_is_fixed?,
       start_date_fixed: epic.start_date_fixed,
-      start_date_from_milestones: epic.start_date_from_milestones,
-      start_date_sourcing_milestone_title: epic.start_date_sourcing_milestone&.title,
+      start_date_from_milestones: epic.start_date_from_inherited_source,
+      start_date_sourcing_milestone_title: epic.start_date_from_inherited_source_title,
       start_date_sourcing_milestone_dates: {
-        start_date: epic.start_date_sourcing_milestone&.start_date,
-        due_date: epic.start_date_sourcing_milestone&.due_date
+        start_date: epic.start_date_from_inherited_source,
+        due_date: epic.due_date_from_inherited_source
       }
     }
   end
 
+  # todo:
+  # same renaming applies here
   def due_dates
     {
       due_date: epic.due_date,
       due_date_is_fixed: epic.due_date_is_fixed?,
       due_date_fixed: epic.due_date_fixed,
-      due_date_from_milestones: epic.due_date_from_milestones,
-      due_date_sourcing_milestone_title: epic.due_date_sourcing_milestone&.title,
+      due_date_from_milestones: epic.due_date_from_inherited_source,
+      due_date_sourcing_milestone_title: epic.due_date_from_inherited_source_title,
       due_date_sourcing_milestone_dates: {
-        start_date: epic.due_date_sourcing_milestone&.start_date,
-        due_date: epic.due_date_sourcing_milestone&.due_date
+        start_date: epic.start_date_from_inherited_source,
+        due_date: epic.due_date_from_inherited_source
       }
     }
   end

ee/app/services/ee/issues/update_service.rb

@@ -11,7 +11,7 @@ module EE
         result = super
 
         if issue.previous_changes.include?(:milestone_id) && issue.epic
-          issue.epic.update_start_and_due_dates
+          Epics::UpdateDatesService.new([issue.epic]).execute
         end
 
         result

ee/app/services/ee/milestones/update_service.rb

@@ -6,19 +6,15 @@ module EE
       extend ::Gitlab::Utils::Override
 
       override :execute
-      # rubocop: disable CodeReuse/ActiveRecord
       def execute(milestone)
         super
 
         if saved_change_to_dates?(milestone)
-          ::Epic.update_start_and_due_dates(
-            ::Epic.joins(:issues).where(issues: { milestone_id: milestone.id })
-          )
+          Epics::UpdateDatesService.new(::Epic.in_milestone(milestone.id)).execute
         end
 
         milestone
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       private
 

ee/app/services/epic_issues/create_service.rb

@@ -8,21 +8,16 @@ module EpicIssues
     def relate_issuables(referenced_issue)
       link = EpicIssue.find_or_initialize_by(issue: referenced_issue)
 
-      affected_epics = [issuable]
-
-      if link.persisted?
-        affected_epics << link.epic
-        params = { issue_moved: true, original_epic: link.epic }
+      params = if link.persisted?
+                 { issue_moved: true, original_epic: link.epic }
                else
-        params = {}
+                 {}
                end
 
       link.epic = issuable
       link.move_to_start
       link.save!
 
-      affected_epics.each(&:update_start_and_due_dates)
-
       yield params
     end
     # rubocop: enable CodeReuse/ActiveRecord
@@ -43,6 +38,10 @@ module EpicIssues
       { group: issuable.group }
     end
 
+    def affected_epics(issues)
+      [issuable, Epic.in_issues(issues)].flatten.uniq
+    end
+
     def linkable_issuables(issues)
       @linkable_issues ||= begin
         return [] unless can?(current_user, :admin_epic, issuable.group)

ee/app/services/epic_issues/destroy_service.rb

@@ -4,7 +4,8 @@ module EpicIssues
   class DestroyService < IssuableLinks::DestroyService
     def execute
       result = super
-      link.epic.update_start_and_due_dates
+      Epics::UpdateDatesService.new([link.epic]).execute
+
       result
     end
 

ee/app/services/epic_links/create_service.rb

@@ -12,14 +12,16 @@ module EpicLinks
 
     private
 
+    def affected_epics(epics)
+      [issuable, epics].flatten.uniq
+    end
+
     def relate_issuables(referenced_epic)
       affected_epics = [issuable]
       affected_epics << referenced_epic if referenced_epic.parent
 
       set_child_epic!(referenced_epic)
 
-      affected_epics.each(&:update_start_and_due_dates)
-
       yield
     end
 

ee/app/services/epics/strategies/base_dates_strategy.rb

+# frozen_string_literal: true
+
+module Epics
+  module Strategies
+    class BaseDatesStrategy
+      def initialize(epics)
+        @epics = epics
+      end
+
+      # rubocop: disable CodeReuse/ActiveRecord
+      def source_milestones_query
+        ::Milestone
+          .joins(issues: :epic_issue)
+          .where("epic_issues.epic_id = epics.id")
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+    end
+  end
+end

ee/app/services/epics/strategies/due_date_inherited_strategy.rb

+# frozen_string_literal: true
+
+module Epics
+  module Strategies
+    class DueDateInheritedStrategy < BaseDatesStrategy
+      # rubocop: disable CodeReuse/ActiveRecord
+      def execute
+        @epics.due_date_inherited.update_all(
+          [
+            %{ (end_date, due_date_sourcing_milestone_id, due_date_sourcing_epic_id) = (?) },
+            ::Epic.from_union([max_milestone_due_date, max_child_epics_end_date], alias_as: 'max_date')
+              .select('max_end_date', 'milestone_id', 'epic_id')
+              .order("max_end_date desc")
+              .limit(1)
+          ]
+        )
+      end
+
+      private
+
+      def max_milestone_due_date
+        source_milestones_query
+          .where.not(due_date: nil)
+          .select(
+            "milestones.due_date AS max_end_date",
+            "NULL AS epic_id",
+            "milestones.id AS milestone_id")
+      end
+
+      def max_child_epics_end_date
+        epic_dates = ::Epic.arel_table.alias('epic_dates')
+
+        ::Epic
+          .where.not(epic_dates: { end_date: nil })
+          .where("epic_dates.parent_id = epics.id")
+          .select(
+            "epic_dates.end_date AS max_end_date",
+            "epic_dates.id AS epic_id",
+            "NULL AS milestone_id")
+          .from(epic_dates)
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+    end
+  end
+end

ee/app/services/epics/strategies/start_date_inherited_strategy.rb

+# frozen_string_literal: true
+
+module Epics
+  module Strategies
+    class StartDateInheritedStrategy < BaseDatesStrategy
+      # rubocop: disable CodeReuse/ActiveRecord
+      def execute
+        @epics.start_date_inherited.update_all(
+          [
+            %{ (start_date, start_date_sourcing_milestone_id, start_date_sourcing_epic_id) = (?) },
+            ::Epic.from_union([min_milestone_start_date, min_child_epics_start_date], alias_as: 'min_date')
+              .select('min_start_date', 'milestone_id', 'epic_id')
+              .order("min_start_date asc")
+              .limit(1)
+          ]
+        )
+      end
+
+      private
+
+      def min_milestone_start_date
+        source_milestones_query
+          .where.not(start_date: nil)
+          .select(
+            "milestones.start_date AS min_start_date",
+            "NULL AS epic_id",
+            "milestones.id AS milestone_id")
+      end
+
+      def min_child_epics_start_date
+        epic_dates = ::Epic.arel_table.alias('epic_dates')
+
+        ::Epic
+          .where.not(epic_dates: { start_date: nil })
+          .where("epic_dates.parent_id = epics.id")
+          .select(
+            "epic_dates.start_date AS min_start_date",
+            "epic_dates.id AS epic_id",
+            "NULL AS milestone_id")
+          .from(epic_dates)
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+    end
+  end
+end

ee/app/services/epics/update_dates_service.rb

+# frozen_string_literal: true
+
+module Epics
+  class UpdateDatesService < ::BaseService
+    BATCH_SIZE = 100
+
+    STRATEGIES = [
+      Epics::Strategies::StartDateInheritedStrategy,
+      Epics::Strategies::DueDateInheritedStrategy
+    ].freeze
+
+    def initialize(epics)
+      @epics = epics
+      @epics = Epic.for_ids(@epics) unless @epics.is_a?(ActiveRecord::Relation)
+    end
+
+    def execute
+      each_batch do |relation, parent_ids|
+        STRATEGIES.each do |strategy|
+          strategy.new(relation).execute
+        end
+
+        if parent_ids.any? && Feature.enabled?(:epics_update_dates_upstream, default_enabled: true)
+          Epics::UpdateEpicsDatesWorker.perform_async(parent_ids)
+        end
+      end
+    end
+
+    private
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def each_batch
+      @epics.in_batches(of: BATCH_SIZE) do |relation| # rubocop: disable Cop/InBatches
+        parent_ids = relation.has_parent.distinct.pluck(:parent_id)
+
+        yield(relation, parent_ids)
+      end
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end

ee/app/services/epics/update_service.rb

@@ -17,7 +17,7 @@ module Epics
       update_task_event(epic) || update(epic)
 
       if saved_change_to_epic_dates?(epic)
-        epic.update_start_and_due_dates
+        Epics::UpdateDatesService.new([epic]).execute
         epic.reset
       end
 

ee/app/services/issuable_links/create_service.rb

@@ -37,11 +37,17 @@ module IssuableLinks
     def create_links
       objects = linkable_issuables(referenced_issuables)
 
+      # it is important that this is not called after relate_issuables, as it relinks epic to the issuable
+      # see EpicLinks::EpicIssues#relate_issuables
+      affected_epics = affected_epics(objects)
+
       objects.each do |referenced_object|
         relate_issuables(referenced_object) do |params|
           create_notes(referenced_object, params)
         end
       end
+
+      Epics::UpdateDatesService.new(affected_epics).execute unless affected_epics.blank?
     end
 
     def referenced_issuables
@@ -68,6 +74,10 @@ module IssuableLinks
       references(extractor)
     end
 
+    def affected_epics(issues)
+      []
+    end
+
     def references(extractor)
       extractor.issues
     end

ee/app/workers/all_queues.yml

@@ -75,3 +75,4 @@
 - refresh_license_compliance_checks
 - repository_update_mirror
 - repository_push_audit_event
+- epics:epics_update_epics_dates

ee/app/workers/epics/update_epics_dates_worker.rb

+# frozen_string_literal: true
+
+module Epics
+  class UpdateEpicsDatesWorker
+    include ApplicationWorker
+
+    queue_namespace :epics
+    feature_category :agile_portfolio_management
+
+    def perform(epic_ids)
+      return if epic_ids.blank?
+
+      Epics::UpdateDatesService.new(Epic.for_ids(epic_ids)).execute
+    end
+  end
+end

ee/changelogs/unreleased/7332-inherit-epic-dates.yml

+---
+title: Inherit children epics start and due dates
+merge_request: 14366
+author:
+type: changed

ee/lib/ee/api/entities.rb

@@ -288,11 +288,13 @@ module EE
         expose :author, using: ::API::Entities::UserBasic
         expose :start_date
         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 :start_date_fixed, :start_date_from_inherited_source, if: can_admin_epic
+        expose :start_date_from_milestones, if: can_admin_epic # @deprecated in favor of start_date_from_inherited_source
+        expose :end_date # @deprecated in favor of due_date
         expose :end_date, as: :due_date
         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 :due_date_fixed, :due_date_from_inherited_source, if: can_admin_epic
+        expose :due_date_from_milestones, if: can_admin_epic # @deprecated in favor of due_date_from_inherited_source
         expose :state
         expose :web_edit_url, if: can_admin_epic # @deprecated
         expose :web_url

ee/spec/fixtures/api/schemas/public_api/v4/epic.json

@@ -30,11 +30,13 @@
     "start_date": { "type": ["date", "null"] },
     "start_date_fixed": { "type": ["date", "null"] },
     "start_date_from_milestones": { "type": ["date", "null"] },
+    "start_date_from_inherited_source": { "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_from_inherited_source": { "type": ["date", "null"] },
     "due_date_is_fixed": { "type": "boolean" },
     "state": { "type": "string" },
     "created_at": { "type": ["string", "null"] },

ee/spec/frontend/epic/components/sidebar_items/__snapshots__/sidebar_date_picker_spec.js.snap

@@ -86,7 +86,7 @@ exports[`SidebarDatePicker renders expected template 1`] = `
       <span
         class="prepend-left-5"
       >
-        From milestones:
+        Inherited:
       </span>
        
       <span

ee/spec/frontend/epic/components/sidebar_items/sidebar_date_picker_spec.js

@@ -112,12 +112,12 @@ describe('SidebarDatePicker', () => {
   it('returns full date string in words when `dateFromMilestones` is defined', () => {
     createComponent({ dateFromMilestones: new Date(2018, 0, 1) });
 
-    expect(wrapper.text()).toContain('From milestones: Jan 1, 2018');
+    expect(wrapper.text()).toContain('Inherited: Jan 1, 2018');
   });
 
   it('returns `None` when `dateFromMilestones` is not defined', () => {
     createComponent();
-    expect(wrapper.text()).toContain('From milestones: None');
+    expect(wrapper.text()).toContain('Inherited: None');
   });
 
   it('passes correct popover options to directive', () => {

ee/spec/models/epic_spec.rb

@@ -324,260 +324,6 @@ describe Epic do
     end
   end
 
-  describe '#update_start_and_due_dates' do
-    def update_and_reload_subject
-      subject.update_start_and_due_dates
-      subject.reload
-    end
-
-    context 'fixed date is set' do
-      subject { create(:epic, :use_fixed_dates, start_date: nil, end_date: nil) }
-
-      it 'updates to fixed date' do
-        update_and_reload_subject
-
-        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, group: group) }
-
-      let(:milestone1) do
-        create(
-          :milestone,
-          start_date: Date.new(2000, 1, 1),
-          due_date: Date.new(2000, 1, 10),
-          group: group
-        )
-      end
-      let(:milestone2) do
-        create(
-          :milestone,
-          start_date: Date.new(2000, 1, 3),
-          due_date: Date.new(2000, 1, 20),
-          group: group
-        )
-      end
-
-      context 'multiple milestones' do
-        before do
-          issue1 = create(:issue, project: project, milestone: milestone1)
-          issue2 = create(:issue, project: project, milestone: milestone2)
-
-          create(:epic_issue, epic: subject, issue: issue1)
-          create(:epic_issue, epic: subject, issue: issue2)
-        end
-
-        context 'complete start and due dates' do
-          it 'updates to milestone dates' do
-            update_and_reload_subject
-
-            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) do
-            create(
-              :milestone,
-              start_date: Date.new(2000, 1, 1),
-              due_date: nil,
-              group: group
-            )
-          end
-          let(:milestone2) do
-            create(
-              :milestone,
-              start_date: Date.new(2000, 1, 3),
-              due_date: nil,
-              group: group
-            )
-          end
-
-          it 'updates to milestone dates' do
-            update_and_reload_subject
-
-            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) do
-            create(
-              :milestone,
-              start_date: nil,
-              due_date: nil,
-              group: group
-            )
-          end
-          let(:milestone2) do
-            create(
-              :milestone,
-              start_date: nil,
-              due_date: nil,
-              group: group
-            )
-          end
-
-          it 'updates to milestone dates' do
-            update_and_reload_subject
-
-            expect(subject.start_date).to eq(nil)
-            expect(subject.due_date).to eq(nil)
-          end
-        end
-      end
-
-      context 'without milestone' do
-        before do
-          create(:epic_issue, epic: subject)
-        end
-
-        it 'updates to milestone dates' do
-          update_and_reload_subject
-
-          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
-
-      context 'single milestone' do
-        before do
-          epic_issue1 = create(:epic_issue, epic: subject)
-          epic_issue1.issue.update(milestone: milestone1, project: project)
-        end
-
-        context 'complete start and due dates' do
-          it 'updates to milestone dates' do
-            update_and_reload_subject
-
-            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) do
-            create(
-              :milestone,
-              start_date: Date.new(2000, 1, 1),
-              due_date: nil,
-              group: group
-            )
-          end
-
-          it 'updates to milestone dates' do
-            update_and_reload_subject
-
-            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) do
-            create(
-              :milestone,
-              start_date: nil,
-              due_date: nil,
-              group: group
-            )
-          end
-
-          it 'updates to milestone dates' do
-            update_and_reload_subject
-
-            expect(subject.start_date).to eq(nil)
-            expect(subject.due_date).to eq(nil)
-          end
-        end
-      end
-    end
-  end
-
-  describe '.update_start_and_due_dates' do
-    def link_epic_to_milestone(epic, milestone)
-      create(:issue, epic: epic, milestone: milestone, project: project)
-    end
-
-    it 'updates in bulk' do
-      milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10), group: group)
-      milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30), group: group)
-
-      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_start_and_due_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), group: group) }
-      let!(:epics) { [create(:epic, group: group)] }
-
-      def setup_control_group
-        link_epic_to_milestone(epics[0], milestone)
-
-        ActiveRecord::QueryRecorder.new do
-          described_class.update_start_and_due_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_start_and_due_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_start_and_due_dates(described_class.where(id: epics.map(&:id)))
-        end.not_to exceed_query_limit(control_count)
-      end
-    end
-  end
-
   describe '.deepest_relationship_level' do
     context 'when there are no epics' do
       it 'returns nil' do

ee/spec/services/ee/issues/update_service_spec.rb

@@ -19,8 +19,8 @@ describe Issues::UpdateService do
       context 'updating milestone' do
         let(:milestone) { create(:milestone, project: project) }
 
-        it 'calls epic#update_start_and_due_dates' do
-          expect(epic).to receive(:update_start_and_due_dates).twice
+        it 'calls UpdateDatesService' do
+          expect(Epics::UpdateDatesService).to receive(:new).with([epic]).and_call_original.twice
 
           update_issue(milestone: milestone)
           update_issue(milestone_id: nil)
@@ -59,8 +59,8 @@ describe Issues::UpdateService do
       end
 
       context 'updating other fields' do
-        it 'does not call epic#update_start_and_due_dates' do
-          expect(epic).not_to receive(:update_start_and_due_dates)
+        it 'does not call UpdateDatesService' do
+          expect(Epics::UpdateDatesService).not_to receive(:new)
           update_issue(title: 'foo')
         end
       end

ee/spec/services/epic_issues/create_service_spec.rb

@@ -25,7 +25,7 @@ describe EpicIssues::CreateService do
       let(:created_link) { EpicIssue.find_by!(issue_id: issue.id) }
 
       it 'creates a new relationship and updates epic' do
-        expect(epic).to receive(:update_start_and_due_dates)
+        expect(Epics::UpdateDatesService).to receive(:new).with([epic]).and_call_original
         expect { subject }.to change(EpicIssue, :count).by(1)
 
         expect(created_link).to have_attributes(epic: epic)
@@ -262,14 +262,12 @@ describe EpicIssues::CreateService do
         end
 
         it 'updates both old and new epic milestone dates' do
+          expect(Epics::UpdateDatesService).to receive(:new).with([another_epic, issue.epic]).and_call_original
           allow(EpicIssue).to receive(:find_or_initialize_by).with(issue: issue).and_wrap_original { |m, *args|
             existing_epic_issue = m.call(*args)
-            expect(existing_epic_issue.epic).to receive(:update_start_and_due_dates)
             existing_epic_issue
           }
 
-          expect(another_epic).to receive(:update_start_and_due_dates)
-
           subject
         end
 

ee/spec/services/epic_issues/destroy_service_spec.rb

@@ -79,8 +79,8 @@ describe EpicIssues::DestroyService do
       end
 
       context 'refresh epic dates' do
-        it 'calls epic#update_start_and_due_dates' do
-          expect(epic).to receive(:update_start_and_due_dates)
+        it 'calls UpdateDatesService' do
+          expect(Epics::UpdateDatesService).to receive(:new).with([epic_issue.epic]).and_call_original
 
           subject
         end

ee/spec/services/epics/update_service_spec.rb

@@ -198,16 +198,18 @@ describe Epics::UpdateService do
 
     context 'refresh epic dates' do
       context 'date fields are updated' do
-        it 'calls epic#update_start_and_due_dates' do
-          expect(epic).to receive(:update_start_and_due_dates)
+        it 'calls UpdateDatesService' do
+          expect(Epics::UpdateDatesService).to receive(:new).with([epic]).and_call_original
 
           update_epic(start_date_is_fixed: true, start_date_fixed: Date.today)
+          epic.reload
+          expect(epic.start_date).to eq(epic.start_date_fixed)
         end
       end
 
       context 'date fields are not updated' do
-        it 'does not call epic#update_start_and_due_dates' do
-          expect(epic).not_to receive(:update_start_and_due_dates)
+        it 'does not call UpdateDatesService' do
+          expect(Epics::UpdateDatesService).not_to receive(:new)
 
           update_epic(title: 'foo')
         end

lib/gitlab/sql/union.rb

@@ -29,7 +29,7 @@ module Gitlab
         end
 
         if fragments.any?
-          fragments.join("\n#{union_keyword}\n")
+          "(" + fragments.join(")\n#{union_keyword}\n(") + ")"
         else
           'NULL'
         end

locale/gitlab.pot

@@ -7427,9 +7427,6 @@ msgstr ""
 msgid "From merge request merge until deploy to production"
 msgstr ""
 
-msgid "From milestones:"
-msgstr ""
-
 msgid "From the Kubernetes cluster details view, install Runner from the applications list"
 msgstr ""
 
@@ -9019,6 +9016,9 @@ msgstr ""
 msgid "Information about additional Pages templates and how to install them can be found in our %{pages_getting_started_guide}."
 msgstr ""
 
+msgid "Inherited:"
+msgstr ""
+
 msgid "Inline"
 msgstr ""
 

spec/lib/gitlab/sql/recursive_cte_spec.rb

@@ -20,7 +20,7 @@ describe Gitlab::SQL::RecursiveCTE do
         [rel1.except(:order).to_sql, rel2.except(:order).to_sql]
       end
 
-      expect(sql).to eq("#{name} AS (#{sql1}\nUNION\n#{sql2})")
+      expect(sql).to eq("#{name} AS ((#{sql1})\nUNION\n(#{sql2}))")
     end
   end
 

spec/lib/gitlab/sql/union_spec.rb

@@ -14,7 +14,7 @@ describe Gitlab::SQL::Union do
     it 'returns a String joining relations together using a UNION' do
       union = described_class.new([relation_1, relation_2])
 
-      expect(union.to_sql).to eq("#{to_sql(relation_1)}\nUNION\n#{to_sql(relation_2)}")
+      expect(union.to_sql).to eq("(#{to_sql(relation_1)})\nUNION\n(#{to_sql(relation_2)})")
     end
 
     it 'skips Model.none segements' do
@@ -22,7 +22,7 @@ describe Gitlab::SQL::Union do
       union = described_class.new([empty_relation, relation_1, relation_2])
 
       expect {User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error
-      expect(union.to_sql).to eq("#{to_sql(relation_1)}\nUNION\n#{to_sql(relation_2)}")
+      expect(union.to_sql).to eq("(#{to_sql(relation_1)})\nUNION\n(#{to_sql(relation_2)})")
     end
 
     it 'uses UNION ALL when removing duplicates is disabled' do

spec/models/concerns/from_union_spec.rb

@@ -15,7 +15,7 @@ describe FromUnion do
     it 'selects from the results of the UNION' do
       query = model.from_union([model.where(id: 1), model.where(id: 2)])
 
-      expect(query.to_sql).to match(/FROM \(SELECT.+UNION.+SELECT.+\) users/m)
+      expect(query.to_sql).to match(/FROM \(\(SELECT.+\)\nUNION\n\(SELECT.+\)\) users/m)
     end
 
     it 'supports the use of a custom alias for the sub query' do
@@ -24,7 +24,7 @@ describe FromUnion do
         alias_as: 'kittens'
       )
 
-      expect(query.to_sql).to match(/FROM \(SELECT.+UNION.+SELECT.+\) kittens/m)
+      expect(query.to_sql).to match(/FROM \(\(SELECT.+\)\nUNION\n\(SELECT.+\)\) kittens/m)
     end
 
     it 'supports keeping duplicate rows' do
@@ -34,7 +34,7 @@ describe FromUnion do
       )
 
       expect(query.to_sql)
-        .to match(/FROM \(SELECT.+UNION ALL.+SELECT.+\) users/m)
+        .to match(/FROM \(\(SELECT.+\)\nUNION ALL\n\(SELECT.+\)\) users/m)
     end
   end
 end