Merge branch '238631-keep-alive-regression' into 'master'

Fix regression introduced due to keep-alive

Closes #238631

See merge request gitlab-org/gitlab!43289

app/assets/javascripts/ide/components/ide_review.vue

@@ -23,26 +23,32 @@ export default {
     },
   },
   mounted() {
-    if (this.activeFile && this.activeFile.pending && !this.activeFile.deleted) {
-      this.$router.push(this.getUrlForPath(this.activeFile.path), () => {
-        this.updateViewer('editor');
-      });
-    } else if (this.activeFile && this.activeFile.deleted) {
-      this.resetOpenFiles();
-    }
-
-    this.$nextTick(() => {
-      this.updateViewer(this.currentMergeRequestId ? viewerTypes.mr : viewerTypes.diff);
-    });
+    this.initialize();
+  },
+  activated() {
+    this.initialize();
   },
   methods: {
     ...mapActions(['updateViewer', 'resetOpenFiles']),
+    initialize() {
+      if (this.activeFile && this.activeFile.pending && !this.activeFile.deleted) {
+        this.$router.push(this.getUrlForPath(this.activeFile.path), () => {
+          this.updateViewer(viewerTypes.edit);
+        });
+      } else if (this.activeFile && this.activeFile.deleted) {
+        this.resetOpenFiles();
+      }
+
+      this.$nextTick(() => {
+        this.updateViewer(this.currentMergeRequestId ? viewerTypes.mr : viewerTypes.diff);
+      });
+    },
   },
 };
 </script>
 
 <template>
-  <ide-tree-list :viewer-type="viewer" header-class="ide-review-header">
+  <ide-tree-list header-class="ide-review-header">
     <template #header>
       <div class="ide-review-button-holder">
         {{ __('Review') }}

app/assets/javascripts/ide/components/ide_tree.vue

 <script>
 import { mapState, mapGetters, mapActions } from 'vuex';
-import { modalTypes } from '../constants';
+import { modalTypes, viewerTypes } from '../constants';
 import IdeTreeList from './ide_tree_list.vue';
 import Upload from './new_dropdown/upload.vue';
 import NewEntryButton from './new_dropdown/button.vue';
@@ -18,15 +18,10 @@ export default {
     ...mapGetters(['currentProject', 'currentTree', 'activeFile', 'getUrlForPath']),
   },
   mounted() {
-    if (!this.activeFile) return;
-
-    if (this.activeFile.pending && !this.activeFile.deleted) {
-      this.$router.push(this.getUrlForPath(this.activeFile.path), () => {
-        this.updateViewer('editor');
-      });
-    } else if (this.activeFile.deleted) {
-      this.resetOpenFiles();
-    }
+    this.initialize();
+  },
+  activated() {
+    this.initialize();
   },
   methods: {
     ...mapActions(['updateViewer', 'createTempEntry', 'resetOpenFiles']),
@@ -36,12 +31,27 @@ export default {
     createNewFolder() {
       this.$refs.newModal.open(modalTypes.tree);
     },
+    initialize() {
+      this.$nextTick(() => {
+        this.updateViewer(viewerTypes.edit);
+      });
+
+      if (!this.activeFile) return;
+
+      if (this.activeFile.pending && !this.activeFile.deleted) {
+        this.$router.push(this.getUrlForPath(this.activeFile.path), () => {
+          this.updateViewer(viewerTypes.edit);
+        });
+      } else if (this.activeFile.deleted) {
+        this.resetOpenFiles();
+      }
+    },
   },
 };
 </script>
 
 <template>
-  <ide-tree-list viewer-type="editor">
+  <ide-tree-list>
     <template #header>
       {{ __('Edit') }}
       <div class="ide-tree-actions ml-auto d-flex">

app/assets/javascripts/ide/components/ide_tree_list.vue

