build_service_spec.rb 16.4 KB
Newer Older
1 2
require 'spec_helper'

3
describe MergeRequests::BuildService do
4
  using RSpec::Parameterized::TableSyntax
5
  include RepoHelpers
6
  include ProjectForksHelper
7

8
  let(:project) { create(:project, :repository) }
9 10
  let(:source_project) { nil }
  let(:target_project) { nil }
11
  let(:user) { create(:user) }
12 13
  let(:issue_confidential) { false }
  let(:issue) { create(:issue, project: project, title: 'A bug', confidential: issue_confidential) }
14 15 16
  let(:description) { nil }
  let(:source_branch) { 'feature-branch' }
  let(:target_branch) { 'master' }
17 18
  let(:milestone_id) { nil }
  let(:label_ids) { [] }
19 20
  let(:merge_request) { service.execute }
  let(:compare) { double(:compare, commits: commits) }
21 22
  let(:commit_1) { double(:commit_1, sha: 'f00ba7', safe_message: "Initial commit\n\nCreate the app") }
  let(:commit_2) { double(:commit_2, sha: 'f00ba7', safe_message: 'This is a bad commit message!') }
23 24
  let(:commits) { nil }

25 26 27 28 29 30 31 32 33 34 35 36
  let(:params) do
    {
      description: description,
      source_branch: source_branch,
      target_branch: target_branch,
      source_project: source_project,
      target_project: target_project,
      milestone_id: milestone_id,
      label_ids: label_ids
    }
  end

37
  let(:service) do
38
    described_class.new(project, user, params)
39 40 41
  end

  before do
42
    project.add_guest(user)
43
  end
44

45
  def stub_compare
46
    allow(CompareService).to receive_message_chain(:new, :execute).and_return(compare)
47 48
    allow(project).to receive(:commit).and_return(commit_1)
    allow(project).to receive(:commit).and_return(commit_2)
49 50
  end

51 52
  describe '#execute' do
    it 'calls the compare service with the correct arguments' do
53
      allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true)
54 55 56 57 58 59 60 61 62 63 64
      expect(CompareService).to receive(:new)
                                  .with(project, Gitlab::Git::BRANCH_REF_PREFIX + source_branch)
                                  .and_call_original

      expect_any_instance_of(CompareService).to receive(:execute)
                                                  .with(project, Gitlab::Git::BRANCH_REF_PREFIX + target_branch)
                                                  .and_call_original

      merge_request
    end

65 66 67 68 69 70 71 72 73 74 75 76 77
    it 'does not assign force_remove_source_branch' do
      expect(merge_request.force_remove_source_branch?).to be_falsey
    end

    context 'with force_remove_source_branch parameter' do
      let(:mr_params) { params.merge(force_remove_source_branch: '1') }
      let(:merge_request) { described_class.new(project, user, mr_params).execute }

      it 'assigns force_remove_source_branch' do
        expect(merge_request.force_remove_source_branch?).to be_truthy
      end
    end

78 79 80 81 82 83 84 85 86 87 88 89
    context 'missing source branch' do
      let(:source_branch) { '' }

      it 'forbids the merge request from being created' do
        expect(merge_request.can_be_created).to eq(false)
      end

      it 'adds an error message to the merge request' do
        expect(merge_request.errors).to contain_exactly('You must select source and target branch')
      end
    end

90 91 92
    context 'when target branch is missing' do
      let(:target_branch) { nil }
      let(:commits) { Commit.decorate([commit_1], project) }
93

94 95 96 97
      before do
        stub_compare
      end

98 99 100
      it 'creates compare object with target branch as default branch' do
        expect(merge_request.compare).to be_present
        expect(merge_request.target_branch).to eq(project.default_branch)
101
      end
102 103 104 105

      it 'allows the merge request to be created' do
        expect(merge_request.can_be_created).to eq(true)
      end
106 107
    end

Artem Sidorenko's avatar
Artem Sidorenko committed
108 109
    context 'same source and target branch' do
      let(:source_branch) { 'master' }
110 111 112 113

      it 'forbids the merge request from being created' do
        expect(merge_request.can_be_created).to eq(false)
      end
