summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2016-11-22 13:27:32 +0000
committerSean McGivern <sean@mcgivern.me.uk>2016-11-22 13:27:32 +0000
commitd4d522b683a9f31cc4e0715d943eae30118293d2 (patch)
tree31b58d0841a668174b803337a9d5005bbb64ff77
parentb1b5060dbad15975184ec20a1914c7c48fc804db (diff)
downloadgitlab-ce-d4d522b683a9f31cc4e0715d943eae30118293d2.tar.gz
Revert "Merge branch 'hide-empty-merge-request-diffs' into 'master'"revert-7a5e653f
This reverts merge request !7568
-rw-r--r--app/controllers/projects/merge_requests_controller.rb8
-rw-r--r--app/models/merge_request_diff.rb6
-rw-r--r--app/services/merge_requests/refresh_service.rb21
-rw-r--r--app/views/projects/merge_requests/widget/_open.html.haml4
-rw-r--r--changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml4
-rw-r--r--changelogs/unreleased/hide-empty-merge-request-diffs.yml4
-rw-r--r--spec/features/merge_requests/deleted_source_branch_spec.rb30
-rw-r--r--spec/features/merge_requests/merge_request_versions_spec.rb7
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb10
9 files changed, 37 insertions, 57 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index dbbd2ad849e..b343ba0b744 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -82,12 +82,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@merge_request_diff =
if params[:diff_id]
- @merge_request.merge_request_diffs.viewable.find(params[:diff_id])
+ @merge_request.merge_request_diffs.find(params[:diff_id])
else
@merge_request.merge_request_diff
end
- @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
+ @merge_request_diffs = @merge_request.merge_request_diffs.select_without_diff
@comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
if params[:start_sha].present?
@@ -417,7 +417,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
response = {
title: merge_request.title,
- sha: (merge_request.diff_head_commit.short_id if merge_request.diff_head_sha),
+ sha: merge_request.diff_head_commit.short_id,
status: status,
coverage: coverage
}
@@ -564,7 +564,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def define_pipelines_vars
@pipelines = @merge_request.all_pipelines
- if @pipelines.present? && @merge_request.commits.present?
+ if @pipelines.present?
@pipeline = @pipelines.first
@statuses = @pipeline.statuses.relevant
end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 58a24eb84cb..dd65a9a8b86 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -11,9 +11,6 @@ class MergeRequestDiff < ActiveRecord::Base
belongs_to :merge_request
- serialize :st_commits
- serialize :st_diffs
-
state_machine :state, initial: :empty do
state :collected
state :overflow
@@ -25,7 +22,8 @@ class MergeRequestDiff < ActiveRecord::Base
state :overflow_diff_lines_limit
end
- scope :viewable, -> { without_state(:empty) }
+ serialize :st_commits
+ serialize :st_diffs
# All diff information is collected from repository after object is created.
# It allows you to override variables like head_commit_sha before getting diff.
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 22596b4014a..4a7e6930842 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -60,15 +60,7 @@ module MergeRequests
merge_requests = filter_merge_requests(merge_requests)
merge_requests.each do |merge_request|
- if merge_request.source_branch == @branch_name || force_push?
- merge_request.reload_diff
- else
- mr_commit_ids = merge_request.commits.map(&:id)
- push_commit_ids = @commits.map(&:id)
- matches = mr_commit_ids & push_commit_ids
- merge_request.reload_diff if matches.any?
- end
-
+ reload_diff(merge_request) unless branch_removed?
merge_request.mark_as_unchecked
end
end
@@ -173,5 +165,16 @@ module MergeRequests
def branch_removed?
Gitlab::Git.blank_ref?(@newrev)
end
+
+ def reload_diff(merge_request)
+ if merge_request.source_branch == @branch_name || force_push?
+ merge_request.reload_diff
+ else
+ mr_commit_ids = merge_request.commits.map(&:id)
+ push_commit_ids = @commits.map(&:id)
+ matches = mr_commit_ids & push_commit_ids
+ merge_request.reload_diff if matches.any?
+ end
+ end
end
end
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index 20c93930abc..ac26aa569ba 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/app/views/projects/merge_requests/widget/_open.html.haml
@@ -9,10 +9,10 @@
- if @project.archived?
= render 'projects/merge_requests/widget/open/archived'
- - elsif @merge_request.branch_missing?
- = render 'projects/merge_requests/widget/open/missing_branch'
- elsif @merge_request.commits.blank?
= render 'projects/merge_requests/widget/open/nothing'
+ - elsif @merge_request.branch_missing?
+ = render 'projects/merge_requests/widget/open/missing_branch'
- elsif @merge_request.unchecked?
= render 'projects/merge_requests/widget/open/check'
- elsif @merge_request.cannot_be_merged? && !resolved_conflicts
diff --git a/changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml b/changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml
new file mode 100644
index 00000000000..a6bee989f6d
--- /dev/null
+++ b/changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml
@@ -0,0 +1,4 @@
+---
+title: Do not create a MergeRequestDiff record when source branch is deleted
+merge_request: 7481
+author:
diff --git a/changelogs/unreleased/hide-empty-merge-request-diffs.yml b/changelogs/unreleased/hide-empty-merge-request-diffs.yml
deleted file mode 100644
index e32a51b555a..00000000000
--- a/changelogs/unreleased/hide-empty-merge-request-diffs.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Fix errors happening when source branch of merge request is removed and then restored
-merge_request: 7568
-author:
diff --git a/spec/features/merge_requests/deleted_source_branch_spec.rb b/spec/features/merge_requests/deleted_source_branch_spec.rb
deleted file mode 100644
index 778b3a90cf3..00000000000
--- a/spec/features/merge_requests/deleted_source_branch_spec.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-require 'spec_helper'
-
-describe 'Deleted source branch', feature: true, js: true do
- let(:user) { create(:user) }
- let(:merge_request) { create(:merge_request) }
-
- before do
- login_as user
- merge_request.project.team << [user, :master]
- merge_request.update!(source_branch: 'this-branch-does-not-exist')
- visit namespace_project_merge_request_path(
- merge_request.project.namespace,
- merge_request.project, merge_request
- )
- end
-
- it 'shows a message about missing source branch' do
- expect(page).to have_content(
- 'Source branch this-branch-does-not-exist does not exist'
- )
- end
-
- it 'hides Discussion, Commits and Changes tabs' do
- within '.merge-request-details' do
- expect(page).to have_no_content('Discussion')
- expect(page).to have_no_content('Commits')
- expect(page).to have_no_content('Changes')
- end
- end
-end
diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb
index 09451f41de4..23cee891bac 100644
--- a/spec/features/merge_requests/merge_request_versions_spec.rb
+++ b/spec/features/merge_requests/merge_request_versions_spec.rb
@@ -3,12 +3,11 @@ require 'spec_helper'
feature 'Merge Request versions', js: true, feature: true do
let(:merge_request) { create(:merge_request, importing: true) }
let(:project) { merge_request.source_project }
- let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
- let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) }
- let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
before do
login_as :admin
+ merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+ merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e')
visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
end
@@ -54,7 +53,7 @@ feature 'Merge Request versions', js: true, feature: true do
project.namespace,
project,
merge_request.iid,
- diff_id: merge_request_diff3.id,
+ diff_id: 2,
start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9'
)
end
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index e515bc9f89c..0220f7e1db2 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -227,6 +227,16 @@ describe MergeRequests::RefreshService, services: true do
end
end
+ context 'when the source branch is deleted' do
+ it 'does not create a MergeRequestDiff record' do
+ refresh_service = service.new(@project, @user)
+
+ expect do
+ refresh_service.execute(@oldrev, Gitlab::Git::BLANK_SHA, 'refs/heads/master')
+ end.not_to change { MergeRequestDiff.count }
+ end
+ end
+
def reload_mrs
@merge_request.reload
@fork_merge_request.reload