Merge branch 'ide-switching-file-changes' into 'master'

Fixed the IDE not showing the correct changes

Closes #5091

See merge request gitlab-org/gitlab-ee!4805

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

@@ -19,9 +19,6 @@ export default {
     shouldHideEditor() {
       return this.activeFile && this.activeFile.binary && !this.activeFile.raw;
     },
-    activeFileChanged() {
-      return this.activeFile && this.activeFile.changed;
-    },
   },
   watch: {
     activeFile(oldVal, newVal) {
@@ -29,13 +26,6 @@ export default {
         this.initMonaco();
       }
     },
-    activeFileChanged(newVal) {
-      if (!this.editor) return;
-
-      if (!newVal && this.model) {
-        this.model.setValue(this.model.getOriginalModel().getValue());
-      }
-    },
     leftPanelCollapsed() {
       this.editor.updateDimensions();
     },
@@ -92,11 +82,13 @@ export default {
 
       this.editor.attachModel(this.model);
 
-      this.model.onChange((m) => {
-        if (this.model.file.active) {
+      this.model.onChange((model) => {
+        const { file } = this.model;
+
+        if (file.active) {
           this.changeFileContent({
-            file: this.model.file,
-            content: m.getValue(),
+            file,
+            content: model.getValue(),
           });
         }
       });

app/assets/javascripts/ide/lib/common/model_manager.js

@@ -12,9 +12,13 @@ export default class ModelManager {
     return this.models.has(path);
   }
 
+  getModel(path) {
+    return this.models.get(path);
+  }
+
   addModel(file) {
     if (this.hasCachedModel(file.path)) {
-      return this.models.get(file.path);
+      return this.getModel(file.path);
     }
 
     const model = new Model(this.monaco, file);

app/assets/javascripts/ide/lib/diff/controller.js

@@ -59,7 +59,8 @@ export default class DirtyDiffController {
 
   decorate({ data }) {
     const decorations = data.changes.map(change => getDecorator(change));
-    this.decorationsController.addDecorations(data.path, 'dirtyDiff', decorations);
+    const model = this.modelManager.getModel(data.path);
+    this.decorationsController.addDecorations(model, 'dirtyDiff', decorations);
   }
 
   dispose() {

app/assets/javascripts/ide/lib/editor.js

@@ -9,6 +9,8 @@ import gitlabTheme from 'ee/ide/lib/themes/gl_theme'; // eslint-disable-line imp
 
 export default class Editor {
   static create(monaco) {
+    if (this.editorInstance) return this.editorInstance;
+
     this.editorInstance = new Editor(monaco);
 
     return this.editorInstance;
@@ -20,18 +22,14 @@ export default class Editor {
     this.instance = null;
     this.dirtyDiffController = null;
     this.disposable = new Disposable();
-
-    this.disposable.add(
-      this.modelManager = new ModelManager(this.monaco),
-      this.decorationsController = new DecorationsController(this),
-    );
+    this.modelManager = new ModelManager(this.monaco);
+    this.decorationsController = new DecorationsController(this);
 
     this.setupMonacoTheme();
 
     this.debouncedUpdate = _.debounce(() => {
       this.updateDimensions();
     }, 200);
-    window.addEventListener('resize', this.debouncedUpdate, false);
   }
 
   createInstance(domElement) {
@@ -50,6 +48,8 @@ export default class Editor {
           this.modelManager, this.decorationsController,
         ),
       );
+
+      window.addEventListener('resize', this.debouncedUpdate, false);
     }
   }
 

ee/changelogs/unreleased/ide-switching-file-changes.yml

+---
+title: Fixed IDE not showing the correct changes and diff markers
+merge_request:
+author:
+type: fixed

spec/javascripts/repo/components/repo_editor_spec.js

@@ -2,6 +2,7 @@ import Vue from 'vue';
 import store from '~/ide/stores';
 import repoEditor from '~/ide/components/repo_editor.vue';
 import monacoLoader from '~/ide/monaco_loader';
+import Editor from '~/ide/lib/editor';
 import { file, resetStore } from '../helpers';
 
 describe('RepoEditor', () => {
@@ -32,6 +33,8 @@ describe('RepoEditor', () => {
     vm.$destroy();
 
     resetStore(vm.$store);
+
+    Editor.editorInstance.modelManager.dispose();
   });
 
   it('renders an ide container', (done) => {
@@ -58,16 +61,46 @@ describe('RepoEditor', () => {
     });
   });
 
-  describe('computed', () => {
-    describe('activeFileChanged', () => {
-      it('returns false when file has no changes', () => {
-        expect(vm.activeFileChanged).toBeFalsy();
-      });
+  describe('setupEditor', () => {
+    it('creates new model', () => {
+      spyOn(vm.editor, 'createModel').and.callThrough();
+
+      Editor.editorInstance.modelManager.dispose();
+
+      vm.setupEditor();
+
+      expect(vm.editor.createModel).toHaveBeenCalledWith(vm.$store.getters.activeFile);
+      expect(vm.model).not.toBeNull();
+    });
+
+    it('attaches model to editor', () => {
+      spyOn(vm.editor, 'attachModel').and.callThrough();
+
+      Editor.editorInstance.modelManager.dispose();
+
+      vm.setupEditor();
+
+      expect(vm.editor.attachModel).toHaveBeenCalledWith(vm.model);
+    });
+
+    it('adds callback methods', () => {
+      spyOn(vm.editor, 'onPositionChange').and.callThrough();
+
+      Editor.editorInstance.modelManager.dispose();
+
+      vm.setupEditor();
+
+      expect(vm.editor.onPositionChange).toHaveBeenCalled();
+      expect(vm.model.events.size).toBe(1);
+    });
+
+    it('updates state when model content changed', (done) => {
+      vm.model.setValue('testing 123');
 
-      it('returns true when file has changes', () => {
-        vm.$store.getters.activeFile.changed = true;
+      setTimeout(() => {
+        expect(vm.$store.getters.activeFile.content).toBe('testing 123');
 
-        expect(vm.activeFileChanged).toBeTruthy();
+        done();
       });
     });
   });

spec/javascripts/repo/lib/common/model_manager_spec.js

@@ -61,6 +61,14 @@ describe('Multi-file editor library model manager', () => {
     });
   });
 
+  describe('getModel', () => {
+    it('returns cached model', () => {
+      instance.addModel(file('path-name'));
+
+      expect(instance.getModel('path-name')).not.toBeNull();
+    });
+  });
+
   describe('dispose', () => {
     it('clears cached models', () => {
       instance.addModel(file());

spec/javascripts/repo/lib/diff/controller_spec.js

@@ -22,7 +22,7 @@ describe('Multi-file editor library dirty diff controller', () => {
       modelManager = new ModelManager(monaco);
       decorationsController = new DecorationsController(editorInstance);
 
-      model = modelManager.addModel(file());
+      model = modelManager.addModel(file('path'));
 
       controller = new DirtyDiffController(modelManager, decorationsController);
 
@@ -128,7 +128,7 @@ describe('Multi-file editor library dirty diff controller', () => {
 
       controller.decorate({ data: { changes: [], path: 'path' } });
 
-      expect(controller.decorationsController.addDecorations).toHaveBeenCalledWith('path', 'dirtyDiff', jasmine.anything());
+      expect(controller.decorationsController.addDecorations).toHaveBeenCalledWith(model, 'dirtyDiff', jasmine.anything());
     });
 
     it('adds decorations into editor', () => {

spec/javascripts/repo/lib/editor_spec.js

@@ -22,6 +22,10 @@ describe('Multi-file editor library', () => {
     expect(editor.editorInstance).not.toBeNull();
   });
 
+  it('creates instance returns cached instance', () => {
+    expect(editor.create(monaco)).toEqual(instance);
+  });
+
   describe('createInstance', () => {
     let el;
 
@@ -42,6 +46,12 @@ describe('Multi-file editor library', () => {
 
       expect(instance.dirtyDiffController).not.toBeNull();
     });
+
+    it('creates model manager', () => {
+      instance.createInstance(el);
+
+      expect(instance.modelManager).not.toBeNull();
+    });
   });
 
   describe('createModel', () => {
@@ -124,5 +134,21 @@ describe('Multi-file editor library', () => {
 
       expect(instance.instance).toBeNull();
     });
+
+    it('does not dispose modelManager', () => {
+      spyOn(instance.modelManager, 'dispose');
+
+      instance.dispose();
+
+      expect(instance.modelManager.dispose).not.toHaveBeenCalled();
+    });
+
+    it('does not dispose decorationsController', () => {
+      spyOn(instance.decorationsController, 'dispose');
+
+      instance.dispose();
+
+      expect(instance.decorationsController.dispose).not.toHaveBeenCalled();
+    });
   });
 });