From 9f3cb094409680423e8a237f8375a4a0fa4bfe18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jarka=20Kos=CC=8Canova=CC=81?= <jajaplz@gmail.com>
Date: Mon, 13 Dec 2021 16:47:32 +0100
Subject: [PATCH] Add validation for epic issue group hieararchy

- Removes the validation checks from services and
adds a validation to the model instead
- Modify the epic_issues factory to create
valid associations
- Fix specs with invalid records

Changelog: added
EE: true
---
 ee/app/models/epic_issue.rb                   | 10 ++-
 .../services/ee/issues/export_csv_service.rb  |  2 +-
 ee/app/services/ee/issues/move_service.rb     |  7 --
 ee/spec/factories/epic_issues.rb              | 30 ++++++-
 ee/spec/finders/epics_finder_spec.rb          | 11 +--
 ee/spec/lib/ee/gitlab/usage_data_spec.rb      |  3 +-
 .../bulk_epic_aggregate_loader_spec.rb        |  9 +-
 .../models/concerns/epic_tree_sorting_spec.rb | 16 ++--
 ee/spec/models/epic_issue_spec.rb             | 86 +++++++++++--------
 ee/spec/models/epic_spec.rb                   | 32 +++----
 .../group/epic/epic_aggregate_query_spec.rb   | 13 ++-
 .../projects/issues_controller_spec.rb        |  5 +-
 ee/spec/serializers/issue_serializer_spec.rb  |  9 +-
 .../services/epic_issues/list_service_spec.rb |  2 +-
 .../epic_issues/update_service_spec.rb        |  3 +-
 .../epics/descendant_count_service_spec.rb    |  2 +-
 .../epics/tree_reorder_service_spec.rb        | 24 ++----
 .../epics/update_dates_service_spec.rb        | 21 ++---
 .../issues/export_csv_service_spec.rb         | 84 +++++++++++-------
 .../milestones/update_service_spec.rb         |  5 +-
 locale/gitlab.pot                             |  3 +
 21 files changed, 220 insertions(+), 157 deletions(-)

diff --git a/ee/app/models/epic_issue.rb b/ee/app/models/epic_issue.rb
index d68e6443b4dc1a..6f97629a48c929 100644
--- a/ee/app/models/epic_issue.rb
+++ b/ee/app/models/epic_issue.rb
@@ -17,6 +17,7 @@ class EpicIssue < ApplicationRecord
   scope :in_epic, ->(epic_id) { where(epic_id: epic_id) }
 
   validate :validate_confidential_epic
+  validate :validate_group_structure
 
   def epic_tree_root?
     false
@@ -30,9 +31,12 @@ def self.epic_tree_node_query(node)
     select(selection).in_epic(node.parent_ids)
   end
 
-  # TODO add this method to validate records (see https://gitlab.com/gitlab-org/gitlab/-/issues/339514)
-  def epic_and_issue_at_same_group_hierarchy?
-    epic.group.self_and_hierarchy.include?(issue.project.group)
+  def validate_group_structure
+    return unless issue && epic
+    return if issue.project.group&.id == epic.group_id
+    return if issue.project.ancestors.include?(epic.group)
+
+    errors.add :issue, _('Cannot assign an issue that does not belong under the same group (or descendant) as the epic.')
   end
 
   private
diff --git a/ee/app/services/ee/issues/export_csv_service.rb b/ee/app/services/ee/issues/export_csv_service.rb
index 3310efb606c277..f0605e880f299c 100644
--- a/ee/app/services/ee/issues/export_csv_service.rb
+++ b/ee/app/services/ee/issues/export_csv_service.rb
@@ -28,7 +28,7 @@ def epic_issue_safe(attribute)
           epic = issue.epic
 
           next if epic.nil?
-          next unless issue.epic_issue.epic_and_issue_at_same_group_hierarchy? # TODO This check can be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/339514
+          next unless issue.epic_issue.valid? # TODO This check can be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/339514
 
           epic[attribute]
         end
diff --git a/ee/app/services/ee/issues/move_service.rb b/ee/app/services/ee/issues/move_service.rb
index 4a820fe7369ac5..1944b07aff48b2 100644
--- a/ee/app/services/ee/issues/move_service.rb
+++ b/ee/app/services/ee/issues/move_service.rb
@@ -19,7 +19,6 @@ def update_old_entity
       def rewrite_epic_issue
         return unless epic_issue = original_entity.epic_issue
         return unless can?(current_user, :update_epic, epic_issue.epic.group)
-        return unless epic_and_issue_at_same_group_hierarchy?(new_entity, epic_issue.epic)
 
         updated = epic_issue.update(issue_id: new_entity.id)
 
@@ -28,12 +27,6 @@ def rewrite_epic_issue
         original_entity.reset
       end
 
