notes_controller_spec.rb 26.4 KB
Newer Older
1 2
# frozen_string_literal: true

3
require 'spec_helper'
4 5

describe Projects::NotesController do
6 7
  include ProjectForksHelper

8
  let(:user)    { create(:user) }
9
  let(:project) { create(:project) }
10 11 12
  let(:issue)   { create(:issue, project: project) }
  let(:note)    { create(:note, noteable: issue, project: project) }

Douwe Maan's avatar
Douwe Maan committed
13 14 15 16 17 18 19 20
  let(:request_params) do
    {
      namespace_id: project.namespace,
      project_id: project,
      id: note
    }
  end

21
  describe 'GET index' do
Douwe Maan's avatar
Douwe Maan committed
22 23 24 25 26 27 28 29 30 31
    let(:request_params) do
      {
        namespace_id: project.namespace,
        project_id: project,
        target_type: 'issue',
        target_id: issue.id,
        format: 'json'
      }
    end

32
    let(:parsed_response) { json_response.with_indifferent_access }
Douwe Maan's avatar
Douwe Maan committed
33 34 35 36
    let(:note_json) { parsed_response[:notes].first }

    before do
      sign_in(user)
37
      project.add_developer(user)
Douwe Maan's avatar
Douwe Maan committed
38 39 40
    end

    it 'passes last_fetched_at from headers to NotesFinder' do
Douwe Maan's avatar
Douwe Maan committed
41 42
      last_fetched_at = 3.hours.ago.to_i

Douwe Maan's avatar
Douwe Maan committed
43 44 45
      request.headers['X-Last-Fetched-At'] = last_fetched_at

      expect(NotesFinder).to receive(:new)
46
        .with(anything, hash_including(last_fetched_at: last_fetched_at))
Douwe Maan's avatar
Douwe Maan committed
47 48
        .and_call_original

blackst0ne's avatar
blackst0ne committed
49
      get :index, params: request_params
Douwe Maan's avatar
Douwe Maan committed
50 51
    end

52 53 54 55 56 57 58 59
    context 'when user notes_filter is present' do
      let(:notes_json) { parsed_response[:notes] }
      let!(:comment) { create(:note, noteable: issue, project: project) }
      let!(:system_note) { create(:note, noteable: issue, project: project, system: true) }

      it 'filters system notes by comments' do
        user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issue)

blackst0ne's avatar
blackst0ne committed
60
        get :index, params: request_params
61 62 63 64 65 66 67 68

        expect(notes_json.count).to eq(1)
        expect(notes_json.first[:id].to_i).to eq(comment.id)
      end

      it 'returns all notes' do
        user.set_notes_filter(UserPreference::NOTES_FILTERS[:all_notes], issue)

blackst0ne's avatar
blackst0ne committed
69
        get :index, params: request_params
70 71 72 73 74 75 76 77 78

        expect(notes_json.map { |note| note[:id].to_i }).to contain_exactly(comment.id, system_note.id)
      end

      it 'does not merge label event notes' do
        user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issue)

        expect(ResourceEvents::MergeIntoNotesService).not_to receive(:new)

blackst0ne's avatar
blackst0ne committed
79
        get :index, params: request_params
80 81 82
      end
    end

Douwe Maan's avatar
Douwe Maan committed
83
    context 'for a discussion note' do
Douwe Maan's avatar
Douwe Maan committed
84 85 86
      let(:project) { create(:project, :repository) }
      let!(:note) { create(:discussion_note_on_merge_request, project: project) }

Felipe Artur's avatar
Felipe Artur committed
87
      let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id, html: true) }
Douwe Maan's avatar
Douwe Maan committed
88

Douwe Maan's avatar
Douwe Maan committed
89
      it 'responds with the expected attributes' do
blackst0ne's avatar
blackst0ne committed
90
        get :index, params: params
Douwe Maan's avatar
Douwe Maan committed
91 92 93 94

        expect(note_json[:id]).to eq(note.id)
        expect(note_json[:discussion_html]).not_to be_nil
        expect(note_json[:diff_discussion_html]).to be_nil