Artem Sidorenko's avatar
Artem Sidorenko committed
114 115 116 117 118 119 120 121 122

      it 'adds an error message to the merge request' do
        expect(merge_request.errors).to contain_exactly('You must select different branches')
      end
    end

    context 'no commits in the diff' do
      let(:commits) { [] }

123 124 125 126
      before do
        stub_compare
      end

Artem Sidorenko's avatar
Artem Sidorenko committed
127 128 129 130 131 132 133
      it 'allows the merge request to be created' do
        expect(merge_request.can_be_created).to eq(true)
      end

      it 'adds a WIP prefix to the merge request title' do
        expect(merge_request.title).to eq('WIP: Feature branch')
      end
134 135 136
    end

    context 'one commit in the diff' do
137
      let(:commits) { Commit.decorate([commit_1], project) }
138
      let(:commit_description) { commit_1.safe_message.split(/\n+/, 2).last }
139

140 141 142 143
      before do
        stub_compare
      end

144 145 146 147 148 149 150 151 152
      it 'allows the merge request to be created' do
        expect(merge_request.can_be_created).to eq(true)
      end

      it 'uses the title of the commit as the title of the merge request' do
        expect(merge_request.title).to eq(commit_1.safe_message.split("\n").first)
      end

      it 'uses the description of the commit as the description of the merge request' do
153
        expect(merge_request.description).to eq(commit_description)
154 155 156 157 158 159 160 161 162 163 164
      end

      context 'merge request already has a description set' do
        let(:description) { 'Merge request description' }

        it 'keeps the description from the initial params' do
          expect(merge_request.description).to eq(description)
        end
      end

      context 'commit has no description' do
165
        let(:commits) { Commit.decorate([commit_2], project) }
166 167 168 169 170 171 172 173 174 175

        it 'uses the title of the commit as the title of the merge request' do
          expect(merge_request.title).to eq(commit_2.safe_message)
        end

        it 'sets the description to nil' do
          expect(merge_request.description).to be_nil
        end
      end

176 177 178 179 180 181 182 183
      context 'when the source branch matches an issue' do
        where(:issue_tracker, :source_branch, :closing_message) do
          :jira                 | 'FOO-123-fix-issue' | 'Closes FOO-123'
          :jira                 | 'fix-issue'         | nil
          :custom_issue_tracker | '123-fix-issue'     | 'Closes #123'
          :custom_issue_tracker | 'fix-issue'         | nil
          :internal             | '123-fix-issue'     | 'Closes #123'
          :internal             | 'fix-issue'         | nil
184 185
        end

186 187 188 189 190 191 192
        with_them do
          before do
            if issue_tracker == :internal
              issue.update!(iid: 123)
            else
              create(:"#{issue_tracker}_service", project: project)
            end
193 194
          end

195 196 197 198
          it 'uses the title of the commit as the title of the merge request' do
            expect(merge_request.title).to eq('Initial commit')
          end

199 200
          it 'appends the closing description' do
            expected_description = [commit_description, closing_message].compact.join("\n\n")
201

202
            expect(merge_request.description).to eq(expected_description)
203 204
          end
        end
205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231

        context 'when the source branch matches an internal issue' do
          let(:label) { create(:label, project: project) }
          let(:milestone) { create(:milestone, project: project) }
          let(:source_branch) { '123-fix-issue' }

          before do
            issue.update!(iid: 123, labels: [label], milestone: milestone)
          end

          it 'assigns the issue label and milestone' do
            expect(merge_request.milestone).to eq(milestone)
            expect(merge_request.labels).to match_array([label])
          end

          context 'when milestone_id and label_ids are shared in the params' do
            let(:label2) { create(:label, project: project) }
            let(:milestone2) { create(:milestone, project: project) }
            let(:label_ids) { [label2.id] }
            let(:milestone_id) { milestone2.id }

            it 'assigns milestone_id and label_ids instead of issue labels and milestone' do
              expect(merge_request.milestone).to eq(milestone2)
              expect(merge_request.labels).to match_array([label2])
            end
          end
        end