-      def epic_and_issue_at_same_group_hierarchy?(new_issue, epic)
-        temp_epic_issue = EpicIssue.new(issue_id: new_issue.id, epic_id: epic.id)
-
-        temp_epic_issue.epic_and_issue_at_same_group_hierarchy?
-      end
-
       def track_epic_issue_moved_from_project
         return unless original_entity.epic_issue
 
diff --git a/ee/spec/factories/epic_issues.rb b/ee/spec/factories/epic_issues.rb
index 478e88816a3a2f..f5e7d289928fb5 100644
--- a/ee/spec/factories/epic_issues.rb
+++ b/ee/spec/factories/epic_issues.rb
@@ -2,8 +2,34 @@
 
 FactoryBot.define do
   factory :epic_issue do
-    epic
-    issue
+    transient do
+      epic { nil }
+      issue { nil }
+    end
+
     relative_position { RelativePositioning::START_POSITION }
+
+    after(:build) do |epic_issue, evaluator|
+      epic = evaluator.epic
+      issue = evaluator.issue
+
+      if epic.present? && issue.nil?
+        epic_issue.epic = epic
+        epic_issue.issue = create(:issue, project: create(:project, group: epic.group))
+      elsif issue.present? && epic.nil?
+        unless issue.project.group.present?
+          raise 'Failed to create epic_issue. The issue needs to exist under a project with a parent group'
+        end
+
+        epic_issue.issue = issue
+        epic_issue.epic = create(:epic, group: issue.project.group)
+      elsif issue.nil? && epic.nil?
+        epic_issue.epic = create(:epic)
+        epic_issue.issue = create(:issue, project: create(:project, group: epic_issue.epic.group))
+      else
+        epic_issue.epic = epic
+        epic_issue.issue = issue
+      end
+    end
   end
 end
diff --git a/ee/spec/finders/epics_finder_spec.rb b/ee/spec/finders/epics_finder_spec.rb
index 7ac9086d04c01b..a07da7741dafe3 100644
--- a/ee/spec/finders/epics_finder_spec.rb
+++ b/ee/spec/finders/epics_finder_spec.rb
@@ -510,14 +510,11 @@ def epics(params = {})
           let_it_be(:base_epic2) { create(:epic, group: base_group) }
           let_it_be(:base_group_milestone) { create(:milestone, group: base_group) }
           let_it_be(:base_project_milestone) { create(:milestone, project: base_group_project) }
-          let_it_be(:project2) { base_group_project }
 
-          shared_examples 'filtered by milestone' do |milestone_type|
+          shared_examples 'filtered by milestone' do
             it 'returns expected epics' do
-              project3 = milestone_type == :group ? project2 : project
-
               create(:issue, project: project, milestone: milestone, epic: epic)
-              create(:issue, project: project3, milestone: milestone, epic: epic2)
+              create(:issue, project: project, milestone: milestone, epic: epic2)
 
               params[:milestone_title] = milestone.title
 
@@ -537,11 +534,11 @@ def epics(params = {})
               }
             end
 
-            it_behaves_like 'filtered by milestone', :group do
+            it_behaves_like 'filtered by milestone' do
               let_it_be(:milestone) { base_group_milestone }
             end
 
-            it_behaves_like 'filtered by milestone', :project do
+            it_behaves_like 'filtered by milestone' do
               let_it_be(:milestone) { base_project_milestone }
             end
 
diff --git a/ee/spec/lib/ee/gitlab/usage_data_spec.rb b/ee/spec/lib/ee/gitlab/usage_data_spec.rb
index 334ba79ddd1965..d9dd990f080f55 100644
--- a/ee/spec/lib/ee/gitlab/usage_data_spec.rb
+++ b/ee/spec/lib/ee/gitlab/usage_data_spec.rb
@@ -12,8 +12,9 @@
   end
 
   describe '.data' do
+    let_it_be(:group) { create(:group) }
     # using Array.new to create a different creator User for each of the projects
-    let_it_be(:projects) { Array.new(3) { create(:project, :repository, creator: create(:user, group_view: :security_dashboard)) } }
+    let_it_be(:projects) { Array.new(3) { create(:project, :repository, group: group, creator: create(:user, group_view: :security_dashboard)) } }
     let(:count_data) { subject[:counts] }
 
     let_it_be(:board) { create(:board, project: projects[0]) }
diff --git a/ee/spec/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader_spec.rb b/ee/spec/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader_spec.rb
index 080fa4177c044e..b86841e3c3b1bd 100644
--- a/ee/spec/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader_spec.rb
+++ b/ee/spec/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader_spec.rb
@@ -5,17 +5,18 @@
 RSpec.describe Gitlab::Graphql::Loaders::BulkEpicAggregateLoader do
   include_context 'includes EpicAggregate constants'
 
