Use EpicsFinder when fetching epics

This is part of the work to implement confidential epics.

Use EpicsFinder to fetch epics on autocomplete and check
for right permissions when exposing issues epics on API.

ee/app/finders/epics_finder.rb

@@ -50,7 +50,8 @@ class EpicsFinder < IssuableFinder
   end
 
   def execute
-    raise ArgumentError, 'group_id argument is missing' unless group
+    raise ArgumentError, 'group_id argument is missing' unless params[:group_id]
+    return Epic.none unless Ability.allowed?(current_user, :read_epic, group)
 
     items = init_collection
     items = by_created_at(items)
@@ -72,19 +73,6 @@ class EpicsFinder < IssuableFinder
     sort(items)
   end
 
-  def group
-    return unless params[:group_id]
-    return @group if defined?(@group)
-
-    group = Group.find(params[:group_id])
-
-    unless Ability.allowed?(current_user, :read_epic, group)
-      raise ActiveRecord::RecordNotFound.new("Could not find a Group with ID #{params[:group_id]}")
-    end
-
-    @group = group
-  end
-
   # rubocop: disable CodeReuse/ActiveRecord
   def init_collection
     groups = if params[:iids].present?
@@ -104,6 +92,13 @@ class EpicsFinder < IssuableFinder
 
   private
 
+  def group
+    return unless params[:group_id]
+    return @group if defined?(@group)
+
+    @group = Group.find(params[:group_id])
+  end
+
   def starts_with_iid(items)
     return items unless params[:iid_starts_with].present?
 

ee/app/services/groups/autocomplete_service.rb

@@ -23,15 +23,11 @@ module Groups
     # rubocop: enable CodeReuse/ActiveRecord
 
     def epics
-      # TODO: change to EpicsFinder once frontend supports epics from external groups.
+      # TODO: use include_descendant_groups: true optional parameter once frontend supports epics from external groups.
       # See https://gitlab.com/gitlab-org/gitlab/issues/6837
-      DeclarativePolicy.user_scope do
-        if Ability.allowed?(current_user, :read_epic, group)
-          group.epics.select(:iid, :title)
-        else
-          []
-        end
-      end
+      EpicsFinder.new(current_user, group_id: group.id)
+        .execute
+        .select(:iid, :title)
     end
 
     # rubocop: disable CodeReuse/ActiveRecord

ee/lib/ee/api/entities/issue.rb

@@ -7,12 +7,18 @@ module EE
         extend ActiveSupport::Concern
 
         prepended do
-          with_options if: -> (issue, options) { ::Ability.allowed?(options[:current_user], :read_epic, issue.project&.group) } do
+          with_options if: -> (issue, _) { issue.project.group&.feature_available?(:epics) } do
             expose :epic_iid do |issue|
-              issue.epic&.iid
+              authorized_epic_for(issue)&.iid
             end
 
-            expose :epic, using: EpicBaseEntity
+            expose :epic, using: EpicBaseEntity do |issue|
+              authorized_epic_for(issue)
+            end
+
+            def authorized_epic_for(issue)
+              issue.epic if ::Ability.allowed?(options[:current_user], :read_epic, issue.epic)
+            end
           end
         end
       end

ee/spec/finders/epics_finder_spec.rb

@@ -43,8 +43,8 @@ describe EpicsFinder do
       end
 
       context 'when user can not read epics of a group' do
-        it 'raises an error when group_id param is missing' do
-          expect { epics }.to raise_error { ArgumentError }
+        it 'returns empty collection' do
+          expect(epics).to be_empty
         end
       end
 

ee/spec/requests/api/issues_spec.rb

@@ -44,19 +44,31 @@ describe API::Issues, :mailer do
         subject
 
         expect(response).to have_gitlab_http_status(:success)
-        expect(epic_issue_response_for(epic_issue)['epic_iid']).to eq(epic.iid)
+        expect(epic_issue_response_for(issue_with_epic)['epic_iid']).to eq(epic.iid)
       end
 
       it 'contains epic in response' do
         subject
 
         expect(response).to have_gitlab_http_status(:success)
