diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/assets/stylesheets/sections/merge_requests.scss | 13 | ||||
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 128 | ||||
| -rw-r--r-- | app/models/merge_request_diff.rb | 163 | ||||
| -rw-r--r-- | app/views/projects/commits/_inline_commit.html.haml | 6 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/show/_commits.html.haml | 13 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/show/_diffs.html.haml | 8 |
7 files changed, 194 insertions, 139 deletions
diff --git a/app/assets/stylesheets/sections/merge_requests.scss b/app/assets/stylesheets/sections/merge_requests.scss index d3462d6aaa7..4388da00735 100644 --- a/app/assets/stylesheets/sections/merge_requests.scss +++ b/app/assets/stylesheets/sections/merge_requests.scss @@ -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; - } - } -} diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7d56f091575..a00138fdb35 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -76,7 +76,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController @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 @@ -217,6 +216,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController # 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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index da7aebd944f..712d01626b5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -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 @@ -53,11 +58,8 @@ class MergeRequest < ActiveRecord::Base end state :opened - state :reopened - state :closed - state :merged end @@ -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 @@ -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 @@ -120,8 +117,7 @@ class MergeRequest < ActiveRecord::Base end def reload_code - self.reloaded_commits - self.reloaded_diffs + merge_request_diff.reload_content if opened? end def check_if_can_be_merged @@ -132,42 +128,6 @@ class MergeRequest < ActiveRecord::Base 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 @@ -176,46 +136,13 @@ class MergeRequest < ActiveRecord::Base 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 @@ -225,7 +152,10 @@ class MergeRequest < ActiveRecord::Base 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, @@ -247,10 +177,6 @@ class MergeRequest < ActiveRecord::Base 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 @@ -327,34 +253,4 @@ class MergeRequest < ActiveRecord::Base 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 diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb new file mode 100644 index 00000000000..3ea610197e6 --- /dev/null +++ b/app/models/merge_request_diff.rb @@ -0,0 +1,163 @@ +require Rails.root.join("app/models/commit") + +class MergeRequestDiff < ActiveRecord::Base + # Prevent store of diff + # if commits amount more then 200 + COMMITS_SAFE_SIZE = 200 + + attr_reader :commits, :diffs + + belongs_to :merge_request + + attr_accessible :state, :st_commits, :st_diffs + + delegate :target_branch, :source_branch, to: :merge_request, prefix: nil + + state_machine :state, initial: :empty do + state :collected + state :timeout + state :overflow_commits_safe_size + state :overflow_diff_files_limit + state :overflow_diff_lines_limit + end + + serialize :st_commits + serialize :st_diffs + + after_create :reload_content + + def reload_content + reload_commits + reload_diffs + end + + def diffs + @diffs ||= (load_diffs(st_diffs) || []) + end + + def commits + @commits ||= load_commits(st_commits || []) + end + + def last_commit + commits.first + end + + def last_commit_short_sha + @last_commit_short_sha ||= last_commit.sha[0..10] + 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.respond_to?(:map) + diffs.map(&:to_hash) + end + end + + def load_diffs(raw) + if raw.respond_to?(:map) + raw.map { |hash| Gitlab::Git::Diff.new(hash) } + end + end + + # When Git::Diff is not able to get diff + # because of git timeout it return this value + def broken_diffs + [Gitlab::Git::Diff::BROKEN_DIFF] + end + + # Collect array of Git::Commit objects + # between target and source branches + def unmerged_commits + commits = if merge_request.for_fork? + Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between + else + repository.commits_between(target_branch, source_branch) + end + + if commits.present? + commits = Commit.decorate(commits). + sort_by(&:created_at). + reverse + end + + commits + end + + # Reload all commits related to current merge request from repo + # and save it as array of hashes in st_commits db field + def reload_commits + commit_objects = unmerged_commits + + if commit_objects.present? + self.st_commits = dump_commits(commit_objects) + end + + save + end + + # Reload diffs between branches related to current merge request from repo + # and save it as array of hashes in st_diffs db field + def reload_diffs + new_diffs = [] + + if commits.size.zero? + self.state = :empty + elsif commits.size > COMMITS_SAFE_SIZE + self.state = :overflow_commits_safe_size + else + new_diffs = unmerged_diffs + end + + if new_diffs.any? + if new_diffs.size > Commit::DIFF_HARD_LIMIT_FILES + self.state = :overflow_diff_files_limit + new_diffs = [] + end + + if new_diffs.sum { |diff| diff.diff.lines.count } > Commit::DIFF_HARD_LIMIT_LINES + self.state = :overflow_diff_lines_limit + new_diffs = [] + end + end + + if new_diffs.present? + new_diffs = dump_commits(new_diffs) + self.state = :collected + end + + self.st_diffs = new_diffs + self.save + end + + # Collect array of Git::Diff objects + # between target and source branches + def unmerged_diffs + diffs = if merge_request.for_fork? + Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite + else + Gitlab::Git::Diff.between(repository, source_branch, target_branch) + end + + if diffs == broken_diffs + self.state = :timeout + diffs = [] + end + + diffs ||= [] + diffs + end + + def repository + merge_request.target_project.repository + end +end diff --git a/app/views/projects/commits/_inline_commit.html.haml b/app/views/projects/commits/_inline_commit.html.haml index f5863463fee..b36369b4285 100644 --- a/app/views/projects/commits/_inline_commit.html.haml +++ b/app/views/projects/commits/_inline_commit.html.haml @@ -2,5 +2,7 @@ .commit-row-title = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" - = link_to_gfm truncate(commit.title, length: 40), project_commit_path(project, commit.id), class: "commit-row-message" - #{time_ago_with_tooltip(commit.committed_date)} + %span.str-truncated + = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" + .pull-right + #{time_ago_with_tooltip(commit.committed_date)} diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 7b0e67053a5..8ca1326c96a 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -12,9 +12,16 @@ 8 of #{@commits.count} commits displayed. %strong %a.show-all-commits Click here to show all - %ul.all-commits.hide.well-list - - @commits.each do |commit| - = render "projects/commits/commit", commit: commit, project: @merge_request.source_project + - if @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE + %ul.all-commits.hide.well-list + - @commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE).each do |commit| + = render "projects/commits/inline_commit", commit: commit, project: @merge_request.source_project + %li + other #{@commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE} commits hidden top prevent performance issues. + - else + %ul.all-commits.hide.well-list + - @commits.each do |commit| + = render "projects/commits/inline_commit", commit: commit, project: @merge_request.source_project - else %ul.well-list diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index 25f63804858..2c7507cfb8b 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,10 +1,10 @@ -- if @merge_request.valid_diffs? +- if @merge_request_diff.collected? = render "projects/commits/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project -- elsif @merge_request.broken_diffs? +- elsif @merge_request_diff.empty? + %h4.nothing_here_message Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} +- else %h4.nothing_here_message Can't load diff. You can = link_to "download it", project_merge_request_path(@merge_request.source_project, @merge_request), format: :diff, class: "vlink" instead. -- else - %h4.nothing_here_message Nothing to merge |