Merge branch '33533-go-to-root-if-no-path-on-branch' into 'master'

Feature #33533, redirect to root when path does not exist on branch

Closes #33533

See merge request gitlab-org/gitlab!18169

app/controllers/concerns/redirects_for_missing_path_on_tree.rb

+# frozen_string_literal: true
+
+module RedirectsForMissingPathOnTree
+  def redirect_to_tree_root_for_missing_path(project, ref, path)
+    redirect_to project_tree_path(project, ref), notice: missing_path_on_ref(path, ref)
+  end
+
+  private
+
+  def missing_path_on_ref(path, ref)
+    _('"%{path}" did not exist on "%{ref}"') % { path: truncate_path(path), ref: ref }
+  end
+
+  def truncate_path(path)
+    path.reverse.truncate(60, separator: "/").reverse
+  end
+end

app/controllers/projects/blame_controller.rb

@@ -3,6 +3,7 @@
 # Controller for viewing a file's blame
 class Projects::BlameController < Projects::ApplicationController
   include ExtractsPath
+  include RedirectsForMissingPathOnTree
 
   before_action :require_non_empty_project
   before_action :assign_ref_vars
@@ -11,7 +12,9 @@ class Projects::BlameController < Projects::ApplicationController
   def show
     @blob = @repository.blob_at(@commit.id, @path)
 
-    return render_404 unless @blob
+    unless @blob
+      return redirect_to_tree_root_for_missing_path(@project, @ref, @path)
+    end
 
     environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit }
     @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last

app/controllers/projects/blob_controller.rb

@@ -7,6 +7,7 @@ class Projects::BlobController < Projects::ApplicationController
   include RendersBlob
   include NotesHelper
   include ActionView::Helpers::SanitizeHelper
+  include RedirectsForMissingPathOnTree
   prepend_before_action :authenticate_user!, only: [:edit]
 
   around_action :allow_gitaly_ref_name_caching, only: [:show]
@@ -119,7 +120,7 @@ class Projects::BlobController < Projects::ApplicationController
         end
       end
 
-      return render_404
+      return redirect_to_tree_root_for_missing_path(@project, @ref, @path)
     end
   end
 

app/controllers/projects/tree_controller.rb

@@ -5,6 +5,7 @@ class Projects::TreeController < Projects::ApplicationController
   include ExtractsPath
   include CreatesCommit
   include ActionView::Helpers::SanitizeHelper
+  include RedirectsForMissingPathOnTree
 
   around_action :allow_gitaly_ref_name_caching, only: [:show]
 
@@ -19,12 +20,9 @@ class Projects::TreeController < Projects::ApplicationController
 
     if tree.entries.empty?
       if @repository.blob_at(@commit.id, @path)
-        return redirect_to(
-          project_blob_path(@project,
-                                      File.join(@ref, @path))
-        )
+        return redirect_to project_blob_path(@project, File.join(@ref, @path))
       elsif @path.present?
-        return render_404
+        return redirect_to_tree_root_for_missing_path(@project, @ref, @path)
       end
     end
 

changelogs/unreleased/33533-go-to-root-if-no-path-on-branch.yml

+---
+title: When a user views a file's blame or blob and switches to a branch where the
+  current file does not exist, they will now be redirected to the root of the repository.
+merge_request: 18169
+author: Jesse Hall @jessehall3
+type: changed

locale/gitlab.pot

@@ -62,6 +62,9 @@ msgstr ""
 msgid " or references (e.g. path/to/project!merge_request_id)"
 msgstr ""
 
+msgid "\"%{path}\" did not exist on \"%{ref}\""
+msgstr ""
+
 msgid "%d comment"
 msgid_plural "%d comments"
 msgstr[0] ""

spec/controllers/concerns/redirects_for_missing_path_on_tree_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RedirectsForMissingPathOnTree, type: :controller do
+  controller(ActionController::Base) do
+    include Gitlab::Routing.url_helpers
+    include RedirectsForMissingPathOnTree
+
+    def fake
+      redirect_to_tree_root_for_missing_path(Project.find(params[:project_id]), params[:ref], params[:file_path])
+    end
+  end
+
+  let(:project) { create(:project) }
+
+  before do
+    routes.draw { get 'fake' => 'anonymous#fake' }
+  end
+
+  describe '#redirect_to_root_path' do
+    it 'redirects to the tree path with a notice' do
+      long_file_path = ('a/b/' * 30) + 'foo.txt'
+      truncated_file_path = '...b/' + ('a/b/' * 12) + 'foo.txt'
+      expected_message = "\"#{truncated_file_path}\" did not exist on \"theref\""
+
+      get :fake, params: { project_id: project.id, ref: 'theref', file_path: long_file_path }
+
+      expect(response).to redirect_to project_tree_path(project, 'theref')
+      expect(response.flash[:notice]).to eq(expected_message)
+    end
+  end
+end

