From 1817f877e19628417dd8209f070386b111de59c4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 8 Apr 2017 14:58:08 -0500 Subject: [PATCH] Some code tweaks --- app/controllers/projects/merge_requests_controller.rb | 9 +++------ app/helpers/notes_helper.rb | 2 +- app/models/concerns/discussion_on_diff.rb | 8 -------- app/models/concerns/note_on_diff.rb | 4 ++++ app/models/legacy_diff_discussion.rb | 8 ++++++++ app/views/discussions/_discussion.html.haml | 1 + .../projects/merge_requests/show/_versions.html.haml | 5 +++-- .../merge_requests/merge_request_versions_spec.rb | 2 +- 8 files changed, 21 insertions(+), 18 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index c25d33e12cf..87d684e5c7a 100755 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -576,13 +576,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController @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 } + 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 + @start_sha = start_sha if @start_version end @diffs = diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index c831f89dc19..eab0738a368 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -76,7 +76,7 @@ module NotesHelper diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: diff_id, anchor: discussion.line_code) elsif discussion.for_commit? anchor = discussion.line_code if discussion.diff_discussion? - + namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor) end end diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb index 87db0c810c3..67b1cace3eb 100644 --- a/app/models/concerns/discussion_on_diff.rb +++ b/app/models/concerns/discussion_on_diff.rb @@ -5,8 +5,6 @@ module DiscussionOnDiff included do NUMBER_OF_TRUNCATED_DIFF_LINES = 16 - memoized_values << :active - delegate :line_code, :original_line_code, :diff_file, @@ -29,12 +27,6 @@ module DiscussionOnDiff true end - def active? - return @active if @active.present? - - @active = first_note.active? - end - # Returns an array of at most 16 highlighted lines above a diff note def truncated_diff_lines(highlight: true) lines = highlight ? highlighted_diff_lines : diff_lines diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb index ac4c3099c00..6c27dd5aa5c 100644 --- a/app/models/concerns/note_on_diff.rb +++ b/app/models/concerns/note_on_diff.rb @@ -26,6 +26,10 @@ module NoteOnDiff raise NotImplementedError end + def active?(diff_refs = nil) + raise NotImplementedError + end + private def noteable_diff_refs diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb index cb2651a03f8..e617ce36f56 100644 --- a/app/models/legacy_diff_discussion.rb +++ b/app/models/legacy_diff_discussion.rb @@ -7,6 +7,8 @@ class LegacyDiffDiscussion < Discussion include DiscussionOnDiff + memoized_values << :active + def legacy_diff_discussion? true end @@ -15,6 +17,12 @@ class LegacyDiffDiscussion < Discussion LegacyDiffNote end + def active?(*args) + return @active if @active.present? + + @active = first_note.active?(*args) + end + def collapsed? !active? end diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index f48dc575661..8440fb3d785 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -30,6 +30,7 @@ - else a deleted commit - elsif discussion.diff_discussion? + on = conditional_link_to url.present?, url do - if discussion.active? the diff diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml index 9842f737ff0..434d9b5837f 100644 --- a/app/views/projects/merge_requests/show/_versions.html.haml +++ b/app/views/projects/merge_requests/show/_versions.html.haml @@ -72,13 +72,14 @@ = link_to namespace_project_compare_path(@project.namespace, @project, from: @start_version.base_commit_sha, to: @merge_request_diff.base_commit_sha) do new commits from - %code= @merge_request.target_branch + = succeed '.' do + %code= @merge_request.target_branch - if @diff_notes_disabled .comments-disabled-notif.content-block = icon('info-circle') - if @start_sha - Comment creation is disabled because you're comparing two versions of this merge request. + Comments are 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 is disabled. diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb index 2577336bf0f..2f627004578 100644 --- a/spec/features/merge_requests/merge_request_versions_spec.rb +++ b/spec/features/merge_requests/merge_request_versions_spec.rb @@ -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 'Comment creation is disabled' + expect(page).to have_content 'Comments are disabled' end it 'show diff between new and old version' do