update_service_spec.rb 20.8 KB
Newer Older
1
# coding: utf-8
2 3
require 'spec_helper'

4
describe Issues::UpdateService, :mailer do
5
  let(:user) { create(:user) }
6
  let(:user2) { create(:user) }
Douwe Maan's avatar
Douwe Maan committed
7
  let(:user3) { create(:user) }
8
  let(:project) { create(:project) }
9
  let(:label) { create(:label, project: project) }
10
  let(:label2) { create(:label) }
11 12 13

  let(:issue) do
    create(:issue, title: 'Old title',
14
                   description: "for #{user2.to_reference}",
15
                   assignee_ids: [user3.id],
16 17
                   project: project,
                   author: create(:user))
18
  end
19

20
  before do
21
    project.add_maintainer(user)
22 23
    project.add_developer(user2)
    project.add_developer(user3)
24 25
  end

26
  describe 'execute' do
27
    def find_note(starting_with)
28
      issue.notes.find do |note|
29 30 31 32
        note && note.note.start_with?(starting_with)
      end
    end

33 34 35 36 37 38 39
    def find_notes(action)
      issue
        .notes
        .joins(:system_note_metadata)
        .where(system_note_metadata: { action: action })
    end

40
    def update_issue(opts)
41
      described_class.new(project, user, opts).execute(issue)
42 43
    end

44 45 46
    context 'valid params' do
      let(:opts) do
        {
47
          title: 'New title',
48
          description: 'Also please fix',
49
          assignee_ids: [user2.id],
Nikita Verkhovin's avatar
Nikita Verkhovin committed
50
          state_event: 'close',
51
          label_ids: [label.id],
52 53
          due_date: Date.tomorrow,
          discussion_locked: true
54 55 56
        }
      end

57
      it 'updates the issue with the given params' do
58 59
        expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in)

60 61 62 63 64
        update_issue(opts)

        expect(issue).to be_valid
        expect(issue.title).to eq 'New title'
        expect(issue.description).to eq 'Also please fix'
65
        expect(issue.assignees).to match_array([user2])
66 67 68
        expect(issue).to be_closed
        expect(issue.labels).to match_array [label]
        expect(issue.due_date).to eq Date.tomorrow
69
        expect(issue.discussion_locked).to be_truthy
70 71
      end

72 73 74 75 76 77 78
      it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do
        issue # make sure the issue is created first so our counts are correct.

        expect { update_issue(confidential: true) }
          .to change { project.open_issues_count }.from(1).to(0)
      end

79
      it 'enqueues ConfidentialIssueWorker when an issue is made confidential' do
80
        expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, issue.id)
81 82 83 84 85 86 87 88 89 90 91 92 93

        update_issue(confidential: true)
      end

      it 'does not enqueue ConfidentialIssueWorker when an issue is made non confidential' do
        # set confidentiality to true before the actual update
        issue.update!(confidential: true)

        expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in)

        update_issue(confidential: false)
      end

94 95 96 97 98 99 100
      it 'updates open issue counter for assignees when issue is reassigned' do
        update_issue(assignee_ids: [user2.id])

        expect(user3.assigned_open_issues_count).to eq 0
        expect(user2.assigned_open_issues_count).to eq 1
      end

Valery Sizov's avatar
Valery Sizov committed
101
      it 'sorts issues as specified by parameters' do
102 103
        issue1 = create(:issue, project: project, assignees: [user3])
        issue2 = create(:issue, project: project, assignees: [user3])
Valery Sizov's avatar
Valery Sizov committed
104 105 106 107 108 109

        [issue, issue1, issue2].each do |issue|
          issue.move_to_end
          issue.save
        end

Felipe Artur's avatar
Felipe Artur committed
110
        opts[:move_between_ids] = [issue1.id, issue2.id]
Valery Sizov's avatar
Valery Sizov committed
111 112 113 114 115 116

        update_issue(opts)

        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
      end

Felipe Artur's avatar
Felipe Artur committed
117
      context 'when moving issue between issues from different projects', :nested_groups do
118
        let(:group) { create(:group) }
Felipe Artur's avatar
Felipe Artur committed
119 120
        let(:subgroup) { create(:group, parent: group) }

