Commit 53b18d2d authored by Robert May's avatar Robert May Committed by Dylan Griffith

Paginate the single commit view [RUN ALL RSPEC] [RUN AS-IF-FOSS]

parent 29ec9856
......@@ -23,6 +23,7 @@ class Projects::CommitController < Projects::ApplicationController
end
BRANCH_SEARCH_LIMIT = 1000
COMMIT_DIFFS_PER_PAGE = 75
feature_category :source_code_management
......
......@@ -126,6 +126,14 @@ module CommitsHelper
%w(btn gpg-status-box) + Array(additional_classes)
end
def conditionally_paginate_diff_files(diffs, paginate:, per: Projects::CommitController::COMMIT_DIFFS_PER_PAGE)
if paginate && Feature.enabled?(:paginate_commit_view, @project, type: :development)
Kaminari.paginate_array(diffs.diff_files.to_a).page(params[:page]).per(per)
else
diffs.diff_files
end
end
protected
# Private: Returns a link to a person. If the person has a matching user and
......
......@@ -12,7 +12,7 @@
.container-fluid{ class: [limited_container_width, container_class] }
= render "commit_box"
= render "ci_menu"
= render "projects/diffs/diffs", diffs: @diffs, environment: @environment, diff_page_context: "is-commit"
= render "projects/diffs/diffs", diffs: @diffs, environment: @environment, diff_page_context: "is-commit", paginate_diffs: true
.limited-width-notes
= render "shared/notes/notes_with_form", :autocomplete => true
......
- environment = local_assigns.fetch(:environment, nil)
- show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true)
- 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"
- paginate_diffs = local_assigns.fetch(:paginate_diffs, false) && !load_diff_files_async && Feature.enabled?(:paginate_commit_view, @project, type: :development)
- diff_files = conditionally_paginate_diff_files(diffs, paginate: paginate_diffs)
.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed
.files-changed-inner
......@@ -27,7 +28,6 @@
- if render_overflow_warning?(diffs)
= render 'projects/diffs/warning', diff_files: diffs
.files{ data: { can_create_note: can_create_note } }
- if load_diff_files_async
- url = url_for(safe_params.merge(action: 'diff_files'))
......@@ -36,3 +36,6 @@
%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 }
- if paginate_diffs
= paginate(diff_files, theme: "gitlab")
---
title: Paginate single commit view
merge_request: 52819
author:
type: performance
---
name: paginate_commit_view
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52819
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300540
milestone: '13.9'
type: development
group: group::source code
default_enabled: false
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Commit' do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:user) { create(:user) }
describe "single commit view" do
let(:commit) do
project.repository.commits(nil, limit: 100).find do |commit|
commit.diffs.size > 1
end
end
let(:files) { commit.diffs.diff_files.to_a }
before do
stub_feature_flags(async_commit_diff_files: false)
project.add_maintainer(user)
sign_in(user)
end
describe "commit details" do
before do
visit project_commit_path(project, commit)
end
it "shows the short commit message" do
expect(page).to have_content(commit.title)
end
it "reports the correct number of total changes" do
expect(page).to have_content("Changes #{commit.diffs.size}")
end
end
context "pagination enabled" do
before do
stub_feature_flags(paginate_commit_view: true)
stub_const("Projects::CommitController::COMMIT_DIFFS_PER_PAGE", 1)
visit project_commit_path(project, commit)
end
it "shows an adjusted count for changed files on this page" do
expect(page).to have_content("Showing 1 changed file")
end
it "shows only the first diff on the first page" do
expect(page).to have_selector(".files ##{files[0].file_hash}")
expect(page).not_to have_selector(".files ##{files[1].file_hash}")
end
it "can navigate to the second page" do
within(".files .gl-pagination") do
click_on("2")
end
expect(page).not_to have_selector(".files ##{files[0].file_hash}")
expect(page).to have_selector(".files ##{files[1].file_hash}")
end
end
context "pagination disabled" do
before do
stub_feature_flags(paginate_commit_view: false)
visit project_commit_path(project, commit)
end
it "shows both diffs on the page" do
expect(page).to have_selector(".files ##{files[0].file_hash}")
expect(page).to have_selector(".files ##{files[1].file_hash}")
end
end
end
end
......@@ -176,4 +176,77 @@ RSpec.describe CommitsHelper do
expect(helper.commit_path(project, commit)).to eq(project_commit_path(project, commit))
end
end
describe "#conditionally_paginate_diff_files" do
let(:diffs_collection) { instance_double(Gitlab::Diff::FileCollection::Commit, diff_files: diff_files) }
let(:diff_files) { Gitlab::Git::DiffCollection.new(files) }
let(:page) { nil }
let(:files) do
Array.new(85).map do
{ too_large: false, diff: "" }
end
end
let(:params) do
{
page: page
}
end
subject { helper.conditionally_paginate_diff_files(diffs_collection, paginate: paginate) }
before do
allow(helper).to receive(:params).and_return(params)
end
context "pagination is enabled" do
let(:paginate) { true }
it "has been paginated" do
expect(subject).to be_an(Array)
end
it "can change the number of items per page" do
commits = helper.conditionally_paginate_diff_files(diffs_collection, paginate: paginate, per: 10)
expect(commits).to be_an(Array)
expect(commits.size).to eq(10)
end
context "page 1" do
let(:page) { 1 }
it "has 20 diffs" do
expect(subject.size).to eq(75)
end
end
context "page 2" do
let(:page) { 2 }
it "has the remaining 10 diffs" do
expect(subject.size).to eq(10)
end
end
end
context "pagination is disabled" do
let(:paginate) { false }
it "returns a standard DiffCollection" do
expect(subject).to be_a(Gitlab::Git::DiffCollection)
end
end
context "feature flag is disabled" do
let(:paginate) { true }
it "returns a standard DiffCollection" do
stub_feature_flags(paginate_commit_view: false)
expect(subject).to be_a(Gitlab::Git::DiffCollection)
end
end
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