Merge branch 'pb-reduce-complexity-of-computed-props-pipelines-table' into 'master'

Reduce the complexity of computed props in pipelines_table_row

See merge request gitlab-org/gitlab!53904

app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table_row.vue

@@ -12,11 +12,6 @@ import PipelineUrl from './pipeline_url.vue';
 import PipelineTriggerer from './pipeline_triggerer.vue';
 import PipelinesTimeago from './time_ago.vue';
 
-/**
- * Pipeline table row.
- *
- * Given the received object renders a table row in the pipelines' table.
- */
 export default {
   i18n: {
     cancelTitle: __('Cancel'),
@@ -127,116 +122,30 @@ export default {
 
       return commitAuthorInformation;
     },
-
-    /**
-     * If provided, returns the commit tag.
-     * Needed to render the commit component column.
-     *
-     * @returns {String|Undefined}
-     */
     commitTag() {
-      if (this.pipeline.ref && this.pipeline.ref.tag) {
-        return this.pipeline.ref.tag;
-      }
-      return undefined;
+      return this.pipeline?.ref?.tag;
     },
-
-    /**
-     * If provided, returns the commit ref.
-     * Needed to render the commit component column.
-     *
-     * Matches `path` prop sent in the API to `ref_url` prop needed
-     * in the commit component.
-     *
-     * @returns {Object|Undefined}
-     */
     commitRef() {
-      if (this.pipeline.ref) {
-        return Object.keys(this.pipeline.ref).reduce((accumulator, prop) => {
-          if (prop === 'path') {
-            accumulator.ref_url = this.pipeline.ref[prop];
-          } else {
-            accumulator[prop] = this.pipeline.ref[prop];
-          }
-          return accumulator;
-        }, {});
-      }
-
-      return undefined;
+      return this.pipeline?.ref;
     },
-
-    /**
-     * If provided, returns the commit url.
-     * Needed to render the commit component column.
-     *
-     * @returns {String|Undefined}
-     */
     commitUrl() {
-      if (this.pipeline.commit && this.pipeline.commit.commit_path) {
-        return this.pipeline.commit.commit_path;
-      }
-      return undefined;
+      return this.pipeline?.commit?.commit_path;
     },
-
-    /**
-     * If provided, returns the commit short sha.
-     * Needed to render the commit component column.
-     *
-     * @returns {String|Undefined}
-     */
     commitShortSha() {
-      if (this.pipeline.commit && this.pipeline.commit.short_id) {
-        return this.pipeline.commit.short_id;
-      }
-      return undefined;
+      return this.pipeline?.commit?.short_id;
     },
-
-    /**
-     * If provided, returns the commit title.
-     * Needed to render the commit component column.
-     *
-     * @returns {String|Undefined}
-     */
     commitTitle() {
-      if (this.pipeline.commit && this.pipeline.commit.title) {
-        return this.pipeline.commit.title;
-      }
-      return undefined;
+      return this.pipeline?.commit?.title;
     },
-
-    /**
-     * Timeago components expects a number
-     *
-     * @return {type}  description
-     */
     pipelineDuration() {
-      if (this.pipeline.details && this.pipeline.details.duration) {
-        return this.pipeline.details.duration;
-      }
-
-      return 0;
+      return this.pipeline?.details?.duration ?? 0;
     },
-
-    /**
-     * Timeago component expects a String.
-     *
-     * @return {String}
-     */
     pipelineFinishedAt() {
-      if (this.pipeline.details && this.pipeline.details.finished_at) {
-        return this.pipeline.details.finished_at;
-      }
-
-      return '';
+      return this.pipeline?.details?.finished_at ?? '';
     },
-
     pipelineStatus() {
-      if (this.pipeline.details && this.pipeline.details.status) {
-        return this.pipeline.details.status;
-      }
-      return {};
+      return this.pipeline?.details?.status ?? {};
     },
-
     displayPipelineActions() {
       return (
         this.pipeline.flags.retryable ||
@@ -245,11 +154,9 @@ export default {
         this.pipeline.details.artifacts.length
       );
     },
-
     isChildView() {
       return this.viewType === 'child';
     },
-
     isCancelling() {
       return this.cancelingPipeline === this.pipeline.id;
     },

app/assets/javascripts/vue_shared/components/commit.vue

@@ -133,6 +133,9 @@ export default {
         ? sprintf(__("%{username}'s avatar"), { username: this.author.username })
         : null;
     },
+    refUrl() {
+      return this.commitRef.ref_url || this.commitRef.path;
+    },
   },
 };
 </script>
@@ -156,9 +159,10 @@ export default {
       <gl-link
         v-else
         v-gl-tooltip
-        :href="commitRef.ref_url"
+        :href="refUrl"
         :title="commitRef.name"
         class="ref-name"
+        data-testid="ref-name"
         >{{ commitRef.name }}</gl-link
       >
     </template>

spec/frontend/vue_shared/components/commit_spec.js

@@ -2,6 +2,7 @@ import { shallowMount } from '@vue/test-utils';
 import { GlIcon } from '@gitlab/ui';
 import CommitComponent from '~/vue_shared/components/commit.vue';
 import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
 describe('Commit component', () => {
   let props;
@@ -13,11 +14,14 @@ describe('Commit component', () => {
   };
 
   const findUserAvatar = () => wrapper.find(UserAvatarLink);
+  const findRefName = () => wrapper.findByTestId('ref-name');
 
   const createComponent = (propsData) => {
-    wrapper = shallowMount(CommitComponent, {
+    wrapper = extendedWrapper(
+      shallowMount(CommitComponent, {
         propsData,
-    });
+      }),
+    );
   };
 
   afterEach(() => {
@@ -223,4 +227,20 @@ describe('Commit component', () => {
       expect(wrapper.find('.ref-name').exists()).toBe(false);
     });
   });
+
+  describe('When commitRef has a path property instead of ref_url property', () => {
+    it('should render path as href attribute', () => {
+      props = {
+        commitRef: {
+          name: 'master',
+          path: 'http://localhost/namespace2/gitlabhq/tree/master',
+        },
+      };
+
+      createComponent(props);
+
+      expect(findRefName().exists()).toBe(true);
+      expect(findRefName().attributes('href')).toBe(props.commitRef.path);
+    });
+  });
 });