121 122
        let(:project_1) { create(:project, namespace: group) }
        let(:project_2) { create(:project, namespace: group) }
Felipe Artur's avatar
Felipe Artur committed
123
        let(:project_3) { create(:project, namespace: subgroup) }
124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149

        let(:issue_1) { create(:issue, project: project_1) }
        let(:issue_2) { create(:issue, project: project_2) }
        let(:issue_3) { create(:issue, project: project_3) }

        before do
          group.add_developer(user)
        end

        it 'sorts issues as specified by parameters' do
          # Moving all issues to end here like the last example won't work since
          # all projects only have the same issue count
          # so their relative_position will be the same.
          issue_1.move_to_end
          issue_2.move_after(issue_1)
          issue_3.move_after(issue_2)
          [issue_1, issue_2, issue_3].map(&:save)

          opts[:move_between_ids] = [issue_1.id, issue_2.id]
          opts[:board_group_id] = group.id

          described_class.new(issue_3.project, user, opts).execute(issue_3)
          expect(issue_2.relative_position).to be_between(issue_1.relative_position, issue_2.relative_position)
        end
      end

150 151 152
      context 'when current user cannot admin issues in the project' do
        let(:guest) { create(:user) }
        before do
153
          project.add_guest(guest)
154
        end
155

156 157 158 159 160 161
        it 'filters out params that cannot be set without the :admin_issue permission' do
          described_class.new(project, guest, opts).execute(issue)

          expect(issue).to be_valid
          expect(issue.title).to eq 'New title'
          expect(issue.description).to eq 'Also please fix'
162
          expect(issue.assignees).to match_array [user3]
163 164 165
          expect(issue.labels).to be_empty
          expect(issue.milestone).to be_nil
          expect(issue.due_date).to be_nil
166
          expect(issue.discussion_locked).to be_falsey
167
        end
168
      end
Nikita Verkhovin's avatar
Nikita Verkhovin committed
169

170 171 172 173 174 175 176 177 178 179 180 181 182 183
      context 'with background jobs processed' do
        before do
          perform_enqueued_jobs do
            update_issue(opts)
          end
        end

        it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do
          deliveries = ActionMailer::Base.deliveries
          email = deliveries.last
          recipients = deliveries.last(2).map(&:to).flatten
          expect(recipients).to include(user2.email, user3.email)
          expect(email.subject).to include(issue.title)
        end
184

185
        it 'creates system note about issue reassign' do
186
          note = find_note('assigned to')
187

188
          expect(note).not_to be_nil
189
          expect(note.note).to include "assigned to #{user2.to_reference}"
190
        end
191

192 193
        it 'creates a resource label event' do
          event = issue.resource_label_events.last
194

195 196 197
          expect(event).not_to be_nil
          expect(event.label_id).to eq label.id
          expect(event.user_id).to eq user.id
198 199 200
        end

        it 'creates system note about title change' do
201
          note = find_note('changed title')
202 203

          expect(note).not_to be_nil
204
          expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
205
        end
206 207 208 209 210 211 212

        it 'creates system note about discussion lock' do
          note = find_note('locked this issue')

          expect(note).not_to be_nil
          expect(note.note).to eq 'locked this issue'
        end
213
      end
214 215
    end

blackst0ne's avatar
blackst0ne committed
216 217 218 219 220 221 222 223 224 225 226
    context 'when description changed' do
      it 'creates system note about description change' do
        update_issue(description: 'Changed description')

        note = find_note('changed the description')

        expect(note).not_to be_nil
        expect(note.note).to eq('changed the description')
      end
    end

227 228 229 230 231
    context 'when issue turns confidential' do
      let(:opts) do
        {
          title: 'New title',
          description: 'Also please fix',
232
          assignee_ids: [user2],
233 234 235 236 237
          state_event: 'close',
          label_ids: [label.id],
          confidential: true
        }
      end
238 239

      it 'creates system note about confidentiality change' do
240
        update_issue(confidential: true)
241

242
        note = find_note('made the issue confidential')
243 244

        expect(note).not_to be_nil
245
        expect(note.note).to eq 'made the issue confidential'
246 247
      end

248 249 250
      it 'executes confidential issue hooks' do
        expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks)
        expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks)
251

252
        update_issue(confidential: true)
253
      end
