diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index dd86b940a08c62ae61e011ab4cb45b42854ad823..a03eb8513b6d611335c230f12e5270255daf736e 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -77,12 +77,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController def diffs apply_diff_view_cookie! - @commit = @merge_request.last_commit - @base_commit = @merge_request.diff_base_commit - - # MRs created before 8.4 don't have a diff_base_commit, - # but we need it for the "View file @ ..." link by deleted files - @base_commit ||= @merge_request.first_commit.parent || @merge_request.first_commit + @commit = @merge_request.diff_head_commit + @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit @comments_target = { noteable_type: 'MergeRequest', @@ -134,7 +130,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @target_project = merge_request.target_project @source_project = merge_request.source_project @commits = @merge_request.compare_commits.reverse - @commit = @merge_request.last_commit + @commit = @merge_request.diff_head_commit @base_commit = @merge_request.diff_base_commit @diffs = @merge_request.compare.diffs(diff_options) if @merge_request.compare @diff_notes_disabled = true @@ -212,7 +208,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController return end - if params[:sha] != @merge_request.source_sha + if params[:sha] != @merge_request.diff_head_sha @status = :sha_mismatch return end @@ -274,16 +270,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController status ||= "preparing" else ci_service = @merge_request.source_project.ci_service - status = ci_service.commit_status(merge_request.last_commit.sha, merge_request.source_branch) if ci_service + status = ci_service.commit_status(merge_request.diff_head_sha, merge_request.source_branch) if ci_service if ci_service.respond_to?(:commit_coverage) - coverage = ci_service.commit_coverage(merge_request.last_commit.sha, merge_request.source_branch) + coverage = ci_service.commit_coverage(merge_request.diff_head_sha, merge_request.source_branch) end end response = { title: merge_request.title, - sha: merge_request.last_commit_short_sha, + sha: merge_request.diff_head_commit.short_id, status: status, coverage: coverage } diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 1dd07a2a2204b0368671f57e3b89b05b82432477..c7dedfe925417c6626bb1406b889bf7b8bbfd028 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -27,7 +27,7 @@ module MergeRequestsHelper end def ci_build_details_path(merge_request) - build_url = merge_request.source_project.ci_service.build_page(merge_request.last_commit.sha, merge_request.source_branch) + build_url = merge_request.source_project.ci_service.build_page(merge_request.diff_head_sha, merge_request.source_branch) return nil unless build_url parsed_url = URI.parse(build_url) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4f7e1d2f302fc31029b2883b0cbdfa971c8aac33..cc85421a8159345e0263c73af837625cea4debc7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -16,7 +16,7 @@ class MergeRequest < ActiveRecord::Base serialize :merge_params, Hash - after_create :create_merge_request_diff, unless: :importing + after_create :create_merge_request_diff, unless: :importing? after_update :update_merge_request_diff delegate :commits, :diffs, :real_size, to: :merge_request_diff, prefix: nil @@ -29,10 +29,6 @@ class MergeRequest < ActiveRecord::Base # when creating new merge request attr_accessor :can_be_created, :compare_commits, :compare - # Temporary fields to store target_sha, and base_sha to - # compare when importing pull requests from GitHub - attr_accessor :base_target_sha, :head_source_sha - state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed @@ -169,28 +165,88 @@ class MergeRequest < ActiveRecord::Base reference end - def last_commit - merge_request_diff ? merge_request_diff.last_commit : compare_commits.last - end - def first_commit merge_request_diff ? merge_request_diff.first_commit : compare_commits.first end + def last_commit + merge_request_diff ? merge_request_diff.last_commit : compare_commits.last + end + def diff_size merge_request_diff.size end def diff_base_commit - if merge_request_diff + if persisted? merge_request_diff.base_commit - elsif source_sha - self.target_project.merge_base_commit(self.source_sha, self.target_branch) + elsif diff_start_commit && diff_head_commit + self.target_project.merge_base_commit(diff_start_sha, diff_head_sha) + end + end + + # MRs created before 8.4 don't store a MergeRequestDiff#base_commit_sha, + # but we need to get a commit for the "View file @ ..." link by deleted files, + # so we find the likely one if we can't get the actual one. + # This will not be the actual base commit if the target branch was merged into + # the source branch after the merge request was created, but it is good enough + # for the specific purpose of linking to a commit. + # It is not good enough for use in Gitlab::Git::DiffRefs, which need the + # true base commit. + def likely_diff_base_commit + first_commit.parent || first_commit + end + + def diff_start_commit + if persisted? + merge_request_diff.start_commit + else + target_branch_head end end - def last_commit_short_sha - last_commit.short_id + def diff_head_commit + if persisted? + merge_request_diff.head_commit + else + source_branch_head + end + end + + def diff_start_sha + diff_start_commit.try(:sha) + end + + def diff_base_sha + diff_base_commit.try(:sha) + end + + def diff_head_sha + diff_head_commit.try(:sha) + end + + # When importing a pull request from GitHub, the old and new branches may no + # longer actually exist by those names, but we need to recreate the merge + # request diff with the right source and target shas. + # We use these attributes to force these to the intended values. + attr_writer :target_branch_sha, :source_branch_sha + + def source_branch_head + source_branch_ref = @source_branch_sha || source_branch + source_project.repository.commit(source_branch) if source_branch_ref + end + + def target_branch_head + target_branch_ref = @target_branch_sha || target_branch + target_project.repository.commit(target_branch) if target_branch_ref + end + + def target_branch_sha + target_branch_head.try(:sha) + end + + def source_branch_sha + source_branch_head.try(:sha) end def validate_branches @@ -241,7 +297,7 @@ class MergeRequest < ActiveRecord::Base return unless unchecked? can_be_merged = - !broken? && project.repository.can_be_merged?(source_sha, target_branch) + !broken? && project.repository.can_be_merged?(diff_head_sha, target_branch) if can_be_merged mark_as_mergeable @@ -293,7 +349,7 @@ class MergeRequest < ActiveRecord::Base !source_project.protected_branch?(source_branch) && !source_project.root_ref?(source_branch) && Ability.abilities.allowed?(current_user, :push_code, source_project) && - last_commit == source_project.commit(source_branch) + diff_head_commit == source_branch_head end def should_remove_source_branch? @@ -331,8 +387,8 @@ class MergeRequest < ActiveRecord::Base work_in_progress: work_in_progress? } - if last_commit - attrs.merge!(last_commit: last_commit.hook_attrs) + if diff_head_commit + attrs.merge!(last_commit: diff_head_commit.hook_attrs) end attributes.merge!(attrs) @@ -510,22 +566,6 @@ class MergeRequest < ActiveRecord::Base end end - def target_sha - return @base_target_sha if defined?(@base_target_sha) - - target_project.repository.commit(target_branch).try(:sha) - end - - def source_sha - return @head_source_sha if defined?(@head_source_sha) - - last_commit.try(:sha) || source_tip.try(:sha) - end - - def source_tip - source_branch && source_project.repository.commit(source_branch) - end - def fetch_ref target_project.repository.fetch_ref( source_project.repository.path_to_repo, @@ -558,10 +598,10 @@ class MergeRequest < ActiveRecord::Base def diverged_commits_count cache = Rails.cache.read(:"merge_request_#{id}_diverged_commits") - if cache.blank? || cache[:source_sha] != source_sha || cache[:target_sha] != target_sha + if cache.blank? || cache[:source_sha] != source_branch_sha || cache[:target_sha] != target_branch_sha cache = { - source_sha: source_sha, - target_sha: target_sha, + source_sha: source_branch_sha, + target_sha: target_branch_sha, diverged_commits_count: compute_diverged_commits_count } Rails.cache.write(:"merge_request_#{id}_diverged_commits", cache) @@ -571,9 +611,9 @@ class MergeRequest < ActiveRecord::Base end def compute_diverged_commits_count - return 0 unless source_sha && target_sha + return 0 unless source_branch_sha && target_branch_sha - Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_sha, target_sha).size + Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_branch_sha, target_branch_sha).size end private :compute_diverged_commits_count @@ -582,13 +622,13 @@ class MergeRequest < ActiveRecord::Base end def pipeline - @pipeline ||= source_project.pipeline(last_commit.id, source_branch) if last_commit && source_project end def diff_refs return nil unless diff_base_commit [diff_base_commit, last_commit] + @pipeline ||= source_project.pipeline(diff_head_sha, source_branch) if diff_head_sha && source_project end def merge_commit diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index fd69f915bd20b85ecd83eb14ce02bb0bfef9a6b2..60f4b44a5d1e52c55faacffe8fe4158d642cb89a 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -7,7 +7,7 @@ class MergeRequestDiff < ActiveRecord::Base belongs_to :merge_request - delegate :head_source_sha, :target_branch, :source_branch, to: :merge_request, prefix: nil + delegate :source_branch_sha, :target_branch_sha, :target_branch, :source_branch, to: :merge_request, prefix: nil state_machine :state, initial: :empty do state :collected @@ -40,8 +40,8 @@ class MergeRequestDiff < ActiveRecord::Base @diffs_no_whitespace ||= begin compare = Gitlab::Git::Compare.new( self.repository.raw_repository, - self.base, - self.head, + self.target_branch_sha, + self.source_branch_sha, ) compare.diffs(options) end @@ -63,13 +63,21 @@ class MergeRequestDiff < ActiveRecord::Base end def base_commit - return nil unless self.base_commit_sha + return unless self.base_commit_sha merge_request.target_project.commit(self.base_commit_sha) end - def last_commit_short_sha - @last_commit_short_sha ||= last_commit.short_id + def start_commit + return unless self.start_commit_sha + + merge_request.target_project.commit(self.start_commit_sha) + end + + def head_commit + return last_commit unless self.head_commit_sha + + merge_request.target_project.commit(self.head_commit_sha) end def dump_commits(commits) @@ -166,23 +174,14 @@ class MergeRequestDiff < ActiveRecord::Base merge_request.target_project.repository end - def source_sha - return head_source_sha if head_source_sha.present? - - source_commit = merge_request.source_project.commit(source_branch) - source_commit.try(:sha) - end - - def target_sha - merge_request.target_sha - end + def branch_base_commit + return unless self.source_branch_sha && self.target_branch_sha - def base - self.target_sha || self.target_branch + merge_request.target_project.merge_base_commit(self.source_branch_sha, self.target_branch_sha) end - def head - self.source_sha + def branch_base_sha + branch_base_commit.try(:sha) end def compare @@ -193,8 +192,8 @@ class MergeRequestDiff < ActiveRecord::Base Gitlab::Git::Compare.new( self.repository.raw_repository, - self.base, - self.head + self.target_branch_sha, + self.source_branch_sha ) end end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 27bf08bf7d928a9937cc7ce15953e76669d62512..97bcbacf2b942f025ba1a4d4df5a245157716729 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -144,7 +144,7 @@ class JiraService < IssueTrackerService commit_id = if entity.is_a?(Commit) entity.id elsif entity.is_a?(MergeRequest) - entity.last_commit.id + entity.diff_head_sha end commit_url = build_entity_url(:commit, commit_id) diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 3bec66cea88555b7fd4b18187c3312daa1d2a0d5..f1b1d90c4578f81058ac5de0d88aab343c16b166 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -34,7 +34,7 @@ module MergeRequests committer: committer } - commit_id = repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) + commit_id = repository.merge(current_user, merge_request.diff_head_sha, merge_request.target_branch, options) merge_request.update(merge_commit_sha: commit_id) rescue GitHooksService::PreReceiveError => e merge_request.update(merge_error: e.message) diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb index 870f5705184a7806fe0152373291f56a9ff85ce1..4ad5fb083114400d4b4bd8aef1a573e9cd71227f 100644 --- a/app/services/merge_requests/merge_when_build_succeeds_service.rb +++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb @@ -12,7 +12,7 @@ module MergeRequests merge_request.merge_when_build_succeeds = true merge_request.merge_user = @current_user - SystemNoteService.merge_when_build_succeeds(merge_request, @project, @current_user, merge_request.last_commit) + SystemNoteService.merge_when_build_succeeds(merge_request, @project, @current_user, merge_request.diff_head_commit) end merge_request.save diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index fe0579744b45ae52d69da03a267dfb0f8f75f294..de79c0244284a1953024b7135920509ebf70527e 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -34,10 +34,10 @@ module MergeRequests def close_merge_requests commit_ids = @commits.map(&:id) merge_requests = @project.merge_requests.opened.where(target_branch: @branch_name).to_a - merge_requests = merge_requests.select(&:last_commit) + merge_requests = merge_requests.select(&:diff_head_commit) merge_requests = merge_requests.select do |merge_request| - commit_ids.include?(merge_request.last_commit.id) + commit_ids.include?(merge_request.diff_head_sha) end merge_requests.uniq.select(&:source_project).each do |merge_request| @@ -94,12 +94,10 @@ module MergeRequests merge_request = merge_requests_for_source_branch.first return unless merge_request - last_commit = merge_request.last_commit - begin # Since any number of commits could have been made to the restored branch, # find the common root to see what has been added. - common_ref = @project.repository.merge_base(last_commit.id, @newrev) + common_ref = @project.repository.merge_base(merge_request.diff_head_sha, @newrev) # If the a commit no longer exists in this repo, gitlab_git throws # a Rugged::OdbError. This is fixed in https://gitlab.com/gitlab-org/gitlab_git/merge_requests/52 @commits = @project.repository.commits_between(common_ref, @newrev) if common_ref diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 08a38d283d23889e3096a2d20026aa0d50fc1be9..489c632ae229fd77d72e37f92eb2226b207b84ee 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -7,7 +7,7 @@ CI build = ci_label_for_status(status) for - - commit = @merge_request.last_commit + - commit = @merge_request.diff_head_commit = succeed "." do = link_to @pipeline.short_sha, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, @pipeline.sha), class: "monospace" %span.ci-coverage @@ -24,7 +24,7 @@ CI build = ci_label_for_status(status) for - - commit = @merge_request.last_commit + - commit = @merge_request.diff_head_commit = succeed "." do = link_to commit.short_id, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, commit), class: "monospace" %span.ci-coverage @@ -33,12 +33,12 @@ .ci_widget = icon("spinner spin") - Checking CI status for #{@merge_request.last_commit_short_sha}… + Checking CI status for #{@merge_request.diff_head_commit.short_id}… .ci_widget.ci-not_found{style: "display:none"} = icon("times-circle") - Could not find CI status for #{@merge_request.last_commit_short_sha}. + Could not find CI status for #{@merge_request.diff_head_commit.short_id}. .ci_widget.ci-error{style: "display:none"} = icon("times-circle") - Could not connect to the CI server. Please check your settings and try again. \ No newline at end of file + Could not connect to the CI server. Please check your settings and try again. diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml index 941513febbd8caec991e497959dd1309e979ddb8..bf2e76f008386a00cb8fa6c1c0a35715ed6cc71b 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -2,7 +2,7 @@ = form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f| = hidden_field_tag :authenticity_token, form_authenticity_token - = hidden_field_tag :sha, @merge_request.source_sha + = hidden_field_tag :sha, @merge_request.diff_head_sha .accept-merge-holder.clearfix.js-toggle-container .clearfix .accept-action diff --git a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml index ad898ff153b96cd1d91132bef93e94a7fd2ee90a..2b6b5e05e8654b3924cb8be1e44e46e06275f193 100644 --- a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml +++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml @@ -16,7 +16,7 @@ - if remove_source_branch_button || user_can_cancel_automatic_merge .clearfix.prepend-top-10 - if remove_source_branch_button - = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.source_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do + = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.diff_head_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do = icon('times') Remove Source Branch When Merged diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 640f1720a6cde4b8b71c21ffb72ef33583be96f2..3611c187202000bddb6ba67b4e1b320392819cf2 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -519,7 +519,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step '"Bug NS-05" has CI status' do project = merge_request.source_project project.enable_ci - pipeline = create :ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch + pipeline = create :ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch create :ci_build, pipeline: pipeline end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 0e94efd4acd56359f1d79c71441532771a88df2a..4fcdf8968c91c51ffe676f234260d2c4fb9cb810 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -233,8 +233,8 @@ module API render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable? - if params[:sha] && merge_request.source_sha != params[:sha] - render_api_error!("SHA does not match HEAD of source branch: #{merge_request.source_sha}", 409) + if params[:sha] && merge_request.diff_head_sha != params[:sha] + render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409) end merge_params = { diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index 498b00cb658d2d73845e942a3c20fc44bdf4d767..a4ea2210abdba794f13aa4a869972645229f7712 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -11,10 +11,10 @@ module Gitlab description: description, source_project: source_branch_project, source_branch: source_branch_name, - head_source_sha: source_branch_sha, + source_branch_sha: source_branch_sha, target_project: target_branch_project, target_branch: target_branch_name, - base_target_sha: target_branch_sha, + target_branch_sha: target_branch_sha, state: state, milestone: milestone, author_id: author_id, diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index eff74e1286926d86c07f58515e37f6cc8b38ff05..2d2fb87f14e1393e98e666eb6283b9f4bc849c4a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -212,7 +212,7 @@ describe Projects::MergeRequestsController do context 'when the sha parameter matches the source SHA' do def merge_with_sha - post :merge, base_params.merge(sha: merge_request.source_sha) + post :merge, base_params.merge(sha: merge_request.diff_head_sha) end it 'returns :success' do @@ -229,11 +229,11 @@ describe Projects::MergeRequestsController do context 'when merge_when_build_succeeds is passed' do def merge_when_build_succeeds - post :merge, base_params.merge(sha: merge_request.source_sha, merge_when_build_succeeds: '1') + post :merge, base_params.merge(sha: merge_request.diff_head_sha, merge_when_build_succeeds: '1') end before do - create(:ci_empty_pipeline, project: project, sha: merge_request.source_sha, ref: merge_request.source_branch) + create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) end it 'returns :merge_when_build_succeeds' do diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index b4d2201c7295de7759500cca9e7dac382a3e9bc1..f676200ecf33631f3499e97f3896abe0e431d8d9 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -30,7 +30,7 @@ feature 'Merge request created from fork' do given(:pipeline) do create(:ci_pipeline_with_two_job, project: fork_project, - sha: merge_request.last_commit.id, + sha: merge_request.diff_head_sha, ref: merge_request.source_branch) end diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index c5e6412d7bfdb0593c02c83b65740a1d4a949a7c..96f7b8c993238c902dfb6875016b92835e2b2aed 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -12,7 +12,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do end context "Active build for Merge Request" do - let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } let!(:ci_build) { create(:ci_build, pipeline: pipeline) } before do @@ -47,7 +47,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do merge_user: user, title: "MepMep", merge_when_build_succeeds: true) end - let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } let!(:ci_build) { create(:ci_build, pipeline: pipeline) } before do diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb index 65e9185ec2441c9355102edcdd55ccb402ddc04b..80e8b8fc6429a487c06eafeac5387abd079be9e5 100644 --- a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb +++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb @@ -19,7 +19,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when project has CI enabled' do - let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } context 'when merge requests can only be merged if the build succeeds' do before do diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 9587252b9909de612464efc0d9b78bb15fe07c97..79931ecd134e9eb40da229119a01a3aaa4738daf 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -43,10 +43,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', - head_source_sha: source_sha, + source_branch_sha: source_sha, target_project: project, target_branch: 'master', - base_target_sha: target_sha, + target_branch_sha: target_sha, state: 'opened', milestone: nil, author_id: project.creator_id, @@ -70,10 +70,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', - head_source_sha: source_sha, + source_branch_sha: source_sha, target_project: project, target_branch: 'master', - base_target_sha: target_sha, + target_branch_sha: target_sha, state: 'closed', milestone: nil, author_id: project.creator_id, @@ -97,10 +97,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', - head_source_sha: source_sha, + source_branch_sha: source_sha, target_project: project, target_branch: 'master', - base_target_sha: target_sha, + target_branch_sha: target_sha, state: 'merged', milestone: nil, author_id: project.creator_id, diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index a75eaa4d51fc968feab805e62135d17fcab7d005..1424de9e60be25b17d483da5100982db6e7e5fef 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -125,7 +125,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do ci_pipeline = create(:ci_pipeline, project: project, - sha: merge_request.last_commit.id, + sha: merge_request.diff_head_sha, ref: merge_request.source_branch, statuses: [commit_status]) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ceb4d64698f5ee87fbde99be18acd54ce9966966..bb83676cddfc2602d2ba093ea002065ebdd91835 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -62,7 +62,7 @@ describe MergeRequest, models: true do end end - describe '#target_sha' do + describe '#target_branch_sha' do context 'when the target branch does not exist anymore' do let(:project) { create(:project) } @@ -73,32 +73,32 @@ describe MergeRequest, models: true do end it 'returns nil' do - expect(subject.target_sha).to be_nil + expect(subject.target_branch_sha).to be_nil end end end - describe '#source_sha' do + describe '#source_branch_sha' do let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) } context 'with diffs' do subject { create(:merge_request, :with_diffs) } it 'returns the sha of the source branch last commit' do - expect(subject.source_sha).to eq(last_branch_commit.sha) + expect(subject.source_branch_sha).to eq(last_branch_commit.sha) end end context 'without diffs' do subject { create(:merge_request, :without_diffs) } it 'returns the sha of the source branch last commit' do - expect(subject.source_sha).to eq(last_branch_commit.sha) + expect(subject.source_branch_sha).to eq(last_branch_commit.sha) end end context 'when the merge request is being created' do subject { build(:merge_request, source_branch: nil, compare_commits: []) } it 'returns nil' do - expect(subject.source_sha).to be_nil + expect(subject.source_branch_sha).to be_nil end end end @@ -252,12 +252,14 @@ describe MergeRequest, models: true do end it "can be removed if the last commit is the head of the source branch" do - allow(subject.source_project).to receive(:commit).and_return(subject.last_commit) + allow(subject.source_project).to receive(:commit).and_return(subject.diff_head_commit) expect(subject.can_remove_source_branch?(user)).to be_truthy end it "cannot be removed if the last commit is not also the head of the source branch" do + subject.source_branch = "lfs" + expect(subject.can_remove_source_branch?(user)).to be_falsey end end @@ -363,7 +365,7 @@ describe MergeRequest, models: true do and_return(2) subject.diverged_commits_count - allow(subject).to receive(:source_sha).and_return('123abc') + allow(subject).to receive(:source_branch_sha).and_return('123abc') subject.diverged_commits_count end @@ -373,7 +375,7 @@ describe MergeRequest, models: true do and_return(2) subject.diverged_commits_count - allow(subject).to receive(:target_sha).and_return('123abc') + allow(subject).to receive(:target_branch_sha).and_return('123abc') subject.diverged_commits_count end end @@ -392,11 +394,10 @@ describe MergeRequest, models: true do describe '#pipeline' do describe 'when the source project exists' do - it 'returns the latest commit' do - commit = double(:commit, id: '123abc') + it 'returns the latest pipeline' do pipeline = double(:ci_pipeline, ref: 'master') - allow(subject).to receive(:last_commit).and_return(commit) + allow(subject).to receive(:diff_head_sha).and_return('123abc') expect(subject.source_project).to receive(:pipeline). with('123abc', 'master'). @@ -464,7 +465,7 @@ describe MergeRequest, models: true do context 'when it is not broken and has no conflicts' do it 'is marked as mergeable' do allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { true } + allow(project.repository).to receive(:can_be_merged?).and_return(true) expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') end @@ -481,7 +482,7 @@ describe MergeRequest, models: true do context 'when it has conflicts' do before do allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { false } + allow(project.repository).to receive(:can_be_merged?).and_return(false) end it 'becomes unmergeable' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e89d6de3a25815713b29411eaf635d2ade241ff..1b434a726dcd077dd3ffd4c1cca0234aa1cac997 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -312,7 +312,7 @@ describe Project, models: true do it 'should update merge request commits with new one if pushed to source branch' do project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) merge_request.reload - expect(merge_request.last_commit.id).to eq(commit_id) + expect(merge_request.diff_head_sha).to eq(commit_id) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7975fc64e59b38863604d9d83fe7647ebf2fd5e0..24e49c8def344c24db97e0bf1b175f8bcbc9ea5f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -12,8 +12,8 @@ describe Repository, models: true do end let(:merge_commit) do source_sha = repository.find_branch('feature').target - merge_commit_id = repository.merge(user, source_sha, 'master', commit_options) - repository.commit(merge_commit_id) + merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options) + repository.commit(merge_commit_sha) end describe :branch_names_contains do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 5d81844fb84bd3ba2d6b64e46f188b888b53b33b..4a1b5600bdf3a27245a77bee7223991a1a5062dc 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -439,14 +439,14 @@ describe API::API, api: true do end it "returns 409 if the SHA parameter doesn't match" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha.succ + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha.reverse expect(response).to have_http_status(409) expect(json_response['message']).to start_with('SHA does not match HEAD of source branch') end it "succeeds if the SHA parameter matches" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha expect(response).to have_http_status(200) end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 85dd30bf48c3c0b7d30539fa59433b6af121addd..43693441450f7b80790ecaf1861b80e01408d537 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -213,7 +213,7 @@ describe SystemNoteService, services: true do create(:merge_request, source_project: project, target_project: project) end - subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.last_commit) } + subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.diff_head_commit) } it_behaves_like 'a system note'