From 8056a77d3ec9d3b4b1fd75211d6bc59fa1c65103 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Wed, 23 May 2018 09:36:41 +0000 Subject: [PATCH] Resolve "Web IDE doesn't work for branches with slashes" --- app/assets/javascripts/ide/ide_router.js | 20 +++++++++++-------- .../javascripts/ide/stores/actions/file.js | 4 +++- .../ide/stores/modules/commit/actions.js | 16 ++++++++++----- .../stores/workers/files_decorator_worker.js | 4 ++-- app/helpers/blob_helper.rb | 5 +++-- .../projects/tree/_tree_header.html.haml | 2 +- spec/helpers/blob_helper_spec.rb | 10 ++++++++-- 7 files changed, 40 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/ide/ide_router.js b/app/assets/javascripts/ide/ide_router.js index adca85dc65b..a21cec4e8d8 100644 --- a/app/assets/javascripts/ide/ide_router.js +++ b/app/assets/javascripts/ide/ide_router.js @@ -41,7 +41,7 @@ const router = new VueRouter({ component: EmptyRouterComponent, children: [ { - path: ':targetmode(edit|tree|blob)/:branch/*', + path: ':targetmode(edit|tree|blob)/*', component: EmptyRouterComponent, }, { @@ -63,23 +63,27 @@ router.beforeEach((to, from, next) => { .then(() => { const fullProjectId = `${to.params.namespace}/${to.params.project}`; - if (to.params.branch) { - store.dispatch('setCurrentBranchId', to.params.branch); + const baseSplit = to.params[0].split('/-/'); + const branchId = baseSplit[0].slice(-1) === '/' ? baseSplit[0].slice(0, -1) : baseSplit[0]; + + if (branchId) { + const basePath = baseSplit.length > 1 ? baseSplit[1] : ''; + + store.dispatch('setCurrentBranchId', branchId); store.dispatch('getBranchData', { projectId: fullProjectId, - branchId: to.params.branch, + branchId, }); store .dispatch('getFiles', { projectId: fullProjectId, - branchId: to.params.branch, + branchId, }) .then(() => { - if (to.params[0]) { - const path = - to.params[0].slice(-1) === '/' ? to.params[0].slice(0, -1) : to.params[0]; + if (basePath) { + const path = basePath.slice(-1) === '/' ? basePath.slice(0, -1) : basePath; const treeEntryKey = Object.keys(store.state.entries).find( key => key === path && !store.state.entries[key].pending, ); diff --git a/app/assets/javascripts/ide/stores/actions/file.js b/app/assets/javascripts/ide/stores/actions/file.js index b6baa693104..13aea91d8ba 100644 --- a/app/assets/javascripts/ide/stores/actions/file.js +++ b/app/assets/javascripts/ide/stores/actions/file.js @@ -63,7 +63,9 @@ export const getFileData = ({ state, commit, dispatch }, { path, makeFileActive const file = state.entries[path]; commit(types.TOGGLE_LOADING, { entry: file }); return service - .getFileData(`${gon.relative_url_root ? gon.relative_url_root : ''}${file.url}`) + .getFileData( + `${gon.relative_url_root ? gon.relative_url_root : ''}${file.url.replace('/-/', '/')}`, + ) .then(res => { const pageTitle = decodeURI(normalizeHeaders(res.headers)['PAGE-TITLE']); setPageTitle(pageTitle); diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js index b85246b2502..cd25c3060f2 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/actions.js +++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js @@ -204,17 +204,23 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo dispatch('updateViewer', 'editor', { root: true }); router.push( - `/project/${rootState.currentProjectId}/blob/${getters.branchName}/${ + `/project/${rootState.currentProjectId}/blob/${getters.branchName}/-/${ rootGetters.activeFile.path }`, ); } }) .then(() => dispatch('updateCommitAction', consts.COMMIT_TO_CURRENT_BRANCH)) - .then(() => dispatch('refreshLastCommitData', { - projectId: rootState.currentProjectId, - branchId: rootState.currentBranchId, - }, { root: true })); + .then(() => + dispatch( + 'refreshLastCommitData', + { + projectId: rootState.currentProjectId, + branchId: rootState.currentBranchId, + }, + { root: true }, + ), + ); }) .catch(err => { let errMsg = __('Error committing changes. Please try again.'); diff --git a/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js b/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js index d249b05f47c..0a1c253c637 100644 --- a/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js +++ b/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js @@ -26,7 +26,7 @@ self.addEventListener('message', e => { id: folderPath, name: folderName, path: folderPath, - url: `/${projectId}/tree/${branchId}/${folderPath}/`, + url: `/${projectId}/tree/${branchId}/-/${folderPath}/`, type: 'tree', parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`, tempFile, @@ -64,7 +64,7 @@ self.addEventListener('message', e => { id: path, name: blobName, path, - url: `/${projectId}/blob/${branchId}/${path}`, + url: `/${projectId}/blob/${branchId}/-/${path}`, type: 'blob', parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`, tempFile, diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index e7a36e20050..3db28fd6da3 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -17,7 +17,9 @@ module BlobHelper end def ide_edit_path(project = @project, ref = @ref, path = @path, options = {}) - "#{ide_path}/project#{url_for([project, "edit", "blob", id: [ref, path], script_name: "/"])}" + segments = [ide_path, 'project', project.full_path, 'edit', ref] + segments.concat(['-', path]) if path.present? + File.join(segments) end def edit_blob_button(project = @project, ref = @ref, path = @path, options = {}) @@ -331,7 +333,6 @@ module BlobHelper if !on_top_of_branch?(project, ref) edit_disabled_button_tag(text, common_classes) # This condition only applies to users who are logged in - # Web IDE (Beta) requires the user to have this feature enabled elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) edit_link_tag(text, edit_path, common_classes) elsif can?(current_user, :fork_project, project) && can?(current_user, :create_merge_request_in, project) diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index 8587d3b0c0d..fc8ebfa1fb1 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -82,7 +82,7 @@ - if can_collaborate = succeed " " do - = link_to ide_edit_path(@project, @id, ""), class: 'btn btn-default' do + = link_to ide_edit_path(@project, @ref, @path), class: 'btn btn-default' do = _('Web IDE') = render 'projects/buttons/download', project: @project, ref: @ref diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 8de615ad8c2..a3e010c3206 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -258,13 +258,19 @@ describe BlobHelper do it 'returns full IDE path' do Rails.application.routes.default_url_options[:script_name] = nil - expect(helper.ide_edit_path(project, "master", "")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/master/") + expect(helper.ide_edit_path(project, "master", "")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/master") + end + + it 'returns full IDE path with second -' do + Rails.application.routes.default_url_options[:script_name] = nil + + expect(helper.ide_edit_path(project, "testing/slashes", "readme.md")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/slashes/-/readme.md") end it 'returns IDE path without relative_url_root' do Rails.application.routes.default_url_options[:script_name] = "/gitlab" - expect(helper.ide_edit_path(project, "master", "")).to eq("/gitlab/-/ide/project/#{project.namespace.path}/#{project.path}/edit/master/") + expect(helper.ide_edit_path(project, "master", "")).to eq("/gitlab/-/ide/project/#{project.namespace.path}/#{project.path}/edit/master") end end end