Check for file deleted and readded

In getFileData, getRawFileData actions, and SET_FILE_RAW_DATA mutation,
ignore the early returns for when a file is deleted and readded. This
fixes issues with incorrect diff appearing because raw data wasn't
loaded correctly for deleted and readded files.

app/assets/javascripts/ide/stores/actions/file.js

@@ -61,8 +61,10 @@ export const getFileData = (
   { path, makeFileActive = true, openFile = makeFileActive },
 ) => {
   const file = state.entries[path];
+  const fileDeletedAndReadded = getters.isFileDeletedAndReadded(path);
 
-  if (file.raw || (file.tempFile && !file.prevPath)) return Promise.resolve();
+  if (file.raw || (file.tempFile && !file.prevPath && !fileDeletedAndReadded))
+    return Promise.resolve();
 
   commit(types.TOGGLE_LOADING, { entry: file });
 
@@ -102,11 +104,16 @@ export const setFileMrChange = ({ commit }, { file, mrChange }) => {
 
 export const getRawFileData = ({ state, commit, dispatch, getters }, { path }) => {
   const file = state.entries[path];
+  const stagedFile = state.stagedFiles.find(f => f.path === path);
+
   return new Promise((resolve, reject) => {
+    const fileDeletedAndReadded = getters.isFileDeletedAndReadded(path);
     service
-      .getRawFileData(file)
+      .getRawFileData(fileDeletedAndReadded ? stagedFile : file)
       .then(raw => {
-        if (!(file.tempFile && !file.prevPath)) commit(types.SET_FILE_RAW_DATA, { file, raw });
+        if (!(file.tempFile && !file.prevPath && !fileDeletedAndReadded))
+          commit(types.SET_FILE_RAW_DATA, { file, raw, fileDeletedAndReadded });
+
         if (file.mrChange && file.mrChange.new_file === false) {
           const baseSha =
             (getters.currentMergeRequest && getters.currentMergeRequest.baseCommitSha) || '';
@@ -151,7 +158,7 @@ export const changeFileContent = ({ commit, dispatch, state }, { path, content }
 
   if (file.changed && indexOfChangedFile === -1) {
     commit(types.ADD_FILE_TO_CHANGED, path);
-  } else if (!file.changed && indexOfChangedFile !== -1) {
+  } else if (!file.changed && !file.tempFile && indexOfChangedFile !== -1) {
     commit(types.REMOVE_FILE_FROM_CHANGED, path);
   }
 

app/assets/javascripts/ide/stores/mutations/file.js

@@ -54,27 +54,29 @@ export default {
       }
     });
   },
-  [types.SET_FILE_RAW_DATA](state, { file, raw }) {
+  [types.SET_FILE_RAW_DATA](state, { file, raw, fileDeletedAndReadded = false }) {
     const openPendingFile = state.openFiles.find(
-      f => f.path === file.path && f.pending && !(f.tempFile && !f.prevPath),
+      f =>
+        f.path === file.path && f.pending && !(f.tempFile && !f.prevPath && !fileDeletedAndReadded),
     );
+    const stagedFile = state.stagedFiles.find(f => f.path === file.path);
 
-    if (file.tempFile && file.content === '') {
-      Object.assign(state.entries[file.path], {
-        content: raw,
-      });
+    if (file.tempFile && file.content === '' && !fileDeletedAndReadded) {
+      Object.assign(state.entries[file.path], { content: raw });
+    } else if (fileDeletedAndReadded) {
+      Object.assign(stagedFile, { raw });
     } else {
-      Object.assign(state.entries[file.path], {
-        raw,
-      });
+      Object.assign(state.entries[file.path], { raw });
     }
 
     if (!openPendingFile) return;
 
     if (!openPendingFile.tempFile) {
       openPendingFile.raw = raw;
-    } else if (openPendingFile.tempFile) {
+    } else if (openPendingFile.tempFile && !fileDeletedAndReadded) {
       openPendingFile.content = raw;
+    } else if (fileDeletedAndReadded) {
+      Object.assign(stagedFile, { raw });
     }
   },
   [types.SET_FILE_BASE_RAW_DATA](state, { file, baseRaw }) {

changelogs/unreleased/39498-part-4.yml

+---
+title: "Web IDE: Fix Incorrect diff of deletion and addition of the same file"
+merge_request: 21680
+author:
+type: fixed

spec/frontend/ide/stores/actions/file_spec.js

@@ -201,6 +201,53 @@ describe('IDE store file actions', () => {
       };
     });
 
+    describe('call to service', () => {
+      const callExpectation = serviceCalled => {
+        store.dispatch('getFileData', { path: localFile.path });
+
+        if (serviceCalled) {
+          expect(service.getFileData).toHaveBeenCalled();
+        } else {
+          expect(service.getFileData).not.toHaveBeenCalled();
+        }
+      };
+
+      beforeEach(() => {
+        service.getFileData.mockImplementation(() => new Promise(() => {}));
+      });
+
+      it("isn't called if file.raw exists", () => {
+        localFile.raw = 'raw data';
+
+        callExpectation(false);
+      });
+
+      it("isn't called if file is a tempFile", () => {
+        localFile.raw = '';
+        localFile.tempFile = true;
+
+        callExpectation(false);
+      });
+
+      it('is called if file is a tempFile but also renamed', () => {
+        localFile.raw = '';
+        localFile.tempFile = true;
+        localFile.prevPath = 'old_path';
+
+        callExpectation(true);
+      });
+
+      it('is called if tempFile but file was deleted and readded', () => {
+        localFile.raw = '';
+        localFile.tempFile = true;
+        localFile.prevPath = 'old_path';
+
+        store.state.stagedFiles = [{ ...localFile, deleted: true }];
+
+        callExpectation(true);
+      });
+    });
+
     describe('success', () => {
       beforeEach(() => {
         mock.onGet(`${RELATIVE_URL_ROOT}/test/test/7297abc/${localFile.path}`).replyOnce(
@@ -331,10 +378,10 @@ describe('IDE store file actions', () => {
         mock.onGet(`${RELATIVE_URL_ROOT}/test/test/7297abc/${localFile.path}`).networkError();
       });
 
-      it('dispatches error action', done => {
+      it('dispatches error action', () => {
         const dispatch = jest.fn();
 
-        actions
+        return actions
           .getFileData(
             { state: store.state, commit() {}, dispatch, getters: store.getters },
             { path: localFile.path },
@@ -349,10 +396,7 @@ describe('IDE store file actions', () => {
                 makeFileActive: true,
               },
             });
-
-            done();
-          })
-          .catch(done.fail);
+          });
       });
     });
   });
@@ -445,12 +489,14 @@ describe('IDE store file actions', () => {
         mock.onGet(/(.*)/).networkError();
       });
 
-      it('dispatches error action', done => {
+      it('dispatches error action', () => {
         const dispatch = jest.fn();
 
-        actions
-          .getRawFileData({ state: store.state, commit() {}, dispatch }, { path: tmpFile.path })
-          .then(done.fail)
+        return actions
+          .getRawFileData(
+            { state: store.state, commit() {}, dispatch, getters: store.getters },
+            { path: tmpFile.path },
+          )
           .catch(() => {
             expect(dispatch).toHaveBeenCalledWith('setErrorMessage', {
               text: 'An error occurred whilst loading the file content.',
@@ -460,8 +506,6 @@ describe('IDE store file actions', () => {
                 path: tmpFile.path,
               },
             });
-
-            done();
           });
       });
     });

spec/frontend/ide/stores/mutations/file_spec.js

@@ -11,7 +11,7 @@ describe('IDE store file mutations', () => {
   beforeEach(() => {
     localStore = createStore();
     localState = localStore.state;
-    localFile = { ...file(), type: 'blob' };
+    localFile = { ...file('file'), type: 'blob', content: 'original' };
 
     localState.entries[localFile.path] = localFile;
   });
@@ -139,35 +139,68 @@ describe('IDE store file mutations', () => {
   });
 
   describe('SET_FILE_RAW_DATA', () => {
-    it('sets raw data', () => {
+    const callMutationForFile = f => {
       mutations.SET_FILE_RAW_DATA(localState, {
-        file: localFile,
+        file: f,
         raw: 'testing',
+        fileDeletedAndReadded: localStore.getters.isFileDeletedAndReadded(localFile.path),
       });
+    };
+
+    it('sets raw data', () => {
+      callMutationForFile(localFile);
 
       expect(localFile.raw).toBe('testing');
     });
 
+    it('sets raw data to stagedFile if file was deleted and readded', () => {
+      localState.stagedFiles = [{ ...localFile, deleted: true }];
+      localFile.tempFile = true;
+
+      callMutationForFile(localFile);
+
+      expect(localFile.raw).toBeFalsy();
+      expect(localState.stagedFiles[0].raw).toBe('testing');
+    });
+
+    it("sets raw data to a file's content if tempFile is empty", () => {
+      localFile.tempFile = true;
+      localFile.content = '';
+
+      callMutationForFile(localFile);
+
+      expect(localFile.raw).toBeFalsy();
+      expect(localFile.content).toBe('testing');
+    });
+
     it('adds raw data to open pending file', () => {
       localState.openFiles.push({ ...localFile, pending: true });
 
-      mutations.SET_FILE_RAW_DATA(localState, {
-        file: localFile,
-        raw: 'testing',
-      });
+      callMutationForFile(localFile);
 
       expect(localState.openFiles[0].raw).toBe('testing');
     });
 
-    it('does not add raw data to open pending tempFile file', () => {
-      localState.openFiles.push({ ...localFile, pending: true, tempFile: true });
+    it('sets raw to content of a renamed tempFile', () => {
+      localFile.tempFile = true;
+      localFile.prevPath = 'old_path';
+      localState.openFiles.push({ ...localFile, pending: true });
 
-      mutations.SET_FILE_RAW_DATA(localState, {
-        file: localFile,
-        raw: 'testing',
-      });
+      callMutationForFile(localFile);
 
       expect(localState.openFiles[0].raw).not.toBe('testing');
+      expect(localState.openFiles[0].content).toBe('testing');
+    });
+
+    it('adds raw data to a staged deleted file if unstaged change has a tempFile of the same name', () => {
+      localFile.tempFile = true;
+      localState.openFiles.push({ ...localFile, pending: true });
+      localState.stagedFiles = [{ ...localFile, deleted: true }];
+
+      callMutationForFile(localFile);
+
+      expect(localFile.raw).toBeFalsy();
+      expect(localState.stagedFiles[0].raw).toBe('testing');
     });
   });