232 233 234 235 236 237 238 239 240

        context 'when a milestone is from another project' do
          let(:milestone) { create(:milestone, project: create(:project)) }
          let(:milestone_id) { milestone.id }

          it 'sets milestone to nil' do
            expect(merge_request.milestone).to be_nil
          end
        end
241 242 243 244
      end
    end

    context 'more than one commit in the diff' do
245
      let(:commits) { Commit.decorate([commit_1, commit_2], project) }
246

247 248 249 250
      before do
        stub_compare
      end

251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269
      it 'allows the merge request to be created' do
        expect(merge_request.can_be_created).to eq(true)
      end

      it 'uses the title of the branch as the merge request title' do
        expect(merge_request.title).to eq('Feature branch')
      end

      it 'does not add a description' do
        expect(merge_request.description).to be_nil
      end

      context 'merge request already has a description set' do
        let(:description) { 'Merge request description' }

        it 'keeps the description from the initial params' do
          expect(merge_request.description).to eq(description)
        end
      end
270

271 272 273 274 275 276 277 278 279
      context 'when the source branch matches an issue' do
        where(:issue_tracker, :source_branch, :title, :closing_message) do
          :jira                 | 'FOO-123-fix-issue' | 'Resolve FOO-123 "Fix issue"' | 'Closes FOO-123'
          :jira                 | 'fix-issue'         | 'Fix issue'                   | nil
          :custom_issue_tracker | '123-fix-issue'     | 'Resolve #123 "Fix issue"'    | 'Closes #123'
          :custom_issue_tracker | 'fix-issue'         | 'Fix issue'                   | nil
          :internal             | '123-fix-issue'     | 'Resolve "A bug"'             | 'Closes #123'
          :internal             | 'fix-issue'         | 'Fix issue'                   | nil
          :internal             | '124-fix-issue'     | '124 fix issue'               | nil
280 281
        end

282
        with_them do
283
          before do
284 285 286 287 288
            if issue_tracker == :internal
              issue.update!(iid: 123)
            else
              create(:"#{issue_tracker}_service", project: project)
            end
289
          end
290

291 292
          it 'sets the correct title' do
            expect(merge_request.title).to eq(title)
293
          end
294

295 296
          it 'sets the closing description' do
            expect(merge_request.description).to eq(closing_message)
297 298
          end
        end
299 300
      end

301 302
      context 'when the issue is not accessible to user' do
        let(:source_branch) { "#{issue.iid}-fix-issue" }
303

304
        before do
305
          project.team.truncate
306
        end
307

308 309
        it 'uses branch title as the merge request title' do
          expect(merge_request.title).to eq("#{issue.iid} fix issue")
310
        end
311

312 313
        it 'does not set a description' do
          expect(merge_request.description).to be_nil
314
        end
315
      end
316

317 318 319
      context 'when the issue is confidential' do
        let(:source_branch) { "#{issue.iid}-fix-issue" }
        let(:issue_confidential) { true }
320

321 322
        it 'uses the title of the branch as the merge request title' do
          expect(merge_request.title).to eq("#{issue.iid} fix issue")
323 324
        end

325 326
        it 'does not set a description' do
          expect(merge_request.description).to be_nil
327
        end
328
      end
329
    end
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 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376

    context 'source branch does not exist' do
      before do
        allow(project).to receive(:commit).with(source_branch).and_return(nil)
        allow(project).to receive(:commit).with(target_branch).and_return(commit_1)
      end

      it 'forbids the merge request from being created' do
        expect(merge_request.can_be_created).to eq(false)
      end

      it 'adds an error message to the merge request' do
        expect(merge_request.errors).to contain_exactly('Source branch "feature-branch" does not exist')
      end
    end

    context 'target branch does not exist' do
      before do
        allow(project).to receive(:commit).with(source_branch).and_return(commit_1)
        allow(project).to receive(:commit).with(target_branch).and_return(nil)
      end

      it 'forbids the merge request from being created' do
        expect(merge_request.can_be_created).to eq(false)
      end

      it 'adds an error message to the merge request' do
        expect(merge_request.errors).to contain_exactly('Target branch "master" does not exist')
      end
    end

    context 'both source and target branches do not exist' do
      before do
        allow(project).to receive(:commit).and_return(nil)
      end

      it 'forbids the merge request from being created' do
        expect(merge_request.can_be_created).to eq(false)
      end

      it 'adds both error messages to the merge request' do
        expect(merge_request.errors).to contain_exactly(
          'Source branch "feature-branch" does not exist',
          'Target branch "master" does not exist'
        )
      end
    end
