Merge branch '9497-refactor-groups-epic-controllers' into 'master'

Refactor `Groups::EpicIssuesController` and `Groups::EpicLinksController`

Closes #9497

See merge request gitlab-org/gitlab-ee!10617

ee/app/controllers/concerns/epic_relations.rb

@@ -2,17 +2,26 @@
 
 module EpicRelations
   extend ActiveSupport::Concern
+  include Gitlab::Utils::StrongMemoize
   include IssuableLinks
 
   included do
-    skip_before_action :authorize_destroy_issuable!
-    skip_before_action :authorize_create_epic!
-    skip_before_action :authorize_update_issuable!
-
+    before_action :check_epics_available!
+    before_action :authorize_read_epic!, only: :index
     before_action :authorize_admin_epic!, only: [:create, :destroy, :update]
   end
 
+  def authorize_read_epic!
+    render_404 unless can?(current_user, :read_epic, epic)
+  end
+
   def authorize_admin_epic!
     render_403 unless can?(current_user, :admin_epic, epic)
   end
+
+  def epic
+    strong_memoize(:epic) do
+      group.epics.find_by_iid(params[:epic_id])
+    end
+  end
 end

ee/app/controllers/groups/epic_issues_controller.rb

 # frozen_string_literal: true
 
-class Groups::EpicIssuesController < Groups::EpicsController
+class Groups::EpicIssuesController < Groups::ApplicationController
   include EpicRelations
 
   before_action :authorize_issue_link_association!, only: [:destroy, :update]

ee/app/controllers/groups/epic_links_controller.rb

 # frozen_string_literal: true
 
-class Groups::EpicLinksController < Groups::EpicsController
+class Groups::EpicLinksController < Groups::ApplicationController
   include EpicRelations
 
   before_action :check_nested_support!

ee/spec/controllers/groups/epic_issues_controller_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Groups::EpicIssuesController do
   let(:group) { create(:group, :public) }
-  let(:project) { create(:project, :public, group: group) }
+  let(:project) { create(:project, group: group) }
   let(:milestone) { create(:milestone, project: project) }
   let(:epic) { create(:epic, group: group) }
   let(:user) { create(:user) }
@@ -35,18 +35,30 @@ describe Groups::EpicIssuesController do
     it_behaves_like 'unlicensed epics action'
 
     context 'when epics feature is enabled' do
-      before do
-        group.add_developer(user)
+      context 'when user has access to epic' do
+        before do
+          group.add_developer(user)
 
-        subject
-      end
+          subject
+        end
+
+        it 'returns status 200' do
+          expect(response.status).to eq(200)
+        end
 
-      it 'returns status 200' do
-        expect(response.status).to eq(200)
+        it 'returns the correct json' do
+          expect(JSON.parse(response.body)).to match_schema('related_issues', dir: 'ee')
+        end
       end
 
-      it 'returns the correct json' do
-        expect(JSON.parse(response.body)).to match_schema('related_issues', dir: 'ee')
+      context 'when user does not have access to epic' do
+        it 'returns 404 status' do
+          group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+
+          subject
+
+          expect(response).to have_gitlab_http_status(404)
+        end
       end
     end
   end

ee/spec/controllers/groups/epic_links_controller_spec.rb

@@ -38,16 +38,31 @@ describe Groups::EpicLinksController, :postgresql do
     context 'when epics are enabled' do
       before do
         stub_licensed_features(epics: true)
-        group.add_developer(user)
+      end
+
+      context 'when user has access to epic' do
+        before do
+          group.add_developer(user)
+
+          subject
+        end
 
-        subject
+        it 'returns the correct JSON response' do
+          list_service_response = EpicLinks::ListService.new(parent_epic, user).execute
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to eq(list_service_response.as_json)
+        end
       end
 
-      it 'returns the correct JSON response' do
-        list_service_response = EpicLinks::ListService.new(parent_epic, user).execute
+      context 'when user does not have access to epic' do
+        it 'returns 404 status' do
+          group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
 
-        expect(response).to have_gitlab_http_status(200)
-        expect(json_response).to eq(list_service_response.as_json)
+          subject
+
+          expect(response).to have_gitlab_http_status(404)
+        end
       end
     end
   end