spec/controllers/projects/blame_controller_spec.rb

@@ -25,14 +25,25 @@ describe Projects::BlameController do
           })
     end
 
-    context "valid file" do
+    context "valid branch, valid file" do
       let(:id) { 'master/files/ruby/popen.rb' }
+
       it { is_expected.to respond_with(:success) }
     end
 
-    context "invalid file" do
-      let(:id) { 'master/files/ruby/missing_file.rb'}
-      it { expect(response).to have_gitlab_http_status(404) }
+    context "valid branch, invalid file" do
+      let(:id) { 'master/files/ruby/invalid-path.rb' }
+
+      it 'redirects' do
+        expect(subject)
+            .to redirect_to("/#{project.full_path}/tree/master")
+      end
+    end
+
+    context "invalid branch, valid file" do
+      let(:id) { 'invalid-branch/files/ruby/missing_file.rb'}
+
+      it { is_expected.to respond_with(:not_found) }
     end
   end
 end

spec/controllers/projects/blob_controller_spec.rb

@@ -24,26 +24,34 @@ describe Projects::BlobController do
 
       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) }
+
+        it 'redirects' do
+          expect(subject)
+              .to redirect_to("/#{project.full_path}/tree/master")
+        end
       end
 
       context "invalid branch, valid file" do
         let(:id) { 'invalid-branch/README.md' }
+
         it { is_expected.to respond_with(:not_found) }
       end
 
       context "binary file" do
         let(:id) { 'binary-encoding/encoding/binary-1.bin' }
+
         it { is_expected.to respond_with(:success) }
       end
 
       context "Markdown file" do
         let(:id) { 'master/README.md' }
+
         it { is_expected.to respond_with(:success) }
       end
     end
@@ -104,6 +112,7 @@ describe Projects::BlobController do
 
       context 'redirect to tree' do
         let(:id) { 'markdown/doc' }
+
         it 'redirects' do
           expect(subject)
             .to redirect_to("/#{project.full_path}/tree/markdown/doc")

spec/controllers/projects/tree_controller_spec.rb

@@ -30,46 +30,61 @@ describe Projects::TreeController do
 
     context "valid branch, no path" do
       let(:id) { 'master' }
+
       it { is_expected.to respond_with(:success) }
     end
 
     context "valid branch, valid path" do
       let(:id) { 'master/encoding/' }
+
       it { is_expected.to respond_with(:success) }
     end
 
     context "valid branch, invalid path" do
       let(:id) { 'master/invalid-path/' }
-      it { is_expected.to respond_with(:not_found) }
+
+      it 'redirects' do
+        expect(subject)
+            .to redirect_to("/#{project.full_path}/tree/master")
+      end
     end
 
     context "invalid branch, valid path" do
       let(:id) { 'invalid-branch/encoding/' }
+
       it { is_expected.to respond_with(:not_found) }
     end
 
     context "valid empty branch, invalid path" do
       let(:id) { 'empty-branch/invalid-path/' }
-      it { is_expected.to respond_with(:not_found) }
+
+      it 'redirects' do
+        expect(subject)
+            .to redirect_to("/#{project.full_path}/tree/empty-branch")
+      end
     end
 
     context "valid empty branch" do
       let(:id) { 'empty-branch' }
+
       it { is_expected.to respond_with(:success) }
     end
 
     context "invalid SHA commit ID" do
       let(:id) { 'ff39438/.gitignore' }
+
       it { is_expected.to respond_with(:not_found) }
     end
 
     context "valid SHA commit ID" do
       let(:id) { '6d39438' }
+
       it { is_expected.to respond_with(:success) }
     end
 
     context "valid SHA commit ID with path" do
       let(:id) { '6d39438/.gitignore' }
+
       it { expect(response).to have_gitlab_http_status(302) }
     end
   end
@@ -108,6 +123,7 @@ describe Projects::TreeController do
 
     context 'redirect to blob' do
       let(:id) { 'master/README.md' }
+
       it 'redirects' do
         redirect_url = "/#{project.full_path}/blob/master/README.md"
         expect(subject)

spec/requests/git_http_spec.rb

@@ -843,8 +843,8 @@ describe 'Git HTTP requests' do
             get "/#{project.full_path}/blob/master/info/refs"
           end
 
-          it "returns not found" do
-            expect(response).to have_gitlab_http_status(:not_found)
+          it "redirects" do
+            expect(response).to have_gitlab_http_status(302)
           end
         end
       end