377

378
    context 'upstream project has disabled merge requests' do
379 380
      let(:upstream_project) { create(:project, :merge_requests_disabled) }
      let(:project) { create(:project, forked_from_project: upstream_project) }
381 382 383 384 385 386 387
      let(:commits) { Commit.decorate([commit_1], project) }

      it 'sets target project correctly' do
        expect(merge_request.target_project).to eq(project)
      end
    end

388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405
    context 'target_project is set and accessible by current_user' do
      let(:target_project) { create(:project, :public, :repository)}
      let(:commits) { Commit.decorate([commit_1], project) }

      it 'sets target project correctly' do
        expect(merge_request.target_project).to eq(target_project)
      end
    end

    context 'target_project is set but not accessible by current_user' do
      let(:target_project) { create(:project, :private, :repository)}
      let(:commits) { Commit.decorate([commit_1], project) }

      it 'sets target project correctly' do
        expect(merge_request.target_project).to eq(project)
      end
    end

406 407 408 409 410 411 412 413 414 415
    context 'target_project is set but repo is not accessible by current_user' do
      let(:target_project) do
        create(:project, :public, :repository, repository_access_level: ProjectFeature::PRIVATE)
      end

      it 'sets target project correctly' do
        expect(merge_request.target_project).to eq(project)
      end
    end

416 417 418 419
    context 'source_project is set and accessible by current_user' do
      let(:source_project) { create(:project, :public, :repository)}
      let(:commits) { Commit.decorate([commit_1], project) }

420 421 422 423 424 425 426
      before do
        # To create merge requests _from_ a project the user needs at least
        # developer access
        source_project.add_developer(user)
      end

      it 'sets source project correctly' do
427 428 429 430 431 432 433 434
        expect(merge_request.source_project).to eq(source_project)
      end
    end

    context 'source_project is set but not accessible by current_user' do
      let(:source_project) { create(:project, :private, :repository)}
      let(:commits) { Commit.decorate([commit_1], project) }

435 436 437 438 439 440 441 442 443 444 445
      it 'sets source project correctly' do
        expect(merge_request.source_project).to eq(project)
      end
    end

    context 'source_project is set but the user cannot create merge requests from the project' do
      let(:source_project) do
        create(:project, :public, :repository, merge_requests_access_level: ProjectFeature::PRIVATE)
      end

      it 'sets the source_project correctly' do
446 447 448
        expect(merge_request.source_project).to eq(project)
      end
    end
449

450 451 452 453 454 455 456 457 458 459 460 461 462 463 464 465 466 467 468 469 470 471
    context 'target_project is not in the fork network of source_project' do
      let(:target_project) { create(:project, :public, :repository) }

      it 'adds an error to the merge request' do
        expect(merge_request.errors[:validate_fork]).to contain_exactly('Source project is not a fork of the target project')
      end
    end

    context 'target_project is in the fork network of source project but no longer accessible' do
      let!(:project) { fork_project(target_project, user, namespace: user.namespace, repository: true) }
      let(:source_project) { project }
      let(:target_project) { create(:project, :public, :repository) }

      before do
        target_project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
      end

      it 'sets the target_project correctly' do
        expect(merge_request.target_project).to eq(project)
      end
    end

472 473 474 475 476 477 478
    context 'when specifying target branch in the description' do
      let(:description) { "A merge request targeting another branch\n\n/target_branch with-codeowners" }

      it 'sets the attribute from the quick actions' do
        expect(merge_request.target_branch).to eq('with-codeowners')
      end
    end
479 480
  end
end