Merge branch '220867-hide-status-labels-for-closed-issues-on-epics-tree' into 'master'

Hide health status labels for closed issues on epics tree

See merge request gitlab-org/gitlab!37561

doc/user/group/epics/index.md

@@ -65,7 +65,8 @@ to add an issue to an epic, reorder issues, move issues between epics, or promot
 
 ## Issue health status in Epic tree **(ULTIMATE)**
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/199184) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.10.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/199184) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.10.
+> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/220867) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.3. The health status of a closed issue will be hidden.
 
 You can report on and quickly respond to the health of individual issues and epics by setting a
 red, amber, or green [health status on an issue](../../project/issues/index.md#health-status-ultimate),

ee/app/assets/javascripts/related_items_tree/components/tree_item_body.vue

@@ -127,6 +127,13 @@ export default {
     showHealthStatus() {
       return this.item.healthStatus && this.allowIssuableHealthStatus;
     },
+    showIssueHealthStatus() {
+      return this.isIssue && this.isOpen && this.showHealthStatus;
+    },
+    showEpicHealthStatus() {
+      const { descendantCounts: { openedIssues = 0 } = {} } = this.item;
+      return this.isEpic && this.showHealthStatus && openedIssues > 0;
+    },
   },
   methods: {
     ...mapActions(['setRemoveItemModalProps']),
@@ -143,20 +150,22 @@ export default {
 </script>
 
 <template>
-  <div class="card card-slim sortable-row flex-grow-1">
+  <div class="card card-slim sortable-row gl-flex-grow-1">
     <div
-      class="item-body card-body d-flex align-items-center pr-1 pl-2 py-1"
+      class="item-body card-body gl-display-flex gl-align-items-center gl-pr-2 gl-pl-3 gl-py-2"
       :class="{
         'item-logged-out': !userSignedIn,
         'item-closed': isClosed,
       }"
     >
-      <div class="item-contents d-flex align-items-center flex-wrap flex-grow-1 flex-xl-nowrap">
-        <div class="item-title-wrapper flex-grow-1 mr-2">
-          <div class="item-title d-flex mb-0 pt-1 pb-1">
+      <div
+        class="item-contents gl-display-flex gl-align-items-center gl-flex-wrap gl-flex-grow-1 flex-xl-nowrap"
+      >
+        <div class="item-title-wrapper gl-flex-grow-1 gl-mr-3">
+          <div class="item-title gl-display-flex gl-mb-0 gl-pt-2 gl-pb-2">
             <gl-icon
               ref="stateIconMd"
-              class="d-block mr-2"
+              class="gl-display-block gl-mr-3"
               :class="stateIconClass"
               :name="stateIconName"
               :aria-label="stateText"
@@ -188,13 +197,13 @@ export default {
           </div>
 
           <div
-            class="item-meta d-flex flex-wrap mt-xl-0 flex-xl-nowrap align-items-center pb-1 pt-1 ml-4"
+            class="item-meta gl-display-flex gl-flex-wrap mt-xl-0 flex-xl-nowrap gl-align-items-center gl-py-2 gl-ml-6"
           >
-            <span class="mr-3">{{ itemHierarchy }}</span>
+            <span class="gl-mr-4">{{ itemHierarchy }}</span>
             <gl-tooltip v-if="isEpic" :target="() => $refs.countBadge">
-              <p v-if="allowSubEpics" class="font-weight-bold m-0">
+              <p v-if="allowSubEpics" class="gl-font-weight-bold gl-m-0">
                 {{ __('Epics') }} &#8226;
-                <span class="text-secondary-400 font-weight-normal"
+                <span class="text-secondary-400 gl-font-weight-normal"
                   >{{
                     sprintf(__('%{openedEpics} open, %{closedEpics} closed'), {
                       openedEpics: item.descendantCounts && item.descendantCounts.openedEpics,
@@ -203,9 +212,9 @@ export default {
                   }}
                 </span>
               </p>
-              <p class="font-weight-bold m-0">
+              <p class="gl-font-weight-bold gl-m-0">
                 {{ __('Issues') }} &#8226;
-                <span class="text-secondary-400 font-weight-normal"
+                <span class="text-secondary-400 gl-font-weight-normal"
                   >{{
                     sprintf(__('%{openedIssues} open, %{closedIssues} closed'), {
                       openedIssues: item.descendantCounts && item.descendantCounts.openedIssues,
@@ -219,17 +228,20 @@ export default {
             <div
               v-if="isEpic"
               ref="countBadge"
-              class="issue-count-badge gl-display-inline-flex text-secondary py-0 p-lg-0"
+              class="issue-count-badge text-secondary gl-display-inline-flex gl-py-0 p-lg-0"
             >
-              <span v-if="allowSubEpics" class="d-inline-flex align-items-center mr-2">
-                <gl-icon name="epic" class="mr-1" />
+              <span
+                v-if="allowSubEpics"
+                class="gl-display-inline-flex gl-align-items-center gl-mr-3"
+              >
+                <gl-icon name="epic" class="gl-mr-2" />
                 {{ totalEpicsCount }}
               </span>
               <span
-                class="d-inline-flex align-items-center mr-2"
+                class="gl-display-inline-flex gl-align-items-center gl-mr-3"
                 :class="{ 'ml-2': allowSubEpics }"
               >
-                <gl-icon name="issues" class="mr-1" />
+                <gl-icon name="issues" class="gl-mr-2" />
                 {{ totalIssuesCount }}
               </span>
             </div>
@@ -237,33 +249,39 @@ export default {
             <item-milestone
               v-if="hasMilestone"
               :milestone="item.milestone"
-              class="d-flex align-items-center item-milestone mr-3"
+              class="item-milestone gl-display-flex gl-align-items-center gl-mr-4"
             />
 
             <item-due-date
               v-if="item.dueDate"
               :date="item.dueDate"
               tooltip-placement="top"
-              css-class="item-due-date d-flex align-items-center mr-3"
+              css-class="item-due-date gl-display-flex gl-align-items-center gl-mr-4"
             />
 
             <item-weight
               v-if="item.weight"
               :weight="item.weight"
-              class="item-weight d-flex align-items-center mr-3"
+              class="item-weight gl-display-flex gl-align-items-center gl-mr-4"
               tag-name="span"
             />
 
             <item-assignees
               v-if="hasAssignees"
               :assignees="item.assignees"
-              class="item-assignees d-inline-flex align-items-center mr-3 mb-md-0 flex-xl-grow-0"
+              class="item-assignees gl-display-inline-flex gl-align-items-center gl-mr-4 mb-md-0 flex-xl-grow-0"
             />
 
-            <div v-if="showHealthStatus" class="item-health-status">
-              <epic-health-status v-if="isEpic" :health-status="item.healthStatus" />
-              <issue-health-status v-else-if="isIssue" :health-status="item.healthStatus" />
-            </div>
+            <epic-health-status
+              v-if="showEpicHealthStatus"
+              :health-status="item.healthStatus"
+              data-testid="epic-health-status"
+            />
+            <issue-health-status
+              v-if="showIssueHealthStatus"
+              :health-status="item.healthStatus"
+              data-testid="issue-health-status"
+            />
           </div>
         </div>
 
@@ -273,12 +291,12 @@ export default {
           v-gl-modal-directive="$options.itemRemoveModalId"
           :title="__('Remove')"
           :disabled="itemActionInProgress"
-          class="btn-svg btn-item-remove js-issue-item-remove-button qa-remove-issue-button align-self-start"
+          class="btn-svg btn-item-remove js-issue-item-remove-button qa-remove-issue-button gl-align-self-start"
           @click="handleRemoveClick"
         >
           <gl-icon name="close" class="btn-item-remove-icon" />
         </gl-deprecated-button>
-        <span v-if="showEmptySpacer" class="p-3"></span>
+        <span v-if="showEmptySpacer" class="gl-p-3"></span>
       </div>
     </div>
   </div>

ee/changelogs/unreleased/220867-hide-status-labels-for-closed-issues-on-epics-tree.yml

+---
+title: Hide health status labels for closed issues on epics tree
+merge_request: 37561
+author:
+type: changed

ee/spec/frontend/related_items_tree/components/related_items_tree_header_spec.js

@@ -61,7 +61,7 @@ describe('RelatedItemsTree', () => {
 
       it('returns string containing issue count based on available direct children within state', () => {
         expect(wrapper.find(GlTooltip).text()).toContain(`Issues •
-        1 open, 1 closed`);
+        2 open, 1 closed`);
       });
     });
 
@@ -232,7 +232,7 @@ describe('RelatedItemsTree', () => {
         const issuesEl = wrapper.findAll('.issue-count-badge > span').at(1);
         const issueIcon = issuesEl.find(GlIcon);
 
-        expect(issuesEl.text().trim()).toContain('2');
+        expect(issuesEl.text().trim()).toContain('3');
         expect(issueIcon.isVisible()).toBe(true);
         expect(issueIcon.props('name')).toBe('issues');
       });

ee/spec/frontend/related_items_tree/components/tree_item_body_spec.js

@@ -15,16 +15,40 @@ import ItemAssignees from '~/vue_shared/components/issue/issue_assignees.vue';
 import ItemDueDate from '~/boards/components/issue_due_date.vue';
 import ItemMilestone from '~/vue_shared/components/issue/issue_milestone.vue';
 
-import { mockParentItem, mockInitialConfig, mockQueryResponse, mockIssue1 } from '../mock_data';
+import {
+  mockParentItem,
+  mockInitialConfig,
+  mockQueryResponse,
+  mockIssue1,
+  mockClosedIssue,
+  mockEpic1 as mockOpenEpic,
+  mockEpic2 as mockClosedEpic,
+  mockEpicMeta1,
+  mockEpicMeta2,
+  mockEpicMeta3,
+} from '../mock_data';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
 
-const mockItem = {
-  ...mockIssue1,
-  type: ChildType.Issue,
-  pathIdSeparator: PathIdSeparator.Issue,
-  assignees: epicUtils.extractIssueAssignees(mockIssue1.assignees),
+let mockItem;
+
+const createIssueItem = (mockIssue = mockIssue1) => {
+  return {
+    ...mockIssue,
+    type: ChildType.Issue,
+    pathIdSeparator: PathIdSeparator.Issue,
+    assignees: epicUtils.extractIssueAssignees(mockIssue.assignees),
+  };
+};
+
+const createEpicItem = (mockEpic = mockOpenEpic, mockEpicMeta = mockEpicMeta1) => {
+  return {
+    ...mockEpic,
+    type: ChildType.Epic,
+    pathIdSeparator: PathIdSeparator.Epic,
+    ...mockEpicMeta,
+  };
 };
 
 const createComponent = (parentItem = mockParentItem, item = mockItem) => {
@@ -53,27 +77,22 @@ const createComponent = (parentItem = mockParentItem, item = mockItem) => {
   });
 };
 
-const createEpicComponent = () => {
-  const mockEpicItem = {
-    ...mockItem,
-    type: ChildType.Epic,
-    healthStatus: mockParentItem.healthStatus,
-    descendantCounts: {
-      openedEpics: 0,
-      closedEpics: 0,
-      openedIssues: 0,
-      closedIssues: 0,
-    },
-  };
-
-  return createComponent(mockParentItem, mockEpicItem);
-};
-
 describe('RelatedItemsTree', () => {
   describe('TreeItemBody', () => {
     let wrapper;
 
+    const findCountBadge = () => wrapper.find({ ref: 'countBadge' });
+    const findIssueHealthStatus = () => wrapper.find('[data-testid="issue-health-status"]');
+    const findEpicHealthStatus = () => wrapper.find('[data-testid="epic-health-status"]');
+    const enableHealthStatus = () => {
+      wrapper.vm.$store.commit('SET_INITIAL_CONFIG', {
+        ...mockInitialConfig,
+        allowIssuableHealthStatus: true,
+      });
+    };
+
     beforeEach(() => {
+      mockItem = createIssueItem();
       wrapper = createComponent();
     });
 
@@ -170,26 +189,6 @@ describe('RelatedItemsTree', () => {
         });
       });
 
-      describe('stateIconName', () => {
-        it('returns string `issues` when `item.type` value is `issue`', () => {
-          wrapper.setProps({
-            item: { ...mockItem, type: ChildType.Issue },
-          });
-
-          return wrapper.vm.$nextTick(() => {
-            expect(wrapper.vm.stateIconName).toBe('issues');
-          });
-        });
-
-        it('returns string `epic` when `item.type` value is `epic`', () => {
-          const epicItemWrapper = createEpicComponent();
-
-          expect(epicItemWrapper.vm.stateIconName).toBe('epic');
-
-          epicItemWrapper.destroy();
-        });
-      });
-
       describe('stateIconClass', () => {
         it('returns string `issue-token-state-icon-open gl-text-green-500` when `item.state` value is `opened`', () => {
           wrapper.setProps({
@@ -242,17 +241,28 @@ describe('RelatedItemsTree', () => {
         });
       });
 
-      describe('isEpic', () => {
-        it('returns false when item type is issue', () => {
-          expect(wrapper.vm.isEpic).toBe(false);
+      describe.each`
+        createItem         | itemType   | isEpic   | stateIconName
+        ${createEpicItem}  | ${'epic'}  | ${true}  | ${'epic'}
+        ${createIssueItem} | ${'issue'} | ${false} | ${'issues'}
+      `(`when dependent on item type`, ({ createItem, isEpic, stateIconName, itemType }) => {
+        beforeEach(() => {
+          mockItem = createItem();
+          wrapper = createComponent();
         });
 
-        it('returns true when item type is epic', () => {
-          const epicItemWrapper = createEpicComponent();
+        describe('isEpic', () => {
+          it(`returns ${isEpic} when item type is ${itemType}`, () => {
+            expect(wrapper.vm.isEpic).toBe(isEpic);
+          });
+        });
 
-          expect(epicItemWrapper.vm.isEpic).toBe(true);
+        describe('stateIconName', () => {
+          it(`returns string ${stateIconName}`, async () => {
+            await wrapper.vm.$nextTick();
 
-          epicItemWrapper.destroy();
+            expect(wrapper.vm.stateIconName).toBe(stateIconName);
+          });
         });
       });
     });
@@ -354,35 +364,72 @@ describe('RelatedItemsTree', () => {
         expect(removeButton.attributes('title')).toBe('Remove');
       });
 
-      it('does not render issue count badge when item is issue', () => {
-        expect(wrapper.find('.issue-count-badge').exists()).toBe(false);
-      });
-
-      it('render issue count badge when item is epic', () => {
-        const epicWrapper = createEpicComponent();
-
-        expect(epicWrapper.find('.issue-count-badge').exists()).toBe(true);
+      describe.each`
+        createItem         | countBadgeExists | itemType
+        ${createEpicItem}  | ${true}          | ${'epic'}
+        ${createIssueItem} | ${false}         | ${'issue'}
+      `('issue count badge', ({ createItem, countBadgeExists, itemType }) => {
+        beforeEach(() => {
+          mockItem = createItem();
+          wrapper = createComponent();
+        });
 
-        epicWrapper.destroy();
+        it(`${
+          countBadgeExists ? 'renders' : 'does not render'
+        } issue count badge when item type is ${itemType}`, () => {
+          expect(findCountBadge().exists()).toBe(countBadgeExists);
+        });
       });
 
-      it('renders health status when feature', () => {
-        function testExistence(exists) {
-          const healthStatus = wrapper.find('.item-health-status').exists();
+      describe('health status', () => {
+        it('renders when feature is available', async () => {
+          expect(findIssueHealthStatus().exists()).toBe(false);
 
-          expect(healthStatus).toBe(exists);
-        }
+          enableHealthStatus();
 
-        testExistence(false);
+          await wrapper.vm.$nextTick();
 
-        wrapper.vm.$store.commit('SET_INITIAL_CONFIG', {
-          ...mockInitialConfig,
-          allowIssuableHealthStatus: true,
+          expect(findIssueHealthStatus().exists()).toBe(true);
         });
 
-        return wrapper.vm.$nextTick(() => {
-          testExistence(true);
+        describe.each`
+          mockIssue          | showHealthStatus
+          ${mockIssue1}      | ${true}
+          ${mockClosedIssue} | ${false}
+        `("for '$mockIssue.state' issue", ({ mockIssue, showHealthStatus }) => {
+          beforeEach(() => {
+            mockItem = createIssueItem(mockIssue);
+            wrapper = createComponent();
+            enableHealthStatus();
+          });
+
+          it(`${showHealthStatus ? 'renders' : 'does not render'} health status`, () => {
+            expect(findIssueHealthStatus().exists()).toBe(showHealthStatus);
+          });
         });
+
+        describe.each`
+          mockEpic          | mockEpicMeta     | childIssues        | showHealthStatus
+          ${mockOpenEpic}   | ${mockEpicMeta1} | ${'open issue(s)'} | ${true}
+          ${mockOpenEpic}   | ${mockEpicMeta2} | ${'closed'}        | ${false}
+          ${mockClosedEpic} | ${mockEpicMeta1} | ${'open issue(s)'} | ${true}
+          ${mockClosedEpic} | ${mockEpicMeta2} | ${'closed issues'} | ${false}
+          ${mockClosedEpic} | ${mockEpicMeta3} | ${'no issues'}     | ${false}
+        `(
+          "for '$mockEpic.state' epic with '$childIssues'",
+          ({ mockEpic, mockEpicMeta, showHealthStatus }) => {
+            beforeEach(() => {
+              mockItem = createEpicItem(mockEpic, mockEpicMeta);
+
+              wrapper = createComponent();
+              enableHealthStatus();
+            });
+
+            it(`${showHealthStatus ? 'renders' : 'does not render'} health status`, () => {
+              expect(findEpicHealthStatus().exists()).toBe(showHealthStatus);
+            });
+          },
+        );
       });
     });
   });

ee/spec/frontend/related_items_tree/mock_data.js

 import { TEST_HOST } from 'spec/test_constants';
+import { ChildState } from 'ee/related_items_tree/constants';
 
 export const mockInitialConfig = {
   epicsEndpoint: `${TEST_HOST}/epics`,
@@ -24,7 +25,7 @@ export const mockParentItem = {
   descendantCounts: {
     openedEpics: 1,
     closedEpics: 1,
-    openedIssues: 1,
+    openedIssues: 2,
     closedIssues: 1,
   },
   healthStatus: {
@@ -62,7 +63,7 @@ export const mockEpic1 = {
   id: 'gid://gitlab/Epic/4',
   iid: '4',
   title: 'Quo ea ipsa enim perferendis at omnis officia.',
-  state: 'opened',
+  state: ChildState.Open,
   webPath: '/groups/gitlab-org/-/epics/4',
   reference: '&4',
   relationPath: '/groups/gitlab-org/-/epics/1/links/4',
@@ -88,7 +89,7 @@ export const mockEpic2 = {
   id: 'gid://gitlab/Epic/3',
   iid: '3',
   title: 'A nisi mollitia explicabo quam soluta dolor hic.',
-  state: 'closed',
+  state: ChildState.Closed,
   webPath: '/groups/gitlab-org/-/epics/3',
   reference: '&3',
   relationPath: '/groups/gitlab-org/-/epics/1/links/3',
@@ -110,12 +111,57 @@ export const mockEpic2 = {
   },
 };
 
+// Epic meta data for having some open issues
+export const mockEpicMeta1 = {
+  descendantCounts: {
+    openedEpics: 1,
+    closedEpics: 1,
+    openedIssues: 2,
+    closedIssues: 1,
+  },
+  healthStatus: {
+    issuesOnTrack: 1,
+    issuesAtRisk: 0,
+    issuesNeedingAttention: 1,
+  },
+};
+
+// Epic meta data for having no open issues
+export const mockEpicMeta2 = {
+  descendantCounts: {
+    openedEpics: 0,
+    closedEpics: 1,
+    openedIssues: 0,
+    closedIssues: 2,
+  },
+  healthStatus: {
+    issuesOnTrack: 0,
+    issuesAtRisk: 0,
+    issuesNeedingAttention: 0,
+  },
+};
+
+// Epic meta data for having no child issues
+export const mockEpicMeta3 = {
+  descendantCounts: {
+    openedEpics: 0,
+    closedEpics: 1,
+    openedIssues: 0,
+    closedIssues: 0,
+  },
+  healthStatus: {
+    issuesOnTrack: 0,
+    issuesAtRisk: 0,
+    issuesNeedingAttention: 0,
+  },
+};
+
 export const mockIssue1 = {
   iid: '8',
   epicIssueId: 'gid://gitlab/EpicIssue/3',
   title: 'Nostrum cum mollitia quia recusandae fugit deleniti voluptatem delectus.',
   closedAt: null,
-  state: 'opened',
+  state: ChildState.Open,
   createdAt: '2019-02-18T14:06:41Z',
   confidential: true,
   dueDate: '2019-06-14',
@@ -149,7 +195,7 @@ export const mockIssue2 = {
   epicIssueId: 'gid://gitlab/EpicIssue/4',
   title: 'Dismiss Cipher with no integrity',
   closedAt: null,
-  state: 'opened',
+  state: ChildState.Open,
   createdAt: '2019-02-18T14:13:05Z',
   confidential: false,
   dueDate: null,
@@ -164,12 +210,12 @@ export const mockIssue2 = {
   healthStatus: 'needsAttention',
 };
 
-export const mockIssue3 = {
+export const mockClosedIssue = {
   iid: '42',
   epicIssueId: 'gid://gitlab/EpicIssue/5',
   title: 'View closed issues in epic',
   closedAt: null,
-  state: 'closed',
+  state: ChildState.Closed,
   createdAt: '2019-02-18T14:13:05Z',
   confidential: false,
   dueDate: null,
@@ -186,7 +232,7 @@ export const mockIssue3 = {
 
 export const mockEpics = [mockEpic1, mockEpic2];
 
-export const mockIssues = [mockIssue1, mockIssue2];
+export const mockIssues = [mockIssue1, mockIssue2, mockClosedIssue];
 
 export const mockQueryResponse = {
   data: {
@@ -225,6 +271,9 @@ export const mockQueryResponse = {
             {
               node: mockIssue2,
             },
+            {
+              node: mockClosedIssue,
+            },
           ],
           pageInfo: {
             endCursor: 'def',
@@ -233,9 +282,9 @@ export const mockQueryResponse = {
         },
         descendantCounts: mockParentItem.descendantCounts,
         healthStatus: {
-          atRisk: 1,
+          atRisk: 0,
           needsAttention: 1,
-          onTrack: 0,
+          onTrack: 1,
         },
       },
     },
@@ -274,7 +323,7 @@ export const mockQueryResponse2 = {
         issues: {
           edges: [
             {
-              node: mockIssue3,
+              node: mockClosedIssue,
             },
             {
               node: mockIssue1,