@@ -12,10 +12,6 @@ export default {
     FileTree,
   },
   props: {
-    viewerType: {
-      type: String,
-      required: true,
-    },
     headerClass: {
       type: String,
       required: false,
@@ -29,11 +25,8 @@ export default {
       return !this.currentTree || this.currentTree.loading;
     },
   },
-  mounted() {
-    this.updateViewer(this.viewerType);
-  },
   methods: {
-    ...mapActions(['updateViewer', 'toggleTreeOpen']),
+    ...mapActions(['toggleTreeOpen']),
   },
   IdeFileRow,
 };

app/assets/javascripts/ide/components/repo_commit_section.vue

@@ -26,28 +26,34 @@ export default {
     },
   },
   mounted() {
-    const file =
-      this.lastOpenedFile && this.lastOpenedFile.type !== 'tree'
-        ? this.lastOpenedFile
-        : this.activeFile;
-
-    if (!file) return;
-
-    this.openPendingTab({
-      file,
-      keyPrefix: file.staged ? stageKeys.staged : stageKeys.unstaged,
-    })
-      .then(changeViewer => {
-        if (changeViewer) {
-          this.updateViewer('diff');
-        }
-      })
-      .catch(e => {
-        throw e;
-      });
+    this.initialize();
+  },
+  activated() {
+    this.initialize();
   },
   methods: {
     ...mapActions(['openPendingTab', 'updateViewer', 'updateActivityBarView']),
+    initialize() {
+      const file =
+        this.lastOpenedFile && this.lastOpenedFile.type !== 'tree'
+          ? this.lastOpenedFile
+          : this.activeFile;
+
+      if (!file) return;
+
+      this.openPendingTab({
+        file,
+        keyPrefix: file.staged ? stageKeys.staged : stageKeys.unstaged,
+      })
+        .then(changeViewer => {
+          if (changeViewer) {
+            this.updateViewer('diff');
+          }
+        })
+        .catch(e => {
+          throw e;
+        });
+    },
   },
   stageKeys,
 };

spec/frontend/helpers/keep_alive_component_helper.js

+import Vue from 'vue';
+
+export function keepAlive(KeptAliveComponent) {
+  return Vue.extend({
+    components: {
+      KeptAliveComponent,
+    },
+    data() {
+      return {
+        view: 'KeptAliveComponent',
+      };
+    },
+    methods: {
+      async activate() {
+        this.view = 'KeptAliveComponent';
+        await this.$nextTick();
+      },
+      async deactivate() {
+        this.view = 'div';
+        await this.$nextTick();
+      },
+      async reactivate() {
+        await this.deactivate();
+        await this.activate();
+      },
+    },
+    template: `<keep-alive><component :is="view"></component></keep-alive>`,
+  });
+}

spec/frontend/helpers/keep_alive_component_helper_spec.js

+import { mount } from '@vue/test-utils';
+import { keepAlive } from './keep_alive_component_helper';
+
+const component = {
+  template: '<div>Test Component</div>',
+};
+
+describe('keepAlive', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    wrapper = mount(keepAlive(component));
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('converts a component to a keep-alive component', async () => {
+    const { element } = wrapper.find(component);
+
+    await wrapper.vm.deactivate();
+    expect(wrapper.find(component).exists()).toBe(false);
+
+    await wrapper.vm.activate();
+
+    // assert that when the component is destroyed and re-rendered, the
+    // newly rendered component has the reference to the old component
+    // (i.e. the old component was deactivated and activated)
+    expect(wrapper.find(component).element).toBe(element);
+  });
+});

spec/frontend/ide/components/ide_review_spec.js

 import Vue from 'vue';
+import Vuex from 'vuex';
+import { createLocalVue, mount } from '@vue/test-utils';
 import IdeReview from '~/ide/components/ide_review.vue';
+import EditorModeDropdown from '~/ide/components/editor_mode_dropdown.vue';
 import { createStore } from '~/ide/stores';
-import { createComponentWithStore } from '../../helpers/vue_mount_component_helper';
 import { trimText } from '../../helpers/text_helper';
