From c14642c678eafbb017f2df7a7aa38b73ed9fc372 Mon Sep 17 00:00:00 2001 From: Jacques Erasmus Date: Wed, 24 Mar 2021 08:57:30 +0000 Subject: [PATCH 1/2] Add Blob Page to the repository vue app Added a blob page in preparation for the blob viewer refactor --- .../javascripts/lib/utils/url_utility.js | 12 ++++++++++ .../repository/components/table/row.vue | 22 ++++++++++++++----- .../javascripts/repository/pages/blob.vue | 17 ++++++++++++++ app/assets/javascripts/repository/router.js | 22 ++++++++++++++++++- app/controllers/projects_controller.rb | 4 ++++ .../development/refactor_blob_viewer.yml | 8 +++++++ spec/features/projects_spec.rb | 1 + spec/frontend/lib/utils/url_utility_spec.js | 14 ++++++++++++ .../repository/components/table/row_spec.js | 5 ++++- spec/frontend/repository/router_spec.js | 2 ++ 10 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 app/assets/javascripts/repository/pages/blob.vue create mode 100644 config/feature_flags/development/refactor_blob_viewer.yml diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 5b3aa3cf9dc85b..1df63eb74cfbc4 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -535,3 +535,15 @@ export function getURLOrigin(url) { return null; } } + +/** + * Removes one slash at the start the given path. + * + * Example: //some/path ==> /some/path + * Example: /some/path ==> some/path + * + * @param {string} path + */ +export function stripLeadingSlash(path = '') { + return path.replace(/^\//, ''); +} diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 70918dd55e4b63..8ea5fce92fa7c7 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -12,6 +12,7 @@ import { escapeRegExp } from 'lodash'; import { escapeFileUrl } from '~/lib/utils/url_utility'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import getRefMixin from '../../mixins/get_ref'; import commitQuery from '../../queries/commit.query.graphql'; @@ -41,7 +42,7 @@ export default { }, }, }, - mixins: [getRefMixin], + mixins: [getRefMixin, glFeatureFlagMixin()], props: { id: { type: String, @@ -103,10 +104,21 @@ export default { }; }, computed: { + refactorBlobViewerEnabled() { + return this.glFeatures.refactorBlobViewer; + }, routerLinkTo() { - return this.isFolder - ? { path: `/-/tree/${this.escapedRef}/${escapeFileUrl(this.path)}` } - : null; + const blobRouteConfig = { path: `/-/blob/${this.escapedRef}/${escapeFileUrl(this.path)}` }; + const treeRouteConfig = { path: `/-/tree/${this.escapedRef}/${escapeFileUrl(this.path)}` }; + + if (this.refactorBlobViewerEnabled && this.isBlob) { + return blobRouteConfig; + } + + return this.isFolder ? treeRouteConfig : null; + }, + isBlob() { + return this.type === 'blob'; }, isFolder() { return this.type === 'tree'; @@ -115,7 +127,7 @@ export default { return this.type === 'commit'; }, linkComponent() { - return this.isFolder ? 'router-link' : 'a'; + return this.isFolder || (this.refactorBlobViewerEnabled && this.isBlob) ? 'router-link' : 'a'; }, fullPath() { return this.path.replace(new RegExp(`^${escapeRegExp(this.currentPath)}/`), ''); diff --git a/app/assets/javascripts/repository/pages/blob.vue b/app/assets/javascripts/repository/pages/blob.vue new file mode 100644 index 00000000000000..c492f9663644aa --- /dev/null +++ b/app/assets/javascripts/repository/pages/blob.vue @@ -0,0 +1,17 @@ + + + diff --git a/app/assets/javascripts/repository/router.js b/app/assets/javascripts/repository/router.js index ad6e32d7055c7b..37761cd21380c9 100644 --- a/app/assets/javascripts/repository/router.js +++ b/app/assets/javascripts/repository/router.js @@ -1,7 +1,8 @@ import { escapeRegExp } from 'lodash'; import Vue from 'vue'; import VueRouter from 'vue-router'; -import { joinPaths } from '../lib/utils/url_utility'; +import { joinPaths, stripLeadingSlash } from '../lib/utils/url_utility'; +import BlobPage from './pages/blob.vue'; import IndexPage from './pages/index.vue'; import TreePage from './pages/tree.vue'; @@ -15,6 +16,13 @@ export default function createRouter(base, baseRef) { }), }; + const blobPathRoute = { + component: BlobPage, + props: (route) => ({ + path: route.params.path ? stripLeadingSlash(route.params.path) : '/', + }), + }; + return new VueRouter({ mode: 'history', base: joinPaths(gon.relative_url_root || '', base), @@ -31,6 +39,18 @@ export default function createRouter(base, baseRef) { path: `(/-)?/tree/${escapeRegExp(baseRef)}/:path*`, ...treePathRoute, }, + { + name: 'blobPathDecoded', + // Sometimes the ref needs decoding depending on how the backend sends it to us + path: `(/-)?/blob/${decodeURI(baseRef)}/:path*`, + ...blobPathRoute, + }, + { + name: 'blobPath', + // Support without decoding as well just in case the ref doesn't need to be decoded + path: `(/-)?/blob/${escapeRegExp(baseRef)}/:path*`, + ...blobPathRoute, + }, { path: '/', name: 'projectRoot', diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 00390d90e8db97..3d9712885deff7 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -35,6 +35,10 @@ class ProjectsController < Projects::ApplicationController push_frontend_feature_flag(:allow_editing_commit_messages, @project) end + before_action do + push_frontend_feature_flag(:refactor_blob_viewer, @project) + end + layout :determine_layout feature_category :projects, [ diff --git a/config/feature_flags/development/refactor_blob_viewer.yml b/config/feature_flags/development/refactor_blob_viewer.yml new file mode 100644 index 00000000000000..231e26840234cd --- /dev/null +++ b/config/feature_flags/development/refactor_blob_viewer.yml @@ -0,0 +1,8 @@ +--- +name: refactor_blob_viewer +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57326 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324351 +milestone: '13.11' +type: development +group: group::source code +default_enabled: false diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 4730679feb86d4..34601cab24fac8 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -290,6 +290,7 @@ let(:project) { create(:forked_project_with_submodules) } before do + stub_feature_flags(refactor_blob_viewer: false) project.add_maintainer(user) sign_in user visit project_path(project) diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index e12cd8b0e3745b..c0361c5b32bf12 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -921,4 +921,18 @@ describe('URL utility', () => { expect(urlUtils.encodeSaferUrl(input)).toBe(input); }); }); + + describe('stripLeadingSlash', () => { + it.each` + path | expected + ${'//'} | ${'/'} + ${'//index.html'} | ${'/index.html'} + ${'/'} | ${''} + ${'//foo/bar'} | ${'/foo/bar'} + ${'/foo/bar/'} | ${'foo/bar/'} + ${'//foo/bar/index.html'} | ${'/foo/bar/index.html'} + `('strips one slash at the start of $path => $expected', ({ path, expected }) => { + expect(urlUtils.stripLeadingSlash(path)).toBe(expected); + }); + }); }); diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index 69cb69de5df5f9..3ebffbedcdba8d 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -19,6 +19,9 @@ function factory(propsData = {}) { projectPath: 'gitlab-org/gitlab-ce', url: `https://test.com`, }, + provide: { + glFeatures: { refactorBlobViewer: true }, + }, mocks: { $router, }, @@ -81,7 +84,7 @@ describe('Repository table row component', () => { it.each` type | component | componentName ${'tree'} | ${RouterLinkStub} | ${'RouterLink'} - ${'file'} | ${'a'} | ${'hyperlink'} + ${'blob'} | ${RouterLinkStub} | ${'RouterLink'} ${'commit'} | ${'a'} | ${'hyperlink'} `('renders a $componentName for type $type', ({ type, component }) => { factory({ diff --git a/spec/frontend/repository/router_spec.js b/spec/frontend/repository/router_spec.js index 3c7dda05ca3d8f..3354b2315fca9c 100644 --- a/spec/frontend/repository/router_spec.js +++ b/spec/frontend/repository/router_spec.js @@ -1,3 +1,4 @@ +import BlobPage from '~/repository/pages/blob.vue'; import IndexPage from '~/repository/pages/index.vue'; import TreePage from '~/repository/pages/tree.vue'; import createRouter from '~/repository/router'; @@ -11,6 +12,7 @@ describe('Repository router spec', () => { ${'/-/tree/master'} | ${'master'} | ${TreePage} | ${'TreePage'} ${'/-/tree/master/app/assets'} | ${'master'} | ${TreePage} | ${'TreePage'} ${'/-/tree/123/app/assets'} | ${'master'} | ${null} | ${'null'} + ${'/-/blob/master/file.md'} | ${'master'} | ${BlobPage} | ${'BlobPage'} `('sets component as $componentName for path "$path"', ({ path, component, branch }) => { const router = createRouter('', branch); -- GitLab From b13ebeaf7c0705dbe1cbdbf5f90dcb9a6a7741a7 Mon Sep 17 00:00:00 2001 From: Jacques Erasmus Date: Mon, 29 Mar 2021 09:30:43 +0000 Subject: [PATCH 2/2] Address code review comments Addressed maintainer review comments --- app/assets/javascripts/lib/utils/url_utility.js | 12 ------------ app/assets/javascripts/repository/router.js | 4 ++-- app/controllers/projects_controller.rb | 2 +- spec/frontend/lib/utils/url_utility_spec.js | 14 -------------- 4 files changed, 3 insertions(+), 29 deletions(-) diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 1df63eb74cfbc4..5b3aa3cf9dc85b 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -535,15 +535,3 @@ export function getURLOrigin(url) { return null; } } - -/** - * Removes one slash at the start the given path. - * - * Example: //some/path ==> /some/path - * Example: /some/path ==> some/path - * - * @param {string} path - */ -export function stripLeadingSlash(path = '') { - return path.replace(/^\//, ''); -} diff --git a/app/assets/javascripts/repository/router.js b/app/assets/javascripts/repository/router.js index 37761cd21380c9..c7f7451fb55f72 100644 --- a/app/assets/javascripts/repository/router.js +++ b/app/assets/javascripts/repository/router.js @@ -1,7 +1,7 @@ import { escapeRegExp } from 'lodash'; import Vue from 'vue'; import VueRouter from 'vue-router'; -import { joinPaths, stripLeadingSlash } from '../lib/utils/url_utility'; +import { joinPaths } from '../lib/utils/url_utility'; import BlobPage from './pages/blob.vue'; import IndexPage from './pages/index.vue'; import TreePage from './pages/tree.vue'; @@ -19,7 +19,7 @@ export default function createRouter(base, baseRef) { const blobPathRoute = { component: BlobPage, props: (route) => ({ - path: route.params.path ? stripLeadingSlash(route.params.path) : '/', + path: route.params.path, }), }; diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3d9712885deff7..4f28207564aec1 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -36,7 +36,7 @@ class ProjectsController < Projects::ApplicationController end before_action do - push_frontend_feature_flag(:refactor_blob_viewer, @project) + push_frontend_feature_flag(:refactor_blob_viewer, @project, default_enabled: :yaml) end layout :determine_layout diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index c0361c5b32bf12..e12cd8b0e3745b 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -921,18 +921,4 @@ describe('URL utility', () => { expect(urlUtils.encodeSaferUrl(input)).toBe(input); }); }); - - describe('stripLeadingSlash', () => { - it.each` - path | expected - ${'//'} | ${'/'} - ${'//index.html'} | ${'/index.html'} - ${'/'} | ${''} - ${'//foo/bar'} | ${'/foo/bar'} - ${'/foo/bar/'} | ${'foo/bar/'} - ${'//foo/bar/index.html'} | ${'/foo/bar/index.html'} - `('strips one slash at the start of $path => $expected', ({ path, expected }) => { - expect(urlUtils.stripLeadingSlash(path)).toBe(expected); - }); - }); }); -- GitLab