-  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:ancestor) { create(:group, :public) }
+  let_it_be(:group) { create(:group, :public, parent: ancestor) }
   let_it_be(:subgroup) { create(:group, :private, parent: group)}
 
   let_it_be(:project) { create(:project, namespace: group) }
   let_it_be(:subproject) { create(:project, namespace: subgroup) }
 
-  let_it_be(:parent_epic) { create(:epic, group: group, title: 'parent epic') }
-  let_it_be(:epic_with_issues) { create(:epic, group: subgroup, parent: parent_epic, state: :opened, title: 'epic with issues') }
+  let_it_be(:parent_epic) { create(:epic, group: ancestor, title: 'parent epic') }
+  let_it_be(:epic_with_issues) { create(:epic, group: group, parent: parent_epic, state: :opened, title: 'epic with issues') }
 
   # closed, no issues
-  let_it_be(:epic_without_issues) { create(:epic, group: subgroup, parent: parent_epic, state: :closed, title: 'epic without issues') }
+  let_it_be(:epic_without_issues) { create(:epic, group: group, parent: parent_epic, state: :closed, title: 'epic without issues') }
 
   # open, public
   let_it_be(:issue1) { create(:issue, project: project, weight: 1, state: :opened) }
diff --git a/ee/spec/models/concerns/epic_tree_sorting_spec.rb b/ee/spec/models/concerns/epic_tree_sorting_spec.rb
index ce75868c1cd93b..e2a70cd3b6a9a0 100644
--- a/ee/spec/models/concerns/epic_tree_sorting_spec.rb
+++ b/ee/spec/models/concerns/epic_tree_sorting_spec.rb
@@ -4,11 +4,15 @@
 
 RSpec.describe EpicTreeSorting do
   let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
   let_it_be(:base_epic) { create(:epic, group: group) }
+  let_it_be(:issue1) { create(:issue, project: project) }
+  let_it_be(:issue2) { create(:issue, project: project) }
+  let_it_be(:issue3) { create(:issue, project: project) }
 
-  let!(:epic_issue1) { create(:epic_issue, epic: base_epic, relative_position: 10) }
-  let!(:epic_issue2) { create(:epic_issue, epic: base_epic, relative_position: 500) }
-  let!(:epic_issue3) { create(:epic_issue, epic: base_epic, relative_position: 1002) }
+  let!(:epic_issue1) { create(:epic_issue, epic: base_epic, issue: issue1, relative_position: 10) }
+  let!(:epic_issue2) { create(:epic_issue, epic: base_epic, issue: issue2, relative_position: 500) }
+  let!(:epic_issue3) { create(:epic_issue, epic: base_epic, issue: issue3, relative_position: 1002) }
   let!(:epic1) { create(:epic, parent: base_epic, group: group, relative_position: 100) }
   let!(:epic2) { create(:epic, parent: base_epic, group: group, relative_position: 1000) }
   let!(:epic3) { create(:epic, parent: base_epic, group: group, relative_position: 1001) }
@@ -103,9 +107,9 @@ def polymorphic_ident(obj)
   end
 
   describe '#move_sequence' do
-    let!(:epic_issue1) { create(:epic_issue, epic: base_epic, relative_position: 1000) }
-    let!(:epic_issue2) { create(:epic_issue, epic: base_epic, relative_position: 1001) }
-    let!(:epic_issue3) { create(:epic_issue, epic: base_epic, relative_position: 1004) }
+    let!(:epic_issue1) { create(:epic_issue, epic: base_epic, issue: issue1, relative_position: 1000) }
+    let!(:epic_issue2) { create(:epic_issue, epic: base_epic, issue: issue2, relative_position: 1001) }
+    let!(:epic_issue3) { create(:epic_issue, epic: base_epic, issue: issue3, relative_position: 1004) }
     let!(:epic1) { create(:epic, parent: base_epic, group: group, relative_position: 1002) }
     let!(:epic2) { create(:epic, parent: base_epic, group: group, relative_position: 1003) }
     let!(:epic3) { create(:epic, parent: base_epic, group: group, relative_position: 1005) }
diff --git a/ee/spec/models/epic_issue_spec.rb b/ee/spec/models/epic_issue_spec.rb
index 1e870dbe1af220..e80af54d3e4e44 100644
--- a/ee/spec/models/epic_issue_spec.rb
+++ b/ee/spec/models/epic_issue_spec.rb
@@ -3,11 +3,17 @@
 require 'spec_helper'
 
 RSpec.describe EpicIssue do
+  let_it_be(:ancestor) { create(:group) }
+  let_it_be(:group) { create(:group, parent: ancestor) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:epic) { create(:epic, group: group) }
+  let_it_be(:issue) { create(:issue, project: project) }
+
   describe 'validations' do
