From 7a2b5cbb51c8fda57c7fc7935df981c29bae500e Mon Sep 17 00:00:00 2001
From: Eduardo Bonet <ebonet@gitlab.com>
Date: Mon, 15 May 2023 18:29:14 +0000
Subject: [PATCH] Adds CI information to candidate detail

If a candidate is associated to a ci_build, render some information
about that CI on the detail page

Changelog: added
---
 app/helpers/projects/ml/experiments_helper.rb |  21 ----
 .../ml/candidate_details_presenter.rb         |  88 ++++++++++++++
 .../projects/ml/candidates/show.html.haml     |   3 +-
 .../projects/ml/experiments_helper_spec.rb    |  35 ------
 .../ml/candidate_details_presenter_spec.rb    | 111 ++++++++++++++++++
 5 files changed, 201 insertions(+), 57 deletions(-)
 create mode 100644 app/presenters/ml/candidate_details_presenter.rb
 create mode 100644 spec/presenters/ml/candidate_details_presenter_spec.rb

diff --git a/app/helpers/projects/ml/experiments_helper.rb b/app/helpers/projects/ml/experiments_helper.rb
index 6e5e13ef0b600..7aef208447ab4 100644
--- a/app/helpers/projects/ml/experiments_helper.rb
+++ b/app/helpers/projects/ml/experiments_helper.rb
@@ -5,27 +5,6 @@ module ExperimentsHelper
       require 'json'
       include ActionView::Helpers::NumberHelper
 
