Addresses aesthetic details on text and naming

app/controllers/projects/issue_links_controller.rb

@@ -7,10 +7,10 @@ module Projects
     end
 
     def create
-      opts = params.slice(:issue_references)
-      result = IssueLinks::CreateService.new(issue, current_user, opts).execute
+      create_params = params.slice(:issue_references)
+      result = IssueLinks::CreateService.new(issue, current_user, create_params).execute
 
-      render json: { result: result, issues: issues }, status: result[:http_status]
+      render json: { message: result[:message], issues: issues }, status: result[:http_status]
     end
 
     def destroy
@@ -18,9 +18,9 @@ module Projects
 
       return render_403 unless can?(current_user, :admin_issue_link, issue_link.target.project)
 
-      result = IssueLinks::DestroyService.new(issue_link, current_user).execute
+      IssueLinks::DestroyService.new(issue_link, current_user).execute
 
-      render json: { result: result, issues: issues }
+      render json: { issues: issues }
     end
 
     private

app/models/issue_link.rb

@@ -10,7 +10,7 @@ class IssueLink < ActiveRecord::Base
   private
 
   def check_self_relation
-    return unless source || target
+    return unless source && target
 
     if source == target
       errors.add(:source, 'cannot be related to itself')

app/models/system_note_metadata.rb

@@ -2,7 +2,8 @@ class SystemNoteMetadata < ActiveRecord::Base
   ICON_TYPES = %w[
     commit description merge confidential visible label assignee cross_reference
     title time_tracking branch milestone discussion task moved opened closed merged
-    outdated approved unapproved relate unrelate
+    outdated
+    approved unapproved relate unrelate
   ].freeze
 
   validates :note, presence: true

app/services/issue_links/create_service.rb

@@ -9,13 +9,13 @@ module IssueLinks
         return error('No Issue found for given reference', 401)
       end
 
-      create_issue_link
+      create_issue_links
       success
     end
 
     private
 
-    def create_issue_link
+    def create_issue_links
       referenced_issues.each do |referenced_issue|
         create_notes(referenced_issue) if relate_issues(referenced_issue)
       end

app/services/issue_links/destroy_service.rb

@@ -8,19 +8,19 @@ module IssueLinks
     end
 
     def execute
-      remove_relation!
-      create_notes!
+      remove_relation
+      create_notes
 
       success(message: 'Relation was removed')
     end
 
     private
 
-    def remove_relation!
+    def remove_relation
       @issue_link.destroy!
     end
 
-    def create_notes!
+    def create_notes
       SystemNoteService.unrelate_issue(@issue, @referenced_issue, current_user)
       SystemNoteService.unrelate_issue(@referenced_issue, @issue, current_user)
     end

spec/models/issue_link_spec.rb

@@ -18,12 +18,25 @@ describe IssueLink do
     end
 
     context 'self relation' do
-      it 'invalidates object' do
-        issue = create :issue
-        issue_link = build :issue_link, source: issue, target: issue
+      let(:issue) { create :issue }
 
-        expect(issue_link).to be_invalid
-        expect(issue_link.errors[:source]).to include('cannot be related to itself')
+      context 'cannot be validated' do
+        it 'does not invalidate object with self relation error' do
+          issue_link = build :issue_link, source: issue, target: nil
+
+          issue_link.valid?
+
+          expect(issue_link.errors[:source]).to be_empty
+        end
+      end
+
+      context 'can be invalidated' do
+        it 'invalidates object' do
+          issue_link = build :issue_link, source: issue, target: issue
+
+          expect(issue_link).to be_invalid
+          expect(issue_link.errors[:source]).to include('cannot be related to itself')
+        end
       end
     end
   end

spec/requests/projects/issue_links_controller_spec.rb

@@ -19,17 +19,10 @@ describe Projects::IssueLinksController do
       login_as user
     end
 
-    subject do
-      get namespace_project_issue_links_path(namespace_id: issue.project.namespace,
-                                             project_id: issue.project,
-                                             issue_id: issue,
-                                             format: :json)
-    end
-
     it 'returns JSON response' do
       list_service_response = IssueLinks::ListService.new(issue, user).execute
 
-      subject
+      get namespace_project_issue_links_path(issue_links_params)
 
       expect(response).to have_http_status(200)
       expect(json_response).to eq(list_service_response.as_json)
@@ -38,87 +31,70 @@ describe Projects::IssueLinksController do
 
   describe 'POST /*namespace_id/:project_id/issues/:issue_id/links' do
     let(:issue_b) { create :issue, project: project }
-    let(:issue_references) { [issue_b.to_reference] }
-    let(:user_role) { :developer }
 
     before do
       project.team << [user, user_role]
       login_as user
     end
 
-    subject do
-      post namespace_project_issue_links_path(namespace_id: issue.project.namespace,
-                                              project_id: issue.project,
-                                              issue_id: issue,
-                                              issue_references: issue_references,
-                                              format: :json)
-    end
-
     context 'with success' do
+      let(:user_role) { :developer }
+      let(:issue_references) { [issue_b.to_reference] }
+
       it 'returns success JSON' do
