Merge branch 'id-code-nav-fields-for-mrs' into 'master'

Expose code_navigation_path and definition_path_prefix for merge request diffs

See merge request gitlab-org/gitlab!28349

app/controllers/projects/blob_controller.rb

@@ -208,24 +208,12 @@ class Projects::BlobController < Projects::ApplicationController
       .last_for_path(@repository, @ref, @path).sha
   end
 
-  def set_code_navigation_build
-    return if Feature.disabled?(:code_navigation, @project)
-
-    artifact =
-      Ci::JobArtifact
-        .for_sha(@blob.commit_id, @project.id)
-        .for_job_name(Ci::Build::CODE_NAVIGATION_JOB_NAME)
-        .last
-
-    @code_navigation_build = artifact&.job
-  end
-
   def show_html
     environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit }
     environment_params[:find_latest] = true
     @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last
     @last_commit = @repository.last_commit_for_path(@commit.id, @blob.path)
-    set_code_navigation_build
+    @code_navigation_path = Gitlab::CodeNavigationPath.new(@project, @blob.commit_id).full_json_path_for(@blob.path)
 
     render 'show'
   end

app/models/ci/build.rb

@@ -33,8 +33,6 @@ module Ci
       scheduler_failure: 2
     }.freeze
 
-    CODE_NAVIGATION_JOB_NAME = 'code_navigation'
-
     has_one :deployment, as: :deployable, class_name: 'Deployment'
     has_one :resource, class_name: 'Ci::Resource', inverse_of: :build
     has_many :trace_sections, class_name: 'Ci::BuildTraceSection'

app/serializers/diff_file_entity.rb

@@ -64,6 +64,10 @@ class DiffFileEntity < DiffFileBaseEntity
   # Used for parallel diffs
   expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, options) { parallel_diff_view?(options, diff_file) && diff_file.text? }
 
+  expose :code_navigation_path, if: -> (diff_file) { options[:code_navigation_path] } do |diff_file|
+    options[:code_navigation_path].full_json_path_for(diff_file.new_path)
+  end
+
   private
 
   def parallel_diff_view?(options, diff_file)

app/serializers/diffs_entity.rb

@@ -70,13 +70,21 @@ class DiffsEntity < Grape::Entity
 
   expose :diff_files do |diffs, options|
     submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
-    DiffFileEntity.represent(diffs.diff_files, options.merge(submodule_links: submodule_links))
+    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))
   end
 
   expose :merge_request_diffs, using: MergeRequestDiffEntity, if: -> (_, options) { options[:merge_request_diffs]&.any? } do |diffs|
     options[:merge_request_diffs]
   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)
+  end
+
   def merge_request
     options[:merge_request]
   end

app/serializers/paginated_diff_entity.rb

@@ -10,7 +10,11 @@ class PaginatedDiffEntity < Grape::Entity
 
   expose :diff_files do |diffs, options|
     submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
-    DiffFileEntity.represent(diffs.diff_files, options.merge(submodule_links: submodule_links))
+    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))
   end
 
   expose :pagination do

app/views/projects/blob/_blob.html.haml

@@ -9,9 +9,8 @@
   = render "projects/blob/auxiliary_viewer", blob: blob
 
 #blob-content-holder.blob-content-holder
-  - if @code_navigation_build
-    - code_nav_url = raw_project_job_artifacts_url(@project, @code_navigation_build, path: "lsif/#{blob.path}")
-    #js-code-navigation{ data: { code_nav_url: "#{code_nav_url}.json", definition_path_prefix: project_blob_path(@project, @ref) } }
+  - if @code_navigation_path
+    #js-code-navigation{ data: { code_nav_url: @code_navigation_path, definition_path_prefix: project_blob_path(@project, @ref) } }
   %article.file-holder
     = render 'projects/blob/header', blob: blob
     = render 'projects/blob/content', blob: blob

lib/gitlab/code_navigation_path.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class CodeNavigationPath
+    include Gitlab::Utils::StrongMemoize
+    include Gitlab::Routing
+
+    CODE_NAVIGATION_JOB_NAME = 'code_navigation'
+
+    def initialize(project, commit_sha)
+      @project = project
+      @commit_sha = commit_sha
+    end
+
+    def full_json_path_for(path)
+      return if Feature.disabled?(:code_navigation, project)
+      return unless build
+
+      raw_project_job_artifacts_path(project, build, path: "lsif/#{path}.json")
+    end
+
+    private
+
+    attr_reader :project, :commit_sha
+
+    def build
+      strong_memoize(:build) do
+        artifact = ::Ci::JobArtifact
+          .for_sha(commit_sha, project.id)
+          .for_job_name(CODE_NAVIGATION_JOB_NAME)
+          .last
+
+        artifact&.job
+      end
+    end
+  end
+end

