Improve UI for Candidate CI info

- Includes the title of the MR, if exists
- Displays a labeled avatar for the CI user, if exists

Changelog: changed

app/assets/javascripts/ml/experiment_tracking/routes/candidates/show/components/candidate_detail_row.vue

 <script>
-import { GlLink } from '@gitlab/ui';
-
 export default {
   name: 'CandidateDetailRow',
-  components: {
-    GlLink,
-  },
   props: {
     label: {
       type: String,
       required: true,
     },
-    text: {
-      type: [String, Number],
-      required: true,
-    },
-    href: {
-      type: String,
-      required: false,
-      default: '',
-    },
     sectionLabel: {
       type: String,
       required: false,
@@ -34,8 +20,7 @@ export default {
     <td class="gl-text-secondary gl-font-weight-bold">{{ sectionLabel }}</td>
     <td class="gl-font-weight-bold">{{ label }}</td>
     <td>
-      <gl-link v-if="href" :href="href">{{ text }}</gl-link>
-      <template v-else>{{ text }}</template>
+      <slot></slot>
     </td>
   </tr>
 </template>

app/assets/javascripts/ml/experiment_tracking/routes/candidates/show/ml_candidates_show.vue

 <script>
+import { GlAvatarLabeled, GlLink } from '@gitlab/ui';
 import ModelExperimentsHeader from '~/ml/experiment_tracking/components/model_experiments_header.vue';
 import DeleteButton from '~/ml/experiment_tracking/components/delete_button.vue';
 import DetailRow from './components/candidate_detail_row.vue';
@@ -29,6 +30,8 @@ export default {
     ModelExperimentsHeader,
     DeleteButton,
     DetailRow,
+    GlAvatarLabeled,
+    GlLink,
   },
   props: {
     candidate: {
@@ -94,52 +97,51 @@ export default {
       <tbody>
         <tr class="divider"></tr>
 
-        <detail-row
-          :label="$options.i18n.ID_LABEL"
-          :section-label="$options.i18n.INFO_LABEL"
-          :text="info.iid"
-        />
+        <detail-row :label="$options.i18n.ID_LABEL" :section-label="$options.i18n.INFO_LABEL">
+          {{ info.iid }}
+        </detail-row>
 
-        <detail-row :label="$options.i18n.MLFLOW_ID_LABEL" :text="info.eid" />
+        <detail-row :label="$options.i18n.MLFLOW_ID_LABEL">{{ info.eid }}</detail-row>
 
-        <detail-row :label="$options.i18n.STATUS_LABEL" :text="info.status" />
+        <detail-row :label="$options.i18n.STATUS_LABEL">{{ info.status }}</detail-row>
 
-        <detail-row
-          :label="$options.i18n.EXPERIMENT_LABEL"
-          :text="info.experiment_name"
-          :href="info.path_to_experiment"
-        />
+        <detail-row :label="$options.i18n.EXPERIMENT_LABEL">
+          <gl-link :href="info.path_to_experiment">
+            {{ info.experiment_name }}
+          </gl-link>
+        </detail-row>
 
-        <detail-row
-          v-if="info.path_to_artifact"
-          :label="$options.i18n.ARTIFACTS_LABEL"
-          :href="info.path_to_artifact"
-          :text="$options.i18n.ARTIFACTS_LABEL"
-        />
+        <detail-row v-if="info.path_to_artifact" :label="$options.i18n.ARTIFACTS_LABEL">
+          <gl-link :href="info.path_to_artifact">
+            {{ $options.i18n.ARTIFACTS_LABEL }}
+          </gl-link>
+        </detail-row>
 
         <template v-if="ciJob">
           <tr class="divider"></tr>
 
           <detail-row
             :label="$options.i18n.JOB_LABEL"
-            :text="ciJob.name"
-            :href="ciJob.path"
             :section-label="$options.i18n.CI_SECTION_LABEL"
-          />
+          >
+            <gl-link :href="ciJob.path">
+              {{ ciJob.name }}
+            </gl-link>
+          </detail-row>
 
-          <detail-row
-            v-if="ciJob.user"
-            :label="$options.i18n.CI_USER_LABEL"
-            :href="ciJob.user.path"
-            :text="ciJob.user.username"
-          />
+          <detail-row v-if="ciJob.user" :label="$options.i18n.CI_USER_LABEL">
+            <gl-avatar-labeled label="" :size="24" :src="ciJob.user.avatar">
+              <gl-link :href="ciJob.user.path">
+                {{ ciJob.user.name }}
+              </gl-link>
+            </gl-avatar-labeled>
+          </detail-row>
 
-          <detail-row
-            v-if="ciJob.merge_request"
-            :label="$options.i18n.CI_MR_LABEL"
-            :text="ciJob.merge_request.title"
-            :href="ciJob.merge_request.path"
-          />
+          <detail-row v-if="ciJob.merge_request" :label="$options.i18n.CI_MR_LABEL">
+            <gl-link :href="ciJob.merge_request.path">
+              !{{ ciJob.merge_request.iid }} {{ ciJob.merge_request.title }}
+            </gl-link>
+          </detail-row>
         </template>
 
         <template v-for="{ sectionName, sectionValues } in sections">
@@ -150,8 +152,9 @@ export default {
             :key="item.name"
             :label="item.name"
             :section-label="index === 0 ? sectionName : ''"
-            :text="item.value"
-          />
+          >
+            {{ item.value }}
+          </detail-row>
         </template>
       </tbody>
     </table>

app/presenters/ml/candidate_details_presenter.rb

@@ -53,7 +53,9 @@ def user_info(user)
       {
         user: {
           path: user_path(user),
-          username: user.username
+          username: user.username,
+          name: user.name,
+          avatar: user.avatar_url
         }
       }
     end
@@ -64,6 +66,7 @@ def mr_info(mr)
       {
         merge_request: {
           path: project_merge_request_path(mr.project, mr),
+          iid: mr.iid,
           title: mr.title
         }
       }

spec/frontend/ml/experiment_tracking/routes/candidates/show/components/candidate_detail_row_spec.js

 import { shallowMount } from '@vue/test-utils';
-import { GlLink } from '@gitlab/ui';
 import DetailRow from '~/ml/experiment_tracking/routes/candidates/show/components/candidate_detail_row.vue';
 
 describe('CandidateDetailRow', () => {
@@ -9,14 +8,14 @@ describe('CandidateDetailRow', () => {
 
   let wrapper;
 
-  const createWrapper = (href = '') => {
+  const createWrapper = ({ slots = {} } = {}) => {
     wrapper = shallowMount(DetailRow, {
-      propsData: { sectionLabel: 'Section', label: 'Item', text: 'Text', href },
+      propsData: { sectionLabel: 'Section', label: 'Item' },
+      slots,
     });
   };
 
   const findCellAt = (index) => wrapper.findAll('td').at(index);
-  const findLink = () => findCellAt(ROW_VALUE_CELL).findComponent(GlLink);
 
   beforeEach(() => createWrapper());
 
@@ -28,22 +27,15 @@ describe('CandidateDetailRow', () => {
     expect(findCellAt(ROW_LABEL_CELL).text()).toBe('Item');
   });
 
-  describe('No href', () => {
-    it('Renders text', () => {
-      expect(findCellAt(ROW_VALUE_CELL).text()).toBe('Text');
-    });
-
-    it('Does not render as link', () => {
-      expect(findLink().exists()).toBe(false);
-    });
+  it('renders nothing on item cell', () => {
+    expect(findCellAt(ROW_VALUE_CELL).text()).toBe('');
   });
 
-  describe('With href', () => {
-    beforeEach(() => createWrapper('LINK'));
+  describe('With slot', () => {
+    beforeEach(() => createWrapper({ slots: { default: 'Some content' } }));
 
-    it('Renders link', () => {
-      expect(findLink().attributes().href).toBe('LINK');
-      expect(findLink().text()).toBe('Text');
+    it('Renders slot', () => {
+      expect(findCellAt(ROW_VALUE_CELL).text()).toBe('Some content');
     });
   });
 });

spec/frontend/ml/experiment_tracking/routes/candidates/show/ml_candidates_show_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { GlAvatarLabeled, GlLink } from '@gitlab/ui';
 import MlCandidatesShow from '~/ml/experiment_tracking/routes/candidates/show';
 import DetailRow from '~/ml/experiment_tracking/routes/candidates/show/components/candidate_detail_row.vue';
 import { TITLE_LABEL } from '~/ml/experiment_tracking/routes/candidates/show/translations';
@@ -9,6 +10,7 @@ import { newCandidate } from './mock_data';
 describe('MlCandidatesShow', () => {
   let wrapper;
   const CANDIDATE = newCandidate();
+  const USER_ROW = 6;
 
   const createWrapper = (createCandidate = () => CANDIDATE) => {
     wrapper = shallowMount(MlCandidatesShow, {
@@ -19,8 +21,12 @@ describe('MlCandidatesShow', () => {
   const findDeleteButton = () => wrapper.findComponent(DeleteButton);
   const findHeader = () => wrapper.findComponent(ModelExperimentsHeader);
   const findNthDetailRow = (index) => wrapper.findAllComponents(DetailRow).at(index);
+  const findLinkInNthDetailRow = (index) => findNthDetailRow(index).findComponent(GlLink);
   const findSectionLabel = (label) => wrapper.find(`[sectionLabel='${label}']`);
   const findLabel = (label) => wrapper.find(`[label='${label}']`);
+  const findCiUserDetailRow = () => findNthDetailRow(USER_ROW);
+  const findCiUserAvatar = () => findCiUserDetailRow().findComponent(GlAvatarLabeled);
+  const findCiUserAvatarNameLink = () => findCiUserAvatar().findComponent(GlLink);
 
   describe('Header', () => {
     beforeEach(() => createWrapper());
@@ -42,36 +48,64 @@ describe('MlCandidatesShow', () => {
     describe('All info available', () => {
       beforeEach(() => createWrapper());
 
+      const mrText = `!${CANDIDATE.info.ci_job.merge_request.iid} ${CANDIDATE.info.ci_job.merge_request.title}`;
       const expectedTable = [
-        ['Info', 'ID', CANDIDATE.info.iid, ''],
-        ['', 'MLflow run ID', CANDIDATE.info.eid, ''],
-        ['', 'Status', CANDIDATE.info.status, ''],
-        ['', 'Experiment', CANDIDATE.info.experiment_name, CANDIDATE.info.path_to_experiment],
-        ['', 'Artifacts', 'Artifacts', CANDIDATE.info.path_to_artifact],
-        ['CI', 'Job', CANDIDATE.info.ci_job.name, CANDIDATE.info.ci_job.path],
-        ['', 'Triggered by', CANDIDATE.info.ci_job.user.username, CANDIDATE.info.ci_job.user.path],
-        [
-          '',
-          'Merge request',
-          CANDIDATE.info.ci_job.merge_request.title,
-          CANDIDATE.info.ci_job.merge_request.path,
-        ],
-        ['Parameters', CANDIDATE.params[0].name, CANDIDATE.params[0].value, ''],
-        ['', CANDIDATE.params[1].name, CANDIDATE.params[1].value, ''],
-        ['Metrics', CANDIDATE.metrics[0].name, CANDIDATE.metrics[0].value, ''],
-        ['', CANDIDATE.metrics[1].name, CANDIDATE.metrics[1].value, ''],
-        ['Metadata', CANDIDATE.metadata[0].name, CANDIDATE.metadata[0].value, ''],
-        ['', CANDIDATE.metadata[1].name, CANDIDATE.metadata[1].value, ''],
+        ['Info', 'ID', CANDIDATE.info.iid],
+        ['', 'MLflow run ID', CANDIDATE.info.eid],
+        ['', 'Status', CANDIDATE.info.status],
+        ['', 'Experiment', CANDIDATE.info.experiment_name],
+        ['', 'Artifacts', 'Artifacts'],
+        ['CI', 'Job', CANDIDATE.info.ci_job.name],
+        ['', 'Triggered by', 'CI User'],
+        ['', 'Merge request', mrText],
+        ['Parameters', CANDIDATE.params[0].name, CANDIDATE.params[0].value],
+        ['', CANDIDATE.params[1].name, CANDIDATE.params[1].value],
+        ['Metrics', CANDIDATE.metrics[0].name, CANDIDATE.metrics[0].value],
+        ['', CANDIDATE.metrics[1].name, CANDIDATE.metrics[1].value],
+        ['Metadata', CANDIDATE.metadata[0].name, CANDIDATE.metadata[0].value],
+        ['', CANDIDATE.metadata[1].name, CANDIDATE.metadata[1].value],
       ].map((row, index) => [index, ...row]);
 
       it.each(expectedTable)(
         'row %s is created correctly',
-        (index, sectionLabel, label, text, href) => {
-          const row = findNthDetailRow(index);
+        (rowIndex, sectionLabel, label, text) => {
+          const row = findNthDetailRow(rowIndex);
 
-          expect(row.props()).toMatchObject({ sectionLabel, label, text, href });
+          expect(row.props()).toMatchObject({ sectionLabel, label });
+          expect(row.text()).toBe(text);
         },
       );
+
+      describe('Table links', () => {
+        const linkRows = [
+          [3, CANDIDATE.info.path_to_experiment],
+          [4, CANDIDATE.info.path_to_artifact],
+          [5, CANDIDATE.info.ci_job.path],
+          [7, CANDIDATE.info.ci_job.merge_request.path],
+        ];
+
+        it.each(linkRows)('row %s is created correctly', (rowIndex, href) => {
+          expect(findLinkInNthDetailRow(rowIndex).attributes().href).toBe(href);
+        });
+      });
+
+      describe('CI triggerer', () => {
+        it('renders user row', () => {
+          const avatar = findCiUserAvatar();
+          expect(avatar.props()).toMatchObject({
+            label: '',
+          });
+          expect(avatar.attributes().src).toEqual('/img.png');
+        });
+
+        it('renders user name', () => {
+          const nameLink = findCiUserAvatarNameLink();
+
+          expect(nameLink.attributes().href).toEqual('path/to/ci/user');
+          expect(nameLink.text()).toEqual('CI User');
+        });
+      });
+
       it('does not render params', () => {
         expect(findSectionLabel('Parameters').exists()).toBe(true);
       });

spec/frontend/ml/experiment_tracking/routes/candidates/show/mock_data.js

@@ -24,11 +24,14 @@ export const newCandidate = () => ({
       path: 'path/to/job',
       merge_request: {
         path: 'path/to/mr',
+        iid: 1,
         title: 'Some MR',
       },
       user: {
         path: 'path/to/ci/user',
+        name: 'CI User',
         username: 'ciuser',
+        avatar: '/img.png',
       },
     },
   },

spec/presenters/ml/candidate_details_presenter_spec.rb

@@ -3,8 +3,8 @@
 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(:user) { create(:user, :with_avatar) } # rubocop:disable RSpec/FactoryBot/AvoidCreate
+  let_it_be(:project) { create(:project, :private, creator: user) } # rubocop:disable RSpec/FactoryBot/AvoidCreate
   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
@@ -74,7 +74,9 @@
           'name' => 'test',
           'user' => {
             'path' => "/#{pipeline.user.username}",
-            'username' => pipeline.user.username
+            'name' => pipeline.user.name,
+            'username' => pipeline.user.username,
+            'avatar' => user.avatar_url
           }
         }
 
@@ -100,6 +102,7 @@
         it 'generates the correct ci' do
           expected_info = {
             'path' => "/#{project.full_path}/-/merge_requests/#{mr.iid}",
+            'iid' => mr.iid,
             'title' => mr.title
           }