254 255 256 257

      it 'does not update assignee_id with unauthorized users' do
        project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
        update_issue(confidential: true)
258 259
        non_member = create(:user)
        original_assignees = issue.assignees
260

261
        update_issue(assignee_ids: [non_member.id])
262

263
        expect(issue.reload.assignees).to eq(original_assignees)
264
      end
265
    end
266

267 268
    context 'todos' do
      let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) }
269 270 271

      context 'when the title change' do
        before do
272
          update_issue(title: 'New title')
273 274
        end

275 276
        it 'marks pending todos as done' do
          expect(todo.reload.done?).to eq true
277
        end
278 279 280 281

        it 'does not create any new todos' do
          expect(Todo.count).to eq(1)
        end
282 283 284 285
      end

      context 'when the description change' do
        before do
286
          update_issue(description: "Also please fix #{user2.to_reference} #{user3.to_reference}")
287 288
        end

289 290
        it 'marks todos as done' do
          expect(todo.reload.done?).to eq true
291
        end
292 293 294 295

        it 'creates only 1 new todo' do
          expect(Todo.count).to eq(2)
        end
296 297 298 299
      end

      context 'when is reassigned' do
        before do
300
          update_issue(assignees: [user2])
301 302
        end

303 304
        it 'marks previous assignee todos as done' do
          expect(todo.reload.done?).to eq true
305
        end
306

307
        it 'creates a todo for new assignee' do
308 309 310 311
          attributes = {
            project: project,
            author: user,
            user: user2,
312 313 314
            target_id: issue.id,
            target_type: issue.class.name,
            action: Todo::ASSIGNED,
315 316 317
            state: :pending
          }

318
          expect(Todo.where(attributes).count).to eq 1
319 320 321
        end
      end

322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345
      context 'when a new assignee added' do
        subject { update_issue(assignees: issue.assignees + [user2]) }

        it 'creates only 1 new todo' do
          expect { subject }.to change { Todo.count }.by(1)
        end

        it 'creates a todo for new assignee' do
          subject

          attributes = {
            project: project,
            author: user,
            user: user2,
            target_id: issue.id,
            target_type: issue.class.name,
            action: Todo::ASSIGNED,
            state: :pending
          }

          expect(Todo.where(attributes).count).to eq(1)
        end
      end

346 347 348 349 350 351 352 353 354 355 356 357 358
      context 'when the milestone is removed' do
        let!(:non_subscriber) { create(:user) }

        let!(:subscriber) do
          create(:user) do |u|
            issue.toggle_subscription(u, project)
            project.add_developer(u)
          end
        end

        it_behaves_like 'system notes for milestones'

        it 'sends notifications for subscribers of changed milestone' do
359
          issue.milestone = create(:milestone, project: project)
360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381

          issue.save

          perform_enqueued_jobs do
            update_issue(milestone_id: "")
          end

          should_email(subscriber)
          should_not_email(non_subscriber)
        end
      end

      context 'when the milestone is changed' do
        let!(:non_subscriber) { create(:user) }

        let!(:subscriber) do
          create(:user) do |u|
            issue.toggle_subscription(u, project)
            project.add_developer(u)
          end
        end

Felipe Artur's avatar
Felipe Artur committed
382
        it 'marks todos as done' do
383
          update_issue(milestone: create(:milestone, project: project))
384

385
          expect(todo.reload.done?).to eq true
386
        end
Felipe Artur's avatar
Felipe Artur committed
387 388

        it_behaves_like 'system notes for milestones'
389 390 391

        it 'sends notifications for subscribers of changed milestone' do
          perform_enqueued_jobs do
392
            update_issue(milestone: create(:milestone, project: project))
393 394 395 396 397
          end

          should_email(subscriber)
          should_not_email(non_subscriber)
        end
398 399 400 401
      end

      context 'when the labels change' do
        before do
402 403 404
          Timecop.freeze(1.minute.from_now) do
            update_issue(label_ids: [label.id])
          end
405 406
        end

407 408
        it 'marks todos as done' do
          expect(todo.reload.done?).to eq true
409
        end
410 411 412 413

        it 'updates updated_at' do
          expect(issue.reload.updated_at).to be > Time.now
        end
414
      end
415
    end
416

