diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 90d823423f35fca5b220dd94ce60b0f9f732fad8..2b2764d2e349a9198ca6b91ceb6ed6ab2741fbb7 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -31,7 +31,7 @@ def show respond_to do |format| format.html do - render + render locals: { pagination_params: params.permit(:page) } end format.diff do send_git_diff(@project.repository, @commit.diff_refs) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 243cc7a346cc6b3680e52be3064c4430d6f50fe2..3ced5f21b24c78b0a96c53cb25a51bb0fee46c11 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -34,7 +34,7 @@ def index def show apply_diff_view_cookie! - render + render locals: { pagination_params: params.permit(:page) } end def diff_for_path diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index c78e906e05292357f289d8a6b50446d7fa0e752a..3c3179f6fbeaa6a96051789aae3c13034e423297 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -135,9 +135,9 @@ def commit_signature_badge_classes(additional_classes) %w(btn gpg-status-box) + Array(additional_classes) end - def conditionally_paginate_diff_files(diffs, paginate:, per:) + def conditionally_paginate_diff_files(diffs, paginate:, page:, per:) if paginate - Kaminari.paginate_array(diffs.diff_files.to_a).page(params[:page]).per(per) + Kaminari.paginate_array(diffs.diff_files.to_a).page(page).per(per) else diffs.diff_files end diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index e22d33e3c72980046a8394c2260126bbb5859a7c..c26f24dd52cecff98a3bdb1a71f7687173d2e7ab 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -16,6 +16,7 @@ diffs: @diffs, environment: @environment, diff_page_context: "is-commit", + page: pagination_params[:page], paginate_diffs: true, paginate_diffs_per_page: Projects::CommitController::COMMIT_DIFFS_PER_PAGE diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index 1fc067b6be1c5d6f9edf2f94db56db391239c9bc..cb2c2d488e81d75fe37b981636abb10fd6493482 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -13,6 +13,7 @@ diffs: @diffs, environment: @environment, diff_page_context: "is-compare", + page: pagination_params[:page], paginate_diffs: true, paginate_diffs_per_page: Projects::CompareController::COMMIT_DIFFS_PER_PAGE - else diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 7e3f9c7664e4850c757daca7876e2dd056d950e8..f504d049f23006d7d33dae80d8f8bb815a081b2f 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -5,7 +5,8 @@ - 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 - paginate_diffs_per_page = local_assigns.fetch(:paginate_diffs_per_page, nil) -- diff_files = conditionally_paginate_diff_files(diffs, paginate: paginate_diffs, per: paginate_diffs_per_page) +- page = local_assigns.fetch(:page, nil) +- diff_files = conditionally_paginate_diff_files(diffs, paginate: paginate_diffs, page: page, per: paginate_diffs_per_page) .content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed .files-changed-inner diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 509e10a10c72e464afc98fe3f0c7a52ffd79ad34..a72c98552a5b904387c09cf1d0b4371f07388b8f 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -60,6 +60,22 @@ def go(extra_params = {}) end end + context 'with valid page' do + it 'responds with 200' do + go(id: commit.id, page: 1) + + expect(response).to be_ok + end + end + + context 'with invalid page' do + it 'does not return an error' do + go(id: commit.id, page: ['invalid']) + + expect(response).to be_ok + end + end + it 'handles binary files' do go(id: TestEnv::BRANCH_SHA['binary-encoding'], format: 'html') diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 62b93a2728b21ccad811b63be26932228aa9cbf3..9821618df8de5617678d8add6d5d65c27f546541 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -58,11 +58,13 @@ from_project_id: from_project_id, from: from_ref, to: to_ref, - w: whitespace + w: whitespace, + page: page } end let(:whitespace) { nil } + let(:page) { nil } context 'when the refs exist in the same project' do context 'when we set the white space param' do @@ -196,6 +198,34 @@ expect(response).to have_gitlab_http_status(:found) end end + + context 'when page is valid' do + let(:from_project_id) { nil } + let(:from_ref) { '08f22f25' } + let(:to_ref) { '66eceea0' } + let(:page) { 1 } + + it 'shows the diff' do + show_request + + expect(response).to be_successful + expect(assigns(:diffs).diff_files.first).to be_present + expect(assigns(:commits).length).to be >= 1 + end + end + + context 'when page is not valid' do + let(:from_project_id) { nil } + let(:from_ref) { '08f22f25' } + let(:to_ref) { '66eceea0' } + let(:page) { ['invalid'] } + + it 'does not return an error' do + show_request + + expect(response).to be_successful + end + end end describe 'GET diff_for_path' do diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index 98db185c1803aefc849bfb52fce49a3c3ee7a996..961e7688202b2a4a3034925396f34e5d400db6e9 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -163,13 +163,7 @@ end end - let(:params) do - { - page: page - } - end - - subject { helper.conditionally_paginate_diff_files(diffs_collection, paginate: paginate, per: Projects::CommitController::COMMIT_DIFFS_PER_PAGE) } + subject { helper.conditionally_paginate_diff_files(diffs_collection, paginate: paginate, page: page, per: Projects::CommitController::COMMIT_DIFFS_PER_PAGE) } before do allow(helper).to receive(:params).and_return(params) @@ -183,7 +177,7 @@ end it "can change the number of items per page" do - commits = helper.conditionally_paginate_diff_files(diffs_collection, paginate: paginate, per: 10) + commits = helper.conditionally_paginate_diff_files(diffs_collection, page: page, paginate: paginate, per: 10) expect(commits).to be_an(Array) expect(commits.size).to eq(10) diff --git a/spec/views/projects/commit/show.html.haml_spec.rb b/spec/views/projects/commit/show.html.haml_spec.rb index e23ffe300c5b7f10302b9dcaf77bed56250bfeb4..59182f6e757ade2c2c4622061c138fdefc73d932 100644 --- a/spec/views/projects/commit/show.html.haml_spec.rb +++ b/spec/views/projects/commit/show.html.haml_spec.rb @@ -25,6 +25,7 @@ allow(view).to receive(:can_collaborate_with_project?).and_return(false) allow(view).to receive(:current_ref).and_return(project.repository.root_ref) allow(view).to receive(:diff_btn).and_return('') + allow(view).to receive(:pagination_params).and_return({}) end context 'inline diff view' do