Commit 5f0535ac authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Improve merge request version feature

* Use version numbers instead of sha as more user-friendly
* Make it clear that we compare later version (left) with older one (right)
* Improve wording of version switch panel
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent ffdfdc27
......@@ -383,10 +383,6 @@
a.btn-link {
color: $gl-dark-link-color;
}
.compare-dots {
margin: 0 $btn-side-margin;
}
}
.merge-request-details {
......
......@@ -95,7 +95,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
if params[:start_sha].present?
@start_sha = params[:start_sha]
validate_start_sha
@start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
unless @start_version
render_404
end
end
respond_to do |format|
......@@ -554,10 +558,4 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@diffs = @merge_request_diff.diffs(diff_options)
end
def validate_start_sha
unless @comparable_diffs.map(&:head_commit_sha).include?(@start_sha)
render_404
end
end
end
......@@ -4,6 +4,6 @@ module GitHelper
end
def short_sha(text)
text[0...8]
Commit.truncate_sha(text)
end
end
......@@ -106,4 +106,8 @@ module MergeRequestsHelper
project.namespace, project, merge_request,
diff_id: merge_request_diff.id, start_sha: start_sha)
end
def version_index(merge_request_diff)
@merge_request_diffs.size - @merge_request_diffs.index(merge_request_diff)
end
end
- if @merge_request_diffs.size > 1
.mr-version-controls
Version
Changes between
%span.dropdown.inline.mr-version-dropdown
%a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} }
%strong.monospace<
%strong
- if @merge_request_diff.latest?
Latest:&nbsp;
#{short_sha(@merge_request_diff.head_commit_sha)}
latest version
- else
version #{version_index(@merge_request_diff)}
%span.caret
%ul.dropdown-menu.dropdown-menu-selectable
- @merge_request_diffs.each_with_index do |merge_request_diff, i|
- @merge_request_diffs.each do |merge_request_diff|
%li
= link_to merge_request_version_path(@project, @merge_request, merge_request_diff), class: ('is-active' if merge_request_diff == @merge_request_diff) do
%strong.monospace
- if i.zero?
Latest:
%strong
- if merge_request_diff.latest?
latest version
- else
#{@merge_request_diffs.size - i}.
#{short_sha(merge_request_diff.head_commit_sha)}
%br
version #{version_index(merge_request_diff)}
.monospace #{short_sha(merge_request_diff.head_commit_sha)}
%small
#{number_with_delimiter(merge_request_diff.commits.count)} #{'commit'.pluralize(merge_request_diff.commits.count)},
= time_ago_with_tooltip(merge_request_diff.created_at)
- if @merge_request_diff.base_commit_sha
%span.compare-dots ...
Compared with
&nbsp;and&nbsp;
%span.dropdown.inline.mr-version-compare-dropdown
%a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} }
%strong.monospace<
%strong
- if @start_sha
#{short_sha(@start_sha)}
version #{version_index(@start_version)}
- else
Base: #{short_sha(@merge_request_diff.base_commit_sha)}
#{@merge_request.target_branch}
%span.caret
%ul.dropdown-menu.dropdown-menu-selectable
- @comparable_diffs.each_with_index do |merge_request_diff, i|
- @comparable_diffs.each do |merge_request_diff|
%li
= link_to merge_request_version_path(@project, @merge_request, @merge_request_diff, merge_request_diff.head_commit_sha), class: ('is-active' if merge_request_diff.head_commit_sha == @start_sha) do
%strong.monospace
#{@comparable_diffs.size - i}. #{short_sha(merge_request_diff.head_commit_sha)}
%br
= link_to merge_request_version_path(@project, @merge_request, @merge_request_diff, merge_request_diff.head_commit_sha), class: ('is-active' if merge_request_diff == @start_version) do
%strong
- if merge_request_diff.latest?
latest version
- else
version #{version_index(merge_request_diff)}
.monospace #{short_sha(merge_request_diff.head_commit_sha)}
%small
= time_ago_with_tooltip(merge_request_diff.created_at)
%li
= link_to merge_request_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless @start_sha) do
%strong.monospace
Base: #{short_sha(@merge_request_diff.base_commit_sha)}
%strong
#{@merge_request.target_branch} (base)
.monospace #{short_sha(@merge_request_diff.base_commit_sha)}
- unless @merge_request_diff.latest? && !@start_sha
.prepend-top-10
......
......@@ -12,7 +12,7 @@ feature 'Merge Request versions', js: true, feature: true do
it 'show the latest version of the diff' do
page.within '.mr-version-dropdown' do
expect(page).to have_content 'Latest: 5937ac0a'
expect(page).to have_content 'latest version'
end
expect(page).to have_content '8 changed files'
......@@ -22,13 +22,13 @@ feature 'Merge Request versions', js: true, feature: true do
before do
page.within '.mr-version-dropdown' do
find('.btn-link').click
click_link '6f6d7e7e'
click_link 'version 1'
end
end
it 'should show older version' do
page.within '.mr-version-dropdown' do
expect(page).to have_content '6f6d7e7e'
expect(page).to have_content 'version 1'
end
expect(page).to have_content '5 changed files'
......@@ -43,13 +43,13 @@ feature 'Merge Request versions', js: true, feature: true do
before do
page.within '.mr-version-compare-dropdown' do
find('.btn-link').click
click_link '6f6d7e7e'
click_link 'version 1'
end
end
it 'should has correct value in the compare dropdown' do
page.within '.mr-version-compare-dropdown' do
expect(page).to have_content '6f6d7e7e'
expect(page).to have_content 'version 1'
end
end
......
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