+import { keepAlive } from '../../helpers/keep_alive_component_helper';
 import { file } from '../helpers';
 import { projectData } from '../mock_data';
 
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
 describe('IDE review mode', () => {
-  const Component = Vue.extend(IdeReview);
-  let vm;
+  let wrapper;
   let store;
 
   beforeEach(() => {
@@ -21,15 +26,53 @@ describe('IDE review mode', () => {
       loading: false,
     });
 
-    vm = createComponentWithStore(Component, store).$mount();
+    wrapper = mount(keepAlive(IdeReview), {
+      store,
+      localVue,
+    });
   });
 
   afterEach(() => {
-    vm.$destroy();
+    wrapper.destroy();
   });
 
   it('renders list of files', () => {
-    expect(vm.$el.textContent).toContain('fileName');
+    expect(wrapper.text()).toContain('fileName');
+  });
+
+  describe('activated', () => {
+    let inititializeSpy;
+
+    beforeEach(async () => {
+      inititializeSpy = jest.spyOn(wrapper.find(IdeReview).vm, 'initialize');
+      store.state.viewer = 'editor';
+
+      await wrapper.vm.reactivate();
+    });
+
+    it('re initializes the component', () => {
+      expect(inititializeSpy).toHaveBeenCalled();
+    });
+
+    it('updates viewer to "diff" by default', () => {
+      expect(store.state.viewer).toBe('diff');
+    });
+
+    describe('merge request is defined', () => {
+      beforeEach(async () => {
+        store.state.currentMergeRequestId = '1';
+        store.state.projects.abcproject.mergeRequests['1'] = {
+          iid: 123,
+          web_url: 'testing123',
+        };
+
+        await wrapper.vm.reactivate();
+      });
+
+      it('updates viewer to "mrdiff"', async () => {
+        expect(store.state.viewer).toBe('mrdiff');
+      });
+    });
   });
 
   describe('merge request', () => {
@@ -40,32 +83,27 @@ describe('IDE review mode', () => {
         web_url: 'testing123',
       };
 
-      return vm.$nextTick();
+      return wrapper.vm.$nextTick();
     });
 
     it('renders edit dropdown', () => {
-      expect(vm.$el.querySelector('.btn')).not.toBe(null);
+      expect(wrapper.find(EditorModeDropdown).exists()).toBe(true);
     });
 
-    it('renders merge request link & IID', () => {
+    it('renders merge request link & IID', async () => {
       store.state.viewer = 'mrdiff';
 
-      return vm.$nextTick(() => {
-        const link = vm.$el.querySelector('.ide-review-sub-header');
+      await wrapper.vm.$nextTick();
 
-        expect(link.querySelector('a').getAttribute('href')).toBe('testing123');
-        expect(trimText(link.textContent)).toBe('Merge request (!123)');
-      });
+      expect(trimText(wrapper.text())).toContain('Merge request (!123)');
     });
 
-    it('changes text to latest changes when viewer is not mrdiff', () => {
+    it('changes text to latest changes when viewer is not mrdiff', async () => {
       store.state.viewer = 'diff';
 
-      return vm.$nextTick(() => {
-        expect(trimText(vm.$el.querySelector('.ide-review-sub-header').textContent)).toBe(
-          'Latest changes',
-        );
-      });
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.text()).toContain('Latest changes');
     });
   });
 });

spec/frontend/ide/components/ide_tree_list_spec.js

@@ -38,15 +38,9 @@ describe('IDE tree list', () => {
     beforeEach(() => {
       bootstrapWithTree();
 
-      jest.spyOn(vm, 'updateViewer');
-
       vm.$mount();
     });
 
-    it('updates viewer on mount', () => {
-      expect(vm.updateViewer).toHaveBeenCalledWith('edit');
-    });
-
     it('renders loading indicator', done => {
       store.state.trees['abcproject/master'].loading = true;
 
@@ -67,8 +61,6 @@ describe('IDE tree list', () => {
     beforeEach(() => {
       bootstrapWithTree(emptyBranchTree);
 
-      jest.spyOn(vm, 'updateViewer');
-
       vm.$mount();
     });
 

spec/frontend/ide/components/ide_tree_spec.js

 import Vue from 'vue';
+import Vuex from 'vuex';
+import { mount, createLocalVue } from '@vue/test-utils';
 import IdeTree from '~/ide/components/ide_tree.vue';
 import { createStore } from '~/ide/stores';
-import { createComponentWithStore } from '../../helpers/vue_mount_component_helper';
+import { keepAlive } from '../../helpers/keep_alive_component_helper';
 import { file } from '../helpers';
 import { projectData } from '../mock_data';
 
-describe('IdeRepoTree', () => {
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+describe('IdeTree', () => {
   let store;
-  let vm;
+  let wrapper;
 
   beforeEach(() => {
     store = createStore();
 
-    const IdeRepoTree = Vue.extend(IdeTree);
-
     store.state.currentProjectId = 'abcproject';
     store.state.currentBranchId = 'master';
     store.state.projects.abcproject = { ...projectData };
@@ -22,14 +25,36 @@ describe('IdeRepoTree', () => {
       loading: false,
     });
 
-    vm = createComponentWithStore(IdeRepoTree, store).$mount();
+    wrapper = mount(keepAlive(IdeTree), {
+      store,
+      localVue,
+    });
   });
 
   afterEach(() => {
-    vm.$destroy();
+    wrapper.destroy();
   });
 
   it('renders list of files', () => {
-    expect(vm.$el.textContent).toContain('fileName');
+    expect(wrapper.text()).toContain('fileName');
+  });
+
+  describe('activated', () => {
+    let inititializeSpy;
+
+    beforeEach(async () => {
+      inititializeSpy = jest.spyOn(wrapper.find(IdeTree).vm, 'initialize');
+      store.state.viewer = 'diff';
+
+      await wrapper.vm.reactivate();
+    });
+
+    it('re initializes the component', () => {
+      expect(inititializeSpy).toHaveBeenCalled();
+    });
+
+    it('updates viewer to "editor" by default', () => {
+      expect(store.state.viewer).toBe('editor');
+    });
   });
 });

spec/frontend/ide/components/repo_commit_section_spec.js

 import { mount } from '@vue/test-utils';
 import { createStore } from '~/ide/stores';
 import { createRouter } from '~/ide/ide_router';
+import { keepAlive } from '../../helpers/keep_alive_component_helper';
 import RepoCommitSection from '~/ide/components/repo_commit_section.vue';
 import EmptyState from '~/ide/components/commit_sidebar/empty_state.vue';
 import { stageKeys } from '~/ide/constants';
@@ -14,7 +15,7 @@ describe('RepoCommitSection', () => {
   let store;
 
   function createComponent() {
-    wrapper = mount(RepoCommitSection, { store });
+    wrapper = mount(keepAlive(RepoCommitSection), { store });
   }
 
   function setupDefaultState() {
@@ -64,6 +65,7 @@ describe('RepoCommitSection', () => {
 
   afterEach(() => {
     wrapper.destroy();
+    wrapper = null;
   });
 
   describe('empty state', () => {
@@ -168,4 +170,21 @@ describe('RepoCommitSection', () => {
       expect(wrapper.find(EmptyState).exists()).toBe(false);
     });
   });
+
+  describe('activated', () => {
+    let inititializeSpy;
+
+    beforeEach(async () => {
+      createComponent();
+
+      inititializeSpy = jest.spyOn(wrapper.find(RepoCommitSection).vm, 'initialize');
+      store.state.viewer = 'diff';
+
+      await wrapper.vm.reactivate();
+    });
+
+    it('re initializes the component', () => {
+      expect(inititializeSpy).toHaveBeenCalled();
+    });
+  });
 });