95
        expect(note_json[:discussion_line_code]).to be_nil
Douwe Maan's avatar
Douwe Maan committed
96 97 98 99 100 101 102
      end
    end

    context 'for a diff discussion note' do
      let(:project) { create(:project, :repository) }
      let!(:note) { create(:diff_note_on_merge_request, project: project) }

Felipe Artur's avatar
Felipe Artur committed
103
      let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id, html: true) }
Douwe Maan's avatar
Douwe Maan committed
104

Douwe Maan's avatar
Douwe Maan committed
105
      it 'responds with the expected attributes' do
blackst0ne's avatar
blackst0ne committed
106
        get :index, params: params
Douwe Maan's avatar
Douwe Maan committed
107 108 109 110

        expect(note_json[:id]).to eq(note.id)
        expect(note_json[:discussion_html]).not_to be_nil
        expect(note_json[:diff_discussion_html]).not_to be_nil
111
        expect(note_json[:discussion_line_code]).not_to be_nil
Douwe Maan's avatar
Douwe Maan committed
112 113 114 115 116 117 118 119 120 121
      end
    end

    context 'for a commit note' do
      let(:project) { create(:project, :repository) }
      let!(:note) { create(:note_on_commit, project: project) }

      context 'when displayed on a merge request' do
        let(:merge_request) { create(:merge_request, source_project: project) }

Felipe Artur's avatar
Felipe Artur committed
122
        let(:params) { request_params.merge(target_type: 'merge_request', target_id: merge_request.id, html: true) }
Douwe Maan's avatar
Douwe Maan committed
123

Douwe Maan's avatar
Douwe Maan committed
124
        it 'responds with the expected attributes' do
blackst0ne's avatar
blackst0ne committed
125
          get :index, params: params
Douwe Maan's avatar
Douwe Maan committed
126 127 128 129

          expect(note_json[:id]).to eq(note.id)
          expect(note_json[:discussion_html]).not_to be_nil
          expect(note_json[:diff_discussion_html]).to be_nil
130
          expect(note_json[:discussion_line_code]).to be_nil
Douwe Maan's avatar
Douwe Maan committed
131 132 133 134
        end
      end

      context 'when displayed on the commit' do
Felipe Artur's avatar
Felipe Artur committed
135
        let(:params) { request_params.merge(target_type: 'commit', target_id: note.commit_id, html: true) }
Douwe Maan's avatar
Douwe Maan committed
136

Douwe Maan's avatar
Douwe Maan committed
137
        it 'responds with the expected attributes' do
blackst0ne's avatar
blackst0ne committed
138
          get :index, params: params
Douwe Maan's avatar
Douwe Maan committed
139 140 141 142

          expect(note_json[:id]).to eq(note.id)
          expect(note_json[:discussion_html]).to be_nil
          expect(note_json[:diff_discussion_html]).to be_nil
143
          expect(note_json[:discussion_line_code]).to be_nil
Douwe Maan's avatar
Douwe Maan committed
144
        end
145 146 147 148 149 150 151 152

        context 'when user cannot read commit' do
          before do
            allow(Ability).to receive(:allowed?).and_call_original
            allow(Ability).to receive(:allowed?).with(user, :download_code, project).and_return(false)
          end

          it 'renders 404' do
blackst0ne's avatar
blackst0ne committed
153
            get :index, params: params
154 155 156 157

            expect(response).to have_gitlab_http_status(404)
          end
        end
Douwe Maan's avatar
Douwe Maan committed
158 159 160 161
      end
    end

    context 'for a regular note' do
Douwe Maan's avatar
Douwe Maan committed
162 163
      let!(:note) { create(:note_on_merge_request, project: project) }

Felipe Artur's avatar
Felipe Artur committed
164
      let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id, html: true) }
Douwe Maan's avatar
Douwe Maan committed
165

