From f6a038b38ae6c256fb9d1f1cbe184b0d8bbb5fde Mon Sep 17 00:00:00 2001 From: Denys Mishunov Date: Tue, 26 Mar 2019 16:23:15 +0100 Subject: [PATCH 1/2] Create a new file if URL references non-existent one --- .../javascripts/ide/stores/actions/project.js | 5 +++++ .../ide/stores/actions/project_spec.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/assets/javascripts/ide/stores/actions/project.js b/app/assets/javascripts/ide/stores/actions/project.js index 06ed5c0b572..4b10d148ebf 100644 --- a/app/assets/javascripts/ide/stores/actions/project.js +++ b/app/assets/javascripts/ide/stores/actions/project.js @@ -147,6 +147,11 @@ export const openBranch = ({ dispatch, state }, { projectId, branchId, basePath if (treeEntry) { dispatch('handleTreeEntryAction', treeEntry); + } else { + dispatch('createTempEntry', { + name: path, + type: 'blob', + }); } } }) diff --git a/spec/javascripts/ide/stores/actions/project_spec.js b/spec/javascripts/ide/stores/actions/project_spec.js index 7b0963713fb..cd519eaed7c 100644 --- a/spec/javascripts/ide/stores/actions/project_spec.js +++ b/spec/javascripts/ide/stores/actions/project_spec.js @@ -279,5 +279,22 @@ describe('IDE store project actions', () => { .then(done) .catch(done.fail); }); + + it('creates a new file supplied via URL if the file does not exist yet', done => { + openBranch(store, { ...branch, basePath: 'not-existent.md' }) + .then(() => { + expect(store.dispatch).not.toHaveBeenCalledWith( + 'handleTreeEntryAction', + jasmine.anything(), + ); + + expect(store.dispatch).toHaveBeenCalledWith('createTempEntry', { + name: 'not-existent.md', + type: 'blob', + }); + }) + .then(done) + .catch(done.fail); + }); }); }); From 5e0423ebc55089978291d0823574c8498994c3b9 Mon Sep 17 00:00:00 2001 From: Denys Mishunov Date: Tue, 26 Mar 2019 17:28:33 +0100 Subject: [PATCH 2/2] Set tree list accounting for already-opened files Since we can create files from URL now, this means that these files will not exist in the tree returned from API: they exist on the client only before the first commit. In order to still show the newly-created files in the tree, we should not override the tree, but merge the tree existing on the client and the one coming from API. Changelog entry Moved trees merging into mutation --- .../javascripts/ide/stores/mutations/tree.js | 14 +- app/assets/javascripts/ide/stores/utils.js | 28 ++++ .../unreleased/57668-create-file-from-url.yml | 5 + .../ide/stores/actions/tree_spec.js | 33 ++++- .../ide/stores/mutations/tree_spec.js | 61 +++++++-- spec/javascripts/ide/stores/utils_spec.js | 125 ++++++++++++++++++ 6 files changed, 249 insertions(+), 17 deletions(-) create mode 100644 changelogs/unreleased/57668-create-file-from-url.yml diff --git a/app/assets/javascripts/ide/stores/mutations/tree.js b/app/assets/javascripts/ide/stores/mutations/tree.js index eac7441ee54..359943b4ab7 100644 --- a/app/assets/javascripts/ide/stores/mutations/tree.js +++ b/app/assets/javascripts/ide/stores/mutations/tree.js @@ -1,5 +1,5 @@ import * as types from '../mutation_types'; -import { sortTree } from '../utils'; +import { sortTree, mergeTrees } from '../utils'; export default { [types.TOGGLE_TREE_OPEN](state, path) { @@ -23,9 +23,15 @@ export default { }); }, [types.SET_DIRECTORY_DATA](state, { data, treePath }) { - Object.assign(state.trees[treePath], { - tree: data, - }); + const selectedTree = state.trees[treePath]; + + // If we opened files while loading the tree, we need to merge them + // Otherwise, simply overwrite the tree + const tree = !selectedTree.tree.length + ? data + : selectedTree.loading && mergeTrees(selectedTree.tree, data); + + Object.assign(selectedTree, { tree }); }, [types.SET_LAST_COMMIT_URL](state, { tree = state, url }) { Object.assign(tree, { diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 0b2a18e9c8a..3ab8f3f11be 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -170,3 +170,31 @@ export const filePathMatches = (filePath, path) => filePath.indexOf(`${path}/`) export const getChangesCountForFiles = (files, path) => files.filter(f => filePathMatches(f.path, path)).length; + +export const mergeTrees = (fromTree, toTree) => { + if (!fromTree || !fromTree.length) { + return toTree; + } + + const recurseTree = (n, t) => { + if (!n) { + return t; + } + const existingTreeNode = t.find(el => el.path === n.path); + + if (existingTreeNode && n.tree.length > 0) { + existingTreeNode.opened = true; + recurseTree(n.tree[0], existingTreeNode.tree); + } else if (!existingTreeNode) { + const sorted = sortTree(t.concat(n)); + t.splice(0, t.length + 1, ...sorted); + } + return t; + }; + + for (let i = 0, l = fromTree.length; i < l; i += 1) { + recurseTree(fromTree[i], toTree); + } + + return toTree; +}; diff --git a/changelogs/unreleased/57668-create-file-from-url.yml b/changelogs/unreleased/57668-create-file-from-url.yml new file mode 100644 index 00000000000..b6033fa24ca --- /dev/null +++ b/changelogs/unreleased/57668-create-file-from-url.yml @@ -0,0 +1,5 @@ +--- +title: Implemented support for creation of new files from URL in Web IDE +merge_request: 26622 +author: +type: added diff --git a/spec/javascripts/ide/stores/actions/tree_spec.js b/spec/javascripts/ide/stores/actions/tree_spec.js index fbb676aab33..5ed9b9003a7 100644 --- a/spec/javascripts/ide/stores/actions/tree_spec.js +++ b/spec/javascripts/ide/stores/actions/tree_spec.js @@ -1,6 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; import testAction from 'spec/helpers/vuex_action_helper'; -import { showTreeEntry, getFiles } from '~/ide/stores/actions/tree'; +import { showTreeEntry, getFiles, setDirectoryData } from '~/ide/stores/actions/tree'; import * as types from '~/ide/stores/mutation_types'; import axios from '~/lib/utils/axios_utils'; import store from '~/ide/stores'; @@ -206,4 +206,35 @@ describe('Multi-file store tree actions', () => { ); }); }); + + describe('setDirectoryData', () => { + it('sets tree correctly if there are no opened files yet', done => { + const treeFile = file({ name: 'README.md' }); + store.state.trees['abcproject/master'] = {}; + + testAction( + setDirectoryData, + { projectId: 'abcproject', branchId: 'master', treeList: [treeFile] }, + store.state, + [ + { + type: types.SET_DIRECTORY_DATA, + payload: { + treePath: 'abcproject/master', + data: [treeFile], + }, + }, + { + type: types.TOGGLE_LOADING, + payload: { + entry: {}, + forceValue: false, + }, + }, + ], + [], + done, + ); + }); + }); }); diff --git a/spec/javascripts/ide/stores/mutations/tree_spec.js b/spec/javascripts/ide/stores/mutations/tree_spec.js index 67e9f7509da..7f9c978aa46 100644 --- a/spec/javascripts/ide/stores/mutations/tree_spec.js +++ b/spec/javascripts/ide/stores/mutations/tree_spec.js @@ -26,17 +26,11 @@ describe('Multi-file store tree mutations', () => { }); describe('SET_DIRECTORY_DATA', () => { - const data = [ - { - name: 'tree', - }, - { - name: 'submodule', - }, - { - name: 'blob', - }, - ]; + let data; + + beforeEach(() => { + data = [file('tree'), file('foo'), file('blob')]; + }); it('adds directory data', () => { localState.trees['project/master'] = { @@ -52,7 +46,7 @@ describe('Multi-file store tree mutations', () => { expect(tree.tree.length).toBe(3); expect(tree.tree[0].name).toBe('tree'); - expect(tree.tree[1].name).toBe('submodule'); + expect(tree.tree[1].name).toBe('foo'); expect(tree.tree[2].name).toBe('blob'); }); @@ -65,6 +59,49 @@ describe('Multi-file store tree mutations', () => { expect(localState.trees['project/master'].loading).toBe(true); }); + + it('does not override tree already in state, but merges the two with correct order', () => { + const openedFile = file('new'); + + localState.trees['project/master'] = { + loading: true, + tree: [openedFile], + }; + + mutations.SET_DIRECTORY_DATA(localState, { + data, + treePath: 'project/master', + }); + + const { tree } = localState.trees['project/master']; + + expect(tree.length).toBe(4); + expect(tree[0].name).toBe('blob'); + expect(tree[1].name).toBe('foo'); + expect(tree[2].name).toBe('new'); + expect(tree[3].name).toBe('tree'); + }); + + it('returns tree unchanged if the opened file is already in the tree', () => { + const openedFile = file('foo'); + localState.trees['project/master'] = { + loading: true, + tree: [openedFile], + }; + + mutations.SET_DIRECTORY_DATA(localState, { + data, + treePath: 'project/master', + }); + + const { tree } = localState.trees['project/master']; + + expect(tree.length).toBe(3); + + expect(tree[0].name).toBe('tree'); + expect(tree[1].name).toBe('foo'); + expect(tree[2].name).toBe('blob'); + }); }); describe('REMOVE_ALL_CHANGES_FILES', () => { diff --git a/spec/javascripts/ide/stores/utils_spec.js b/spec/javascripts/ide/stores/utils_spec.js index 9f18034f8a3..c4f122efdda 100644 --- a/spec/javascripts/ide/stores/utils_spec.js +++ b/spec/javascripts/ide/stores/utils_spec.js @@ -235,4 +235,129 @@ describe('Multi-file store utils', () => { ]); }); }); + + describe('mergeTrees', () => { + let fromTree; + let toTree; + + beforeEach(() => { + fromTree = [file('foo')]; + toTree = [file('bar')]; + }); + + it('merges simple trees with sorting the result', () => { + toTree = [file('beta'), file('alpha'), file('gamma')]; + const res = utils.mergeTrees(fromTree, toTree); + + expect(res.length).toEqual(4); + expect(res[0].name).toEqual('alpha'); + expect(res[1].name).toEqual('beta'); + expect(res[2].name).toEqual('foo'); + expect(res[3].name).toEqual('gamma'); + expect(res[2]).toBe(fromTree[0]); + }); + + it('handles edge cases', () => { + expect(utils.mergeTrees({}, []).length).toEqual(0); + + let res = utils.mergeTrees({}, toTree); + + expect(res.length).toEqual(1); + expect(res[0].name).toEqual('bar'); + + res = utils.mergeTrees(fromTree, []); + + expect(res.length).toEqual(1); + expect(res[0].name).toEqual('foo'); + expect(res[0]).toBe(fromTree[0]); + }); + + it('merges simple trees without producing duplicates', () => { + toTree.push(file('foo')); + + const res = utils.mergeTrees(fromTree, toTree); + + expect(res.length).toEqual(2); + expect(res[0].name).toEqual('bar'); + expect(res[1].name).toEqual('foo'); + expect(res[1]).not.toBe(fromTree[0]); + }); + + it('merges nested tree into the main one without duplicates', () => { + fromTree[0].tree.push({ + ...file('alpha'), + path: 'foo/alpha', + tree: [ + { + ...file('beta.md'), + path: 'foo/alpha/beta.md', + }, + ], + }); + + toTree.push({ + ...file('foo'), + tree: [ + { + ...file('alpha'), + path: 'foo/alpha', + tree: [ + { + ...file('gamma.md'), + path: 'foo/alpha/gamma.md', + }, + ], + }, + ], + }); + + const res = utils.mergeTrees(fromTree, toTree); + + expect(res.length).toEqual(2); + expect(res[1].name).toEqual('foo'); + + const finalBranch = res[1].tree[0].tree; + + expect(finalBranch.length).toEqual(2); + expect(finalBranch[0].name).toEqual('beta.md'); + expect(finalBranch[1].name).toEqual('gamma.md'); + }); + + it('marks correct folders as opened as the parsing goes on', () => { + fromTree[0].tree.push({ + ...file('alpha'), + path: 'foo/alpha', + tree: [ + { + ...file('beta.md'), + path: 'foo/alpha/beta.md', + }, + ], + }); + + toTree.push({ + ...file('foo'), + tree: [ + { + ...file('alpha'), + path: 'foo/alpha', + tree: [ + { + ...file('gamma.md'), + path: 'foo/alpha/gamma.md', + }, + ], + }, + ], + }); + + const res = utils.mergeTrees(fromTree, toTree); + + expect(res[1].name).toEqual('foo'); + expect(res[1].opened).toEqual(true); + + expect(res[1].tree[0].name).toEqual('alpha'); + expect(res[1].tree[0].opened).toEqual(true); + }); + }); });