Commit a6c8b8ac authored by David Kim's avatar David Kim Committed by Imre Farkas

Load commit diff_files asynchronously

This is an attempt to improve the commit page load performance.
This improve the user interaction timing, but it does not load
diff_files in batches yet.
parent 64788bd0
......@@ -7,23 +7,47 @@ import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation';
import MiniPipelineGraph from '~/mini_pipeline_graph_dropdown';
import initNotes from '~/init_notes';
import initChangesDropdown from '~/init_changes_dropdown';
import initDiffNotes from '~/diff_notes/diff_notes_bundle';
import { fetchCommitMergeRequests } from '~/commit_merge_requests';
import '~/sourcegraph/load';
import { handleLocationHash } from '~/lib/utils/common_utils';
import axios from '~/lib/utils/axios_utils';
import syntaxHighlight from '~/syntax_highlight';
import flash from '~/flash';
import { __ } from '~/locale';
document.addEventListener('DOMContentLoaded', () => {
const hasPerfBar = document.querySelector('.with-performance-bar');
const performanceHeight = hasPerfBar ? 35 : 0;
new Diff();
new ZenMode();
new ShortcutsNavigation();
new MiniPipelineGraph({
container: '.js-commit-pipeline-graph',
}).bindEvents();
initNotes();
initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight + performanceHeight);
// eslint-disable-next-line no-jquery/no-load
$('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath);
fetchCommitMergeRequests();
initDiffNotes();
const filesContainer = $('.js-diffs-batch');
const initAfterPageLoad = () => {
new Diff();
new ZenMode();
new ShortcutsNavigation();
new MiniPipelineGraph({
container: '.js-commit-pipeline-graph',
}).bindEvents();
initNotes();
initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight + performanceHeight);
// eslint-disable-next-line no-jquery/no-load
$('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath);
fetchCommitMergeRequests();
};
if (filesContainer.length) {
const batchPath = filesContainer.data('diffFilesPath');
axios
.get(batchPath)
.then(({ data }) => {
filesContainer.html($(data.html));
syntaxHighlight(filesContainer);
handleLocationHash();
initAfterPageLoad();
})
.catch(() => {
flash(__('An error occurred while retrieving diff files'));
});
} else {
initAfterPageLoad();
}
});
......@@ -15,8 +15,8 @@ class Projects::CommitController < Projects::ApplicationController
before_action :authorize_download_code!
before_action :authorize_read_pipeline!, only: [:pipelines]
before_action :commit
before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines, :merge_requests]
before_action :define_note_vars, only: [:show, :diff_for_path]
before_action :define_commit_vars, only: [:show, :diff_for_path, :diff_files, :pipelines, :merge_requests]
before_action :define_note_vars, only: [:show, :diff_for_path, :diff_files]
before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
BRANCH_SEARCH_LIMIT = 1000
......@@ -41,6 +41,10 @@ class Projects::CommitController < Projects::ApplicationController
render_diff_for_path(@commit.diffs(diff_options))
end
def diff_files
render json: { html: view_to_html_string('projects/commit/diff_files', diffs: @diffs, environment: @environment) }
end
# rubocop: disable CodeReuse/ActiveRecord
def pipelines
@pipelines = @commit.pipelines.order(id: :desc)
......
- diff_files = diffs.diff_files
= render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment, diff_page_context: 'is-commit' }
......@@ -3,6 +3,7 @@
- can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project)
- diff_files = diffs.diff_files
- diff_page_context = local_assigns.fetch(:diff_page_context, nil)
- load_diff_files_async = Feature.enabled?(:async_commit_diff_files, @project) && diff_page_context == "is-commit"
.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed
.files-changed-inner
......@@ -26,5 +27,12 @@
- if render_overflow_warning?(diffs)
= render 'projects/diffs/warning', diff_files: diffs
.files{ data: { can_create_note: can_create_note } }
= render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment, diff_page_context: diff_page_context }
- if load_diff_files_async
- url = url_for(safe_params.merge(action: 'diff_files'))
.js-diffs-batch{ data: { diff_files_path: url } }
.text-center
%span.spinner.spinner-md
- else
= render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment, diff_page_context: diff_page_context }
......@@ -17,6 +17,7 @@ resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
post :revert
post :cherry_pick
get :diff_for_path
get :diff_files
get :merge_requests
end
end
......
......@@ -2786,6 +2786,9 @@ msgstr ""
msgid "An error occurred while retrieving diff"
msgstr ""
msgid "An error occurred while retrieving diff files"
msgstr ""
msgid "An error occurred while saving LDAP override status. Please try again."
msgstr ""
......
......@@ -737,6 +737,7 @@ RSpec.describe 'Copy as GFM', :js do
context 'inline diff' do
before do
visit project_commit_path(project, sample_commit.id, view: 'inline')
wait_for_requests
end
it_behaves_like 'copying code from a diff'
......@@ -745,6 +746,7 @@ RSpec.describe 'Copy as GFM', :js do
context 'parallel diff' do
before do
visit project_commit_path(project, sample_commit.id, view: 'parallel')
wait_for_requests
end
it_behaves_like 'copying code from a diff'
......
......@@ -8,30 +8,35 @@ RSpec.describe 'Commit diff', :js do
let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) }
before do
project.add_maintainer(user)
sign_in user
using RSpec::Parameterized::TableSyntax
where(:view, :async_diff_file_loading) do
'inline' | true
'inline' | false
'parallel' | true
'parallel' | false
end
%w(inline parallel).each do |view|
context "#{view} view" do
before do
visit project_commit_path(project, sample_commit.id, view: view)
end
with_them do
before do
stub_feature_flags(async_commit_diff_files: async_diff_file_loading)
project.add_maintainer(user)
sign_in user
visit project_commit_path(project, sample_commit.id, view: view)
end
it "adds comment to diff" do
diff_line_num = first('.diff-line-num.new')
it "adds comment to diff" do
diff_line_num = first('.diff-line-num.new')
diff_line_num.hover
diff_line_num.find('.js-add-diff-note-button').click
diff_line_num.hover
diff_line_num.find('.js-add-diff-note-button').click
page.within(first('.diff-viewer')) do
find('.js-note-text').set 'test comment'
page.within(first('.diff-viewer')) do
find('.js-note-text').set 'test comment'
click_button 'Comment'
click_button 'Comment'
expect(page).to have_content('test comment')
end
expect(page).to have_content('test comment')
end
end
end
......
......@@ -30,7 +30,7 @@ RSpec.describe 'Project > Commit > View user status' do
end
end
describe 'status for a diff note on the commit' do
describe 'status for a diff note on the commit', :js do
let(:note) { create(:diff_note_on_commit, project: project) }
it_behaves_like 'showing user status' do
......
......@@ -41,7 +41,7 @@ RSpec.describe 'User browses commits' do
.and have_selector('ul.breadcrumb a', count: 4)
end
it 'renders diff links to both the previous and current image' do
it 'renders diff links to both the previous and current image', :js do
visit project_commit_path(project, sample_image_commit.id)
links = page.all('.file-actions a')
......
......@@ -14,6 +14,12 @@ RSpec.describe 'projects/commit/show.html.haml' do
assign(:notes, [])
assign(:diffs, commit.diffs)
controller.params[:controller] = 'projects/commit'
controller.params[:action] = 'show'
controller.params[:namespace_id] = project.namespace.to_param
controller.params[:project_id] = project.to_param
controller.params[:id] = commit.id
allow(view).to receive(:current_user).and_return(nil)
allow(view).to receive(:can?).and_return(false)
allow(view).to receive(:can_collaborate_with_project?).and_return(false)
......
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