Douwe Maan's avatar
Douwe Maan committed
166
      it 'responds with the expected attributes' do
blackst0ne's avatar
blackst0ne committed
167
        get :index, params: params
Douwe Maan's avatar
Douwe Maan committed
168 169 170 171 172

        expect(note_json[:id]).to eq(note.id)
        expect(note_json[:html]).not_to be_nil
        expect(note_json[:discussion_html]).to be_nil
        expect(note_json[:diff_discussion_html]).to be_nil
173
        expect(note_json[:discussion_line_code]).to be_nil
Douwe Maan's avatar
Douwe Maan committed
174 175
      end
    end
176 177 178 179 180 181 182 183 184 185 186

    context 'with cross-reference system note', :request_store do
      let(:new_issue) { create(:issue) }
      let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" }

      before do
        note
        create(:discussion_note_on_issue, :system, noteable: issue, project: issue.project, note: cross_reference)
      end

      it 'filters notes that the user should not see' do
blackst0ne's avatar
blackst0ne committed
187
        get :index, params: request_params
188 189

        expect(parsed_response[:notes].count).to eq(1)
190
        expect(note_json[:id]).to eq(note.id.to_s)
191 192 193 194
      end

      it 'does not result in N+1 queries' do
        # Instantiate the controller variables to ensure QueryRecorder has an accurate base count
blackst0ne's avatar
blackst0ne committed
195
        get :index, params: request_params
196 197 198 199

        RequestStore.clear!

        control_count = ActiveRecord::QueryRecorder.new do
blackst0ne's avatar
blackst0ne committed
200
          get :index, params: request_params
201 202 203 204 205 206
        end.count

        RequestStore.clear!

        create_list(:discussion_note_on_issue, 2, :system, noteable: issue, project: issue.project, note: cross_reference)

blackst0ne's avatar
blackst0ne committed
207
        expect { get :index, params: request_params }.not_to exceed_query_limit(control_count)
208 209
      end
    end
210 211
  end

212 213
  describe 'POST create' do
    let(:merge_request) { create(:merge_request) }
214
    let(:project) { merge_request.source_project }
215
    let(:note_text) { 'some note' }
216 217
    let(:request_params) do
      {
218
        note: { note: note_text, noteable_id: merge_request.id, noteable_type: 'MergeRequest' },
219 220
        namespace_id: project.namespace,
        project_id: project,
Douwe Maan's avatar
Douwe Maan committed
221 222 223
        merge_request_diff_head_sha: 'sha',
        target_type: 'merge_request',
        target_id: merge_request.id
224 225 226 227 228 229 230 231 232
      }.merge(extra_request_params)
    end
    let(:extra_request_params) { {} }

    let(:project_visibility) { Gitlab::VisibilityLevel::PUBLIC }
    let(:merge_requests_access_level) { ProjectFeature::ENABLED }

    def create!
      post :create, params: request_params
233 234 235
    end

    before do
236 237
      project.update_attribute(:visibility_level, project_visibility)
      project.project_feature.update(merge_requests_access_level: merge_requests_access_level)
238 239 240
      sign_in(user)
    end

241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261
    describe 'making the creation request' do
      before do
        create!
      end

      context 'the project is publically available' do
        context 'for HTML' do
          it "returns status 302" do
            expect(response).to have_gitlab_http_status(302)
          end
        end

        context 'for JSON' do
          let(:extra_request_params) { { format: :json } }

          it "returns status 200 for json" do
            expect(response).to have_gitlab_http_status(200)
          end
        end
      end

262 263 264 265 266 267 268 269 270 271 272
      context 'the note does not have commands_only errors' do
        context 'for empty note' do
          let(:note_text) { '' }
          let(:extra_request_params) { { format: :json } }

          it "returns status 422 for json" do
            expect(response).to have_gitlab_http_status(422)
          end
        end
      end

273 274
      context 'the project is a private project' do
        let(:project_visibility) { Gitlab::VisibilityLevel::PRIVATE }
275

