diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-04-06 17:13:28 -0500 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-04-08 14:37:46 -0500 |
commit | 50eae640dbbfa42a42e8b6966bd739cfda3adabc (patch) | |
tree | 1471ce67569d038e64a7e6f616d0d24be945aab9 | |
parent | f47b737456f848784fddb5958c3fa781d2ede2f1 (diff) | |
download | gitlab-ce-50eae640dbbfa42a42e8b6966bd739cfda3adabc.tar.gz |
Fix specs and make tweaks
-rwxr-xr-x | app/controllers/projects/merge_requests_controller.rb | 65 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 5 | ||||
-rw-r--r-- | app/views/discussions/_discussion.html.haml | 9 | ||||
-rw-r--r-- | app/views/projects/merge_requests/show/_versions.html.haml | 2 | ||||
-rw-r--r-- | spec/features/merge_requests/merge_request_versions_spec.rb | 4 |
7 files changed, 50 insertions, 39 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index bba3e007610..c25d33e12cf 100755 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -100,39 +100,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController respond_to do |format| format.html { define_discussion_vars } format.json do - @merge_request_diff = - if params[:diff_id] - @merge_request.merge_request_diffs.viewable.find(params[:diff_id]) - else - @merge_request.merge_request_diff - end - + define_diff_vars define_diff_comment_vars - @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff - @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id } - - if params[:start_sha].present? - @start_sha = params[:start_sha] - @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha } - - unless @start_version - @start_sha = @merge_request_diff.head_commit_sha - @start_version = @merge_request_diff - end - end - @environment = @merge_request.environments_for(current_user).last - @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha - - @diffs = - if @start_sha - @merge_request_diff.compare_with(@start_sha).diffs(diff_options) - else - @merge_request_diff.diffs(diff_options) - end - render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } end end @@ -144,16 +116,18 @@ class Projects::MergeRequestsController < Projects::ApplicationController def diff_for_path if params[:id] merge_request + define_diff_vars define_diff_comment_vars else build_merge_request + @diffs = @merge_request.diffs(diff_options) @diff_notes_disabled = true @grouped_diff_discussions = {} end define_commit_vars - render_diff_for_path(@merge_request.diffs(diff_options)) + render_diff_for_path(@diffs) end def commits @@ -590,12 +564,43 @@ class Projects::MergeRequestsController < Projects::ApplicationController @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit end + def define_diff_vars + @merge_request_diff = + if params[:diff_id] + @merge_request.merge_request_diffs.viewable.find(params[:diff_id]) + else + @merge_request.merge_request_diff + end + + @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff + @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id } + + if params[:start_sha].present? + @start_sha = params[:start_sha] + @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha } + + unless @start_version + @start_sha = @merge_request_diff.head_commit_sha + @start_version = @merge_request_diff + end + end + + @diffs = + if @start_sha + @merge_request_diff.compare_with(@start_sha).diffs(diff_options) + else + @merge_request_diff.diffs(diff_options) + end + end + def define_diff_comment_vars @new_diff_note_attrs = { noteable_type: 'MergeRequest', noteable_id: @merge_request.id } + @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha + @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs? @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@merge_request_diff.diff_refs) diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index b244ae4fcbf..6f4ba79b80b 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -68,6 +68,8 @@ module NotesHelper elsif merge_request_diff = discussion.latest_merge_request_diff diff_id = merge_request_diff.id else + # If the discussion is not active, and we cannot find the latest + # merge request diff for this discussion, we return no path at all. return end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 95e41106b49..1e24c03e5b6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -369,7 +369,7 @@ class MergeRequest < ActiveRecord::Base def merge_request_diff_for(diff_refs) @merge_request_diffs_by_diff_refs ||= Hash.new do |h, diff_refs| - h[diff_refs] = merge_request_diffs.viewable.select_without_diff.with_diff_refs(diff_refs).take + h[diff_refs] = merge_request_diffs.viewable.select_without_diff.find_by_diff_refs(diff_refs) end @merge_request_diffs_by_diff_refs[diff_refs] diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index bf9289086f0..3a26b4ee401 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -26,12 +26,15 @@ class MergeRequestDiff < ActiveRecord::Base end scope :viewable, -> { without_state(:empty) } - scope :with_diff_refs, ->(diff_refs) { where(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha) } # 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? + def self.find_by_diff_refs(diff_refs) + where(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha) + end + def self.select_without_diff select(column_names - ['st_diffs']) end diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 6f74948f498..0ee27b6ff20 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -26,14 +26,15 @@ - commit = discussion.noteable - if commit commit - = link_to commit.short_id, discussion_diff_path(discussion), class: 'monospace' + = link_to commit.short_id, url, class: 'monospace' - else a deleted commit - else - - unless discussion.active? - a now outdated portion of = conditional_link_to url.present?, url do - the diff + - if discussion.active? + the diff + - else + an outdated diff = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago") = render "discussions/headline", discussion: discussion diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml index 47f45e8e061..9842f737ff0 100644 --- a/app/views/projects/merge_requests/show/_versions.html.haml +++ b/app/views/projects/merge_requests/show/_versions.html.haml @@ -80,7 +80,7 @@ - if @start_sha Comment creation is disabled because you're comparing two versions of this merge request. - else - Discussions on this old version of the merge request are displayed but comment creation has been disabled. + Discussions on this old version of the merge request are displayed but comment creation is disabled. .pull-right = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm' diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb index 04e85ed3f73..2577336bf0f 100644 --- a/spec/features/merge_requests/merge_request_versions_spec.rb +++ b/spec/features/merge_requests/merge_request_versions_spec.rb @@ -37,7 +37,7 @@ feature 'Merge Request versions', js: true, feature: true do end it 'show the message about disabled comments' do - expect(page).to have_content 'Comments are disabled' + expect(page).to have_content 'comment creation is disabled' end end @@ -66,7 +66,7 @@ feature 'Merge Request versions', js: true, feature: true do end it 'show the message about disabled comments' do - expect(page).to have_content 'Comments are disabled' + expect(page).to have_content 'Comment creation is disabled' end it 'show diff between new and old version' do |