Merge branch '351520_fix_undefined_method_error' into 'master'

Fix undefined method error for Compare, Commit controllers

See merge request gitlab-org/gitlab!84176

app/controllers/projects/commit_controller.rb

@@ -31,7 +31,7 @@ class Projects::CommitController < Projects::ApplicationController
 
     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)

app/controllers/projects/compare_controller.rb

@@ -34,7 +34,7 @@ class Projects::CompareController < Projects::ApplicationController
   def show
     apply_diff_view_cookie!
 
-    render
+    render locals: { pagination_params: params.permit(:page) }
   end
 
   def diff_for_path

app/helpers/commits_helper.rb

@@ -135,9 +135,9 @@ module CommitsHelper
     %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

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
 

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

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

spec/controllers/projects/commit_controller_spec.rb

@@ -60,6 +60,22 @@ RSpec.describe Projects::CommitController do
       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')
 

spec/controllers/projects/compare_controller_spec.rb

@@ -58,11 +58,13 @@ RSpec.describe Projects::CompareController do
         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 @@ RSpec.describe Projects::CompareController do
         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

spec/helpers/commits_helper_spec.rb

@@ -163,13 +163,7 @@ RSpec.describe CommitsHelper do
       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 @@ RSpec.describe CommitsHelper do
       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)

spec/views/projects/commit/show.html.haml_spec.rb

@@ -25,6 +25,7 @@ RSpec.describe 'projects/commit/show.html.haml' do
     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