276 277 278 279 280 281 282 283 284 285
        [{}, { format: :json }].each do |extra|
          context "format is #{extra[:format]}" do
            let(:extra_request_params) { extra }

            it "returns status 404" do
              expect(response).to have_gitlab_http_status(404)
            end
          end
        end
      end
286 287
    end

288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307
    context 'the user is a developer on a private project' do
      let(:project_visibility) { Gitlab::VisibilityLevel::PRIVATE }

      before do
        project.add_developer(user)
      end

      context 'HTML requests' do
        it "returns status 302 (redirect)" do
          create!

          expect(response).to have_gitlab_http_status(302)
        end
      end

      context 'JSON requests' do
        let(:extra_request_params) { { format: :json } }

        it "returns status 200" do
          create!
308

309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356
          expect(response).to have_gitlab_http_status(200)
        end
      end

      context 'the return_discussion param is set' do
        let(:extra_request_params) { { format: :json, return_discussion: 'true' } }

        it 'returns discussion JSON when the return_discussion param is set' do
          create!

          expect(response).to have_gitlab_http_status(200)
          expect(json_response).to have_key 'discussion'
          expect(json_response.dig('discussion', 'notes', 0, 'note')).to eq(request_params[:note][:note])
        end
      end

      context 'when creating a note with quick actions' do
        context 'with commands that return changes' do
          let(:note_text) { "/award :thumbsup:\n/estimate 1d\n/spend 3h" }
          let(:extra_request_params) { { format: :json } }

          it 'includes changes in commands_changes ' do
            create!

            expect(response).to have_gitlab_http_status(200)
            expect(json_response['commands_changes']).to include('emoji_award', 'time_estimate', 'spend_time')
            expect(json_response['commands_changes']).not_to include('target_project', 'title')
          end
        end

        context 'with commands that do not return changes' do
          let(:issue) { create(:issue, project: project) }
          let(:other_project) { create(:project) }
          let(:note_text) { "/move #{other_project.full_path}\n/title AAA" }
          let(:extra_request_params) { { format: :json, target_id: issue.id, target_type: 'issue' } }

          before do
            other_project.add_developer(user)
          end

          it 'does not include changes in commands_changes' do
            create!

            expect(response).to have_gitlab_http_status(200)
            expect(json_response['commands_changes']).not_to include('target_project', 'title')
          end
        end
      end
357 358
    end

359 360 361 362 363 364
    context 'when the internal project prohibits non-members from accessing merge requests' do
      let(:project_visibility) { Gitlab::VisibilityLevel::INTERNAL }
      let(:merge_requests_access_level) { ProjectFeature::PRIVATE }

      it "prevents a non-member user from creating a note on one of the project's merge requests" do
        create!
365

366 367 368 369 370 371 372 373 374 375 376 377
        expect(response).to have_gitlab_http_status(404)
      end

      context 'when the user is a team member' do
        before do
          project.add_developer(user)
        end

        it 'can add comments' do
          expect { create! }.to change { project.notes.count }.by(1)
        end
      end
378