-    let(:epic) { build(:epic) }
-    let(:confidential_epic) { build(:epic, :confidential) }
-    let(:issue) { build(:issue) }
-    let(:confidential_issue) { build(:issue, :confidential) }
+    let(:epic) { build(:epic, group: group) }
+    let(:confidential_epic) { build(:epic, :confidential, group: group) }
+    let(:issue) { build(:issue, project: project) }
+    let(:confidential_issue) { build(:issue, :confidential, project: project) }
 
     it 'is valid to add non-confidential issue to non-confidential epic' do
       expect(build(:epic_issue, epic: epic, issue: issue)).to be_valid
@@ -24,11 +30,49 @@
     it 'is not valid to add non-confidential issue to confidential epic' do
       expect(build(:epic_issue, epic: confidential_epic, issue: issue)).not_to be_valid
     end
+
+    context 'group hierarchy' do
+      let(:issue) { build(:issue, project: project) }
+      let(:error_message) do
+        'Issue Cannot assign an issue that does not belong under the same group (or descendant) as the epic.'
+      end
+
+      subject { described_class.new(epic: epic, issue: issue) }
+
+      context 'when epic and issue belong to the same group' do
+        let_it_be(:project) { project }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'when epic is in an ancestor group' do
+        let_it_be(:project) { create(:project, group: create(:group, parent: group)) }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'when epic is in a descendant group' do
+        let_it_be(:project) { create(:project, group: ancestor) }
+
+        it 'is invalid' do
+          expect(subject).not_to be_valid
+          expect(subject.errors.full_messages).to include(error_message)
+        end
+      end
+
+      context 'when epic and issue are at different group hierarchies' do
+        let_it_be(:project) { create(:project, group: create(:group)) }
+
+        it 'is invalid' do
+          expect(subject).not_to be_valid
+          expect(subject.errors.full_messages).to include(error_message)
+        end
+      end
+    end
   end
 
   context "relative positioning" do
     it_behaves_like "a class that supports relative positioning" do
-      let_it_be(:epic) { create(:epic) }
       let(:factory) { :epic_tree_node }
       let(:default_params) { { parent: epic, group: epic.group } }
 
@@ -38,9 +82,8 @@ def as_item(item)
     end
 
     context 'with a mixed tree level' do
-      let_it_be(:epic) { create(:epic) }
-      let_it_be_with_reload(:left) { create(:epic_issue, epic: epic, relative_position: 100) }
-      let_it_be_with_reload(:middle) { create(:epic, group: epic.group, parent: epic, relative_position: 101) }
+      let_it_be_with_reload(:left) { create(:epic_issue, epic: epic, issue: issue, relative_position: 100) }
+      let_it_be_with_reload(:middle) { create(:epic, group: group, parent: epic, relative_position: 101) }
       let_it_be_with_reload(:right) { create(:epic_issue, epic: epic, relative_position: 102) }
 
       it 'can create space to the right' do
@@ -77,31 +120,4 @@ def as_item(item)
       end
     end
   end
-
-  describe '#epic_and_issue_at_same_group_hierarchy?' do
-    let_it_be(:group) { create(:group) }
-    let_it_be(:group_a) { create(:group, parent: group) }
-    let_it_be(:group_b) { create(:group, parent: group) }
-    let_it_be(:group_a_project_1) { create(:project, group: group) }
-
-    let(:epic) { build(:epic, group: group_a) }
-
-    subject { described_class.new(epic: epic, issue: issue).epic_and_issue_at_same_group_hierarchy? }
-
-    context 'when epic and issue are at same group hierarchy' do
-      let_it_be(:group_a_project_2) { create(:project, group: group_a) }
-
-      let(:issue) { build(:issue, project: group_a_project_2) }
-
-      it { is_expected.to eq(true) }
-    end
-
-    context 'when epic and issue are at different group hierarchies' do
-      let_it_be(:group_b_project_1) { create(:project, group: group_b) }
-
-      let(:issue) { build(:issue, project: group_b_project_1) }
-
-      it { is_expected.to eq(false) }
-    end
-  end
 end
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index 7a063e30b3ec51..a3570008c979dd 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -110,12 +110,12 @@
       let_it_be(:milestone) { create(:milestone, project: project) }
 
       it 'returns epics which have an issue in the milestone' do
-        issue1 = create(:issue, project: project, milestone: milestone)
-        issue2 = create(:issue, project: project, milestone: milestone)
-        other_issue = create(:issue, project: project)
-        epic1 = create(:epic_issue, issue: issue1).epic
-        epic2 = create(:epic_issue, issue: issue2).epic
-        create(:epic_issue, issue: other_issue)
+        epic1 = create(:epic, group: group)
+        epic2 = create(:epic, group: group)
+        epic3 = create(:epic, group: group)
+        create(:issue, project: project, milestone: milestone, epic: epic1)
+        create(:issue, project: project, milestone: milestone, epic: epic2)
+        create(:issue, project: project, epic: epic3)
 
         expect(described_class.in_milestone(milestone.id)).to match_array([epic1, epic2])
       end