-        subject
+        post namespace_project_issue_links_path(issue_links_params(issue_references: issue_references))
 
         list_service_response = IssueLinks::ListService.new(issue, user).execute
 
         expect(response).to have_http_status(200)
-        expect(json_response['result']).to eq('status' => 'success')
-        expect(json_response['issues']).to eq(list_service_response.as_json)
+        expect(json_response).to eq('message' => nil,
+                                    'issues' => list_service_response.as_json)
       end
     end
 
     context 'with failure' do
       context 'when unauthorized' do
         let(:user_role) { :guest }
+        let(:issue_references) { [issue_b.to_reference] }
 
         it 'returns 403' do
-          subject
+          post namespace_project_issue_links_path(issue_links_params(issue_references: issue_references))
 
           expect(response).to have_http_status(403)
         end
       end
 
       context 'when failing service result' do
+        let(:user_role) { :developer }
         let(:issue_references) { ['#999'] }
 
         it 'returns failure JSON' do
-          subject
+          post namespace_project_issue_links_path(issue_links_params(issue_references: issue_references))
 
           list_service_response = IssueLinks::ListService.new(issue, user).execute
 
           expect(response).to have_http_status(401)
-          expect(json_response['result']).to eq('message' => 'No Issue found for given reference',
-                                                'status' => 'error',
-                                                'http_status' => 401)
-          expect(json_response['issues']).to eq(list_service_response.as_json)
+          expect(json_response).to eq('message' => 'No Issue found for given reference', 'issues' => list_service_response.as_json)
         end
       end
     end
   end
 
   describe 'DELETE /*namespace_id/:project_id/issues/:issue_id/link/:id' do
-    let(:referenced_issue) { create :issue, project: project }
     let(:issue_link) { create :issue_link, target: referenced_issue }
-    let(:current_project_user_role) { :developer }
-
-    subject do
-      delete namespace_project_issue_link_path(namespace_id: issue.project.namespace,
-                                               project_id: issue.project,
-                                               issue_id: issue,
-                                               id: issue_link.id,
-                                               format: :json)
-    end
 
     before do
-      project.team << [user, current_project_user_role]
+      project.team << [user, user_role]
       login_as user
     end
 
     context 'when unauthorized' do
       context 'when no authorization on current project' do
-        let(:current_project_user_role) { :guest }
+        let(:referenced_issue) { create :issue, project: project }
+        let(:user_role) { :guest }
 
         it 'returns 403' do
-          subject
+          delete namespace_project_issue_link_path(issue_links_params(id: issue_link.id))
 
           expect(response).to have_http_status(403)
         end
@@ -127,10 +103,10 @@ describe Projects::IssueLinksController do
       context 'when no authorization on the related issue project' do
         # unauthorized project issue
         let(:referenced_issue) { create :issue }
-        let(:current_project_user_role) { :developer }
+        let(:user_role) { :developer }
 
         it 'returns 403' do
-          subject
+          delete namespace_project_issue_link_path(issue_links_params(id: issue_link.id))
 
           expect(response).to have_http_status(403)
         end
@@ -138,16 +114,23 @@ describe Projects::IssueLinksController do
     end
 
     context 'when authorized' do
-      let(:current_project_user_role) { :developer }
+      let(:referenced_issue) { create :issue, project: project }
+      let(:user_role) { :developer }
 
       it 'returns success JSON' do
-        subject
+        delete namespace_project_issue_link_path(issue_links_params(id: issue_link.id))
 
         list_service_response = IssueLinks::ListService.new(issue, user).execute
 
-        expect(json_response['result']).to eq('message' => 'Relation was removed', 'status' => 'success')
-        expect(json_response['issues']).to eq(list_service_response.as_json)
+        expect(json_response).to eq('issues' => list_service_response.as_json)
       end
     end
   end
+
+  def issue_links_params(opts = {})
+    opts.reverse_merge(namespace_id: issue.project.namespace,
+                       project_id: issue.project,
+                       issue_id: issue,
+                       format: :json)
+  end
 end

spec/services/issue_links/create_service_spec.rb

@@ -19,7 +19,7 @@ describe IssueLinks::CreateService, service: true do
 
     subject { described_class.new(issue, user, params).execute }
 
-    context 'when empty reference list' do
+    context 'when the reference list is empty' do
       let(:params) do
         { issue_references: [] }
       end
@@ -61,7 +61,7 @@ describe IssueLinks::CreateService, service: true do
       end
     end
 
-    context 'when any Issue to relate' do
+    context 'when there is an issue to relate' do
       let(:issue_a) { create :issue, project: project }
       let(:another_project) { create :empty_project, namespace: project.namespace }
       let(:another_project_issue) { create :issue, project: another_project }
@@ -84,7 +84,7 @@ describe IssueLinks::CreateService, service: true do
         expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue)
       end
 
-      it 'returns success message with Issue reference' do
+      it 'returns success status' do
         is_expected.to eq(status: :success)
       end
 
@@ -117,7 +117,7 @@ describe IssueLinks::CreateService, service: true do
         { issue_references: [issue_b.to_reference, issue_a.to_reference] }
       end
 
-      it 'returns success' do
+      it 'returns success status' do
         is_expected.to eq(status: :success)
       end