379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451
      # Illustration of the attack vector for posting comments to discussions that should
      # be inaccessible.
      #
      # This relies on posting a note to a commit that is not necessarily even in the
      # merge request, with a value of :in_reply_to_discussion_id that points to a
      # discussion on a merge_request that should not be accessible.
      context 'when the request includes a :in_reply_to_discussion_id designed to fool us' do
        let(:commit) { create(:commit, project: project) }

        let(:existing_comment) do
          create(:note_on_commit,
                 note: 'first',
                 project: project,
                 commit_id: merge_request.commit_shas.first)
        end

        let(:discussion) { existing_comment.discussion }

        # see !60465 for details of the structure of this request
        let(:request_params) do
          { "utf8" => "✓",
            "authenticity_token" => "1",
            "view" => "inline",
            "line_type" => "",
            "merge_request_diff_head_sha" => "",
            "in_reply_to_discussion_id" => discussion.id,
            "note_project_id" => project.id,
            "project_id" => project.id,
            "namespace_id" => project.namespace,
            "target_type" => "commit",
            "target_id" => commit.id,
            "note" => {
              "noteable_type" => "",
              "noteable_id" => "",
              "commit_id" => "",
              "type" => "",
              "line_code" => "",
              "position" => "",
              "note" => "ThisReplyWillGoToMergeRequest"
            } }
        end

        it 'prevents the request from adding notes to the spoofed discussion' do
          expect { create! }.not_to change { discussion.notes.count }
        end

        it 'returns an error to the user' do
          create!
          expect(response).to have_gitlab_http_status(404)
        end
      end
    end

    context 'when the public project prohibits non-members from accessing merge requests' do
      let(:project_visibility) { Gitlab::VisibilityLevel::PUBLIC }
      let(:merge_requests_access_level) { ProjectFeature::PRIVATE }

      it "prevents a non-member user from creating a note on one of the project's merge requests" do
        create!

        expect(response).to have_gitlab_http_status(404)
      end

      context 'when the user is a team member' do
        before do
          project.add_developer(user)
          create!
        end

        it 'can add comments' do
          expect(response).to be_redirect
        end
      end
452 453
    end

454 455
    context 'when merge_request_diff_head_sha present' do
      before do
456
        service_params = ActionController::Parameters.new({
457
          note: 'some note',
458
          noteable_id: merge_request.id,
459
          noteable_type: 'MergeRequest',
460 461
          commit_id: nil,
          merge_request_diff_head_sha: 'sha'
462
        }).permit!
463 464 465 466 467

        expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true))
      end

      it "returns status 302 for html" do
468
        create!
469

470
        expect(response).to have_gitlab_http_status(302)
471 472
      end
    end
473

474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489
    context 'when creating a comment on a commit with SHA1 starting with a large number' do
      let(:commit) { create(:commit, project: project, id: '842616594688d2351480dfebd67b3d8d15571e6d') }

      it 'creates a note successfully' do
        expect do
          post :create, params: {
            note: { note: 'some note', commit_id: commit.id },
            namespace_id: project.namespace,
            project_id: project,
            target_type: 'commit',
            target_id: commit.id
          }
        end.to change { Note.count }.by(1)
      end
    end

490
    context 'when creating a commit comment from an MR fork' do
491
      let(:project) { create(:project, :repository, :public) }
492

493 494
      let(:forked_project) do
        fork_project(project, nil, repository: true)
495 496 497
      end

      let(:merge_request) do
498
        create(:merge_request, source_project: forked_project, target_project: project, source_branch: 'feature', target_branch: 'master')
499 500 501
      end

      let(:existing_comment) do
502
        create(:note_on_commit, note: 'a note', project: forked_project, commit_id: merge_request.commit_shas.first)
503 504
      end

505 506 507 508 509 510
      let(:note_project_id) do
        forked_project.id
      end

      let(:request_params) do
        {
511 512 513 514 515
          note: { note: 'some other note', noteable_id: merge_request.id },
          namespace_id: project.namespace,
          project_id: project,
          target_type: 'merge_request',
          target_id: merge_request.id,
516
          note_project_id: note_project_id,
517
          in_reply_to_discussion_id: existing_comment.discussion_id
518 519 520 521 522 523 524
        }
      end

      let(:fork_visibility) { Gitlab::VisibilityLevel::PUBLIC }

      before do
        forked_project.update_attribute(:visibility_level, fork_visibility)
525 526 527
      end

      context 'when the note_project_id is not correct' do
528 529 530
        let(:note_project_id) do
          project.id && Project.maximum(:id).succ
        end
531

532
        it 'returns a 404', :sidekiq_might_not_need_inline do
533
          create!
534
          expect(response).to have_gitlab_http_status(404)
535 536 537 538
        end
      end

      context 'when the user has no access to the fork' do
539
        let(:fork_visibility) { Gitlab::VisibilityLevel::PRIVATE }
540

541
        it 'returns a 404', :sidekiq_might_not_need_inline do
