Merge branch 'not-found-in-commits' into 'master'

Renders 404 in commits controller if no commits are found

Closes #37620

See merge request gitlab-org/gitlab-ce!14610

app/controllers/projects/commits_controller.rb

@@ -48,6 +48,8 @@ class Projects::CommitsController < Projects::ApplicationController
   private
 
   def set_commits
+    render_404 unless request.format == :atom || @repository.blob_at(@commit.id, @path) || @repository.tree(@commit.id, @path).entries.present?
+
     @limit, @offset = (params[:limit] || 40).to_i, (params[:offset] || 0).to_i
     search = params[:search]
 

changelogs/unreleased/not-found-in-commits.yml

+---
+title: Renders 404 in commits controller if no commits are found for a given path
+merge_request: 14610
+author: Guilherme Vieira
+type: fixed

features/steps/shared/paths.rb

@@ -222,7 +222,7 @@ module SharedPaths
   end
 
   step "I visit my project's commits page for a specific path" do
-    visit project_commits_path(@project, root_ref + "/app/models/project.rb", { limit: 5 })
+    visit project_commits_path(@project, root_ref + "/files/ruby/regex.rb", { limit: 5 })
   end
 
   step 'I visit my project\'s commits stats page' do

spec/controllers/projects/commits_controller_spec.rb

@@ -10,9 +10,36 @@ describe Projects::CommitsController do
   end
 
   describe "GET show" do
-    context "when the ref name ends in .atom" do
-      render_views
+    render_views
+
+    context 'with file path' do
+      before do
+        get(:show,
+            namespace_id: project.namespace,
+            project_id: project,
+            id: id)
+      end
+
+      context "valid branch, valid file" do
+        let(:id) { 'master/README.md' }
+
+        it { is_expected.to respond_with(:success) }
+      end
+
+      context "valid branch, invalid file" do
+        let(:id) { 'master/invalid-path.rb' }
 
+        it { is_expected.to respond_with(:not_found) }
+      end
+
+      context "invalid branch, valid file" do
+        let(:id) { 'invalid-branch/README.md' }
+
+        it { is_expected.to respond_with(:not_found) }
+      end
+    end
+
+    context "when the ref name ends in .atom" do
       context "when the ref does not exist with the suffix" do
         it "renders as atom" do
           get(:show,