Merge branch 'id-code-nav-500-error' into 'master'

Return code navigation path for nil diff_refs

See merge request gitlab-org/gitlab!33850

app/serializers/diffs_entity.rb

@@ -69,11 +69,9 @@ class DiffsEntity < Grape::Entity
 
   expose :diff_files do |diffs, options|
     submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
-    code_navigation_path =
-      Gitlab::CodeNavigationPath.new(merge_request.project, diffs.diff_refs.head_sha)
 
     DiffFileEntity.represent(diffs.diff_files,
-      options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path))
+      options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs)))
   end
 
   expose :merge_request_diffs, using: MergeRequestDiffEntity, if: -> (_, options) { options[:merge_request_diffs]&.any? } do |diffs|
@@ -81,7 +79,7 @@ class DiffsEntity < Grape::Entity
   end
 
   expose :definition_path_prefix, if: -> (diff_file) { Feature.enabled?(:code_navigation, merge_request.project) } do |diffs|
-    project_blob_path(merge_request.project, diffs.diff_refs.head_sha)
+    project_blob_path(merge_request.project, diffs.diff_refs&.head_sha)
   end
 
   def merge_request
@@ -90,6 +88,12 @@ class DiffsEntity < Grape::Entity
 
   private
 
+  def code_navigation_path(diffs)
+    return unless Feature.enabled?(:code_navigation, merge_request.project)
+
+    Gitlab::CodeNavigationPath.new(merge_request.project, diffs.diff_refs&.head_sha)
+  end
+
   def commit_ids
     @commit_ids ||= merge_request.recent_commits.map(&:id)
   end

app/serializers/paginated_diff_entity.rb

@@ -10,11 +10,9 @@ class PaginatedDiffEntity < Grape::Entity
 
   expose :diff_files do |diffs, options|
     submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
-    code_navigation_path =
-      Gitlab::CodeNavigationPath.new(merge_request.project, diffs.diff_refs.head_sha)
 
     DiffFileEntity.represent(diffs.diff_files,
-      options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path))
+      options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs)))
   end
 
   expose :pagination do
@@ -38,6 +36,12 @@ class PaginatedDiffEntity < Grape::Entity
 
   private
 
+  def code_navigation_path(diffs)
+    return unless Feature.enabled?(:code_navigation, merge_request.project)
+
+    Gitlab::CodeNavigationPath.new(merge_request.project, diffs.diff_refs&.head_sha)
+  end
+
   %i[current_page next_page total_pages].each do |method|
     define_method method do
       pagination_data[method]

changelogs/unreleased/id-code-nav-500-error.yml

+---
+title: Return code navigation path for nil diff_refs
+merge_request: 33850
+author:
+type: fixed

spec/lib/gitlab/code_navigation_path_spec.rb

@@ -12,6 +12,7 @@ describe Gitlab::CodeNavigationPath do
 
     let(:commit_sha) { sha }
     let(:path) { 'lib/app.rb' }
+    let(:lsif_path) { "/#{project.full_path}/-/jobs/#{job.id}/artifacts/raw/lsif/#{path}.json?file_type=lsif" }
 
     subject { described_class.new(project, commit_sha).full_json_path_for(path) }
 
@@ -21,7 +22,15 @@ describe Gitlab::CodeNavigationPath do
 
     context 'when a pipeline exist for a sha' do
       it 'returns path to a file in the artifact' do
-        expect(subject).to eq("/#{project.full_path}/-/jobs/#{job.id}/artifacts/raw/lsif/#{path}.json?file_type=lsif")
+        expect(subject).to eq(lsif_path)
+      end
+
+      context 'when passed commit sha is nil' do
+        let(:commit_sha) { nil }
+
+        it 'returns path to a file in the artifact' do
+          expect(subject).to eq(lsif_path)
+        end
       end
     end
 
@@ -29,7 +38,7 @@ describe Gitlab::CodeNavigationPath do
       let(:commit_sha) { project.commit.id }
 
       it 'returns path to a file in the artifact' do
-        expect(subject).to eq("/#{project.full_path}/-/jobs/#{job.id}/artifacts/raw/lsif/#{path}.json?file_type=lsif")
+        expect(subject).to eq(lsif_path)
       end
     end
 

spec/serializers/diffs_entity_spec.rb

@@ -68,5 +68,15 @@ describe DiffsEntity do
         end
       end
     end
+
+    context 'when code_navigation feature flag is disabled' do
+      it 'does not include code navigation properties' do
+        stub_feature_flags(code_navigation: false)
+
+        expect(Gitlab::CodeNavigationPath).not_to receive(:new)
+
+        expect(subject).not_to include(:definition_path_prefix)
+      end
+    end
   end
 end

spec/serializers/paginated_diff_entity_spec.rb

@@ -30,4 +30,14 @@ describe PaginatedDiffEntity do
       total_pages: 7
     )
   end
+
+  context 'when code_navigation feature flag is disabled' do
+    it 'does not execute Gitlab::CodeNavigationPath' do
+      stub_feature_flags(code_navigation: false)
+
+      expect(Gitlab::CodeNavigationPath).not_to receive(:new)
+
+      subject
+    end
+  end
 end