417 418
    context 'when the issue is relabeled' do
      let!(:non_subscriber) { create(:user) }
419

420
      let!(:subscriber) do
421
        create(:user) do |u|
422
          label.toggle_subscription(u, project)
423
          project.add_developer(u)
424 425
        end
      end
426

427
      it 'sends notifications for subscribers of newly added labels' do
428 429 430
        opts = { label_ids: [label.id] }

        perform_enqueued_jobs do
431
          @issue = described_class.new(project, user, opts).execute(issue)
432 433 434 435 436 437
        end

        should_email(subscriber)
        should_not_email(non_subscriber)
      end

438
      context 'when issue has the `label` label' do
439 440 441
        before do
          issue.labels << label
        end
442

443 444
        it 'does not send notifications for existing labels' do
          opts = { label_ids: [label.id, label2.id] }
445

446
          perform_enqueued_jobs do
447
            @issue = described_class.new(project, user, opts).execute(issue)
448
          end
449

450 451 452
          should_not_email(subscriber)
          should_not_email(non_subscriber)
        end
453

454 455
        it 'does not send notifications for removed labels' do
          opts = { label_ids: [label2.id] }
456

457
          perform_enqueued_jobs do
458
            @issue = described_class.new(project, user, opts).execute(issue)
459
          end
460

461 462
          should_not_email(subscriber)
          should_not_email(non_subscriber)
463 464 465 466
        end
      end
    end

467
    context 'when issue has tasks' do
468 469 470
      before do
        update_issue(description: "- [ ] Task 1\n- [ ] Task 2")
      end
471

472
      it { expect(issue.tasks?).to eq(true) }
473

474 475
      it_behaves_like 'updating a single task'

476
      context 'when tasks are marked as completed' do
477 478 479
        before do
          update_issue(description: "- [x] Task 1\n- [X] Task 2")
        end
480 481

        it 'creates system note about task status change' do
482 483
          note1 = find_note('marked the task **Task 1** as completed')
          note2 = find_note('marked the task **Task 2** as completed')
484 485 486

          expect(note1).not_to be_nil
          expect(note2).not_to be_nil
487 488 489

          description_notes = find_notes('description')
          expect(description_notes.length).to eq(1)
490 491 492 493 494
        end
      end

      context 'when tasks are marked as incomplete' do
        before do
495 496
          update_issue(description: "- [x] Task 1\n- [X] Task 2")
          update_issue(description: "- [ ] Task 1\n- [ ] Task 2")
497 498 499
        end

        it 'creates system note about task status change' do
500 501
          note1 = find_note('marked the task **Task 1** as incomplete')
          note2 = find_note('marked the task **Task 2** as incomplete')
502 503 504

          expect(note1).not_to be_nil
          expect(note2).not_to be_nil
505 506 507

          description_notes = find_notes('description')
          expect(description_notes.length).to eq(1)
508 509 510 511 512
        end
      end

      context 'when tasks position has been modified' do
        before do
513 514
          update_issue(description: "- [x] Task 1\n- [X] Task 2")
          update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2")
515 516
        end

517 518 519
        it 'does not create a system note for the task' do
          task_note = find_note('marked the task **Task 2** as incomplete')
          description_notes = find_notes('description')
520

521 522
          expect(task_note).to be_nil
          expect(description_notes.length).to eq(2)
523 524
        end
      end
525 526 527

      context 'when a Task list with a completed item is totally replaced' do
        before do
528 529
          update_issue(description: "- [ ] Task 1\n- [X] Task 2")
          update_issue(description: "- [ ] One\n- [ ] Two\n- [ ] Three")
530 531 532
        end

        it 'does not create a system note referencing the position the old item' do
533 534
          task_note = find_note('marked the task **Two** as incomplete')
          description_notes = find_notes('description')
535

536 537
          expect(task_note).to be_nil
          expect(description_notes.length).to eq(2)
538 539
        end

540
        it 'does not generate a new note at all' do
541
          expect do
542
            update_issue(description: "- [ ] One\n- [ ] Two\n- [ ] Three")
543
          end.not_to change { Note.count }
544 545
        end
      end
546
    end
547 548 549

    context 'updating labels' do
      let(:label3) { create(:label, project: project) }
550
      let(:result) { described_class.new(project, user, params).execute(issue).reload }
