Merge branch '60859-upload-after-delete' into 'master'

Resolve "Cannot delete and upload file of the same name in the Web IDE"

Closes #60859

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

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

@@ -62,7 +62,7 @@ export const createTempEntry = (
   new Promise(resolve => {
     const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
 
-    if (state.entries[name]) {
+    if (state.entries[name] && !state.entries[name].deleted) {
       flash(
         `The name "${name.split('/').pop()}" is already taken in this directory.`,
         'alert',
@@ -208,6 +208,7 @@ export const deleteEntry = ({ commit, dispatch, state }, path) => {
   }
 
   commit(types.DELETE_ENTRY, path);
+  dispatch('stageChange', path);
 
   dispatch('triggerFilesChange');
 };

app/assets/javascripts/ide/stores/modules/commit/actions.js

@@ -186,6 +186,8 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
 
           commit(rootTypes.CLEAR_STAGED_CHANGES, null, { root: true });
 
+          commit(rootTypes.CLEAR_REPLACED_FILES, null, { root: true });
+
           setTimeout(() => {
             commit(rootTypes.SET_LAST_COMMIT_MSG, '', { root: true });
           }, 5000);

app/assets/javascripts/ide/stores/mutation_types.js

@@ -60,6 +60,8 @@ export const CLEAR_STAGED_CHANGES = 'CLEAR_STAGED_CHANGES';
 export const STAGE_CHANGE = 'STAGE_CHANGE';
 export const UNSTAGE_CHANGE = 'UNSTAGE_CHANGE';
 
+export const CLEAR_REPLACED_FILES = 'CLEAR_REPLACED_FILES';
+
 export const UPDATE_FILE_AFTER_COMMIT = 'UPDATE_FILE_AFTER_COMMIT';
 export const ADD_PENDING_TAB = 'ADD_PENDING_TAB';
 export const REMOVE_PENDING_TAB = 'REMOVE_PENDING_TAB';

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

@@ -56,6 +56,11 @@ export default {
       stagedFiles: [],
     });
   },
+  [types.CLEAR_REPLACED_FILES](state) {
+    Object.assign(state, {
+      replacedFiles: [],
+    });
+  },
   [types.SET_ENTRIES](state, entries) {
     Object.assign(state, {
       entries,
@@ -70,6 +75,13 @@ export default {
         Object.assign(state.entries, {
           [key]: entry,
         });
+      } else if (foundEntry.deleted) {
+        Object.assign(state.entries, {
+          [key]: {
+            ...entry,
+            replaces: true,
+          },
+        });
       } else {
         const tree = entry.tree.filter(
           f => foundEntry.tree.find(e => e.path === f.path) === undefined,
@@ -144,6 +156,7 @@ export default {
       raw: file.content,
       changed: Boolean(changedFile),
       staged: false,
+      replaces: false,
       prevPath: '',
       moved: false,
       lastCommitSha: lastCommit.commit.id,

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

@@ -170,12 +170,16 @@ export default {
       entries: Object.assign(state.entries, {
         [path]: Object.assign(state.entries[path], {
           staged: true,
-          changed: false,
         }),
       }),
     });
 
     if (stagedFile) {
+      Object.assign(state, {
+        replacedFiles: state.replacedFiles.concat({
+          ...stagedFile,
+        }),
+      });
       Object.assign(stagedFile, {
         ...state.entries[path],
       });

app/assets/javascripts/ide/stores/state.js

@@ -6,6 +6,7 @@ export default () => ({
   currentMergeRequestId: '',
   changedFiles: [],
   stagedFiles: [],
+  replacedFiles: [],
   endpoints: {},
   lastCommitMsg: '',
   lastCommitPath: '',

app/assets/javascripts/ide/stores/utils.js

@@ -18,6 +18,7 @@ export const dataStructure = () => ({
   active: false,
   changed: false,
   staged: false,
+  replaces: false,
   lastCommitPath: '',
   lastCommitSha: '',
   lastCommit: {
@@ -119,7 +120,7 @@ export const commitActionForFile = file => {
     return commitActionTypes.move;
   } else if (file.deleted) {
     return commitActionTypes.delete;
-  } else if (file.tempFile) {
+  } else if (file.tempFile && !file.replaces) {
     return commitActionTypes.create;
   }
 
@@ -151,7 +152,8 @@ export const createCommitPayload = ({
     previous_path: f.prevPath === '' ? undefined : f.prevPath,
     content: f.prevPath ? null : f.content || undefined,
     encoding: f.base64 ? 'base64' : 'text',
-    last_commit_id: newBranch || f.deleted || f.prevPath ? undefined : f.lastCommitSha,
+    last_commit_id:
+      newBranch || f.deleted || f.prevPath || f.replaces ? undefined : f.lastCommitSha,
   })),
   start_sha: newBranch ? rootGetters.lastCommit.short_id : undefined,
 });

changelogs/unreleased/60859-upload-after-delete.yml

+---
+title: In WebIDE allow adding new entries of the same name as deleted entry
+merge_request: 30239
+author:
+type: fixed

spec/javascripts/ide/stores/actions_spec.js

@@ -10,6 +10,7 @@ import actions, {
   deleteEntry,
   renameEntry,
   getBranchData,
+  createTempEntry,
 } from '~/ide/stores/actions';
 import axios from '~/lib/utils/axios_utils';
 import store from '~/ide/stores';
@@ -247,18 +248,30 @@ describe('Multi-file store actions', () => {
       });
 
       it('sets tmp file as active', done => {
-        store
-          .dispatch('createTempEntry', {
+        testAction(
+          createTempEntry,
+          {
             name: 'test',
             branchId: 'mybranch',
             type: 'blob',
-          })
-          .then(f => {
-            expect(f.active).toBeTruthy();
-
-            done();
-          })
-          .catch(done.fail);
+          },
+          store.state,
+          [
+            { type: types.CREATE_TMP_ENTRY, payload: jasmine.any(Object) },
+            { type: types.TOGGLE_FILE_OPEN, payload: 'test' },
+            { type: types.ADD_FILE_TO_CHANGED, payload: 'test' },
+          ],
+          [
+            {
+              type: 'setFileActive',
+              payload: 'test',
+            },
+            {
+              type: 'triggerFilesChange',
+            },
+          ],
+          done,
+        );
       });
 
       it('creates flash message if file already exists', done => {
@@ -488,7 +501,11 @@ describe('Multi-file store actions', () => {
         'path',
         store.state,
         [{ type: types.DELETE_ENTRY, payload: 'path' }],
-        [{ type: 'burstUnusedSeal' }, { type: 'triggerFilesChange' }],
+        [
+          { type: 'burstUnusedSeal' },
+          { type: 'stageChange', payload: 'path' },
+          { type: 'triggerFilesChange' },
+        ],
         done,
       );
     });
@@ -515,7 +532,11 @@ describe('Multi-file store actions', () => {
         'testFolder/entry-to-delete',
         store.state,
         [{ type: types.DELETE_ENTRY, payload: 'testFolder/entry-to-delete' }],
-        [{ type: 'burstUnusedSeal' }, { type: 'triggerFilesChange' }],
+        [
+          { type: 'burstUnusedSeal' },
+          { type: 'stageChange', payload: 'testFolder/entry-to-delete' },
+          { type: 'triggerFilesChange' },
+        ],
         done,
       );
     });

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

@@ -315,6 +315,19 @@ describe('IDE store file mutations', () => {
       expect(localState.stagedFiles.length).toBe(1);
       expect(localState.stagedFiles[0].raw).toEqual('testing 123');
     });
+
+    it('adds already-staged file to `replacedFiles`', () => {
+      localFile.raw = 'already-staged';
+
+      mutations.STAGE_CHANGE(localState, localFile.path);
+
+      localFile.raw = 'testing 123';
+
+      mutations.STAGE_CHANGE(localState, localFile.path);
+
+      expect(localState.replacedFiles.length).toBe(1);
+      expect(localState.replacedFiles[0].raw).toEqual('already-staged');
+    });
   });
 
   describe('UNSTAGE_CHANGE', () => {

spec/javascripts/ide/stores/mutations_spec.js

@@ -79,6 +79,16 @@ describe('Multi-file store mutations', () => {
     });
   });
 
+  describe('CLEAR_REPLACED_FILES', () => {
+    it('clears replacedFiles array', () => {
+      localState.replacedFiles.push('a');
+
+      mutations.CLEAR_REPLACED_FILES(localState);
+
+      expect(localState.replacedFiles.length).toBe(0);
+    });
+  });
+
   describe('UPDATE_VIEWER', () => {
     it('sets viewer state', () => {
       mutations.UPDATE_VIEWER(localState, 'diff');
@@ -109,6 +119,62 @@ describe('Multi-file store mutations', () => {
     });
   });
 
+  describe('CREATE_TMP_ENTRY', () => {
+    beforeEach(() => {
+      localState.currentProjectId = 'gitlab-ce';
+      localState.currentBranchId = 'master';
+      localState.trees['gitlab-ce/master'] = {
+        tree: [],
+      };
+    });
+
+    it('creates temp entry in the tree', () => {
+      const tmpFile = file('test');
+      mutations.CREATE_TMP_ENTRY(localState, {
+        data: {
+          entries: {
+            test: {
+              ...tmpFile,
+              tempFile: true,
+              changed: true,
+            },
+          },
+          treeList: [tmpFile],
+        },
+        projectId: 'gitlab-ce',
+        branchId: 'master',
+      });
+
+      expect(localState.trees['gitlab-ce/master'].tree.length).toEqual(1);
+      expect(localState.entries.test.tempFile).toEqual(true);
+    });
+
+    it('marks entry as replacing previous entry if the old one has been deleted', () => {
+      const tmpFile = file('test');
+      localState.entries.test = {
+        ...tmpFile,
+        deleted: true,
+      };
+      mutations.CREATE_TMP_ENTRY(localState, {
+        data: {
+          entries: {
+            test: {
+              ...tmpFile,
+              tempFile: true,
+              changed: true,
+            },
+          },
+          treeList: [tmpFile],
+        },
+        projectId: 'gitlab-ce',
+        branchId: 'master',
+      });
+
+      expect(localState.trees['gitlab-ce/master'].tree.length).toEqual(1);
+      expect(localState.entries.test.replaces).toEqual(true);
+    });
+  });
+
   describe('UPDATE_TEMP_FLAG', () => {
     beforeEach(() => {
       localState.entries.test = {
@@ -252,6 +318,7 @@ describe('Multi-file store mutations', () => {
         permalink: `${gl.TEST_HOST}/testing-123`,
         commitsPath: `${gl.TEST_HOST}/testing-123`,
         blamePath: `${gl.TEST_HOST}/testing-123`,
+        replaces: true,
       };
       localState.entries.test = f;
       localState.changedFiles.push(f);
@@ -262,6 +329,7 @@ describe('Multi-file store mutations', () => {
       expect(f.permalink).toBe(`${gl.TEST_HOST}/test`);
       expect(f.commitsPath).toBe(`${gl.TEST_HOST}/test`);
       expect(f.blamePath).toBe(`${gl.TEST_HOST}/test`);
+      expect(f.replaces).toBe(false);
     });
   });
 

spec/javascripts/ide/stores/utils_spec.js

@@ -92,6 +92,16 @@ describe('Multi-file store utils', () => {
             path: 'deletedFile',
             deleted: true,
           },
+          {
+            ...file('renamedFile'),
+            path: 'renamedFile',
+            prevPath: 'prevPath',
+          },
+          {
+            ...file('replacingFile'),
+            path: 'replacingFile',
+            replaces: true,
+          },
         ],
         currentBranchId: 'master',
       };
@@ -131,6 +141,22 @@ describe('Multi-file store utils', () => {
             last_commit_id: undefined,
             previous_path: undefined,
           },
+          {
+            action: commitActionTypes.move,
+            file_path: 'renamedFile',
+            content: null,
+            encoding: 'text',
+            last_commit_id: undefined,
+            previous_path: 'prevPath',
+          },
+          {
+            action: commitActionTypes.update,
+            file_path: 'replacingFile',
+            content: undefined,
+            encoding: 'text',
+            last_commit_id: undefined,
+            previous_path: undefined,
+          },
         ],
         start_sha: undefined,
       });