Skip to content

Commit

Permalink
Merge branch 'improve/mr_diff'
Browse files Browse the repository at this point in the history 
Signed-off-by: Dmitriy Zaporozhets <[email protected]>

Conflicts:
	features/steps/project/project_fork.rb
	features/steps/project/project_forked_merge_requests.rb
	features/steps/project/project_issue_tracker.rb
	features/steps/project/project_markdown_render.rb
	features/steps/shared/project.rb
  • Loading branch information
@dzaporozhets
dzaporozhets committed Jan 23, 2014
2 parents 68590fd + 573f1f4 commit 28e1363
Show file tree
Hide file tree
Showing 61 changed files with 492 additions and 386 deletions.
13 changes: 0 additions & 13 deletions app/assets/stylesheets/sections/merge_requests.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,3 @@
.merge-request-form-info {
padding-top: 15px;
}

.merge-request-branches {
.commit-row-message {
font-weight: normal !important;
}

.select2-container .select2-single {
span {
font-weight: bold;
color: #555;
}
}
}
2 changes: 1 addition & 1 deletion app/controllers/projects/merge_requests_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ def create
@merge_request.author = current_user
@target_branches ||= []
if @merge_request.save
@merge_request.reload_code
redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.'
else
@source_project = @merge_request.source_project
Expand Down Expand Up @@ -217,6 +216,7 @@ def define_show_vars
# or from cache if already merged
@commits = @merge_request.commits

@merge_request_diff = @merge_request.merge_request_diff
@allowed_to_merge = allowed_to_merge?
@show_merge_controls = @merge_request.opened? && @commits.any? && @allowed_to_merge
end
Expand Down
128 changes: 12 additions & 116 deletions app/models/merge_request.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class MergeRequest < ActiveRecord::Base
belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"
belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"

has_one :merge_request_diff, dependent: :destroy
after_create :create_merge_request_diff

delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil

attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event, :description

attr_accessor :should_remove_source_branch
Expand All @@ -53,11 +58,8 @@ class MergeRequest < ActiveRecord::Base
end

state :opened

state :reopened

state :closed

state :merged
end

Expand All @@ -75,15 +77,10 @@ class MergeRequest < ActiveRecord::Base
end

state :unchecked

state :can_be_merged

state :cannot_be_merged
end

serialize :st_commits
serialize :st_diffs

validates :source_project, presence: true, unless: :allow_broken
validates :source_branch, presence: true
validates :target_project, presence: true
Expand All @@ -105,7 +102,7 @@ class MergeRequest < ActiveRecord::Base
scope :closed, -> { with_states(:closed, :merged) }

def validate_branches
if target_project==source_project && target_branch == source_branch
if target_project == source_project && target_branch == source_branch
errors.add :branch_conflict, "You can not use same project/branch for source and target"
end

Expand All @@ -120,8 +117,7 @@ def validate_branches
end

def reload_code
self.reloaded_commits
self.reloaded_diffs
merge_request_diff.reload_content if opened?
end

def check_if_can_be_merged
Expand All @@ -132,42 +128,6 @@ def check_if_can_be_merged
end
end

def diffs
@diffs ||= (load_diffs(st_diffs) || [])
end

def reloaded_diffs
if opened? && unmerged_diffs.any?
self.st_diffs = dump_diffs(unmerged_diffs)
self.save
end
end

def broken_diffs?
diffs == broken_diffs
rescue
true
end

def valid_diffs?
!broken_diffs?
end

def unmerged_diffs
diffs = if for_fork?
Gitlab::Satellite::MergeAction.new(author, self).diffs_between_satellite
else
Gitlab::Git::Diff.between(target_project.repository, source_branch, target_branch)
end

diffs ||= []
diffs
end

def last_commit
commits.first
end

def merge_event
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
end
Expand All @@ -176,46 +136,13 @@ def closed_event
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
end

def commits
load_commits(st_commits || [])
end

def probably_merged?
unmerged_commits.empty? &&
commits.any? && opened?
end

def reloaded_commits
if opened? && unmerged_commits.any?
self.st_commits = dump_commits(unmerged_commits)
save

end
commits
end

def unmerged_commits
if for_fork?
commits = Gitlab::Satellite::MergeAction.new(self.author, self).commits_between
else
commits = target_project.repository.commits_between(self.target_branch, self.source_branch)
end

if commits.present?
commits = Commit.decorate(commits).
sort_by(&:created_at).
reverse
end
commits
end

def merge!(user_id)
self.author_id_of_changes = user_id
self.merge
end

def automerge!(current_user, commit_message = nil)
if Gitlab::Satellite::MergeAction.new(current_user, self).merge!(commit_message) && self.unmerged_commits.empty?
if Gitlab::Satellite::MergeAction.new(current_user, self).merge!(commit_message)
self.merge!(current_user.id)
true
end
Expand All @@ -225,7 +152,10 @@ def automerge!(current_user, commit_message = nil)
end

def mr_and_commit_notes
commit_ids = commits.map(&:id)
# Fetch comments only from last 100 commits
commits_for_notes_limit = 100
commit_ids = commits.last(commits_for_notes_limit).map(&:id)

project.notes.where(
"(noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR (noteable_type = 'Commit' AND commit_id IN (:commit_ids))",
mr_id: id,
Expand All @@ -247,10 +177,6 @@ def to_patch(current_user)
Gitlab::Satellite::MergeAction.new(current_user, self).format_patch
end

def last_commit_short_sha
@last_commit_short_sha ||= last_commit.sha[0..10]
end

def for_fork?
target_project != source_project
end
Expand Down Expand Up @@ -327,34 +253,4 @@ def merge_commit_message
message << description.to_s
message
end

private

def dump_commits(commits)
commits.map(&:to_hash)
end

def load_commits(array)
array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash)) }
end

def dump_diffs(diffs)
if diffs == broken_diffs
broken_diffs
elsif diffs.respond_to?(:map)
diffs.map(&:to_hash)
end
end

def load_diffs(raw)
if raw == broken_diffs
broken_diffs
elsif raw.respond_to?(:map)
raw.map { |hash| Gitlab::Git::Diff.new(hash) }
end
end

def broken_diffs
[Gitlab::Git::Diff::BROKEN_DIFF]
end
end
Loading

0 comments on commit 28e1363

Please sign in to comment.