@@ -153,16 +153,15 @@
     end
 
     it 'is valid if epic is confidential and has only confidential issues' do
-      issue = create(:issue, :confidential)
-      epic = create(:epic_issue, issue: issue).epic
-
-      epic.confidential = true
+      epic = create(:epic, :confidential, group: group)
+      create(:issue, :confidential, project: project, epic: epic)
 
       expect(epic).to be_valid
     end
 
     it 'is not valid if epic is confidential and has non-confidential issues' do
-      epic = create(:epic_issue).epic
+      epic = create(:epic, group: group)
+      create(:issue, project: project, epic: epic)
 
       epic.confidential = true
 
@@ -700,7 +699,7 @@ def epics(order_by)
     end
 
     it 'has child issues' do
-      create(:epic_issue, epic: epic, issue: create(:issue))
+      create(:epic_issue, epic: epic)
 
       expect(epic.has_issues?).to be_truthy
     end
@@ -756,8 +755,9 @@ def epics(order_by)
   end
 
   context "relative positioning" do
-    let_it_be(:parent) { create(:epic) }
     let_it_be(:group) { create(:group) }
+    let_it_be(:parent) { create(:epic, group: group) }
+    let_it_be(:project) { create(:project, group: group) }
 
     context 'there is no parent' do
       let_it_be(:factory) { :epic }
@@ -783,9 +783,9 @@ def as_item(item)
     let_it_be(:epic2) { create(:epic, group: group, parent: epic1) }
     let_it_be(:epic3) { create(:epic, group: group, parent: epic2, state: :closed) }
     let_it_be(:epic4) { create(:epic, group: group) }
-    let_it_be(:issue1) { create(:issue, weight: 2) }
-    let_it_be(:issue2) { create(:issue, weight: 3) }
-    let_it_be(:issue3) { create(:issue, state: :closed) }
+    let_it_be(:issue1) { create(:issue, weight: 2, project: project) }
+    let_it_be(:issue2) { create(:issue, weight: 3, project: project) }
+    let_it_be(:issue3) { create(:issue, state: :closed, project: project) }
     let_it_be(:epic_issue1) { create(:epic_issue, epic: epic2, issue: issue1, relative_position: 5) }
     let_it_be(:epic_issue2) { create(:epic_issue, epic: epic2, issue: issue2, relative_position: 2) }
     let_it_be(:epic_issue3) { create(:epic_issue, epic: epic3, issue: issue3) }
diff --git a/ee/spec/requests/api/graphql/group/epic/epic_aggregate_query_spec.rb b/ee/spec/requests/api/graphql/group/epic/epic_aggregate_query_spec.rb
index b671d5f9ceee8f..3a8ea1e22aec91 100644
--- a/ee/spec/requests/api/graphql/group/epic/epic_aggregate_query_spec.rb
+++ b/ee/spec/requests/api/graphql/group/epic/epic_aggregate_query_spec.rb
@@ -6,8 +6,9 @@
   include GraphqlHelpers
 
   let_it_be(:current_user) { create(:user) }
-  let_it_be(:group) { create(:group, :public) }
-  let_it_be(:parent_epic) { create(:epic, group: group, title: 'parent epic') }
+  let_it_be(:ancestor) { create(:group, :public)}
+  let_it_be(:group) { create(:group, :public, parent: ancestor) }
+  let_it_be(:parent_epic) { create(:epic, group: ancestor, title: 'parent epic') }
 
   let(:target_epic) { parent_epic }
   let(:query) do
@@ -39,13 +40,11 @@
     subject { graphql_data.dig('group', 'epics', 'nodes') }
 
     let_it_be(:subgroup) { create(:group, :private, parent: group)}
-    let_it_be(:subsubgroup) { create(:group, :private, parent: subgroup)}
-
     let_it_be(:project) { create(:project, namespace: group) }
 
-    let_it_be(:epic_with_issues) { create(:epic, group: subgroup, parent: parent_epic, title: 'epic with issues') }
-    let_it_be(:epic_without_issues) { create(:epic, :closed, group: subgroup, parent: parent_epic, title: 'epic without issues') }
-    let_it_be(:closed_epic) { create(:epic, :closed, group: subgroup, parent: parent_epic, title: 'closed epic') }
+    let_it_be(:epic_with_issues) { create(:epic, group: group, parent: parent_epic, title: 'epic with issues') }
+    let_it_be(:epic_without_issues) { create(:epic, :closed, group: group, parent: parent_epic, title: 'epic without issues') }
+    let_it_be(:closed_epic) { create(:epic, :closed, group: group, parent: parent_epic, title: 'closed epic') }
 
     let_it_be(:issue1) { create(:issue, project: project, weight: 5, state: :opened) }
     let_it_be(:issue2) { create(:issue, project: project, weight: 7, state: :closed) }
diff --git a/ee/spec/requests/projects/issues_controller_spec.rb b/ee/spec/requests/projects/issues_controller_spec.rb
index f4bb415f20af97..fdbd10be29b9c1 100644
--- a/ee/spec/requests/projects/issues_controller_spec.rb
+++ b/ee/spec/requests/projects/issues_controller_spec.rb
@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Projects::IssuesController do
-  let_it_be(:issue) { create(:issue) }
   let_it_be(:group) { create(:group) }
-  let_it_be(:project) { issue.project }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:issue) { create(:issue, project: project) }
   let_it_be(:user) { issue.author }
   let_it_be(:blocking_issue) { create(:issue, project: project) }
   let_it_be(:blocked_by_issue) { create(:issue, project: project) }
@@ -52,6 +52,7 @@ def get_show
     end
 
     it 'exposes the escalation_policies licensed feature setting' do
+      project.add_guest(user)
       stub_licensed_features(escalation_policies: true)
 
       get_show
diff --git a/ee/spec/serializers/issue_serializer_spec.rb b/ee/spec/serializers/issue_serializer_spec.rb
index 437f11e61aec1a..fe6775b4feae3f 100644
--- a/ee/spec/serializers/issue_serializer_spec.rb
+++ b/ee/spec/serializers/issue_serializer_spec.rb
@@ -3,8 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe IssueSerializer do
-  let(:resource) { create(:issue) }
-  let(:user)     { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:resource) { create(:issue, project: project) }
+  let_it_be(:user) { create(:user) }
+
   let(:json_entity) do
     described_class.new(current_user: user)
       .represent(resource, serializer: serializer)
@@ -14,7 +17,7 @@
   before do
     stub_licensed_features(epics: true)
 
-    create(:epic, :use_fixed_dates).tap do |epic|
+    create(:epic, :use_fixed_dates, group: group).tap do |epic|
       create(:epic_issue, issue: resource, epic: epic)
     end
 
diff --git a/ee/spec/services/epic_issues/list_service_spec.rb b/ee/spec/services/epic_issues/list_service_spec.rb
index 105dc312bbdb20..60e58f880dba32 100644
--- a/ee/spec/services/epic_issues/list_service_spec.rb
+++ b/ee/spec/services/epic_issues/list_service_spec.rb
@@ -48,7 +48,7 @@
         group.add_developer(user)
         list_service = described_class.new(epic, user)
 
-        new_group = create(:group, :private)
+        new_group = create(:group, :private, parent: group)
         new_group.add_developer(user)
         new_project = create(:project, namespace: new_group)
         milestone = create(:milestone, project: project)
diff --git a/ee/spec/services/epic_issues/update_service_spec.rb b/ee/spec/services/epic_issues/update_service_spec.rb
index 79f0197bd181de..436bcc41a178bb 100644
--- a/ee/spec/services/epic_issues/update_service_spec.rb
+++ b/ee/spec/services/epic_issues/update_service_spec.rb
@@ -6,8 +6,9 @@
   describe '#execute' do
     let_it_be(:user) { create(:user) }
     let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
     let_it_be(:epic) { create(:epic, group: group) }
-    let_it_be(:issues) { create_list(:issue, 4) }
+    let_it_be(:issues) { create_list(:issue, 4, project: project) }
     let_it_be(:epic_issue1, reload: true) { create(:epic_issue, epic: epic, issue: issues[0], relative_position: 3) }
     let_it_be(:epic_issue2, reload: true) { create(:epic_issue, epic: epic, issue: issues[1], relative_position: 600) }
     let_it_be(:epic_issue3, reload: true) { create(:epic_issue, epic: epic, issue: issues[2], relative_position: 1200) }
diff --git a/ee/spec/services/epics/descendant_count_service_spec.rb b/ee/spec/services/epics/descendant_count_service_spec.rb
index 93f0e5bf65de50..93b90dc1a0840a 100644
--- a/ee/spec/services/epics/descendant_count_service_spec.rb
+++ b/ee/spec/services/epics/descendant_count_service_spec.rb
@@ -10,7 +10,7 @@
   let_it_be(:epic1) { create(:epic, group: subgroup, parent: parent_epic, state: :opened) }
   let_it_be(:epic2) { create(:epic, group: subgroup, parent: parent_epic, state: :closed) }
 
-  let_it_be(:project) { create(:project, :private, group: group)}
+  let_it_be(:project) { create(:project, :private, group: subgroup)}
   let_it_be(:issue1) { create(:issue, project: project, state: :opened, health_status: :on_track) }
   let_it_be(:issue2) { create(:issue, project: project, state: :closed, health_status: :on_track) }
   let_it_be(:issue3) { create(:issue, project: project, state: :opened) }
