Merge branch '61335-fix-file-icon-status' into 'master'

Resolve "Web IDE uncommitted files use wrong icon in tree"

Closes #61335

See merge request gitlab-org/gitlab-ce!31560

5 files changed, 129 insertions, 72 deletions

diff --git a/app/assets/javascripts/ide/components/file_row_extra.vue b/app/assets/javascripts/ide/components/file_row_extra.vue
index 80a6ab9598a..7254c50a568 100644
--- a/app/assets/javascripts/ide/components/file_row_extra.vue
+++ b/app/assets/javascripts/ide/components/file_row_extra.vue
@@ -87,7 +87,6 @@ export default {
       :file="file"
       :show-tooltip="true"
       :show-staged-icon="true"
-      :force-modified-icon="true"
     />
     <new-dropdown
       :type="file.type"
diff --git a/app/assets/javascripts/vue_shared/components/changed_file_icon.vue b/app/assets/javascripts/vue_shared/components/changed_file_icon.vue
index cdf2d1020ba..beb2ac09992 100644
--- a/app/assets/javascripts/vue_shared/components/changed_file_icon.vue
+++ b/app/assets/javascripts/vue_shared/components/changed_file_icon.vue
@@ -26,11 +26,6 @@ export default {
       required: false,
       default: false,
     },
-    forceModifiedIcon: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     size: {
       type: Number,
       required: false,
@@ -48,8 +43,6 @@ export default {
       // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
       const suffix = !this.file.changed && this.file.staged && !this.showStagedIcon ? '-solid' : '';
 
-      if (this.forceModifiedIcon) return `file-modified${suffix}`;
-
       return `${getCommitIconMap(this.file).icon}${suffix}`;
     },
     changedIconClass() {
@@ -88,7 +81,7 @@ export default {
     v-gl-tooltip.right
     :title="tooltipTitle"
     :class="{ 'ml-auto': isCentered }"
-    class="file-changed-icon"
+    class="file-changed-icon d-inline-block"
   >
     <icon v-if="showIcon" :name="changedIcon" :size="size" :css-classes="changedIconClass" />
   </span>
diff --git a/changelogs/unreleased/61335-fix-file-icon-status.yml b/changelogs/unreleased/61335-fix-file-icon-status.yml
new file mode 100644
index 00000000000..d524d91b246
--- /dev/null
+++ b/changelogs/unreleased/61335-fix-file-icon-status.yml
@@ -0,0 +1,5 @@
+---
+title: Fix IDE new files icon in tree
+merge_request: 31560
+author:
+type: fixed
diff --git a/spec/frontend/vue_shared/components/changed_file_icon_spec.js b/spec/frontend/vue_shared/components/changed_file_icon_spec.js
new file mode 100644
index 00000000000..806602877ef
--- /dev/null
+++ b/spec/frontend/vue_shared/components/changed_file_icon_spec.js
@@ -0,0 +1,123 @@
+import { shallowMount } from '@vue/test-utils';
+import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
+import Icon from '~/vue_shared/components/icon.vue';
+
+const changedFile = () => ({ changed: true });
+const stagedFile = () => ({ changed: false, staged: true });
+const changedAndStagedFile = () => ({ changed: true, staged: true });
+const newFile = () => ({ changed: true, tempFile: true });
+const unchangedFile = () => ({ changed: false, tempFile: false, staged: false, deleted: false });
+
+describe('Changed file icon', () => {
+  let wrapper;
+
+  const factory = (props = {}) => {
+    wrapper = shallowMount(ChangedFileIcon, {
+      propsData: {
+        file: changedFile(),
+        showTooltip: true,
+        ...props,
+      },
+      sync: false,
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  const findIcon = () => wrapper.find(Icon);
+  const findIconName = () => findIcon().props('name');
+  const findIconClasses = () =>
+    findIcon()
+      .props('cssClasses')
+      .split(' ');
+  const findTooltipText = () => wrapper.attributes('data-original-title');
+
+  it('with isCentered true, adds center class', () => {
+    factory({
+      isCentered: true,
+    });
+
+    expect(wrapper.classes('ml-auto')).toBe(true);
+  });
+
+  it('with isCentered false, does not center', () => {
+    factory({
+      isCentered: false,
+    });
+
+    expect(wrapper.classes('ml-auto')).toBe(false);
+  });
+
+  it('with showTooltip false, does not show tooltip', () => {
+    factory({
+      showTooltip: false,
+    });
+
+    expect(findTooltipText()).toBeFalsy();
+  });
+
+  describe.each`
+    file                      | iconName                 | tooltipText                           | desc
+    ${changedFile()}          | ${'file-modified'}       | ${'Unstaged modification'}            | ${'with file changed'}
+    ${stagedFile()}           | ${'file-modified-solid'} | ${'Staged modification'}              | ${'with file staged'}
+    ${changedAndStagedFile()} | ${'file-modified'}       | ${'Unstaged and staged modification'} | ${'with file changed and staged'}
+    ${newFile()}              | ${'file-addition'}       | ${'Unstaged addition'}                | ${'with file new'}
+  `('$desc', ({ file, iconName, tooltipText }) => {
+    beforeEach(() => {
+      factory({ file });
+    });
+
+    it('renders icon', () => {
+      expect(findIconName()).toBe(iconName);
+      expect(findIconClasses()).toContain(iconName);
+    });
+
+    it('renders tooltip text', () => {
+      expect(findTooltipText()).toBe(tooltipText);
+    });
+  });
+
+  describe('with file unchanged', () => {
+    beforeEach(() => {
+      factory({
+        file: unchangedFile(),
+      });
+    });
+
+    it('does not show icon', () => {
+      expect(findIcon().exists()).toBe(false);
+    });
+
+    it('does not have tooltip text', () => {
+      expect(findTooltipText()).toBe('');
+    });
+  });
+
+  it('with size set, sets icon size', () => {
+    const size = 8;
+
+    factory({
+      file: changedFile(),
+      size,
+    });
+
+    expect(findIcon().props('size')).toBe(size);
+  });
+
+  // NOTE: It looks like 'showStagedIcon' behavior is backwards to what the name suggests
+  // https://gitlab.com/gitlab-org/gitlab-ce/issues/66071
+  it.each`
+    showStagedIcon | iconName                 | desc
+    ${false}       | ${'file-modified-solid'} | ${'with showStagedIcon false, renders staged icon'}
+    ${true}        | ${'file-modified'}       | ${'with showStagedIcon true, renders regular icon'}
+  `('$desc', ({ showStagedIcon, iconName }) => {
+    factory({
+      file: stagedFile(),
+      showStagedIcon,
+    });
+
+    expect(findIconName()).toEqual(iconName);
+  });
+});
diff --git a/spec/javascripts/vue_shared/components/changed_file_icon_spec.js b/spec/javascripts/vue_shared/components/changed_file_icon_spec.js
deleted file mode 100644
index 634ba8403d5..00000000000
--- a/spec/javascripts/vue_shared/components/changed_file_icon_spec.js
+++ /dev/null
@@ -1,63 +0,0 @@
-import Vue from 'vue';
-import changedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
-import createComponent from 'spec/helpers/vue_mount_component_helper';
-
-describe('Changed file icon', () => {
-  let vm;
-
-  function factory(props = {}) {
-    const component = Vue.extend(changedFileIcon);
-
-    vm = createComponent(component, {
-      ...props,
-      file: {
-        tempFile: false,
-        changed: true,
-      },
-    });
-  }
-
-  afterEach(() => {
-    vm.$destroy();
-  });
-
-  it('centers icon', () => {
-    factory({
-      isCentered: true,
-    });
-
-    expect(vm.$el.classList).toContain('ml-auto');
-  });
-
-  describe('changedIcon', () => {
-    it('equals file-modified when not a temp file and has changes', () => {
-      factory();
-
-      expect(vm.changedIcon).toBe('file-modified');
-    });
-
-    it('equals file-addition when a temp file', () => {
-      factory();
-
-      vm.file.tempFile = true;
-
-      expect(vm.changedIcon).toBe('file-addition');
-    });
-  });
-
-  describe('changedIconClass', () => {
-    it('includes file-modified when not a temp file', () => {
-      factory();
-
-      expect(vm.changedIconClass).toContain('file-modified');
-    });
-
-    it('includes file-addition when a temp file', () => {
-      factory();
-
-      vm.file.tempFile = true;
-
-      expect(vm.changedIconClass).toContain('file-addition');
-    });
-  });
-});