551 552 553 554 555 556 557 558 559 560 561 562 563 564 565 566

      context 'when add_label_ids and label_ids are passed' do
        let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } }

        it 'ignores the label_ids parameter' do
          expect(result.label_ids).not_to include(label.id)
        end

        it 'adds the passed labels' do
          expect(result.label_ids).to include(label3.id)
        end
      end

      context 'when remove_label_ids and label_ids are passed' do
        let(:params) { { label_ids: [], remove_label_ids: [label.id] } }

567
        before do
Lin Jen-Shin's avatar
Lin Jen-Shin committed
568
          issue.update(labels: [label, label3])
569
        end
570 571 572 573 574 575 576 577 578 579 580 581 582

        it 'ignores the label_ids parameter' do
          expect(result.label_ids).not_to be_empty
        end

        it 'removes the passed labels' do
          expect(result.label_ids).not_to include(label.id)
        end
      end

      context 'when add_label_ids and remove_label_ids are passed' do
        let(:params) { { add_label_ids: [label3.id], remove_label_ids: [label.id] } }

583
        before do
Lin Jen-Shin's avatar
Lin Jen-Shin committed
584
          issue.update(labels: [label])
585
        end
586 587 588 589 590 591 592 593 594 595

        it 'adds the passed labels' do
          expect(result.label_ids).to include(label3.id)
        end

        it 'removes the passed labels' do
          expect(result.label_ids).not_to include(label.id)
        end
      end
    end
596

597 598 599 600 601 602 603 604 605 606 607 608 609 610 611 612 613 614 615 616 617 618 619 620 621 622 623 624 625
    context 'updating asssignee_id' do
      it 'does not update assignee when assignee_id is invalid' do
        update_issue(assignee_ids: [-1])

        expect(issue.reload.assignees).to eq([user3])
      end

      it 'unassigns assignee when user id is 0' do
        update_issue(assignee_ids: [0])

        expect(issue.reload.assignees).to be_empty
      end

      it 'does not update assignee_id when user cannot read issue' do
        update_issue(assignee_ids: [create(:user).id])

        expect(issue.reload.assignees).to eq([user3])
      end

      context "when issuable feature is private" do
        levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC]

        levels.each do |level|
          it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
            assignee = create(:user)
            project.update(visibility_level: level)
            feature_visibility_attr = :"#{issue.model_name.plural}_access_level"
            project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE)

626
            expect { update_issue(assignee_ids: [assignee.id]) }.not_to change { issue.assignees }
627 628 629 630 631
          end
        end
      end
    end

632 633
    context 'updating mentions' do
      let(:mentionable) { issue }
634
      include_examples 'updating mentions', described_class
635
    end
636

637
    context 'duplicate issue' do
638
      let(:canonical_issue) { create(:issue, project: project) }
639

640 641 642
      context 'invalid canonical_issue_id' do
        it 'does not call the duplicate service' do
          expect(Issues::DuplicateService).not_to receive(:new)
643

644
          update_issue(canonical_issue_id: 123456789)
645 646 647
        end
      end

648 649 650 651
      context 'valid canonical_issue_id' do
        it 'calls the duplicate service with both issues' do
          expect_any_instance_of(Issues::DuplicateService)
            .to receive(:execute).with(issue, canonical_issue)
652

653
          update_issue(canonical_issue_id: canonical_issue.id)
654 655 656 657
        end
      end
    end

658 659 660 661 662
    context 'move issue to another project' do
      let(:target_project) { create(:project) }

      context 'valid project' do
        before do
663
          target_project.add_maintainer(user)
664 665 666
        end

        it 'calls the move service with the proper issue and project' do
667
          move_stub = instance_double(Issues::MoveService)
668 669 670 671 672 673 674 675 676 677
          allow(Issues::MoveService).to receive(:new).and_return(move_stub)
          allow(move_stub).to receive(:execute).with(issue, target_project).and_return(issue)

          expect(move_stub).to receive(:execute).with(issue, target_project)

          update_issue(target_project: target_project)
        end
      end
    end

678 679 680 681
    include_examples 'issuable update service' do
      let(:open_issuable) { issue }
      let(:closed_issuable) { create(:closed_issue, project: project) }
    end
682 683
  end
end