diff --git a/ee/spec/services/epics/tree_reorder_service_spec.rb b/ee/spec/services/epics/tree_reorder_service_spec.rb
index 3cf340050f157c..137391b6f23731 100644
--- a/ee/spec/services/epics/tree_reorder_service_spec.rb
+++ b/ee/spec/services/epics/tree_reorder_service_spec.rb
@@ -5,7 +5,8 @@
 RSpec.describe Epics::TreeReorderService do
   describe '#execute' do
     let(:user) { create(:user) }
-    let(:group) { create(:group) }
+    let(:ancestor) { create(:group) }
+    let(:group) { create(:group, parent: ancestor) }
     let(:project) { create(:project, group: group) }
     let(:epic) { create(:epic, group: group) }
     let(:issue1) { create(:issue, project: project) }
@@ -149,8 +150,7 @@
           end
 
           context 'when user does not have permissions to admin the previous parent' do
-            let(:other_group) { create(:group) }
-            let(:other_epic) { create(:epic, group: other_group) }
+            let(:other_epic) { create(:epic, group: ancestor) }
             let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
             before do
@@ -161,16 +161,14 @@
           end
 
           context 'when user does not have permissions to admin the new parent' do
-            let(:other_group) { create(:group) }
-            let(:other_epic) { create(:epic, group: other_group) }
+            let(:other_epic) { create(:epic, group: ancestor) }
             let(:new_parent_id) { GitlabSchema.id_from_object(other_epic) }
 
             it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
           end
 
           context 'when the epics of reordered epic-issue links are not subepics of the base epic' do
-            let(:another_group) { create(:group) }
-            let(:another_epic) { create(:epic, group: another_group) }
+            let(:another_epic) { create(:epic, group: ancestor) }
 
             before do
               epic_issue1.update!(epic: another_epic)
@@ -239,8 +237,7 @@
             end
 
             context 'when user does not have permissions to admin the previous parent' do
-              let(:other_group) { create(:group) }
-              let(:other_epic) { create(:epic, group: other_group) }
+              let(:other_epic) { create(:epic, group: ancestor) }
               let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
               before do
@@ -274,22 +271,19 @@
             end
 
             context 'when user does not have permissions to admin the new parent' do
-              let(:other_group) { create(:group) }
-              let(:other_epic) { create(:epic, group: other_group) }
+              let(:other_epic) { create(:epic, group: ancestor) }
               let(:new_parent_id) { GitlabSchema.id_from_object(other_epic) }
 
               it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
             end
 
-            context 'when user '
-
             context 'when the reordered epics are not subepics of the base epic' do
               let(:another_group) { create(:group) }
               let(:another_epic) { create(:epic, group: another_group) }
 
               before do
-                epic1.update!(group: another_group, parent: another_epic)
-                epic2.update!(group: another_group, parent: another_epic)
+                epic1.update!(group: ancestor, parent: another_epic)
+                epic2.update!(group: ancestor, parent: another_epic)
               end
 
               it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
diff --git a/ee/spec/services/epics/update_dates_service_spec.rb b/ee/spec/services/epics/update_dates_service_spec.rb
index 2ee81cadb3d8aa..5b7a742466e1b4 100644
--- a/ee/spec/services/epics/update_dates_service_spec.rb
+++ b/ee/spec/services/epics/update_dates_service_spec.rb
@@ -3,10 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe Epics::UpdateDatesService do
-  let(:group) { create(:group, :internal) }
-  let(:user) { create(:user) }
-  let(:project) { create(:project, group: group) }
-  let(:epic) { create(:epic, group: group) }
+  let_it_be(:group) { create(:group, :internal) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:epic) { create(:epic, group: group) }
+  let_it_be(:issue) { create(:issue, project: project) }
 
   describe '#execute' do
     context 'fixed date is set' do
@@ -120,7 +121,7 @@
 
       context 'without milestone' do
         before do
-          create(:epic_issue, epic: epic)
+          create(:epic_issue, epic: epic, issue: issue)
         end
 
         it 'updates to milestone dates' do
@@ -136,8 +137,8 @@
 
       context 'single milestone' do
         before do
-          epic_issue1 = create(:epic_issue, epic: epic)
-          epic_issue1.issue.update!(milestone: milestone1, project: project)
+          create(:epic_issue, epic: epic, issue: issue)
+          issue.update!(milestone: milestone1, project: project)
         end
 
         context 'complete start and due dates' do
@@ -202,9 +203,9 @@ def link_epic_to_milestone(epic, milestone)
         milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30), group: group)
 
         epics = [
-          create(:epic),
-          create(:epic),
-          create(:epic, :use_fixed_dates)
+          create(:epic, group: group),
+          create(:epic, group: group),
+          create(:epic, :use_fixed_dates, group: group)
         ]
         old_attributes = epics.map(&:attributes)
 
