Add related issues public API

app/controllers/projects/issue_links_controller.rb

@@ -15,12 +15,9 @@ module Projects
 
     def destroy
       issue_link = IssueLink.find(params[:id])
+      result = IssueLinks::DestroyService.new(issue_link, current_user).execute
 
-      return render_403 unless can?(current_user, :admin_issue_link, issue_link.target.project)
-
-      IssueLinks::DestroyService.new(issue_link, current_user).execute
-
-      render json: { issues: issues }
+      render json: { issues: issues }, status: result[:http_status]
     end
 
     private

app/models/issue.rb

@@ -184,6 +184,19 @@ class Issue < ActiveRecord::Base
     branches_with_iid - branches_with_merge_request
   end
 
+  def related_issues(current_user, preload: nil)
+    related_issues = Issue
+                       .select(['issues.*', 'issue_links.id AS issue_link_id'])
+                       .joins("INNER JOIN issue_links ON
+                                 (issue_links.source_id = issues.id AND issue_links.target_id = #{id})
+                                 OR
+                                 (issue_links.target_id = issues.id AND issue_links.source_id = #{id})")
+                       .preload(preload)
+                       .reorder('issue_link_id')
+
+    Ability.issues_readable_by_user(related_issues, current_user)
+  end
+
   # Returns boolean if a related branch exists for the current issue
   # ignores merge requests branchs
   def has_related_branch?

app/services/issue_links/create_service.rb

@@ -6,7 +6,7 @@ module IssueLinks
 
     def execute
       if referenced_issues.blank?
-        return error('No Issue found for given reference', 401)
+        return error('No Issue found for given params', 401)
       end
 
       create_issue_links
@@ -32,16 +32,26 @@ module IssueLinks
 
     def referenced_issues
       @referenced_issues ||= begin
-        issue_references = params[:issue_references]
-        text = issue_references.join(' ')
+        target_issue = params[:target_issue]
 
-        extractor = Gitlab::ReferenceExtractor.new(@issue.project, @current_user)
-        extractor.analyze(text)
+        issues = if params[:issue_references].present?
+                   extract_issues_from_references
+                 elsif target_issue
+                   [target_issue]
+                 end
 
-        extractor.issues.select do |issue|
-          can?(current_user, :admin_issue_link, issue)
-        end
+        issues&.select { |issue| can?(current_user, :admin_issue_link, issue) }
       end
     end
+
+    def extract_issues_from_references
+      issue_references = params[:issue_references]
+      text = issue_references.join(' ')
+
+      extractor = Gitlab::ReferenceExtractor.new(@issue.project, @current_user)
+      extractor.analyze(text)
+
+      extractor.issues
+    end
   end
 end

app/services/issue_links/destroy_service.rb

@@ -8,6 +8,8 @@ module IssueLinks
     end
 
     def execute
+      return error('Unauthorized', 401) unless permission_to_remove_relation?
+
       remove_relation
       create_notes
 
@@ -24,5 +26,10 @@ module IssueLinks
       SystemNoteService.unrelate_issue(@issue, @referenced_issue, current_user)
       SystemNoteService.unrelate_issue(@referenced_issue, @issue, current_user)
     end
+
+    def permission_to_remove_relation?
+      can?(current_user, :admin_issue_link, @issue) &&
+        can?(current_user, :admin_issue_link, @referenced_issue)
+    end
   end
 end

app/services/issue_links/list_service.rb

@@ -22,16 +22,7 @@ module IssueLinks
     private
 
     def issues
-      related_issues = Issue
-                         .select(['issues.*', 'issue_links.id AS issue_links_id'])
-                         .joins("INNER JOIN issue_links ON
-                                 (issue_links.source_id = issues.id AND issue_links.target_id = #{@issue.id})
-                                 OR
-                                 (issue_links.target_id = issues.id AND issue_links.source_id = #{@issue.id})")
-                         .preload(project: :namespace)
-                         .reorder('issue_links_id')
-
-      Ability.issues_readable_by_user(related_issues, @current_user)
+      @issue.related_issues(@current_user, preload: { project: :namespace })
     end
 
     def destroy_relation_path(issue)
@@ -41,7 +32,7 @@ module IssueLinks
         namespace_project_issue_link_path(@project.namespace,
                                           @issue.project,
                                           @issue.iid,
-                                          issue.issue_links_id)
+                                          issue.issue_link_id)
       end
     end
 

changelogs/unreleased-ee/2633-related-issues-public-api.yml

+---
+title: Add public API for listing, creating and deleting Related Issues
+merge_request:
+author:

lib/api/api.rb

@@ -106,6 +106,7 @@ module API
     mount ::API::Geo
     mount ::API::Internal
     mount ::API::Issues
+    mount ::API::IssueLinks
     mount ::API::Jobs
     mount ::API::Keys
     mount ::API::Labels

lib/api/entities.rb

@@ -320,6 +320,16 @@ module API
       end
     end
 
+    class RelatedIssue < Issue
+      expose :issue_link_id
+    end
+
+    class IssueLink < Grape::Entity
+      expose :id
+      expose :source_id, as: :source_issue_id
+      expose :target_id, as: :target_issue_id
+    end
+
     class IssuableTimeStats < Grape::Entity
       expose :time_estimate
       expose :total_time_spent

lib/api/helpers.rb

@@ -92,8 +92,9 @@ module API
       label || not_found!('Label')
     end
 
-    def find_project_issue(iid)
-      IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
+    def find_project_issue(iid, project_id = nil)
+      project = project_id ? find_project!(project_id) : user_project
+      IssuesFinder.new(current_user, project_id: project.id).find_by!(iid: iid)
     end
 
     def find_project_merge_request(iid)

lib/api/issue_links.rb

+module API
+  class IssueLinks < Grape::API
+    include PaginationParams
+
+    before { authenticate! }
+
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+      requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
+    end
+    resource :projects, requirements: { id: %r{[^/]+} } do
+      desc 'Get related issues' do
+        success Entities::RelatedIssue
+      end
+      get ':id/issues/:issue_iid/links' do
+        source_issue = find_project_issue(params[:issue_iid])
+        related_issues = source_issue.related_issues(current_user)
+
+        present related_issues,
+                with: Entities::RelatedIssue,
+                current_user: current_user,
+                project: user_project
+      end
+
+      desc 'Relate issues' do
+        success Entities::IssueLink
+      end
+      params do
+        requires :target_project_id, type: String, desc: 'The ID of the target project'
+        requires :target_issue_iid, type: Integer, desc: 'The IID of the target issue'
+      end
+      post ':id/issues/:issue_iid/links' do
+        source_issue = find_project_issue(params[:issue_iid])
+
+        target_issue = find_project_issue(declared_params[:target_issue_iid],
+                                          declared_params[:target_project_id])
+
+        create_params = { target_issue: target_issue }
+
+        result = ::IssueLinks::CreateService
+                   .new(source_issue, current_user, create_params)
+                   .execute
+
+        if result[:status] == :success
+          issue_link = IssueLink.find_by!(source: source_issue, target: target_issue)
+
+          present issue_link, with: Entities::IssueLink
+        else
+          render_api_error!(result[:message], result[:http_status])
+        end
+      end
+
+      desc 'Remove issues relation' do
+        success Entities::IssueLink
+      end
+      params do
+        requires :issue_link_id, type: Integer, desc: 'The ID of an issue link'
+      end
+      delete ':id/issues/:issue_iid/links/:issue_link_id' do
+        issue_link = IssueLink.find_by(id: declared_params[:issue_link_id])
+
+        not_found! unless issue_link
+
+        result = ::IssueLinks::DestroyService
+                   .new(issue_link, current_user)
+                   .execute
+
+        if result[:status] == :success
+          present issue_link, with: Entities::IssueLink
+        else
+          render_api_error!(result[:message], result[:http_status])
+        end
+      end
+    end
+  end
+end

spec/models/issue_spec.rb

@@ -329,6 +329,27 @@ describe Issue, models: true do
     end
   end
 
+  describe '#related_issues' do
+    let(:user) { create(:user) }
+    let(:authorized_project) { create(:empty_project) }
+    let(:unauthorized_project) { create(:empty_project) }
+
+    let(:authorized_issue_a) { create(:issue, project: authorized_project) }
+    let(:authorized_issue_b) { create(:issue, project: authorized_project) }
+    let(:unauthorized_issue) { create(:issue, project: unauthorized_project) }
+
+    let!(:issue_link_a) { create(:issue_link, source: authorized_issue_a, target: authorized_issue_b) }
+    let!(:issue_link_b) { create(:issue_link, source: authorized_issue_a, target: unauthorized_issue) }
+
+    before do
+      authorized_project.add_developer(user)
+    end
+
+    it 'returns only authorized related issues for given user' do
+      expect(authorized_issue_a.related_issues(user)).to contain_exactly(authorized_issue_b)
+    end
+  end
+
   describe '#has_related_branch?' do
     let(:issue) { create(:issue, title: "Blue Bell Knoll") }
     subject { issue.has_related_branch? }

spec/requests/api/issue_links_spec.rb

+require 'spec_helper'
+
+describe API::IssueLinks do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+
+  before do
+    project.add_guest(user)
+  end
+
+  describe 'GET /links' do
+    context 'when unauthenticated' do
+      it 'returns 401' do
+        get api("/projects/#{project.id}/issues/#{issue.iid}/links")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+
+    context 'when authenticated' do
+      it 'returns related issues' do
+        target_issue = create(:issue, project: project)
+        create(:issue_link, source: issue, target: target_issue)
+
+        get api("/projects/#{project.id}/issues/#{issue.iid}/links", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+      end
+    end
+  end
+
+  describe 'POST /links' do
+    context 'when unauthenticated' do
+      it 'returns 401' do
+        target_issue = create(:issue)
+
+        post api("/projects/#{project.id}/issues/#{issue.iid}/links"),
+             target_project_id: target_issue.project.id, target_issue_iid: target_issue.iid
+
+        expect(response).to have_http_status(401)
+      end
+    end
+
+    context 'when authenticated' do
+      context 'given target project not found' do
+        it 'returns 404' do
+          target_issue = create(:issue)
+
+          post api("/projects/#{project.id}/issues/#{issue.iid}/links", user),
+               target_project_id: 999, target_issue_iid: target_issue.iid
+
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'given target issue not found' do
+        it 'returns 404' do
+          target_project = create(:empty_project)
+
+          post api("/projects/#{project.id}/issues/#{issue.iid}/links", user),
+               target_project_id: target_project.id, target_issue_iid: 999
+
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'when user does not have write access to given issue' do
+        it 'returns 401' do
+          unauthorized_project = create(:empty_project)
+          target_issue = create(:issue, project: unauthorized_project)
+          unauthorized_project.add_guest(user)
+
+          post api("/projects/#{project.id}/issues/#{issue.iid}/links", user),
+               target_project_id: unauthorized_project.id, target_issue_iid: target_issue.iid
+
+          expect(response).to have_http_status(401)
+          expect(json_response['message']).to eq('No Issue found for given params')
+        end
+      end
+
+      context 'success' do
+        it 'returns 201' do
+          target_issue = create(:issue, project: project)
+          project.add_reporter(user)
+
+          post api("/projects/#{project.id}/issues/#{issue.iid}/links", user),
+               target_project_id: project.id, target_issue_iid: target_issue.iid
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to include('id', 'source_issue_id', 'target_issue_id')
+        end
+
+        it 'returns 201 when sending full path of target project' do
+          target_issue = create(:issue, project: project)
+          project.add_reporter(user)
+
+          post api("/projects/#{project.id}/issues/#{issue.iid}/links", user),
+               target_project_id: project.to_reference(full: true), target_issue_iid: target_issue.iid
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to include('id', 'source_issue_id', 'target_issue_id')
+        end
+      end
+    end
+  end
+
+  describe 'DELETE /links/:issue_link_id' do
+    context 'when unauthenticated' do
+      it 'returns 401' do
+        issue_link = create(:issue_link)
+
+        delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+
+    context 'when authenticated' do
+      context 'when user does not have write access to given issue link' do
+        it 'returns 401' do
+          unauthorized_project = create(:empty_project)
+          target_issue = create(:issue, project: unauthorized_project)
+          issue_link = create(:issue_link, source: issue, target: target_issue)
+          unauthorized_project.add_guest(user)
+
+          delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user)
+
+          expect(response).to have_http_status(401)
+        end
+      end
+
+      context 'issue link not found' do
+        it 'returns 404' do
+          delete api("/projects/#{project.id}/issues/#{issue.iid}/links/999", user)
+
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'success' do
+        it 'returns 200' do
+          target_issue = create(:issue, project: project)
+          issue_link = create(:issue_link, source: issue, target: target_issue)
+          project.add_reporter(user)
+
+          delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user)
+
+          expect(response).to have_http_status(200)
+          expect(json_response).to include('id', 'source_issue_id', 'target_issue_id')
+        end
+      end
+    end
+  end
+end

spec/requests/projects/issue_links_controller_spec.rb

@@ -74,14 +74,14 @@ describe Projects::IssueLinksController do
           list_service_response = IssueLinks::ListService.new(issue, user).execute
 
           expect(response).to have_http_status(401)
-          expect(json_response).to eq('message' => 'No Issue found for given reference', 'issues' => list_service_response.as_json)
+          expect(json_response).to eq('message' => 'No Issue found for given params', 'issues' => list_service_response.as_json)
         end
       end
     end
   end
 
   describe 'DELETE /*namespace_id/:project_id/issues/:issue_id/link/:id' do
-    let(:issue_link) { create :issue_link, target: referenced_issue }
+    let(:issue_link) { create :issue_link, source: issue, target: referenced_issue }
 
     before do
       project.team << [user, user_role]
@@ -105,10 +105,10 @@ describe Projects::IssueLinksController do
         let(:referenced_issue) { create :issue }
         let(:user_role) { :developer }
 
-        it 'returns 403' do
+        it 'returns 401' do
           delete namespace_project_issue_link_path(issue_links_params(id: issue_link.id))
 
-          expect(response).to have_http_status(403)
+          expect(response).to have_http_status(401)
         end
       end
     end

spec/services/issue_links/create_service_spec.rb

@@ -25,7 +25,7 @@ describe IssueLinks::CreateService, service: true do
       end
 
       it 'returns error' do
-        is_expected.to eq(message: 'No Issue found for given reference', status: :error, http_status: 401)
+        is_expected.to eq(message: 'No Issue found for given params', status: :error, http_status: 401)
       end
     end
 
@@ -35,7 +35,7 @@ describe IssueLinks::CreateService, service: true do
       end
 
       it 'returns error' do
-        is_expected.to eq(message: 'No Issue found for given reference', status: :error, http_status: 401)
+        is_expected.to eq(message: 'No Issue found for given params', status: :error, http_status: 401)
       end
 
       it 'no relationship is created' do
@@ -53,7 +53,7 @@ describe IssueLinks::CreateService, service: true do
       it 'returns error' do
         target_issue.project.add_guest(user)
 
-        is_expected.to eq(message: 'No Issue found for given reference', status: :error, http_status: 401)
+        is_expected.to eq(message: 'No Issue found for given params', status: :error, http_status: 401)
       end
 
       it 'no relationship is created' do

spec/services/issue_links/destroy_service_spec.rb

@@ -2,27 +2,58 @@ require 'spec_helper'
 
 describe IssueLinks::DestroyService, service: true do
   describe '#execute' do
+    let(:project) { create :empty_project }
     let(:user) { create :user }
-    let!(:issue_link) { create :issue_link }
 
     subject { described_class.new(issue_link, user).execute }
 
-    it 'removes related issue' do
-      expect { subject }.to change(IssueLink, :count).from(1).to(0)
-    end
+    context 'success' do
+      let(:issue_a) { create :issue, project: project }
+      let(:issue_b) { create :issue, project: project }
+
+      let!(:issue_link) { create :issue_link, source: issue_a, target: issue_b }
+
+      before do
+        project.add_reporter(user)
+      end
+
+      it 'removes related issue' do
+        expect { subject }.to change(IssueLink, :count).from(1).to(0)
+      end
+
+      it 'creates notes' do
+        # Two-way notes creation
+        expect(SystemNoteService).to receive(:unrelate_issue)
+                                       .with(issue_link.source, issue_link.target, user)
+        expect(SystemNoteService).to receive(:unrelate_issue)
+                                       .with(issue_link.target, issue_link.source, user)
 
-    it 'creates notes' do
-      # Two-way notes creation
-      expect(SystemNoteService).to receive(:unrelate_issue)
-        .with(issue_link.source, issue_link.target, user)
-      expect(SystemNoteService).to receive(:unrelate_issue)
-        .with(issue_link.target, issue_link.source, user)
+        subject
+      end
 
-      subject
+      it 'returns success message' do
+        is_expected.to eq(message: 'Relation was removed', status: :success)
+      end
     end
 
-    it 'returns success message' do
-      is_expected.to eq(message: 'Relation was removed', status: :success)
+    context 'failure' do
+      let(:unauthorized_project) { create :empty_project }
+      let(:issue_a) { create :issue, project: project }
+      let(:issue_b) { create :issue, project: unauthorized_project }
+
+      let!(:issue_link) { create :issue_link, source: issue_a, target: issue_b }
+
+      it 'does not remove relation' do
+        expect { subject }.not_to change(IssueLink, :count).from(1)
+      end
+
+      it 'does not create notes' do
+        expect(SystemNoteService).not_to receive(:unrelate_issue)
+      end
+
+      it 'returns error message' do
+        is_expected.to eq(message: 'Unauthorized', status: :error, http_status: 401)
+      end
     end
   end
 end