-      def show_candidate_view_model(candidate)
-        data = {
-          candidate: {
-            params: candidate.params,
-            metrics: candidate.latest_metrics,
-            info: {
-              iid: candidate.iid,
-              eid: candidate.eid,
-              path_to_artifact: link_to_artifact(candidate),
-              experiment_name: candidate.experiment.name,
-              path_to_experiment: link_to_experiment(candidate.project, candidate.experiment),
-              path: link_to_details(candidate),
-              status: candidate.status
-            },
-            metadata: candidate.metadata
-          }
-        }
-
-        Gitlab::Json.generate(data)
-      end
-
       def experiment_as_data(experiment)
         data = {
           name: experiment.name,
diff --git a/app/presenters/ml/candidate_details_presenter.rb b/app/presenters/ml/candidate_details_presenter.rb
new file mode 100644
index 0000000000000..58ec2aee47106
--- /dev/null
+++ b/app/presenters/ml/candidate_details_presenter.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+module Ml
+  class CandidateDetailsPresenter
+    include Rails.application.routes.url_helpers
+
+    def initialize(candidate)
+      @candidate = candidate
+    end
+
+    def present
+      data = {
+        candidate: {
+          info: {
+            iid: candidate.iid,
+            eid: candidate.eid,
+            path_to_artifact: link_to_artifact,
+            experiment_name: candidate.experiment.name,
+            path_to_experiment: link_to_experiment,
+            path: link_to_details,
+            status: candidate.status,
+            ci_job: job_info
+          },
+          params: candidate.params,
+          metrics: candidate.latest_metrics,
+          metadata: candidate.metadata
+        }
+      }
+
+      Gitlab::Json.generate(data)
+    end
+
+    private
+
+    attr_reader :candidate
+
+    def job_info
+      return unless candidate.from_ci?
+
+      build = candidate.ci_build
+
+      {
+        path: project_job_path(build.project, build),
+        name: build.name,
+        **user_info(build.user) || {},
+        **mr_info(build.pipeline.merge_request) || {}
+      }
+    end
+
+    def user_info(user)
+      return unless user
+
+      {
+        user: {
+          path: user_path(user),
+          username: user.username
+        }
+      }
+    end
+
+    def mr_info(mr)
+      return unless mr
+
+      {
+        merge_request: {
+          path: project_merge_request_path(mr.project, mr),
+          title: mr.title
+        }
+      }
+    end
+
+    def link_to_artifact
+      artifact = candidate.artifact
+
+      return unless artifact.present?
+
+      project_package_path(candidate.project, artifact)
+    end
+
+    def link_to_details
+      project_ml_candidate_path(candidate.project, candidate.iid)
+    end
+
+    def link_to_experiment
+      project_ml_experiment_path(candidate.project, candidate.experiment.iid)
+    end
+  end
+end
diff --git a/app/views/projects/ml/candidates/show.html.haml b/app/views/projects/ml/candidates/show.html.haml
index aea74ecfb487d..5d5059551d3b7 100644
--- a/app/views/projects/ml/candidates/show.html.haml
+++ b/app/views/projects/ml/candidates/show.html.haml
@@ -3,5 +3,6 @@
 - add_to_breadcrumbs experiment.name, project_ml_experiment_path(@project, experiment.iid)
 - breadcrumb_title "Candidate #{@candidate.iid}"
 - add_page_specific_style 'page_bundles/ml_experiment_tracking'
+- presenter = ::Ml::CandidateDetailsPresenter.new(@candidate)
 
-#js-show-ml-candidate{ data: { view_model: show_candidate_view_model(@candidate) } }
+#js-show-ml-candidate{ data: { view_model: presenter.present } }
diff --git a/spec/helpers/projects/ml/experiments_helper_spec.rb b/spec/helpers/projects/ml/experiments_helper_spec.rb
index e0fbc8ac48894..021d518a32944 100644
--- a/spec/helpers/projects/ml/experiments_helper_spec.rb
+++ b/spec/helpers/projects/ml/experiments_helper_spec.rb
@@ -98,41 +98,6 @@
     end
   end
 
-  describe '#show_candidate_view_model' do
-    let(:candidate) { candidate0 }
-
-    subject { Gitlab::Json.parse(helper.show_candidate_view_model(candidate))['candidate'] }
-
-    it 'generates the correct params' do
-      expect(subject['params']).to include(
-        hash_including('name' => 'param1', 'value' => 'p1'),
-        hash_including('name' => 'param2', 'value' => 'p2')
-      )
-    end
-
-    it 'generates the correct metrics' do
-      expect(subject['metrics']).to include(
-        hash_including('name' => 'metric1', 'value' => 0.1),
-        hash_including('name' => 'metric2', 'value' => 0.2),
-        hash_including('name' => 'metric3', 'value' => 0.3)
-      )
-    end
-
-    it 'generates the correct info' do
-      expected_info = {
-        'iid' => candidate.iid,
-        'eid' => candidate.eid,
-        'path_to_artifact' => "/#{project.full_path}/-/packages/#{candidate.artifact.id}",
-        'experiment_name' => candidate.experiment.name,
-        'path_to_experiment' => "/#{project.full_path}/-/ml/experiments/#{experiment.iid}",
-        'status' => 'running',
-        'path' => "/#{project.full_path}/-/ml/candidates/#{candidate.iid}"
-      }
-
-      expect(subject['info']).to include(expected_info)
-    end
-  end
-
   describe '#experiments_as_data' do
     let(:experiments) { [experiment] }
 
diff --git a/spec/presenters/ml/candidate_details_presenter_spec.rb b/spec/presenters/ml/candidate_details_presenter_spec.rb
new file mode 100644
index 0000000000000..d83ffbc71299c
--- /dev/null
+++ b/spec/presenters/ml/candidate_details_presenter_spec.rb
@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ml::CandidateDetailsPresenter, feature_category: :mlops do
+  let_it_be(:project) { create(:project, :private) } # rubocop:disable RSpec/FactoryBot/AvoidCreate
+  let_it_be(:user) { project.creator }
+  let_it_be(:experiment) { create(:ml_experiments, user: user, project: project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate
+  let_it_be(:candidate) do
+    create(:ml_candidates, :with_artifact, experiment: experiment, user: user, project: project) # rubocop:disable RSpec/FactoryBot/AvoidCreate
+  end
+
+  let_it_be(:metrics) do
+    [
+      build_stubbed(:ml_candidate_metrics, name: 'metric1', value: 0.1, candidate: candidate),
+      build_stubbed(:ml_candidate_metrics, name: 'metric2', value: 0.2, candidate: candidate),
+      build_stubbed(:ml_candidate_metrics, name: 'metric3', value: 0.3, candidate: candidate)
+    ]
+  end
+
+  let_it_be(:params) do
+    [
+      build_stubbed(:ml_candidate_params, name: 'param1', value: 'p1', candidate: candidate),
+      build_stubbed(:ml_candidate_params, name: 'param2', value: 'p2', candidate: candidate)
+    ]
+  end
+
+  subject { Gitlab::Json.parse(described_class.new(candidate).present)['candidate'] }
+
+  before do
+    allow(candidate).to receive(:latest_metrics).and_return(metrics)
+    allow(candidate).to receive(:params).and_return(params)
+  end
+
+  describe '#execute' do
+    context 'when candidate has metrics, params and artifacts' do
+      it 'generates the correct params' do
+        expect(subject['params']).to include(
+          hash_including('name' => 'param1', 'value' => 'p1'),
+          hash_including('name' => 'param2', 'value' => 'p2')
+        )
+      end
+
+      it 'generates the correct metrics' do
+        expect(subject['metrics']).to include(
+          hash_including('name' => 'metric1', 'value' => 0.1),
+          hash_including('name' => 'metric2', 'value' => 0.2),
+          hash_including('name' => 'metric3', 'value' => 0.3)
+        )
+      end
+
+      it 'generates the correct info' do
+        expected_info = {
+          'iid' => candidate.iid,
+          'eid' => candidate.eid,
+          'path_to_artifact' => "/#{project.full_path}/-/packages/#{candidate.artifact.id}",
+          'experiment_name' => candidate.experiment.name,
+          'path_to_experiment' => "/#{project.full_path}/-/ml/experiments/#{experiment.iid}",
+          'status' => 'running',
+          'path' => "/#{project.full_path}/-/ml/candidates/#{candidate.iid}"
+        }
+
+        expect(subject['info']).to include(expected_info)
+      end
+    end
+
+    context 'when candidate has job' do
+      let_it_be(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) }
+      let_it_be(:build) { candidate.ci_build = build_stubbed(:ci_build, pipeline: pipeline, user: user) }
+
+      it 'generates the correct ci' do
+        expected_info = {
+          'path' => "/#{project.full_path}/-/jobs/#{build.id}",
+          'name' => 'test',
+          'user' => {
+            'path' => "/#{pipeline.user.username}",
+            'username' => pipeline.user.username
+          }
+        }
+
+        expect(subject.dig('info', 'ci_job')).to include(expected_info)
+      end
+
+      context 'when build user is nil' do
+        it 'does not include build user info' do
+          expected_info = {
+            'path' => "/#{project.full_path}/-/jobs/#{build.id}",
+            'name' => 'test'
+          }
+
+          allow(build).to receive(:user).and_return(nil)
+
+          expect(subject.dig('info', 'ci_job')).to eq(expected_info)
+        end
+      end
+
+      context 'and job is from MR' do
+        let_it_be(:mr) { pipeline.merge_request = build_stubbed(:merge_request, source_project: project) }
+
+        it 'generates the correct ci' do
+          expected_info = {
+            'path' => "/#{project.full_path}/-/merge_requests/#{mr.iid}",
+            'title' => mr.title
+          }
+
+          expect(subject.dig('info', 'ci_job', 'merge_request')).to include(expected_info)
+        end
+      end
+    end
+  end
+end
-- 
GitLab