diff --git a/ee/spec/services/issues/export_csv_service_spec.rb b/ee/spec/services/issues/export_csv_service_spec.rb
index 54dd5e35e1dc23..0d7d6b91eb6103 100644
--- a/ee/spec/services/issues/export_csv_service_spec.rb
+++ b/ee/spec/services/issues/export_csv_service_spec.rb
@@ -15,49 +15,67 @@ def csv
     CSV.parse(subject.csv_data, headers: true)
   end
 
+  shared_examples 'including issues with epics' do
+    context 'with epics disabled' do
+      it 'does not include epics information' do
+        expect(csv[0]).not_to have_key('Epic ID')
+      end
+    end
+
+    context 'with epics enabled' do
+      before do
+        stub_licensed_features(epics: true)
+      end
+
+      specify 'epic ID' do
+        expect(csv[0]['Epic ID']).to eq(epic.id.to_s)
+        expect(csv[1]['Epic ID']).to be_nil
+      end
+
+      specify 'epic Title' do
+        expect(csv[0]['Epic Title']).to eq(epic.title)
+        expect(csv[1]['Epic Title']).to be_nil
+      end
+    end
+  end
+
   context 'includes' do
-    context 'handling epics' do
+    context 'when epic and issue are from the same group' do
       let(:epic) { create(:epic, group: group) }
 
       before do
         create(:epic_issue, issue: issue, epic: epic)
       end
 
-      context 'with epics disabled' do
-        it 'does not include epics information' do
-          expect(csv[0]).not_to have_key('Epic ID')
-        end
+      it_behaves_like 'including issues with epics'
+    end
+
+    context 'when epic is in an ancestor group' do
+      let_it_be(:ancestor) { create(:group) }
+      let_it_be(:epic) { create(:epic, group: ancestor) }
+
+      before do
+        group.update!(parent: ancestor)
+        create(:epic_issue, issue: issue, epic: epic)
       end
 
-      context 'with epics enabled' do
-        before do
-          stub_licensed_features(epics: true)
-        end
-
-        specify 'epic ID' do
-          expect(csv[0]['Epic ID']).to eq(epic.id.to_s)
-          expect(csv[1]['Epic ID']).to be_nil
-        end
-
-        specify 'epic Title' do
-          expect(csv[0]['Epic Title']).to eq(epic.title)
-          expect(csv[1]['Epic Title']).to be_nil
-        end
-
-        context 'when epic and issue are not from same group hierarchy' do
-          let(:epic) { create(:epic) }
-
-          specify 'epic ID' do
-            expect(csv[0]['Epic ID']).to be_nil
-            expect(csv[1]['Epic ID']).to be_nil
-          end
-
-          specify 'epic Title' do
-            expect(csv[0]['Epic Title']).to be_nil
-            expect(csv[1]['Epic Title']).to be_nil
-          end
-        end
+      it_behaves_like 'including issues with epics'
+    end
+  end
+
+  context 'when epic issue is not valid' do
+    let(:epic) { create(:epic, group: group) }
+
+    before do
+      create(:epic_issue, issue: issue, epic: epic)
+
+      allow_next_instance_of(EpicIssue) do |epic_issue|
+        allow(epic_issue).to receive(:valid?).and_return(false)
       end
     end
+
+    it 'does not include epics information' do
+      expect(csv[0]).not_to have_key('Epic ID')
+    end
   end
 end
diff --git a/ee/spec/services/milestones/update_service_spec.rb b/ee/spec/services/milestones/update_service_spec.rb
index 94496ce3c095aa..b7f520c9066258 100644
--- a/ee/spec/services/milestones/update_service_spec.rb
+++ b/ee/spec/services/milestones/update_service_spec.rb
@@ -5,10 +5,11 @@
   describe '#execute' do
     context 'refresh related epic dates' do
       it 'updates milestone sourced dates' do
-        project = create(:project)
+        group = create(:group)
+        project = create(:project, group: group)
         user = build(:user)
         milestone = create(:milestone, project: project)
-        epic = create(:epic)
+        epic = create(:epic, group: group)
         create(:issue, milestone: milestone, epic: epic, project: project)
         due_date = 3.days.from_now.to_date
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 6a7621557f2d61..4c24ab09a7cb38 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7173,6 +7173,9 @@ msgstr ""
 msgid "Cannot assign a confidential epic to a non-confidential issue. Make the issue confidential and try again"
 msgstr ""
 
+msgid "Cannot assign an issue that does not belong under the same group (or descendant) as the epic."
+msgstr ""
+
 msgid "Cannot be merged automatically"
 msgstr ""
 
-- 
GitLab