Skip to content
Snippets Groups Projects
Verified Commit e40ded8c authored by Paul Slaughter's avatar Paul Slaughter :two:
Browse files

Fix discarding renamed directories in Web IDE

**How?**
In the discard, we revert renames by calling the
`renameEntry` action. Unfortunately this action
is not sufficient to rename an entry. In the new_dropdown
modal component you'll see that there's a call to
createTempEntry for the parent directory...

The solution is to move that createTempEntry
into the renameEntry action so that this action is
properly encapsulated.
parent 8cef97d0
No related branches found
No related tags found
2 merge requests!158514Fix CodeReviewMetrics worker failure with kwargs,!22943Fix discarding renamed directories in Web IDE
Pipeline #109114451 passed with warnings
......@@ -81,22 +81,11 @@ export default {
const entryName = parentPath.pop();
parentPath = parentPath.join('/');
const createPromise =
parentPath && !this.entries[parentPath]
? this.createTempEntry({ name: parentPath, type: 'tree' })
: Promise.resolve();
createPromise
.then(() =>
this.renameEntry({
path: this.entryModal.entry.path,
name: entryName,
parentPath,
}),
)
.catch(() =>
flash(__('Error creating a new path'), 'alert', document, null, false, true),
);
this.renameEntry({
path: this.entryModal.entry.path,
name: entryName,
parentPath,
});
}
} else {
this.createTempEntry({
......
......@@ -67,60 +67,53 @@ export const setResizingStatus = ({ commit }, resizing) => {
export const createTempEntry = (
{ state, commit, dispatch },
{ name, type, content = '', base64 = false, binary = false, rawPath = '' },
) =>
new Promise(resolve => {
const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
if (state.entries[name] && !state.entries[name].deleted) {
flash(
`The name "${name.split('/').pop()}" is already taken in this directory.`,
'alert',
document,
null,
false,
true,
);
resolve();
return null;
}
const data = decorateFiles({
data: [fullName],
projectId: state.currentProjectId,
branchId: state.currentBranchId,
type,
tempFile: true,
content,
base64,
binary,
rawPath,
});
const { file, parentPath } = data;
) => {
const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
if (state.entries[name] && !state.entries[name].deleted) {
flash(
`The name "${name.split('/').pop()}" is already taken in this directory.`,
'alert',
document,
null,
false,
true,
);
commit(types.CREATE_TMP_ENTRY, {
data,
projectId: state.currentProjectId,
branchId: state.currentBranchId,
});
return;
}
if (type === 'blob') {
commit(types.TOGGLE_FILE_OPEN, file.path);
commit(types.ADD_FILE_TO_CHANGED, file.path);
dispatch('setFileActive', file.path);
dispatch('triggerFilesChange');
dispatch('burstUnusedSeal');
}
const data = decorateFiles({
data: [fullName],
projectId: state.currentProjectId,
branchId: state.currentBranchId,
type,
tempFile: true,
content,
base64,
binary,
rawPath,
});
const { file, parentPath } = data;
if (parentPath && !state.entries[parentPath].opened) {
commit(types.TOGGLE_TREE_OPEN, parentPath);
}
commit(types.CREATE_TMP_ENTRY, {
data,
projectId: state.currentProjectId,
branchId: state.currentBranchId,
});
resolve(file);
if (type === 'blob') {
commit(types.TOGGLE_FILE_OPEN, file.path);
commit(types.ADD_FILE_TO_CHANGED, file.path);
dispatch('setFileActive', file.path);
dispatch('triggerFilesChange');
dispatch('burstUnusedSeal');
}
return null;
});
if (parentPath && !state.entries[parentPath].opened) {
commit(types.TOGGLE_TREE_OPEN, parentPath);
}
};
export const scrollToTab = () => {
Vue.nextTick(() => {
......@@ -225,8 +218,9 @@ export const deleteEntry = ({ commit, dispatch, state }, path) => {
const entry = state.entries[path];
const { prevPath, prevName, prevParentPath } = entry;
const isTree = entry.type === 'tree';
const prevEntry = prevPath && state.entries[prevPath];
if (prevPath) {
if (prevPath && (!prevEntry || prevEntry.deleted)) {
dispatch('renameEntry', {
path,
name: prevName,
......@@ -259,6 +253,11 @@ export const resetOpenFiles = ({ commit }) => commit(types.RESET_OPEN_FILES);
export const renameEntry = ({ dispatch, commit, state }, { path, name, parentPath }) => {
const entry = state.entries[path];
const newPath = parentPath ? `${parentPath}/${name}` : name;
const existingParent = parentPath && state.entries[parentPath];
if (parentPath && (!existingParent || existingParent.deleted)) {
dispatch('createTempEntry', { name: parentPath, type: 'tree' });
}
commit(types.RENAME_ENTRY, { path, name, parentPath });
......
---
title: Fix discarding renamed directories in Web IDE
merge_request: 22943
author:
type: fixed
......@@ -7176,9 +7176,6 @@ msgstr ""
msgid "Error Tracking"
msgstr ""
msgid "Error creating a new path"
msgstr ""
msgid "Error creating epic"
msgstr ""
......
......@@ -52,19 +52,6 @@ describe('new file modal component', () => {
expect(templateFilesEl instanceof Element).toBeTruthy();
}
});
describe('createEntryInStore', () => {
it('$emits create', () => {
spyOn(vm, 'createTempEntry');
vm.submitForm();
expect(vm.createTempEntry).toHaveBeenCalledWith({
name: 'testing',
type,
});
});
});
});
});
......@@ -149,27 +136,5 @@ describe('new file modal component', () => {
expect(flashSpy).toHaveBeenCalled();
});
it('calls createTempEntry when target path does not exist', () => {
const store = createStore();
store.state.entryModal = {
type: 'rename',
path: 'test-path/test',
entry: {
name: 'test',
type: 'blob',
path: 'test-path1/test',
},
};
vm = createComponentWithStore(Component, store).$mount();
spyOn(vm, 'createTempEntry').and.callFake(() => Promise.resolve());
vm.submitForm();
expect(vm.createTempEntry).toHaveBeenCalledWith({
name: 'test-path1',
type: 'tree',
});
});
});
});
......@@ -258,13 +258,17 @@ describe('Multi-file store actions', () => {
describe('blob', () => {
it('creates temp file', done => {
const name = 'test';
store
.dispatch('createTempEntry', {
name: 'test',
name,
branchId: 'mybranch',
type: 'blob',
})
.then(f => {
.then(() => {
const f = store.state.entries[name];
expect(f.tempFile).toBeTruthy();
expect(store.state.trees['abcproject/mybranch'].tree.length).toBe(1);
......@@ -274,13 +278,17 @@ describe('Multi-file store actions', () => {
});
it('adds tmp file to open files', done => {
const name = 'test';
store
.dispatch('createTempEntry', {
name: 'test',
name,
branchId: 'mybranch',
type: 'blob',
})
.then(f => {
.then(() => {
const f = store.state.entries[name];
expect(store.state.openFiles.length).toBe(1);
expect(store.state.openFiles[0].name).toBe(f.name);
......@@ -290,13 +298,17 @@ describe('Multi-file store actions', () => {
});
it('adds tmp file to changed files', done => {
const name = 'test';
store
.dispatch('createTempEntry', {
name: 'test',
name,
branchId: 'mybranch',
type: 'blob',
})
.then(f => {
.then(() => {
const f = store.state.entries[name];
expect(store.state.changedFiles.length).toBe(1);
expect(store.state.changedFiles[0].name).toBe(f.name);
......@@ -656,36 +668,98 @@ describe('Multi-file store actions', () => {
);
});
it('if renamed, reverts the rename before deleting', () => {
const testEntry = {
path: 'test',
name: 'test',
prevPath: 'lorem/ipsum',
prevName: 'ipsum',
prevParentPath: 'lorem',
};
describe('when renamed', () => {
let testEntry;
store.state.entries = { test: testEntry };
testAction(
deleteEntry,
testEntry.path,
store.state,
[],
[
{
type: 'renameEntry',
payload: {
path: testEntry.path,
name: testEntry.prevName,
parentPath: testEntry.prevParentPath,
},
},
{
type: 'deleteEntry',
payload: testEntry.prevPath,
},
],
);
beforeEach(() => {
testEntry = {
path: 'test',
name: 'test',
prevPath: 'test_old',
prevName: 'test_old',
prevParentPath: '',
};
store.state.entries = { test: testEntry };
});
describe('and previous does not exist', () => {
it('reverts the rename before deleting', done => {
testAction(
deleteEntry,
testEntry.path,
store.state,
[],
[
{
type: 'renameEntry',
payload: {
path: testEntry.path,
name: testEntry.prevName,
parentPath: testEntry.prevParentPath,
},
},
{
type: 'deleteEntry',
payload: testEntry.prevPath,
},
],
done,
);
});
});
describe('and previous exists', () => {
beforeEach(() => {
const oldEntry = {
path: testEntry.prevPath,
name: testEntry.prevName,
};
store.state.entries[oldEntry.path] = oldEntry;
});
it('does not revert rename before deleting', done => {
testAction(
deleteEntry,
testEntry.path,
store.state,
[{ type: types.DELETE_ENTRY, payload: testEntry.path }],
[
{ type: 'burstUnusedSeal' },
{ type: 'stageChange', payload: testEntry.path },
{ type: 'triggerFilesChange' },
],
done,
);
});
it('when previous is deleted, it reverts rename before deleting', done => {
store.state.entries[testEntry.prevPath].deleted = true;
testAction(
deleteEntry,
testEntry.path,
store.state,
[],
[
{
type: 'renameEntry',
payload: {
path: testEntry.path,
name: testEntry.prevName,
parentPath: testEntry.prevParentPath,
},
},
{
type: 'deleteEntry',
payload: testEntry.prevPath,
},
],
done,
);
});
});
});
it('bursts unused seal', done => {
......@@ -970,6 +1044,103 @@ describe('Multi-file store actions', () => {
.then(done)
.catch(done.fail);
});
describe('with file in directory', () => {
const parentPath = 'original-dir';
const newParentPath = 'new-dir';
const fileName = 'test.md';
const filePath = `${parentPath}/${fileName}`;
let rootDir;
beforeEach(() => {
const parentEntry = file(parentPath, parentPath, 'tree');
const fileEntry = file(filePath, filePath, 'blob', parentEntry);
rootDir = {
tree: [],
};
Object.assign(store.state, {
entries: {
[parentPath]: {
...parentEntry,
tree: [fileEntry],
},
[filePath]: fileEntry,
},
trees: {
'/': rootDir,
},
});
});
it('creates new directory', done => {
expect(store.state.entries[newParentPath]).toBeUndefined();
store
.dispatch('renameEntry', { path: filePath, name: fileName, parentPath: newParentPath })
.then(() => {
expect(store.state.entries[newParentPath]).toEqual(
jasmine.objectContaining({
path: newParentPath,
type: 'tree',
tree: jasmine.arrayContaining([
store.state.entries[`${newParentPath}/${fileName}`],
]),
}),
);
})
.then(done)
.catch(done.fail);
});
describe('when new directory exists', () => {
let newDir;
beforeEach(() => {
newDir = file(newParentPath, newParentPath, 'tree');
store.state.entries[newDir.path] = newDir;
rootDir.tree.push(newDir);
});
it('inserts in new directory', done => {
expect(newDir.tree).toEqual([]);
store
.dispatch('renameEntry', {
path: filePath,
name: fileName,
parentPath: newParentPath,
})
.then(() => {
expect(newDir.tree).toEqual([store.state.entries[`${newParentPath}/${fileName}`]]);
})
.then(done)
.catch(done.fail);
});
it('when new directory is deleted, it undeletes it', done => {
store.dispatch('deleteEntry', newParentPath);
expect(store.state.entries[newParentPath].deleted).toBe(true);
expect(rootDir.tree.some(x => x.path === newParentPath)).toBe(false);
store
.dispatch('renameEntry', {
path: filePath,
name: fileName,
parentPath: newParentPath,
})
.then(() => {
expect(store.state.entries[newParentPath].deleted).toBe(false);
expect(rootDir.tree.some(x => x.path === newParentPath)).toBe(true);
})
.then(done)
.catch(done.fail);
});
});
});
});
});
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment