Merge branch...

Merge branch '208151-code-review-analytics-shows-no-data-for-mrs-in-review-for-less-than-an-hour' into 'master'

Code Review Analytics: Show "< 1 hour" for MRs with review time being zero

Closes #208151

See merge request gitlab-org/gitlab!26057

changelogs/unreleased/208151-code-review-analytics-shows-no-data-for-mrs-in-review-for-less-tha.yml

+---
+title: 'Code Review Analytics: Fix review time display'
+merge_request: 26057
+author:
+type: fixed

ee/app/assets/javascripts/analytics/code_review_analytics/components/merge_request_table.vue

@@ -28,6 +28,9 @@ export default {
         timeAgo: escape(getTimeago().format(createdAt)),
       });
     },
+    showReviewTime(value) {
+      return value !== null && value !== '';
+    },
     formatReviewTime(hours) {
       if (hours >= 24) {
         const days = Math.floor(hours / 24);
@@ -113,7 +116,7 @@ export default {
     </template>
 
     <template #cell(review_time)="{ value }">
-      <template v-if="value">
+      <template v-if="showReviewTime(value)">
         {{ formatReviewTime(value) }}
       </template>
       <template v-else>

ee/spec/frontend/analytics/code_review_analytics/components/__snapshots__/merge_request_table_spec.js.snap

@@ -4,9 +4,9 @@ exports[`MergeRequestTable component template matches the snapshot 1`] = `
 <table
   aria-busy="false"
   aria-colcount="7"
-  aria-describedby="__BVID__33__caption_"
+  aria-describedby="__BVID__59__caption_"
   class="table b-table gl-table my-3 b-table-stacked-sm"
-  id="__BVID__33"
+  id="__BVID__59"
   role="table"
 >
   <!---->
@@ -80,7 +80,174 @@ exports[`MergeRequestTable component template matches the snapshot 1`] = `
   </thead>
   <tbody
     role="rowgroup"
-  />
+  >
+    <!---->
+    <tr
+      class=""
+      role="row"
+    >
+      <td
+        aria-colindex="1"
+        class="table-col d-flex align-items-center"
+        data-label="Merge Request"
+        role="cell"
+      >
+        <div>
+          <div
+            class="d-flex flex-column flex-grow align-items-end align-items-sm-start"
+          >
+            <div
+              class="mr-title str-truncated my-2"
+            >
+              <a
+                class="gl-link font-weight-bold text-plain"
+                href="https://gitlab.com/gitlab-org/gitlab/merge_requests/38062"
+                rel="noopener noreferrer"
+                target="_blank"
+              >
+                This is just a super long merge request title that does not fit into one line so it needs to be truncated
+              </a>
+            </div>
+             
+            <ul
+              class="horizontal-list list-items-separated text-secondary mb-0"
+            >
+              <li>
+                !12345
+              </li>
+               
+              <li>
+                opened 1 month ago
+              </li>
+               
+              <li>
+                <span
+                  class="d-flex align-items-center"
+                >
+                  <svg
+                    class="mr-2 gl-icon s16"
+                  >
+                    <use
+                      href="#clock"
+                    />
+                  </svg>
+                  
+            11.1
+          
+                </span>
+              </li>
+            </ul>
+          </div>
+        </div>
+      </td>
+      <td
+        aria-colindex="2"
+        class="text-right table-col d-flex align-items-center d-sm-table-cell"
+        data-label="Review time"
+        role="cell"
+      >
+        <div>
+          
+      2 days
+    
+        </div>
+      </td>
+      <td
+        aria-colindex="3"
+        class="table-col d-flex align-items-center d-sm-table-cell"
+        data-label="Author"
+        role="cell"
+      >
+        <div>
+          <a
+            class="gl-link gl-avatar-link"
+            href="https://gitlab.com/foo"
+            rel="noopener noreferrer"
+            target="_blank"
+            title="foo"
+          >
+            <div
+              class="gl-avatar gl-avatar-identicon gl-avatar-circle gl-avatar-s24 gl-avatar-identicon-bg1"
+            >
+              
+  F
+
+            </div>
+          </a>
+        </div>
+      </td>
+      <td
+        aria-colindex="4"
+        class="table-col d-flex align-items-center d-sm-table-cell"
+        data-label="Approvers"
+        role="cell"
+      >
+        <div>
+          <div>
+            <a
+              class="gl-link gl-avatar-link"
+              href="https://gitlab.com/bar"
+              rel="noopener noreferrer"
+              target="_blank"
+              title="bar"
+            >
+              <div
+                class="gl-avatar gl-avatar-identicon gl-avatar-circle gl-avatar-s24 gl-avatar-identicon-bg1"
+              >
+                
+  
+
+              </div>
+            </a>
+          </div>
+        </div>
+      </td>
+      <td
+        aria-colindex="5"
+        class="text-right table-col d-flex align-items-center d-sm-table-cell"
+        data-label="Comments"
+        role="cell"
+      >
+        <div>
+          21
+        </div>
+      </td>
+      <td
+        aria-colindex="6"
+        class="text-right table-col d-flex align-items-center d-sm-table-cell"
+        data-label="Commits"
+        role="cell"
+      >
+        <div>
+          <span>
+            7
+          </span>
+        </div>
+      </td>
+      <td
+        aria-colindex="7"
+        class="text-right table-col d-flex align-items-center d-sm-table-cell"
+        data-label="Line changes"
+        role="cell"
+      >
+        <div>
+          <span
+            class="font-weight-bold cgreen"
+          >
+             +504 
+          </span>
+           
+          <span
+            class="font-weight-bold cred"
+          >
+             -10 
+          </span>
+        </div>
+      </td>
+    </tr>
+    <!---->
+    <!---->
+  </tbody>
   <!---->
 </table>
 `;

ee/spec/frontend/analytics/code_review_analytics/components/merge_request_table_spec.js

@@ -12,21 +12,12 @@ describe('MergeRequestTable component', () => {
   let wrapper;
   let vuexStore;
 
-  const createStore = (initialState = {}, getters = {}) =>
+  const createStore = (initialState = {}) =>
     new Vuex.Store({
       state: {
         ...createState(),
         ...initialState,
       },
-      actions: {
-        setProjectId: jest.fn(),
-        setPage: jest.fn(),
-        fetchMergeRequests: jest.fn(),
-      },
-      getters: {
-        showMrCount: () => false,
-        ...getters,
-      },
     });
 
   const createComponent = store =>
@@ -35,16 +26,40 @@ describe('MergeRequestTable component', () => {
       store,
     });
 
+  const bootstrap = initialState => {
+    vuexStore = createStore(initialState);
+    wrapper = createComponent(vuexStore);
+  };
+
   afterEach(() => {
     wrapper.destroy();
   });
 
   const findTable = () => wrapper.find(GlTable);
+  const findTableRow = index =>
+    findTable()
+      .findAll('tbody tr')
+      .at(index);
+  const findReviewTimeCol = rowIndex =>
+    findTableRow(rowIndex)
+      .findAll('td')
+      .at(1);
+
+  const updateMergeRequests = (index, attrs) =>
+    mergeRequests.map((item, idx) => {
+      if (idx !== index) {
+        return item;
+      }
+
+      return {
+        ...item,
+        ...attrs,
+      };
+    });
 
   describe('template', () => {
     beforeEach(() => {
-      vuexStore = createStore({ mergeRequests });
-      wrapper = createComponent(vuexStore);
+      bootstrap({ mergeRequests });
     });
 
     it('renders the GlTable component', () => {
@@ -71,20 +86,30 @@ describe('MergeRequestTable component', () => {
 
       tableHeaders.forEach((headerText, i) => expect(headers.at(i).text()).toEqual(headerText));
     });
-  });
 
-  describe('methods', () => {
-    describe('formatReviewTime', () => {
-      it('returns "days" when review time is >= 24', () => {
-        expect(wrapper.vm.formatReviewTime(51)).toBe('2 days');
+    describe('review time column', () => {
+      it('shows "days" when the review time is >= 24', () => {
+        bootstrap({ mergeRequests: updateMergeRequests(0, { review_time: 64 }) });
+
+        expect(findReviewTimeCol(0).text()).toBe('2 days');
+      });
+
+      it('shows "hours" when review time is < 24', () => {
+        bootstrap({ mergeRequests: updateMergeRequests(0, { review_time: 18 }) });
+
+        expect(findReviewTimeCol(0).text()).toBe('18 hours');
       });
 
-      it('returns "hours" when review time is < 18', () => {
-        expect(wrapper.vm.formatReviewTime(18)).toBe('18 hours');
+      it('shows "< 1 hour" when review time is < 1', () => {
+        bootstrap({ mergeRequests: updateMergeRequests(0, { review_time: 0 }) });
+
+        expect(findReviewTimeCol(0).text()).toBe('< 1 hour');
       });
 
-      it('returns "< 1 hour" when review is < 1', () => {
-        expect(wrapper.vm.formatReviewTime(0)).toBe('< 1 hour');
+      it('shows "-" when review time is null', () => {
+        bootstrap({ mergeRequests: updateMergeRequests(0, { review_time: null }) });
+
+        expect(findReviewTimeCol(0).text()).toBe('–');
       });
     });
   });

ee/spec/frontend/analytics/code_review_analytics/mock_data.js

-// eslint-disable-next-line import/prefer-default-export
-export const mergeRequests = [
+const mergeRequests = [
   {
     title:
       'This is just a super long merge request title that does not fit into one line so it needs to be truncated',
@@ -34,3 +33,5 @@ export const mergeRequests = [
     diff_stats: { additions: 504, deletions: 10, total: 514, commits_count: 7 },
   },
 ];
+
+export default mergeRequests;