-        expect(epic_issue_response_for(epic_issue)['epic']).to eq({ "id" => epic.id,
+        expect(epic_issue_response_for(issue_with_epic)['epic']).to eq({ "id" => epic.id,
                                                                     "iid" => epic.iid,
                                                                     "group_id" => epic.group_id,
                                                                     "title" => epic.title,
                                                                     "url" => group_epic_path(epic.group, epic) })
       end
+
+      context 'and epic issue is not present' do
+        it 'exposes epic as nil' do
+          issue_with_epic.epic_issue.destroy
+
+          subject
+
+          response = epic_issue_response_for(issue_with_epic)
+          expect(response['epic']).to eq(nil)
+          expect(response['epic_id']).to eq(nil)
+        end
+      end
     end
 
     context 'without epics feature' do
@@ -68,14 +80,14 @@ describe API::Issues, :mailer do
         subject
 
         expect(response).to have_gitlab_http_status(:success)
-        expect(epic_issue_response_for(epic_issue)).not_to have_key('epic_iid')
+        expect(epic_issue_response_for(issue_with_epic)).not_to have_key('epic_iid')
       end
 
       it 'does not contain epic_iid in response' do
         subject
 
         expect(response).to have_gitlab_http_status(:success)
-        expect(epic_issue_response_for(epic_issue)).not_to have_key('epic')
+        expect(epic_issue_response_for(issue_with_epic)).not_to have_key('epic')
       end
     end
   end
@@ -150,7 +162,7 @@ describe API::Issues, :mailer do
     end
 
     include_examples 'exposes epic' do
-      let!(:epic_issue) { create(:issue, project: group_project, epic: epic) }
+      let!(:issue_with_epic) { create(:issue, project: group_project, epic: epic) }
     end
   end
 
@@ -176,7 +188,7 @@ describe API::Issues, :mailer do
     end
 
     context 'on personal project' do
-      let!(:epic_issue) { create(:issue, project: project, epic: epic) }
+      let!(:issue_with_epic) { create(:issue, project: project, epic: epic) }
 
       before do
         stub_licensed_features(epics: true)
@@ -186,12 +198,12 @@ describe API::Issues, :mailer do
         subject
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(epic_issue_response_for(epic_issue)).not_to have_key('epic_iid')
+        expect(epic_issue_response_for(issue_with_epic)).not_to have_key('epic_iid')
       end
     end
 
     context 'on group project' do
-      let!(:epic_issue) { create(:issue, project: group_project, epic: epic) }
+      let!(:issue_with_epic) { create(:issue, project: group_project, epic: epic) }
 
       subject { get api("/projects/#{group_project.id}/issues", user) }
 
@@ -201,9 +213,9 @@ describe API::Issues, :mailer do
 
   describe 'GET /project/:id/issues/:issue_id' do
     context 'on personal project' do
-      let!(:epic_issue) { create(:issue, project: project, epic: epic) }
+      let!(:issue_with_epic) { create(:issue, project: project, epic: epic) }
 
-      subject { get api("/projects/#{project.id}/issues/#{epic_issue.iid}", user) }
+      subject { get api("/projects/#{project.id}/issues/#{issue_with_epic.iid}", user) }
 
       before do
         stub_licensed_features(epics: true)
@@ -213,14 +225,14 @@ describe API::Issues, :mailer do
         subject
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(epic_issue_response_for(epic_issue)).not_to have_key('epic_iid')
+        expect(epic_issue_response_for(issue_with_epic)).not_to have_key('epic_iid')
       end
     end
 
     context 'on group project' do
-      let!(:epic_issue) { create(:issue, project: group_project, epic: epic) }
+      let!(:issue_with_epic) { create(:issue, project: group_project, epic: epic) }
 
-      subject { get api("/projects/#{group_project.id}/issues/#{epic_issue.iid}", user) }
+      subject { get api("/projects/#{group_project.id}/issues/#{issue_with_epic.iid}", user) }
 
       include_examples 'exposes epic'
     end
@@ -399,8 +411,8 @@ describe API::Issues, :mailer do
 
   describe 'PUT /projects/:id/issues/:issue_id to update epic' do
     it_behaves_like 'with epic parameter' do
-      let(:epic_issue) { create(:issue, project: target_project) }
-      let(:request) { put api("/projects/#{target_project.id}/issues/#{epic_issue.iid}", user), params: params }
+      let(:issue_with_epic) { create(:issue, project: target_project) }
+      let(:request) { put api("/projects/#{target_project.id}/issues/#{issue_with_epic.iid}", user), params: params }
     end
   end
 

ee/spec/services/groups/autocomplete_service_spec.rb

@@ -83,15 +83,19 @@ describe Groups::AutocompleteService do
   end
 
   describe '#epics' do
+    before do
+      stub_licensed_features(epics: true)
+    end
+
     it 'returns nothing if not allowed' do
-      allow(Ability).to receive(:allowed?).with(user, :read_epic, group).and_return(false)
+      guest = create(:user)
 
-      expect(subject.epics).to eq([])
+      epics = described_class.new(group, guest).epics
+
+      expect(epics).to be_empty
     end
 
     it 'returns epics from group' do
-      allow(Ability).to receive(:allowed?).with(user, :read_epic, group).and_return(true)
-
       expect(subject.epics.map(&:iid)).to contain_exactly(epic.iid)
     end
   end