spec/controllers/projects/blob_controller_spec.rb

@@ -118,32 +118,6 @@ describe Projects::BlobController do
         end
       end
     end
-
-    context 'when there is an artifact with code navigation data' do
-      let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
-      let!(:job) { create(:ci_build, pipeline: pipeline, name: Ci::Build::CODE_NAVIGATION_JOB_NAME) }
-      let!(:artifact) { create(:ci_job_artifact, :lsif, job: job) }
-
-      let(:id) { 'master/README.md' }
-
-      it 'assigns code_navigation_build variable' do
-        request
-
-        expect(assigns[:code_navigation_build]).to eq(job)
-      end
-
-      context 'when code_navigation feature is disabled' do
-        before do
-          stub_feature_flags(code_navigation: false)
-        end
-
-        it 'does not assign code_navigation_build variable' do
-          request
-
-          expect(assigns[:code_navigation_build]).to be_nil
-        end
-      end
-    end
   end
 
   describe 'GET diff' do

spec/lib/gitlab/code_navigation_path_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::CodeNavigationPath do
+  context 'when there is an artifact with code navigation data' do
+    let(:project) { create(:project, :repository) }
+    let(:sha) { project.commit.id }
+    let(:build_name) { Gitlab::CodeNavigationPath::CODE_NAVIGATION_JOB_NAME }
+    let(:path) { 'lib/app.rb' }
+    let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha) }
+    let!(:job) { create(:ci_build, pipeline: pipeline, name: build_name) }
+    let!(:artifact) { create(:ci_job_artifact, :lsif, job: job) }
+
+    subject { described_class.new(project, sha).full_json_path_for(path) }
+
+    it 'assigns code_navigation_build variable' do
+      expect(subject).to eq("/#{project.full_path}/-/jobs/#{job.id}/artifacts/raw/lsif/#{path}.json")
+    end
+
+    context 'when code_navigation feature is disabled' do
+      before do
+        stub_feature_flags(code_navigation: false)
+      end
+
+      it 'does not assign code_navigation_build variable' do
+        expect(subject).to be_nil
+      end
+    end
+  end
+end

spec/serializers/diff_file_entity_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe DiffFileEntity do
   include RepoHelpers
 
-  let(:project) { create(:project, :repository) }
+  let_it_be(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:commit) { project.commit(sample_commit.id) }
   let(:diff_refs) { commit.diff_refs }
@@ -21,10 +21,11 @@ describe DiffFileEntity do
   end
 
   context 'when there is a merge request' do
+    let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
     let(:user) { create(:user) }
+    let(:code_navigation_path) { Gitlab::CodeNavigationPath.new(project, project.commit.sha) }
     let(:request) { EntityRequest.new(project: project, current_user: user) }
-    let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
-    let(:entity) { described_class.new(diff_file, options.merge(request: request, merge_request: merge_request)) }
+    let(:entity) { described_class.new(diff_file, options.merge(request: request, merge_request: merge_request, code_navigation_path: code_navigation_path)) }
     let(:exposed_urls) { %i(edit_path view_path context_lines_path) }
 
     it_behaves_like 'diff file entity'
@@ -32,6 +33,7 @@ describe DiffFileEntity do
     it 'exposes additional attributes' do
       expect(subject).to include(*exposed_urls)
       expect(subject).to include(:replaced_view_path)
+      expect(subject).to include(:code_navigation_path)
     end
 
     it 'points all urls to merge request target project' do

spec/serializers/diffs_entity_spec.rb

@@ -23,7 +23,7 @@ describe DiffsEntity do
         :start_version, :latest_diff, :latest_version_path,
         :added_lines, :removed_lines, :render_overflow_warning,
         :email_patch_path, :plain_diff_path, :diff_files,
-        :merge_request_diffs
+        :merge_request_diffs, :definition_path_prefix
       )
     end
   end

spec/serializers/diffs_metadata_entity_spec.rb

@@ -28,8 +28,8 @@ describe DiffsMetadataEntity do
         :start_version, :latest_diff, :latest_version_path,
         :added_lines, :removed_lines, :render_overflow_warning,
         :email_patch_path, :plain_diff_path,
-        :merge_request_diffs,
-        :context_commits,
+        :merge_request_diffs, :context_commits,
+        :definition_path_prefix,
         # Attributes
         :diff_files
       )