Commit 025a4ba7 authored by Abhishek Kumar's avatar Abhishek Kumar

Add 'previously merged commits' compare dropdown

parent d3e70a87
<script> <script>
import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; import { GlDropdown, GlDropdownItem, GlDropdownDivider } from '@gitlab/ui';
import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
export default { export default {
components: { components: {
GlDropdown, GlDropdown,
GlDropdownItem, GlDropdownItem,
GlDropdownDivider,
TimeAgo, TimeAgo,
}, },
props: { props: {
...@@ -24,8 +25,9 @@ export default { ...@@ -24,8 +25,9 @@ export default {
<template> <template>
<gl-dropdown :text="selectedVersionName" data-qa-selector="dropdown_content"> <gl-dropdown :text="selectedVersionName" data-qa-selector="dropdown_content">
<template v-for="version in versions">
<gl-dropdown-divider v-if="version.addDivider" :key="version.id" />
<gl-dropdown-item <gl-dropdown-item
v-for="version in versions"
:key="version.id" :key="version.id"
:class="{ :class="{
'is-active': version.selected, 'is-active': version.selected,
...@@ -53,5 +55,6 @@ export default { ...@@ -53,5 +55,6 @@ export default {
</small> </small>
</div> </div>
</gl-dropdown-item> </gl-dropdown-item>
</template>
</gl-dropdown> </gl-dropdown>
</template> </template>
...@@ -7,6 +7,10 @@ export const selectedTargetIndex = (state) => ...@@ -7,6 +7,10 @@ export const selectedTargetIndex = (state) =>
export const selectedSourceIndex = (state) => state.mergeRequestDiff.version_index; export const selectedSourceIndex = (state) => state.mergeRequestDiff.version_index;
export const selectedContextCommitsDiffs = (state) =>
state.contextCommitsDiff && state.contextCommitsDiff.showing_context_commits_diff;
export const diffCompareDropdownTargetVersions = (state, getters) => { export const diffCompareDropdownTargetVersions = (state, getters) => {
// startVersion only exists if the user has selected a version other // startVersion only exists if the user has selected a version other
// than "base" so if startVersion is null then base must be selected // than "base" so if startVersion is null then base must be selected
...@@ -58,7 +62,7 @@ export const diffCompareDropdownTargetVersions = (state, getters) => { ...@@ -58,7 +62,7 @@ export const diffCompareDropdownTargetVersions = (state, getters) => {
export const diffCompareDropdownSourceVersions = (state, getters) => { export const diffCompareDropdownSourceVersions = (state, getters) => {
// Appended properties here are to make the compare_dropdown_layout easier to reason about // Appended properties here are to make the compare_dropdown_layout easier to reason about
return state.mergeRequestDiffs.map((v, i) => { const versions = state.mergeRequestDiffs.map((v, i) => {
const isLatestVersion = i === 0; const isLatestVersion = i === 0;
return { return {
...@@ -69,7 +73,19 @@ export const diffCompareDropdownSourceVersions = (state, getters) => { ...@@ -69,7 +73,19 @@ export const diffCompareDropdownSourceVersions = (state, getters) => {
versionName: isLatestVersion versionName: isLatestVersion
? __('latest version') ? __('latest version')
: sprintf(__(`version %{versionIndex}`), { versionIndex: v.version_index }), : sprintf(__(`version %{versionIndex}`), { versionIndex: v.version_index }),
selected: v.version_index === getters.selectedSourceIndex, selected: v.version_index === getters.selectedSourceIndex && !getters.selectedContextCommitsDiffs,
}; };
}); });
const { contextCommitsDiff } = state;
if (contextCommitsDiff) {
versions.push({
href: contextCommitsDiff.diffs_path,
commitsText: n__(`%d commit`, `%d commits`, contextCommitsDiff.commits_count),
versionName: __('previously merged commits'),
selected: getters.selectedContextCommitsDiffs,
addDivider: state.mergeRequestDiffs.length > 0,
});
}
return versions;
}; };
...@@ -47,7 +47,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -47,7 +47,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
diffs = @compare.diffs(diff_options) diffs = @compare.diffs(diff_options)
render json: DiffsMetadataSerializer.new(project: @merge_request.project, current_user: current_user) render json: DiffsMetadataSerializer.new(project: @merge_request.project, current_user: current_user)
.represent(diffs, additional_attributes) .represent(diffs, additional_attributes.merge(only_context_commits: show_only_context_commits?))
end end
private private
...@@ -92,7 +92,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -92,7 +92,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def commit def commit
return unless commit_id = params[:commit_id].presence return unless commit_id = params[:commit_id].presence
return unless @merge_request.all_commits.exists?(sha: commit_id) return unless @merge_request.all_commits.exists?(sha: commit_id) || @merge_request.recent_context_commits.map(&:id).include?(commit_id)
@commit ||= @project.commit(commit_id) @commit ||= @project.commit(commit_id)
end end
...@@ -122,6 +122,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -122,6 +122,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
end end
end end
return @merge_request.context_commits_diff if show_only_context_commits? && !@merge_request.context_commits_diff.empty?
return @merge_request.merge_head_diff if render_merge_ref_head_diff? return @merge_request.merge_head_diff if render_merge_ref_head_diff?
if @start_sha if @start_sha
......
...@@ -114,6 +114,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -114,6 +114,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@noteable = @merge_request @noteable = @merge_request
@commits_count = @merge_request.commits_count + @merge_request.context_commits_count @commits_count = @merge_request.commits_count + @merge_request.context_commits_count
@diffs_count = get_diffs_count
@issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar') @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar')
@current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestCurrentUserEntity).to_json @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestCurrentUserEntity).to_json
@show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs
...@@ -386,6 +387,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -386,6 +387,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
private private
def get_diffs_count
if show_only_context_commits?
@merge_request.context_commits_diff.raw_diffs.size
else
@merge_request.diff_size
end
end
def merge_request_update_params def merge_request_update_params
merge_request_params.merge!(params.permit(:merge_request_diff_head_sha)) merge_request_params.merge!(params.permit(:merge_request_diff_head_sha))
end end
......
...@@ -23,14 +23,16 @@ module DiffHelper ...@@ -23,14 +23,16 @@ module DiffHelper
end end
end end
def show_only_context_commits?
!!params[:only_context_commits] || @merge_request&.commits&.empty?
end
def diff_options def diff_options
options = { ignore_whitespace_change: hide_whitespace?, expanded: diffs_expanded? } options = { ignore_whitespace_change: hide_whitespace?, expanded: diffs_expanded? }
if action_name == 'diff_for_path' if action_name == 'diff_for_path'
options[:expanded] = true options[:expanded] = true
options[:paths] = params.values_at(:old_path, :new_path) options[:paths] = params.values_at(:old_path, :new_path)
elsif action_name == 'show'
options[:include_context_commits] = true unless @project.context_commits_enabled?
end end
options options
......
# frozen_string_literal: true
class ContextCommitsDiff
include ActsAsPaginatedDiff
attr_reader :merge_request
def initialize(merge_request)
@merge_request = merge_request
end
def empty?
commits.empty?
end
def commits_count
merge_request.context_commits_count
end
def diffs(diff_options = nil)
Gitlab::Diff::FileCollection::Compare.new(
self,
project: merge_request.project,
diff_options: diff_options,
diff_refs: diff_refs
)
end
def raw_diffs(options = {})
compare.diffs(options.merge(paths: paths))
end
def diff_refs
Gitlab::Diff::DiffRefs.new(
base_sha: commits.last.diff_refs.base_sha,
head_sha: commits.first.diff_refs.head_sha
)
end
private
def compare
@compare ||=
Gitlab::Git::Compare.new(
merge_request.project.repository.raw_repository,
commits.last.diff_refs.base_sha,
commits.first.diff_refs.head_sha
)
end
def commits
@commits ||= merge_request.project.repository.commits_by(oids: merge_request.recent_context_commits.map(&:id))
end
def paths
merge_request.merge_request_context_commit_diff_files.map(&:path)
end
end
\ No newline at end of file
...@@ -1899,6 +1899,11 @@ class MergeRequest < ApplicationRecord ...@@ -1899,6 +1899,11 @@ class MergeRequest < ApplicationRecord
diff_stats.map(&:path).include?(project.ci_config_path_or_default) diff_stats.map(&:path).include?(project.ci_config_path_or_default)
end end
def context_commits_diff
strong_memoize(:context_commits_diff) do
ContextCommitsDiff.new(self)
end
end
private private
def missing_report_error(report_type) def missing_report_error(report_type)
......
...@@ -16,4 +16,8 @@ class MergeRequestContextCommitDiffFile < ApplicationRecord ...@@ -16,4 +16,8 @@ class MergeRequestContextCommitDiffFile < ApplicationRecord
def self.bulk_insert(*args) def self.bulk_insert(*args)
Gitlab::Database.bulk_insert('merge_request_context_commit_diff_files', *args) # rubocop:disable Gitlab/BulkInsert Gitlab::Database.bulk_insert('merge_request_context_commit_diff_files', *args) # rubocop:disable Gitlab/BulkInsert
end end
def path
new_path.presence || old_path
end
end end
...@@ -665,10 +665,6 @@ class MergeRequestDiff < ApplicationRecord ...@@ -665,10 +665,6 @@ class MergeRequestDiff < ApplicationRecord
opening_external_diff do opening_external_diff do
collection = merge_request_diff_files collection = merge_request_diff_files
if options[:include_context_commits]
collection += merge_request.merge_request_context_commit_diff_files
end
if paths = options[:paths] if paths = options[:paths]
collection = collection.where('old_path IN (?) OR new_path IN (?)', paths, paths) collection = collection.where('old_path IN (?) OR new_path IN (?)', paths, paths)
end end
......
# frozen_string_literal: true
class ContextCommitsDiffEntity < Grape::Entity
include Gitlab::Routing
expose :commits_count
expose :showing_context_commits_diff do |_, options|
options[:only_context_commits]
end
expose :diffs_path do |diff|
merge_request = diff.merge_request
project = merge_request.target_project
next unless project
diffs_project_merge_request_path(project, merge_request, only_context_commits: true)
end
end
\ No newline at end of file
...@@ -84,6 +84,15 @@ class DiffsEntity < Grape::Entity ...@@ -84,6 +84,15 @@ class DiffsEntity < Grape::Entity
project_blob_path(merge_request.project, merge_request.diff_head_sha) project_blob_path(merge_request.project, merge_request.diff_head_sha)
end end
expose :context_commits_diff, if: -> (_) { merge_request&.project&.context_commits_enabled? } do |diffs, options|
next unless merge_request.context_commits_diff.commits_count > 0
ContextCommitsDiffEntity.represent(
merge_request.context_commits_diff,
options
)
end
def merge_request def merge_request
options[:merge_request] options[:merge_request]
end end
......
...@@ -40,7 +40,7 @@ ...@@ -40,7 +40,7 @@
= render "projects/merge_requests/tabs/tab", name: "diffs", class: "diffs-tab", id: "diffs-tab", qa_selector: "diffs_tab" do = render "projects/merge_requests/tabs/tab", name: "diffs", class: "diffs-tab", id: "diffs-tab", qa_selector: "diffs_tab" do
= tab_link_for @merge_request, :diffs do = tab_link_for @merge_request, :diffs do
= _("Changes") = _("Changes")
%span.badge.badge-pill.gl-badge.badge-muted.sm= @merge_request.diff_size %span.badge.badge-pill.gl-badge.badge-muted.sm= @diffs_count
.d-flex.flex-wrap.align-items-center.justify-content-lg-end .d-flex.flex-wrap.align-items-center.justify-content-lg-end
#js-vue-discussion-counter #js-vue-discussion-counter
......
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