542
          create!
543
          expect(response).to have_gitlab_http_status(404)
544 545 546
        end
      end

547
      context 'when the user has access to the fork', :sidekiq_might_not_need_inline do
548 549
        let!(:discussion) { forked_project.notes.find_discussion(existing_comment.discussion_id) }
        let(:fork_visibility) { Gitlab::VisibilityLevel::PUBLIC }
550

551 552 553
        it 'is successful' do
          create!
          expect(response).to have_gitlab_http_status(302)
554 555 556
        end

        it 'creates the note' do
557
          expect { create! }.to change { forked_project.notes.count }.by(1)
558 559 560
        end
      end
    end
561

562 563 564 565 566 567 568 569 570 571 572 573 574
    context 'when target_id and noteable_id do not match' do
      let(:locked_issue) { create(:issue, :locked, project: project) }
      let(:issue) {create(:issue, project: project)}

      it 'uses target_id and ignores noteable_id' do
        request_params = {
          note: { note: 'some note', noteable_type: 'Issue', noteable_id: locked_issue.id },
          target_type: 'issue',
          target_id: issue.id,
          project_id: project,
          namespace_id: project.namespace
        }

blackst0ne's avatar
blackst0ne committed
575
        expect { post :create, params: request_params }.to change { issue.notes.count }.by(1)
576 577 578 579 580
          .and change { locked_issue.notes.count }.by(0)
        expect(response).to have_gitlab_http_status(302)
      end
    end

581 582 583 584 585
    context 'when the merge request discussion is locked' do
      before do
        merge_request.update_attribute(:discussion_locked, true)
      end

586 587
      context 'when a noteable is not found' do
        it 'returns 404 status' do
Felipe Artur's avatar
Felipe Artur committed
588
          request_params[:target_id] = 9999
blackst0ne's avatar
blackst0ne committed
589
          post :create, params: request_params.merge(format: :json)
590

591
          expect(response).to have_gitlab_http_status(404)
592 593 594
        end
      end

595
      context 'when a user is a team member' do
596 597 598 599
        before do
          project.add_developer(user)
        end

600
        it 'returns 302 status for html' do
blackst0ne's avatar
blackst0ne committed
601
          post :create, params: request_params
602

603
          expect(response).to have_gitlab_http_status(302)
604 605 606
        end

        it 'returns 200 status for json' do
blackst0ne's avatar
blackst0ne committed
607
          post :create, params: request_params.merge(format: :json)
608

609
          expect(response).to have_gitlab_http_status(200)
610 611 612
        end

        it 'creates a new note' do
blackst0ne's avatar
blackst0ne committed
613
          expect { post :create, params: request_params }.to change { Note.count }.by(1)
614 615 616 617 618
        end
      end

      context 'when a user is not a team member' do
        it 'returns 404 status' do
blackst0ne's avatar
blackst0ne committed
619
          post :create, params: request_params
620

621
          expect(response).to have_gitlab_http_status(404)
622 623 624
        end

        it 'does not create a new note' do
blackst0ne's avatar
blackst0ne committed
625
          expect { post :create, params: request_params }.not_to change { Note.count }
626 627 628
        end
      end
    end
629 630
  end

631
  describe 'PUT update' do
632 633 634 635 636 637 638 639 640 641
    context "should update the note with a valid issue" do
      let(:request_params) do
        {
          namespace_id: project.namespace,
          project_id: project,
          id: note,
          format: :json,
          note: {
            note: "New comment"
          }
642
        }
643
      end
644

645 646 647 648 649 650
      before do
        sign_in(note.author)
        project.add_developer(note.author)
      end

      it "updates the note" do
blackst0ne's avatar
blackst0ne committed
651
        expect { put :update, params: request_params }.to change { note.reload.note }
652
      end
653
    end
654 655 656 657 658 659 660 661
    context "doesnt update the note" do
      let(:issue)   { create(:issue, :confidential, project: project) }
      let(:note)    { create(:note, noteable: issue, project: project) }

      before do
        sign_in(user)
        project.add_guest(user)
      end
