BigW Consortium Gitlab

Skip to content

G
gitlab-ce
Unverified Commit 6515ec09 by Dmitriy Zaporozhets
Options

Chnage the way how merge request diff is initialized and saved

Signed-off-by: 's avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent f8aeb8cd
Showing with 44 additions and 52 deletions
app/models/merge_request.rb
......@@ -11,11 +11,13 @@ class MergeRequest < ActiveRecord::Base
belongs_to :merge_user, class_name: "User"
has_many :merge_request_diffs, dependent: :destroy
has_one :merge_request_diff
has_many :events, as: :target, dependent: :destroy
serialize :merge_params, Hash
after_update :update_merge_request_diff
after_create :ensure_merge_request_diff, unless: :importing?
after_update :reload_diff_if_branch_changed
delegate :commits, :real_size, to: :merge_request_diff, prefix: nil
......@@ -94,7 +96,6 @@ class MergeRequest < ActiveRecord::Base
validates :merge_user, presence: true, if: :merge_when_build_succeeds?
validate :validate_branches, unless: [:allow_broken, :importing?]
validate :validate_fork
validates_associated :merge_request_diff, on: :create, unless: [:allow_broken, :importing?]
scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) }
scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) }
......@@ -282,11 +283,11 @@ class MergeRequest < ActiveRecord::Base
end
end
def create_merge_request_diff
merge_request_diffs.create
def ensure_merge_request_diff
merge_request_diff || create_merge_request_diff
end
def update_merge_request_diff
def reload_diff_if_branch_changed
if source_branch_changed? || target_branch_changed?
reload_diff
end
......@@ -689,8 +690,4 @@ class MergeRequest < ActiveRecord::Base
def keep_around_commit
project.repository.keep_around(self.merge_commit_sha)
end
def merge_request_diff
merge_request_diffs.first
end
end
app/models/merge_request_diff.rb
......@@ -22,28 +22,28 @@ class MergeRequestDiff < ActiveRecord::Base
serialize :st_commits
serialize :st_diffs
validates :start_commit_sha, presence: true, unless: :importing?
validates :head_commit_sha, presence: true, unless: :importing?
# For compatibility with old MergeRequestDiff which
# does not store those variables in database
after_initialize :ensure_commits_sha, if: :persisted?
after_initialize :initialize_commits_sha, unless: :importing?
# All diff information is collected from repository after object is created.
# It allows you to override variables like head_commit_sha before getting diff.
after_create :save_git_content, unless: :importing?
after_save :keep_around_commits, unless: :importing?
# Those variables are used for collecting commits and diff from git repository.
# After object is created those sha are stored in the database.
# However some old MergeRequestDiff records don't
# have those variables in the database so we try to initialize it
def initialize_commits_sha
self.start_commit_sha ||= merge_request.target_branch_sha
self.head_commit_sha ||= persisted? ? last_commit.sha : merge_request.source_branch_sha
self.base_commit_sha ||= find_base_sha
end
# Collect information about commits and diff from repository
# and save it to the database as serialized data
def save_git_content
ensure_commits_sha
save_commits
reload_commits
save_diffs
keep_around_commits
end
def ensure_commits_sha
self.start_commit_sha ||= merge_request.target_branch_sha
self.head_commit_sha ||= last_commit.try(:sha) || merge_request.source_branch_sha
self.base_commit_sha ||= find_base_sha
end
def size
......@@ -52,14 +52,15 @@ class MergeRequestDiff < ActiveRecord::Base
def diffs(options={})
if options[:ignore_whitespace_change]
@diffs_no_whitespace ||= begin
compare = Gitlab::Git::Compare.new(
repository.raw_repository,
start_commit_sha,
head_commit_sha
)
compare.diffs(options)
end
@diffs_no_whitespace ||=
begin
compare = Gitlab::Git::Compare.new(
repository.raw_repository,
start_commit_sha,
head_commit_sha
)
compare.diffs(options)
end
else
@diffs ||= {}
@diffs[options] ||= load_diffs(st_diffs, options)
......@@ -70,6 +71,11 @@ class MergeRequestDiff < ActiveRecord::Base
@commits ||= load_commits(st_commits || [])
end
def reload_commits
@commits = nil
commits
end
def last_commit
commits.first
end
......@@ -192,7 +198,6 @@ class MergeRequestDiff < ActiveRecord::Base
new_attributes[:st_diffs] = new_diffs
update_columns_serialized(new_attributes)
keep_around_commits
end
def project
......@@ -207,6 +212,9 @@ class MergeRequestDiff < ActiveRecord::Base
return unless head_commit_sha && start_commit_sha
project.merge_base_commit(head_commit_sha, start_commit_sha).try(:sha)
rescue Rugged::OdbError
# In case head or start commit does not exist in the repository any more
nil
end
def utf8_st_diffs
......
app/services/merge_requests/create_service.rb
......@@ -16,7 +16,6 @@ module MergeRequests
merge_request.target_project ||= source_project
merge_request.author = current_user
merge_request.merge_params['force_remove_source_branch'] = force_remove_source_branch
merge_request.merge_request_diffs.build
if merge_request.save
merge_request.update_attributes(label_ids: label_params)
......
features/project/merge_requests.feature
......@@ -24,7 +24,7 @@ Feature: Project Merge Requests
Scenario: I should see target branch when it is different from default
Given project "Shop" have "Bug NS-06" open merge request
When I visit project "Shop" merge requests page
Then I should see "other_branch" branch
Then I should see "feature_conflict" branch
Scenario: I should not see the numbers of diverged commits if the branch is rebased on the target
Given project "Shop" have "Bug NS-07" open merge request with rebased branch
......
features/steps/project/merge_requests.rb
......@@ -56,8 +56,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
expect(find('.merge-request-info')).not_to have_content "master"
end
step 'I should see "other_branch" branch' do
expect(page).to have_content "other_branch"
step 'I should see "feature_conflict" branch' do
expect(page).to have_content "feature_conflict"
end
step 'I should see "Bug NS-04" in merge requests' do
......@@ -122,7 +122,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
source_project: project,
target_project: project,
source_branch: 'fix',
target_branch: 'other_branch',
target_branch: 'feature_conflict',
author: project.users.first,
description: "# Description header"
)
......
spec/factories/merge_requests.rb
......@@ -68,11 +68,5 @@ FactoryGirl.define do
factory :closed_merge_request, traits: [:closed]
factory :reopened_merge_request, traits: [:reopened]
factory :merge_request_with_diffs, traits: [:with_diffs]
after :create do |merge_request|
unless merge_request.merge_request_diff
merge_request.create_merge_request_diff
end
end
end
end
spec/models/merge_request_diff_spec.rb
require 'spec_helper'
describe MergeRequestDiff, models: true do
describe 'initialize new object' do
subject { build(:merge_request).merge_request_diffs.build }
it { expect(subject).to be_valid }
it { expect(subject.head_commit_sha).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
end
describe 'create new record' do
subject { create(:merge_request).merge_request_diff }
......@@ -17,6 +8,9 @@ describe MergeRequestDiff, models: true do
it { expect(subject).to be_persisted }
it { expect(subject.commits.count).to eq(5) }
it { expect(subject.diffs.count).to eq(8) }
it { expect(subject.head_commit_sha).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
end
describe '#diffs' do
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment