Merge branch '35473-webide-hash-in-branch-name' into 'master'

Resolve "Web IDE does not work on some branch name"

See merge request gitlab-org/gitlab!24717

app/assets/javascripts/ide/ide_router.js

 import Vue from 'vue';
-import VueRouter from 'vue-router';
+import IdeRouter from '~/ide/ide_router_extension';
 import { joinPaths } from '~/lib/utils/url_utility';
 import flash from '~/flash';
 import store from './stores';
 import { __ } from '~/locale';
 
-Vue.use(VueRouter);
+Vue.use(IdeRouter);
 
 /**
  * Routes below /-/ide/:
@@ -33,7 +33,7 @@ const EmptyRouterComponent = {
   },
 };
 
-const router = new VueRouter({
+const router = new IdeRouter({
   mode: 'history',
   base: joinPaths(gon.relative_url_root || '', '/-/ide/'),
   routes: [

app/assets/javascripts/ide/ide_router_extension.js

+import VueRouter from 'vue-router';
+import { escapeFileUrl } from '~/lib/utils/url_utility';
+
+// To allow special characters (like "#," for example) in the branch names, we
+// should encode all the locations before those get processed by History API.
+// Otherwise, paths get messed up so that the router receives incorrect
+// branchid. The only way to do it consistently and in a more or less
+// future-proof manner is, unfortunately, to monkey-patch VueRouter or, as
+// suggested here, achieve the same more reliably by subclassing VueRouter and
+// update the methods, used in WebIDE.
+//
+// More context: https://gitlab.com/gitlab-org/gitlab/issues/35473
+
+export default class IDERouter extends VueRouter {
+  push(location, onComplete, onAbort) {
+    super.push(escapeFileUrl(location), onComplete, onAbort);
+  }
+  resolve(to, current, append) {
+    return super.resolve(escapeFileUrl(to), current, append);
+  }
+}

app/assets/javascripts/lib/utils/url_utility.js

@@ -194,12 +194,14 @@ export function redirectTo(url) {
   return window.location.assign(url);
 }
 
+export const escapeFileUrl = fileUrl => encodeURIComponent(fileUrl).replace(/%2F/g, '/');
+
 export function webIDEUrl(route = undefined) {
   let returnUrl = `${gon.relative_url_root || ''}/-/ide/`;
   if (route) {
     returnUrl += `project${route.replace(new RegExp(`^${gon.relative_url_root || ''}`), '')}`;
   }
-  return returnUrl;
+  return escapeFileUrl(returnUrl);
 }
 
 /**
@@ -313,8 +315,6 @@ export const setUrlParams = (params, url = window.location.href, clearParams = f
   return urlObj.toString();
 };
 
-export const escapeFileUrl = fileUrl => encodeURIComponent(fileUrl).replace(/%2F/g, '/');
-
 export function urlIsDifferent(url, compare = String(window.location)) {
   return url !== compare;
 }

app/helpers/blob_helper.rb

@@ -27,7 +27,7 @@ module BlobHelper
         "#{current_user.namespace.full_path}/#{project.path}"
       end
 
-    segments = [ide_path, 'project', project_path, 'edit', ref]
+    segments = [ide_path, 'project', project_path, 'edit', encode_ide_path(ref)]
     segments.concat(['-', encode_ide_path(path)]) if path.present?
     File.join(segments)
   end

changelogs/unreleased/35473-webide-hash-in-branch-name.yml

+---
+title: 'WebIDE: Support # in branch names'
+merge_request: 24717
+author:
+type: changed

spec/frontend/ide/ide_router_extension_spec.js

+import VueRouter from 'vue-router';
+import IdeRouter from '~/ide/ide_router_extension';
+
+jest.mock('vue-router');
+
+describe('IDE overrides of VueRouter', () => {
+  const paths = branch => [
+    `${branch}`,
+    `/${branch}`,
+    `/${branch}/-/`,
+    `/edit/${branch}`,
+    `/edit/${branch}/-/`,
+    `/blob/${branch}`,
+    `/blob/${branch}/-/`,
+    `/blob/${branch}/-/src/merge_requests/2`,
+    `/blob/${branch}/-/src/blob/`,
+    `/tree/${branch}/-/src/blob/`,
+    `/tree/${branch}/-/src/tree/`,
+  ];
+  let router;
+
+  beforeEach(() => {
+    VueRouter.mockClear();
+    router = new IdeRouter({
+      mode: 'history',
+    });
+  });
+
+  it.each`
+    path               | expected
+    ${'#-test'}        | ${'%23-test'}
+    ${'#test'}         | ${'%23test'}
+    ${'test#'}         | ${'test%23'}
+    ${'test-#'}        | ${'test-%23'}
+    ${'test-#-hash'}   | ${'test-%23-hash'}
+    ${'test/hash#123'} | ${'test/hash%23123'}
+  `('finds project path when route is $path', ({ path, expected }) => {
+    paths(path).forEach(route => {
+      const expectedPath = route.replace(path, expected);
+
+      router.push(route);
+      expect(VueRouter.prototype.push).toHaveBeenCalledWith(expectedPath, undefined, undefined);
+
+      router.resolve(route);
+      expect(VueRouter.prototype.resolve).toHaveBeenCalledWith(expectedPath, undefined, undefined);
+    });
+  });
+});

spec/frontend/lib/utils/url_utility_spec.js

@@ -28,6 +28,12 @@ describe('URL utility', () => {
       gon.relative_url_root = '';
     });
 
+    it('escapes special characters', () => {
+      expect(urlUtils.webIDEUrl('/gitlab-org/gitlab-#-foss/merge_requests/1')).toBe(
+        '/-/ide/project/gitlab-org/gitlab-%23-foss/merge_requests/1',
+      );
+    });
+
     describe('without relative_url_root', () => {
       it('returns IDE path with route', () => {
         expect(urlUtils.webIDEUrl('/gitlab-org/gitlab-foss/merge_requests/1')).toBe(

spec/helpers/blob_helper_spec.rb

@@ -244,8 +244,8 @@ describe BlobHelper do
     it 'escapes special characters' do
       Rails.application.routes.default_url_options[:script_name] = nil
 
-      expect(helper.ide_edit_path(project, "testing/#hashes", "readme.md#test")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/#hashes/-/readme.md%23test")
-      expect(helper.ide_edit_path(project, "testing/#hashes", "src#/readme.md#test")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/#hashes/-/src%23/readme.md%23test")
+      expect(helper.ide_edit_path(project, "testing/#hashes", "readme.md#test")).to eq("/-/ide/project/#{project.full_path}/edit/testing/%23hashes/-/readme.md%23test")
+      expect(helper.ide_edit_path(project, "testing/#hashes", "src#/readme.md#test")).to eq("/-/ide/project/#{project.full_path}/edit/testing/%23hashes/-/src%23/readme.md%23test")
     end
 
     it 'does not escape "/" character' do