662

663 664 665 666 667 668 669 670 671 672
      it "disallows edits when the issue is confidential and the user has guest permissions" do
        request_params = {
          namespace_id: project.namespace,
          project_id: project,
          id: note,
          format: :json,
          note: {
            note: "New comment"
          }
        }
blackst0ne's avatar
blackst0ne committed
673
        expect { put :update, params: request_params }.not_to change { note.reload.note }
674 675
        expect(response).to have_gitlab_http_status(404)
      end
676 677 678
    end
  end

679 680 681
  describe 'DELETE destroy' do
    let(:request_params) do
      {
682 683 684 685
          namespace_id: project.namespace,
          project_id: project,
          id: note,
          format: :js
686 687 688 689 690 691
      }
    end

    context 'user is the author of a note' do
      before do
        sign_in(note.author)
692
        project.add_developer(note.author)
693 694 695
      end

      it "returns status 200 for html" do
blackst0ne's avatar
blackst0ne committed
696
        delete :destroy, params: request_params
697

698
        expect(response).to have_gitlab_http_status(200)
699 700 701
      end

      it "deletes the note" do
blackst0ne's avatar
blackst0ne committed
702
        expect { delete :destroy, params: request_params }.to change { Note.count }.from(1).to(0)
703 704 705 706 707 708
      end
    end

    context 'user is not the author of a note' do
      before do
        sign_in(user)
709
        project.add_developer(user)
710 711 712
      end

      it "returns status 404" do
blackst0ne's avatar
blackst0ne committed
713
        delete :destroy, params: request_params
714

715
        expect(response).to have_gitlab_http_status(404)
716 717 718 719
      end
    end
  end

720
  describe 'POST toggle_award_emoji' do
721 722
    before do
      sign_in(user)
723
      project.add_developer(user)
724 725
    end

726
    subject { post(:toggle_award_emoji, params: request_params.merge(name: emoji_name)) }
727

728 729
    let(:emoji_name) { 'thumbsup' }

730 731
    it "toggles the award emoji" do
      expect do
732
        subject
Z.J. van de Weg's avatar
Z.J. van de Weg committed
733
      end.to change { note.award_emoji.count }.by(1)
734

735
      expect(response).to have_gitlab_http_status(200)
736 737
    end

Z.J. van de Weg's avatar
Z.J. van de Weg committed
738
    it "removes the already awarded emoji" do
739
      create(:award_emoji, awardable: note, name: emoji_name, user: user)
740

741
      expect { subject }.to change { AwardEmoji.count }.by(-1)
742

743
      expect(response).to have_gitlab_http_status(200)
744
    end
745 746 747 748 749 750 751 752

    it 'marks Todos on the Noteable as done' do
      todo = create(:todo, target: note.noteable, project: project, user: user)

      subject

      expect(todo.reload).to be_done
    end
753
  end
754

Douwe Maan's avatar
Douwe Maan committed
755
  describe "resolving and unresolving" do
756
    let(:project) { create(:project, :repository) }
Douwe Maan's avatar
Douwe Maan committed
757 758
    let(:merge_request) { create(:merge_request, source_project: project) }
    let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) }
759

Douwe Maan's avatar
Douwe Maan committed
760
    describe 'POST resolve' do
761
      before do
Douwe Maan's avatar
Douwe Maan committed
762
        sign_in user
763 764
      end

Douwe Maan's avatar
Douwe Maan committed
765
      context "when the user is not authorized to resolve the note" do
766
        it "returns status 404" do
blackst0ne's avatar
blackst0ne committed
767
          post :resolve, params: request_params
768

769
          expect(response).to have_gitlab_http_status(404)
770 771 772
        end
      end

Douwe Maan's avatar
Douwe Maan committed
773 774
      context "when the user is authorized to resolve the note" do
        before do
775
          project.add_developer(user)
776 777
        end

