Merge branch '209993-add-ability-to-comment-vulnerability-state-changes' into 'master'

Add ability to comment notes created for vulnerabilities

See merge request gitlab-org/gitlab!27794

ee/app/controllers/projects/security/vulnerabilities/notes_controller.rb

@@ -12,7 +12,7 @@ module Projects
         include ToggleAwardEmoji
 
         before_action :not_found, unless: -> { project.first_class_vulnerabilities_enabled? }
-        before_action :vulnerability
+        before_action :authorize_create_note!, only: [:create]
 
         private
 

ee/app/serializers/vulnerability_note_entity.rb

@@ -4,4 +4,8 @@ class VulnerabilityNoteEntity < NoteEntity
   expose :toggle_award_path, if: -> (note, _) { note.emoji_awardable? } do |note|
     toggle_award_emoji_project_security_vulnerability_note_path(note.noteable.project, note.noteable, note)
   end
+
+  expose :path, if: -> (note, _) { note.noteable } do |note|
+    project_security_vulnerability_note_path(note.noteable.project, note.noteable, note)
+  end
 end

ee/config/routes/project.rb

@@ -101,7 +101,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
             end
 
             scope module: :vulnerabilities do
-              resources :notes, only: [:index], concerns: :awardable, constraints: { id: /\d+/ }
+              resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ }
             end
           end
         end

ee/spec/controllers/projects/security/vulnerabilities/notes_controller_spec.rb

@@ -6,7 +6,8 @@ describe Projects::Security::Vulnerabilities::NotesController do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project) }
   let_it_be(:vulnerability) { create(:vulnerability, project: project) }
-  let_it_be(:note) { create(:note, noteable: vulnerability, project: project) }
+
+  let!(:note) { create(:note, noteable: vulnerability, project: project) }
 
   it_behaves_like SecurityDashboardsPermissions do
     let(:vulnerable) { project }
@@ -52,4 +53,229 @@ describe Projects::Security::Vulnerabilities::NotesController do
       end
     end
   end
+
+  describe 'POST create' do
+    let(:note_params) { { note: 'some note' } }
+    let(:extra_params) { {} }
+
+    let(:request_params) do
+      {
+        namespace_id: project.namespace,
+        project_id: project,
+        vulnerability_id: vulnerability,
+        note: note_params,
+        format: :json
+      }
+    end
+
+    subject(:create_note) { post :create, params: request_params.merge(extra_params) }
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    context 'when note is empty' do
+      let(:note_params) { { note: '' } }
+
+      it 'does not create new note' do
+        expect { create_note }.not_to change { Note.count }
+      end
+
+      it 'returns status 422' do
+        create_note
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+      end
+    end
+
+    context 'when note is provided' do
+      let(:note_params) { { note: 'some note' } }
+
+      it 'creates new note' do
+        expect { create_note }.to change { Note.count }.by(1)
+      end
+
+      it 'returns status 200' do
+        create_note
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+
+      context 'when user has no permission to create a note' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'does not create new note' do
+          expect { create_note }.not_to change { Note.count }
+        end
+
+        it 'returns status 403' do
+          create_note
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
+      context 'when replying to the discussion' do
+        let(:extra_params) { { in_reply_to_discussion_id: note.discussion_id } }
+
+        it 'creates new note in reply to discussion' do
+          expect { create_note }.to change { Note.where(discussion_id: note.discussion_id).count }.by(1)
+        end
+
+        it 'returns status 200' do
+          create_note
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+
+        context 'when return_discussion param is set' do
+          let(:extra_params) { { in_reply_to_discussion_id: note.discussion_id, return_discussion: 'true' } }
+          let(:last_returned_note_in_discussion) { json_response.dig('discussion', 'notes').last }
+
+          it 'returns discussion JSON when the return_discussion param is set' do
+            create_note
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response).to have_key 'discussion'
+            expect(last_returned_note_in_discussion['note']).to eq(note_params[:note])
+          end
+        end
+      end
+    end
+  end
+
+  describe 'PUT update' do
+    let(:note_params) { { note: 'some note' } }
+
+    let(:request_params) do
+      {
+        id: note,
+        namespace_id: project.namespace,
+        project_id: project,
+        vulnerability_id: vulnerability,
+        note: note_params,
+        format: :json
+      }
+    end
+
+    subject(:update_note) { put :update, params: request_params }
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    context 'when user is not an author of the note' do
+      it 'returns status 404' do
+        update_note
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when user is an author of the note' do
+      let!(:note) { create(:note, noteable: vulnerability, project: project, author: user) }
+
+      context 'when note is provided' do
+        let(:note_params) { { note: 'some note' } }
+
+        it 'updates note' do
+          expect { update_note }.to change { note.reload.note }.to(note_params[:note])
+        end
+
+        it 'returns status 200' do
+          update_note
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
+  end
+
+  describe 'DELETE destroy' do
+    let(:request_params) do
+      {
+        id: note,
+        namespace_id: project.namespace,
+        project_id: project,
+        vulnerability_id: vulnerability,
+        format: :js
+      }
+    end
+
+    subject(:delete_note) { delete :destroy, params: request_params }
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    context 'when user is not an author of the note' do
+      it 'does not delete the note' do
+        expect { delete_note }.not_to change { Note.count }
+      end
+
+      it 'returns status 404' do
+        delete_note
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when user is an author of the note' do
+      let!(:note) { create(:note, noteable: vulnerability, project: project, author: user) }
+
+      it 'deletes the note' do
+        expect { delete_note }.to change { Note.count }.by(-1)
+      end
+
+      it 'returns status 200' do
+        delete_note
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+    end
+  end
+
+  describe 'POST toggle_award_emoji' do
+    let(:request_params) do
+      {
+        id: note,
+        namespace_id: project.namespace,
+        project_id: project,
+        vulnerability_id: vulnerability,
+        format: :json
+      }
+    end
+
+    subject(:toggle_award_emoji) { post :toggle_award_emoji, params: request_params.merge(name: emoji_name) }
+
+    before do
+      sign_in(user)
+      project.add_developer(user)
+    end
+
+    let(:emoji_name) { 'thumbsup' }
+
+    it 'creates the award emoji' do
+      expect { toggle_award_emoji }.to change { note.award_emoji.count }.by(1)
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    context 'when award emoji was already created' do
+      before do
+        post :toggle_award_emoji, params: request_params.merge(name: emoji_name)
+      end
+
+      it 'deletes the award emoji' do
+        expect { toggle_award_emoji }.to change { AwardEmoji.count }.by(-1)
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+    end
+  end
 end

ee/spec/serializers/vulnerability_note_entity_spec.rb

@@ -18,6 +18,6 @@ describe VulnerabilityNoteEntity do
   it_behaves_like 'note entity'
 
   it 'exposes vulnerability-specific elements' do
-    expect(subject).to include(:toggle_award_path)
+    expect(subject).to include(:toggle_award_path, :path)
   end
 end