Merge branch 'ph/215917/escapeRef' into 'master'

Escape branch name on backend

Closes #215917

See merge request gitlab-org/gitlab!30696

app/assets/javascripts/repository/components/breadcrumbs.vue

@@ -108,14 +108,14 @@ export default {
             return acc.concat({
               name,
               path,
-              to: `/-/tree/${joinPaths(escapeFileUrl(this.ref), path)}`,
+              to: `/-/tree/${joinPaths(this.escapedRef, path)}`,
             });
           },
           [
             {
               name: this.projectShortPath,
               path: '/',
-              to: `/-/tree/${escapeFileUrl(this.ref)}/`,
+              to: `/-/tree/${this.escapedRef}/`,
             },
           ],
         );

app/assets/javascripts/repository/components/table/index.vue

@@ -81,7 +81,7 @@ export default {
         <tbody>
           <parent-row
             v-show="showParentRow"
-            :commit-ref="ref"
+            :commit-ref="escapedRef"
             :path="path"
             :loading-path="loadingPath"
           />

app/assets/javascripts/repository/components/table/parent_row.vue

 <script>
 import { GlLoadingIcon } from '@gitlab/ui';
-import { escapeFileUrl } from '~/lib/utils/url_utility';
 
 export default {
   components: {
@@ -29,7 +28,7 @@ export default {
       return splitArray.map(p => encodeURIComponent(p)).join('/');
     },
     parentRoute() {
-      return { path: `/-/tree/${escapeFileUrl(this.commitRef)}/${this.parentPath}` };
+      return { path: `/-/tree/${this.commitRef}/${this.parentPath}` };
     },
   },
   methods: {

app/assets/javascripts/repository/components/table/row.vue

@@ -99,7 +99,7 @@ export default {
   computed: {
     routerLinkTo() {
       return this.isFolder
-        ? { path: `/-/tree/${escapeFileUrl(this.ref)}/${escapeFileUrl(this.path)}` }
+        ? { path: `/-/tree/${this.escapedRef}/${escapeFileUrl(this.path)}` }
         : null;
     },
     isFolder() {

app/assets/javascripts/repository/index.js

@@ -15,14 +15,15 @@ import { __ } from '../locale';
 export default function setupVueRepositoryList() {
   const el = document.getElementById('js-tree-list');
   const { dataset } = el;
-  const { projectPath, projectShortPath, ref, fullName } = dataset;
-  const router = createRouter(projectPath, ref);
+  const { projectPath, projectShortPath, ref, escapedRef, fullName } = dataset;
+  const router = createRouter(projectPath, escapedRef);
 
   apolloProvider.clients.defaultClient.cache.writeData({
     data: {
       projectPath,
       projectShortPath,
       ref,
+      escapedRef,
       vueFileListLfsBadge: gon.features?.vueFileListLfsBadge || false,
       commits: [],
     },

app/assets/javascripts/repository/log_tree.js

@@ -23,13 +23,13 @@ export function fetchLogsTree(client, path, offset, resolver = null) {
   if (fetchpromise) return fetchpromise;
 
   const { projectPath } = client.readQuery({ query: getProjectPath });
-  const { ref } = client.readQuery({ query: getRef });
+  const { escapedRef } = client.readQuery({ query: getRef });
 
   fetchpromise = axios
     .get(
-      `${gon.relative_url_root}/${projectPath}/-/refs/${encodeURIComponent(
-        ref,
-      )}/logs_tree/${encodeURIComponent(path.replace(/^\//, ''))}`,
+      `${gon.relative_url_root}/${projectPath}/-/refs/${escapedRef}/logs_tree/${encodeURIComponent(
+        path.replace(/^\//, ''),
+      )}`,
       {
         params: { format: 'json', offset },
       },

app/assets/javascripts/repository/mixins/get_ref.js

@@ -4,11 +4,19 @@ export default {
   apollo: {
     ref: {
       query: getRef,
+      manual: true,
+      result({ data, loading }) {
+        if (!loading) {
+          this.ref = data.ref;
+          this.escapedRef = data.escapedRef;
+        }
+      },
     },
   },
   data() {
     return {
       ref: '',
+      escapedRef: '',
     };
   },
 };

app/assets/javascripts/repository/queries/getRef.query.graphql

 query getRef {
   ref @client
+  escapedRef @client
 }

app/assets/javascripts/repository/router.js

@@ -12,7 +12,7 @@ export default function createRouter(base, baseRef) {
     base: joinPaths(gon.relative_url_root || '', base),
     routes: [
       {
-        path: `(/-)?/tree/(${encodeURIComponent(baseRef).replace(/%2F/g, '/')}|${baseRef})/:path*`,
+        path: `(/-)?/tree/${baseRef}/:path*`,
         name: 'treePath',
         component: TreePage,
         props: route => ({

app/helpers/tree_helper.rb

@@ -194,6 +194,7 @@ module TreeHelper
       project_path: project.full_path,
       project_short_path: project.path,
       ref: ref,
+      escaped_ref: ActionDispatch::Journey::Router::Utils.escape_path(ref),
       full_name: project.name_with_namespace
     }
   end

changelogs/unreleased/ph-215917-escapeRef.yml

+---
+title: Fixes branch name not getting escaped correctly on frontend
+merge_request:
+author:
+type: fixed

spec/frontend/repository/components/table/row_spec.js

@@ -26,7 +26,7 @@ function factory(propsData = {}) {
     },
   });
 
-  vm.setData({ ref: 'master' });
+  vm.setData({ escapedRef: 'master' });
 }
 
 describe('Repository table row component', () => {

spec/frontend/repository/log_tree_spec.js

@@ -53,7 +53,7 @@ describe('fetchLogsTree', () => {
     client = {
       readQuery: () => ({
         projectPath: 'gitlab-org/gitlab-foss',
-        ref: 'master',
+        escapedRef: 'master',
         commits: [],
       }),
       writeQuery: jest.fn(),

spec/frontend/repository/router_spec.js

@@ -4,13 +4,12 @@ import createRouter from '~/repository/router';
 
 describe('Repository router spec', () => {
   it.each`
-    path                                     | branch              | component    | componentName
-    ${'/'}                                   | ${'master'}         | ${IndexPage} | ${'IndexPage'}
-    ${'/tree/master'}                        | ${'master'}         | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/master'}                      | ${'master'}         | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/master/app/assets'}           | ${'master'}         | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/feature/test-%23/app/assets'} | ${'feature/test-#'} | ${TreePage}  | ${'TreePage'}
-    ${'/-/tree/123/app/assets'}              | ${'master'}         | ${null}      | ${'null'}
+    path                           | branch      | component    | componentName
+    ${'/'}                         | ${'master'} | ${IndexPage} | ${'IndexPage'}
+    ${'/tree/master'}              | ${'master'} | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/master'}            | ${'master'} | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/master/app/assets'} | ${'master'} | ${TreePage}  | ${'TreePage'}
+    ${'/-/tree/123/app/assets'}    | ${'master'} | ${null}      | ${'null'}
   `('sets component as $componentName for path "$path"', ({ path, component, branch }) => {
     const router = createRouter('', branch);