Douwe Maan's avatar
Douwe Maan committed
778 779 780 781
        context "when the note is not resolvable" do
          before do
            note.update(system: true)
          end
782

Douwe Maan's avatar
Douwe Maan committed
783
          it "returns status 404" do
blackst0ne's avatar
blackst0ne committed
784
            post :resolve, params: request_params
Douwe Maan's avatar
Douwe Maan committed
785

786
            expect(response).to have_gitlab_http_status(404)
Douwe Maan's avatar
Douwe Maan committed
787
          end
788 789
        end

Douwe Maan's avatar
Douwe Maan committed
790 791
        context "when the note is resolvable" do
          it "resolves the note" do
blackst0ne's avatar
blackst0ne committed
792
            post :resolve, params: request_params
793

Douwe Maan's avatar
Douwe Maan committed
794 795 796
            expect(note.reload.resolved?).to be true
            expect(note.reload.resolved_by).to eq(user)
          end
797

Douwe Maan's avatar
Douwe Maan committed
798
          it "sends notifications if all discussions are resolved" do
799 800 801
            expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance|
              expect(instance).to receive(:execute).with(merge_request)
            end
802

blackst0ne's avatar
blackst0ne committed
803
            post :resolve, params: request_params
Douwe Maan's avatar
Douwe Maan committed
804
          end
805

Douwe Maan's avatar
Douwe Maan committed
806
          it "returns the name of the resolving user" do
blackst0ne's avatar
blackst0ne committed
807
            post :resolve, params: request_params.merge(html: true)
Douwe Maan's avatar
Douwe Maan committed
808

809
            expect(json_response["resolved_by"]).to eq(user.name)
Douwe Maan's avatar
Douwe Maan committed
810
          end
811

Douwe Maan's avatar
Douwe Maan committed
812
          it "returns status 200" do
blackst0ne's avatar
blackst0ne committed
813
            post :resolve, params: request_params
814

815
            expect(response).to have_gitlab_http_status(200)
Douwe Maan's avatar
Douwe Maan committed
816 817
          end
        end
818 819 820
      end
    end

Douwe Maan's avatar
Douwe Maan committed
821
    describe 'DELETE unresolve' do
822
      before do
Douwe Maan's avatar
Douwe Maan committed
823 824 825
        sign_in user

        note.resolve!(user)
826 827
      end

Douwe Maan's avatar
Douwe Maan committed
828
      context "when the user is not authorized to resolve the note" do
829
        it "returns status 404" do
blackst0ne's avatar
blackst0ne committed
830
          delete :unresolve, params: request_params
831

832
          expect(response).to have_gitlab_http_status(404)
833 834 835
        end
      end

Douwe Maan's avatar
Douwe Maan committed
836 837
      context "when the user is authorized to resolve the note" do
        before do
838
          project.add_developer(user)
Douwe Maan's avatar
Douwe Maan committed
839 840 841 842 843 844
        end

        context "when the note is not resolvable" do
          before do
            note.update(system: true)
          end
845

Douwe Maan's avatar
Douwe Maan committed
846
          it "returns status 404" do
blackst0ne's avatar
blackst0ne committed
847
            delete :unresolve, params: request_params
848

849
            expect(response).to have_gitlab_http_status(404)
Douwe Maan's avatar
Douwe Maan committed
850
          end
851 852
        end

Douwe Maan's avatar
Douwe Maan committed
853 854
        context "when the note is resolvable" do
          it "unresolves the note" do
blackst0ne's avatar
blackst0ne committed
855
            delete :unresolve, params: request_params
Douwe Maan's avatar
Douwe Maan committed
856 857 858 859 860

            expect(note.reload.resolved?).to be false
          end

          it "returns status 200" do
blackst0ne's avatar
blackst0ne committed
861
            delete :unresolve, params: request_params
862

863
            expect(response).to have_gitlab_http_status(200)
Douwe Maan's avatar
Douwe Maan committed
864
          end
865 866 867 868
        end
      end
    end
  end
869
end