Commit 91344e30 authored by GitLab Bot's avatar GitLab Bot
Browse files

Add latest changes from gitlab-org/gitlab@master

parent dc922a323bbf
......@@ -167,7 +167,7 @@ dropdown-menu-labels dropdown-menu-selectable"
<div class="dropdown-page-one">
<dropdown-header v-if="showCreate" />
<dropdown-search-input />
<div class="dropdown-content"></div>
<div class="dropdown-content" data-qa-selector="labels_dropdown_content"></div>
<div class="dropdown-loading"><gl-loading-icon /></div>
<dropdown-footer
v-if="showCreate"
......
......@@ -14,7 +14,11 @@ export default {
{{ __('Labels') }}
<template v-if="canEdit">
<i aria-hidden="true" class="fa fa-spinner fa-spin block-loading" data-hidden="true"> </i>
<button type="button" class="edit-link btn btn-blank float-right js-sidebar-dropdown-toggle">
<button
type="button"
class="edit-link btn btn-blank float-right js-sidebar-dropdown-toggle"
data-qa-selector="labels_edit_button"
>
{{ __('Edit') }}
</button>
</template>
......
......@@ -14,7 +14,11 @@ def merge_request
end
def merge_request_includes(association)
association.includes(:metrics, :assignees, author: :status) # rubocop:disable CodeReuse/ActiveRecord
association.includes(preloadable_mr_relations) # rubocop:disable CodeReuse/ActiveRecord
end
def preloadable_mr_relations
[:metrics, :assignees, { author: :status }]
end
def merge_request_params
......
......@@ -5,9 +5,9 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
include RendersNotes
before_action :apply_diff_view_cookie!
before_action :commit
before_action :define_diff_vars
before_action :define_diff_comment_vars
before_action :commit, except: :diffs_batch
before_action :define_diff_vars, except: :diffs_batch
before_action :define_diff_comment_vars, except: :diffs_batch
def show
render_diffs
......@@ -17,8 +17,29 @@ def diff_for_path
render_diffs
end
def diffs_batch
return render_404 unless Feature.enabled?(:diffs_batch_load, @merge_request.project)
diffable = @merge_request.merge_request_diff
return render_404 unless diffable
diffs = diffable.diffs_in_batch(params[:page], params[:per_page], diff_options: diff_options)
options = {
merge_request: @merge_request,
pagination_data: diffs.pagination_data
}
render json: PaginatedDiffSerializer.new(current_user: current_user).represent(diffs, options)
end
private
def preloadable_mr_relations
[{ source_project: :namespace }, { target_project: :namespace }]
end
def render_diffs
@environment = @merge_request.environments_for(current_user).last
......
......@@ -17,6 +17,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
before_action :set_issuables_index, only: [:index]
before_action :authenticate_user!, only: [:assign_related_issues]
before_action :check_user_can_push_to_source_branch!, only: [:rebase]
before_action only: [:show] do
push_frontend_feature_flag(:diffs_batch_load, @project)
end
around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]
......
......@@ -297,6 +297,13 @@ def diff_refs_by_sha?
base_commit_sha? && head_commit_sha? && start_commit_sha?
end
def diffs_in_batch(batch_page, batch_size, diff_options:)
Gitlab::Diff::FileCollection::MergeRequestDiffBatch.new(self,
batch_page,
batch_size,
diff_options: diff_options)
end
def diffs(diff_options = nil)
if without_files? && comparison = diff_refs&.compare_in(project)
# It should fetch the repository when diffs are cleaned by the system.
......
......@@ -5,6 +5,7 @@ class MergeRequestDiffFile < ApplicationRecord
include DiffFile
belongs_to :merge_request_diff, inverse_of: :merge_request_diff_files
alias_attribute :index, :relative_order
def utf8_diff
return '' if diff.blank?
......
# frozen_string_literal: true
# Serializes diffs with pagination data.
#
# Avoid adding more keys to this serializer as processing the
# diff file serialization is not cheap.
#
class PaginatedDiffEntity < Grape::Entity
include RequestAwareEntity
expose :diff_files do |diffs, options|
submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
DiffFileEntity.represent(diffs.diff_files, options.merge(submodule_links: submodule_links))
end
expose :pagination do
expose :current_page
expose :next_page
expose :total_pages
expose :next_page_href do |diffs|
next unless next_page
project = merge_request.target_project
diffs_batch_namespace_project_json_merge_request_path(
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: merge_request.iid,
page: next_page,
format: :json
)
end
end
private
%i[current_page next_page total_pages].each do |method|
define_method method do
pagination_data[method]
end
end
def pagination_data
options.fetch(:pagination_data, {})
end
def merge_request
options[:merge_request]
end
end
# frozen_string_literal: true
class PaginatedDiffSerializer < BaseSerializer
entity PaginatedDiffEntity
end
---
title: Introduce diffs_batch JSON endpoint for paginated diffs
merge_request: 17651
author:
type: added
......@@ -281,6 +281,7 @@
get :commits
get :pipelines
get :diffs, to: 'merge_requests/diffs#show'
get :diffs_batch, to: 'merge_requests/diffs#diffs_batch'
get :widget, to: 'merge_requests/content#widget'
get :cached_widget, to: 'merge_requests/content#cached_widget'
end
......
# frozen_string_literal: true
class CreateAnalyticsRepositoryFileCommits < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :analytics_repository_file_commits do |t|
t.references :analytics_repository_file, index: { name: 'index_analytics_repository_file_commits_file_id' }, foreign_key: { on_delete: :cascade }, null: false
t.references :project, index: false, foreign_key: { on_delete: :cascade }, null: false
t.date :committed_date, null: false
t.integer :commit_count, limit: 2, null: false
end
add_index :analytics_repository_file_commits,
[:project_id, :committed_date, :analytics_repository_file_id],
name: 'index_file_commits_on_committed_date_file_id_and_project_id',
unique: true
end
end
# frozen_string_literal: true
class DropUnusedAnalyticsRepositoryFileEdits < ActiveRecord::Migration[5.2]
DOWNTIME = false
def up
# The table was never used, there is no app code that writes or reads the table. Safe to remove.
drop_table :analytics_repository_file_edits
end
def down
create_table :analytics_repository_file_edits do |t|
t.references :project,
index: true,
foreign_key: { on_delete: :cascade }, null: false
t.references :analytics_repository_file,
index: false,
foreign_key: { on_delete: :cascade },
null: false
t.date :committed_date,
null: false
t.integer :num_edits,
null: false,
default: 0
end
add_index :analytics_repository_file_edits,
[:analytics_repository_file_id, :committed_date, :project_id],
name: 'index_file_edits_on_committed_date_file_id_and_project_id',
unique: true
end
end
......@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_09_30_025655) do
ActiveRecord::Schema.define(version: 2019_10_04_134055) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
......@@ -93,13 +93,13 @@
t.index ["project_id"], name: "analytics_repository_languages_on_project_id"
end
create_table "analytics_repository_file_edits", force: :cascade do |t|
t.bigint "project_id", null: false
create_table "analytics_repository_file_commits", force: :cascade do |t|
t.bigint "analytics_repository_file_id", null: false
t.bigint "project_id", null: false
t.date "committed_date", null: false
t.integer "num_edits", default: 0, null: false
t.index ["analytics_repository_file_id", "committed_date", "project_id"], name: "index_file_edits_on_committed_date_file_id_and_project_id", unique: true
t.index ["project_id"], name: "index_analytics_repository_file_edits_on_project_id"
t.integer "commit_count", limit: 2, null: false
t.index ["analytics_repository_file_id"], name: "index_analytics_repository_file_commits_file_id"
t.index ["project_id", "committed_date", "analytics_repository_file_id"], name: "index_file_commits_on_committed_date_file_id_and_project_id", unique: true
end
create_table "analytics_repository_files", force: :cascade do |t|
......@@ -3894,8 +3894,8 @@
add_foreign_key "analytics_cycle_analytics_project_stages", "projects", on_delete: :cascade
add_foreign_key "analytics_language_trend_repository_languages", "programming_languages", on_delete: :cascade
add_foreign_key "analytics_language_trend_repository_languages", "projects", on_delete: :cascade
add_foreign_key "analytics_repository_file_edits", "analytics_repository_files", on_delete: :cascade
add_foreign_key "analytics_repository_file_edits", "projects", on_delete: :cascade
add_foreign_key "analytics_repository_file_commits", "analytics_repository_files", on_delete: :cascade
add_foreign_key "analytics_repository_file_commits", "projects", on_delete: :cascade
add_foreign_key "analytics_repository_files", "projects", on_delete: :cascade
add_foreign_key "application_settings", "namespaces", column: "custom_project_templates_group_id", on_delete: :nullify
add_foreign_key "application_settings", "projects", column: "file_template_project_id", name: "fk_ec757bd087", on_delete: :nullify
......
......@@ -3,19 +3,7 @@
module Gitlab
module Diff
module FileCollection
class MergeRequestDiff < Base
extend ::Gitlab::Utils::Override
def initialize(merge_request_diff, diff_options:)
@merge_request_diff = merge_request_diff
super(merge_request_diff,
project: merge_request_diff.project,
diff_options: diff_options,
diff_refs: merge_request_diff.diff_refs,
fallback_diff_refs: merge_request_diff.fallback_diff_refs)
end
class MergeRequestDiff < MergeRequestDiffBase
def diff_files
diff_files = super
......
# frozen_string_literal: true
module Gitlab
module Diff
module FileCollection
class MergeRequestDiffBase < Base
extend ::Gitlab::Utils::Override
def initialize(merge_request_diff, diff_options:)
@merge_request_diff = merge_request_diff
super(merge_request_diff,
project: merge_request_diff.project,
diff_options: diff_options,
diff_refs: merge_request_diff.diff_refs,
fallback_diff_refs: merge_request_diff.fallback_diff_refs)
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Diff
module FileCollection
# Builds a paginated diff file collection and collects pagination
# metadata.
#
# It doesn't handle caching yet as we're not prepared to write/read
# separate file keys (https://gitlab.com/gitlab-org/gitlab/issues/30550).
#
class MergeRequestDiffBatch < MergeRequestDiffBase
DEFAULT_BATCH_PAGE = 1
DEFAULT_BATCH_SIZE = 20
attr_reader :pagination_data
def initialize(merge_request_diff, batch_page, batch_size, diff_options:)
super(merge_request_diff, diff_options: diff_options)
batch_page ||= DEFAULT_BATCH_PAGE
batch_size ||= DEFAULT_BATCH_SIZE
@paginated_collection = relation.page(batch_page).per(batch_size)
@pagination_data = {
current_page: @paginated_collection.current_page,
next_page: @paginated_collection.next_page,
total_pages: @paginated_collection.total_pages
}
end
override :diffs
def diffs
strong_memoize(:diffs) do
@merge_request_diff.opening_external_diff do
# Avoiding any extra queries.
collection = @paginated_collection.to_a
# The offset collection and calculation is required so that we
# know how much has been loaded in previous batches, collapsing
# the current paginated set accordingly (collection limit calculation).
# See: https://docs.gitlab.com/ee/development/diffs.html#diff-collection-limits
#
offset_index = collection.first&.index
options = diff_options.dup
collection =
if offset_index && offset_index > 0
offset_collection = relation.limit(offset_index) # rubocop:disable CodeReuse/ActiveRecord
options[:offset_index] = offset_index
offset_collection + collection
else
collection
end
Gitlab::Git::DiffCollection.new(collection.map(&:to_hash), options)
end
end
end
private
def relation
@merge_request_diff.merge_request_diff_files
end
end
end
end
end
......@@ -31,6 +31,7 @@ def initialize(iterator, options = {})
@limits = self.class.limits(options)
@enforce_limits = !!options.fetch(:limits, true)
@expanded = !!options.fetch(:expanded, true)
@offset_index = options.fetch(:offset_index, 0)
@line_count = 0
@byte_count = 0
......@@ -128,7 +129,7 @@ def each_gitaly_patch
def each_serialized_patch
i = @array.length
@iterator.each do |raw|
@iterator.each_with_index do |raw, iterator_index|
@empty = false
if @enforce_limits && i >= max_files
......@@ -154,8 +155,12 @@ def each_serialized_patch
break
end
yield @array[i] = diff
i += 1
# We should not yield / memoize diffs before the offset index. Though,
# we still consider the limit buffers for diffs before it.
if iterator_index >= @offset_index
yield @array[i] = diff
i += 1
end
end
end
end
......
......@@ -5,6 +5,49 @@
describe Projects::MergeRequests::DiffsController do
include ProjectForksHelper
shared_examples 'forked project with submodules' do
render_views
let(:project) { create(:project, :repository) }
let(:forked_project) { fork_project_with_submodules(project) }
let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) }
before do
project.add_developer(user)
merge_request.reload
go
end
it 'renders' do
expect(response).to be_successful
expect(response.body).to have_content('Subproject commit')
end
end
shared_examples 'persisted preferred diff view cookie' do
context 'with view param' do
before do
go(view: 'parallel')
end
it 'saves the preferred diff view in a cookie' do
expect(response.cookies['diff_view']).to eq('parallel')
end
end
context 'when the user cannot view the merge request' do
before do
project.team.truncate
go
end
it 'returns a 404' do
expect(response).to have_gitlab_http_status(404)
end
end
end
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
......@@ -51,36 +94,10 @@ def go(extra_params = {})
end
end
context 'with forked projects with submodules' do
render_views
let(:project) { create(:project, :repository) }
let(:forked_project) { fork_project_with_submodules(project) }
let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) }
before do
project.add_developer(user)
merge_request.reload
go
end
it 'renders' do
expect(response).to be_successful
expect(response.body).to have_content('Subproject commit')
end
end
it_behaves_like 'forked project with submodules'
end
context 'with view' do
before do
go(view: 'parallel')
end
it 'saves the preferred diff view in a cookie' do
expect(response.cookies['diff_view']).to eq('parallel')
end
end
it_behaves_like 'persisted preferred diff view cookie'
end
describe 'GET diff_for_path' do
......@@ -154,4 +171,92 @@ def diff_for_path(extra_params = {})
end
end
end
describe 'GET diffs_batch' do
def go(extra_params = {})
params = {
namespace_id: project.namespace.to_param,
project_id: project,
id: merge_request.iid,
format: 'json'
}
get :diffs_batch, params: params.merge(extra_params)
end
context 'when feature is disabled' do
before do
stub_feature_flags(diffs_batch_load: false)
end
it 'returns 404' do
go
expect(response).to have_gitlab_http_status(404)
end
end
context 'when not authorized' do
let(:other_user) { create(:user) }
before do
sign_in(other_user)
end
it 'returns 404' do
go
expect(response).to have_gitlab_http_status(404)
end
end
context 'with default params' do
let(:expected_options) do
{
merge_request: merge_request,
pagination_data: {
current_page: 1,
next_page: nil,
total_pages: 1
}
}
end
it 'serializes paginated merge request diff collection' do
expect_next_instance_of(PaginatedDiffSerializer) do |instance|
expect(instance).to receive(:represent)
.with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiffBatch), expected_options)
.and_call_original
end
go
end
end
context 'with smaller diff batch params' do
let(:expected_options) do
{
merge_request: merge_request,
pagination_data: {
current_page: 2,
next_page: 3,
total_pages: 4
}
}
end