Merge branch 'dz-scope-commit-resource' into 'master'

Move commit resource in repository routing

See merge request gitlab-org/gitlab!24279

app/assets/javascripts/ide/stores/modules/commit/actions.js

@@ -44,7 +44,7 @@ export const setLastCommitMessage = ({ commit, rootGetters }, data) => {
   const commitMsg = sprintf(
     __('Your changes have been committed. Commit %{commitId} %{commitStats}'),
     {
-      commitId: `<a href="${currentProject.web_url}/commit/${data.short_id}" class="commit-sha">${data.short_id}</a>`,
+      commitId: `<a href="${currentProject.web_url}/-/commit/${data.short_id}" class="commit-sha">${data.short_id}</a>`,
       commitStats,
     },
     false,
@@ -56,7 +56,7 @@ export const setLastCommitMessage = ({ commit, rootGetters }, data) => {
 export const updateFilesAfterCommit = ({ commit, dispatch, rootState, rootGetters }, { data }) => {
   const selectedProject = rootGetters.currentProject;
   const lastCommit = {
-    commit_path: `${selectedProject.web_url}/commit/${data.id}`,
+    commit_path: `${selectedProject.web_url}/-/commit/${data.id}`,
     commit: {
       id: data.id,
       message: data.message,

app/assets/javascripts/monitoring/components/charts/time_series.vue

@@ -209,7 +209,7 @@ export default {
             id,
             createdAt: created_at,
             sha,
-            commitUrl: `${this.projectPath}/commit/${sha}`,
+            commitUrl: `${this.projectPath}/-/commit/${sha}`,
             tag,
             tagUrl: tag ? `${this.tagsPath}/${ref.name}` : null,
             ref: ref.name,

app/models/project_services/flowdock_service.rb

@@ -36,7 +36,7 @@ class FlowdockService < Service
       token: token,
       repo: project.repository,
       repo_url: "#{Gitlab.config.gitlab.url}/#{project.full_path}",
-      commit_url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/%s",
+      commit_url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/%s",
       diff_url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/compare/%s...%s"
     )
   end

changelogs/unreleased/dz-scope-commit-resource.yml

+---
+title: Move commit routes under - scope
+merge_request: 24279
+author:
+type: changed

config/routes/project.rb

@@ -301,17 +301,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           defaults: { format: 'json' },
           constraints: { template_type: %r{issue|merge_request}, format: 'json' }
 
-      resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
-        member do
-          get :branches
-          get :pipelines
-          post :revert
-          post :cherry_pick
-          get :diff_for_path
-          get :merge_requests
-        end
-      end
-
       resource :pages, only: [:show, :update, :destroy] do
         resources :domains, except: :index, controller: 'pages_domains', constraints: { id: %r{[^/]+} } do
           member do

config/routes/repository.rb

@@ -10,6 +10,17 @@ resource :repository, only: [:create] do
   end
 end
 
+resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
+  member do
+    get :branches
+    get :pipelines
+    post :revert
+    post :cherry_pick
+    get :diff_for_path
+    get :merge_requests
+  end
+end
+
 # Don't use format parameter as file extension (old 3.0.x behavior)
 # See http://guides.rubyonrails.org/routing.html#route-globbing-and-wildcard-segments
 scope format: false do

ee/spec/features/security/project/internal_access_spec.rb

@@ -44,7 +44,7 @@ describe '[EE] Internal Project Access' do
     it { is_expected.to be_allowed_for(:auditor) }
   end
 
-  describe "GET /:project_path/commit/:sha" do
+  describe "GET /:project_path/-/commit/:sha" do
     subject { project_commit_path(project, project.repository.commit) }
 
     it { is_expected.to be_allowed_for(:auditor) }

ee/spec/features/security/project/private_access_spec.rb

@@ -44,7 +44,7 @@ describe '[EE] Private Project Access' do
     it { is_expected.to be_allowed_for(:auditor) }
   end
 
-  describe "GET /:project_path/commit/:sha" do
+  describe "GET /:project_path/-/commit/:sha" do
     subject { project_commit_path(project, project.repository.commit) }
 
     it { is_expected.to be_allowed_for(:auditor) }

ee/spec/features/security/project/public_access_spec.rb

@@ -44,7 +44,7 @@ describe '[EE] Public Project Access' do
     it { is_expected.to be_allowed_for(:auditor) }
   end
 
-  describe "GET /:project_path/commit/:sha" do
+  describe "GET /:project_path/-/commit/:sha" do
     subject { project_commit_path(project, project.repository.commit) }
 
     it { is_expected.to be_allowed_for(:auditor) }

ee/spec/helpers/ee/emails_helper_spec.rb

@@ -10,7 +10,7 @@ describe EE::EmailsHelper do
       'somedomain.com/groups/agroup/-/epics/231'     | 'View Epic'
       'somedomain.com/aproject/issues/231'           | 'View Issue'
       'somedomain.com/aproject/-/merge_requests/231' | 'View Merge request'
-      'somedomain.com/aproject/commit/al3f231'       | 'View Commit'
+      'somedomain.com/aproject/-/commit/al3f231'     | 'View Commit'
     end
 
     with_them do

lib/gitlab/data_builder/push.rb

@@ -37,7 +37,7 @@ module Gitlab
               id: "c5feabde2d8cd023215af4d2ceeb7a64839fc428",
               message: "Add simple search to projects in public area",
               timestamp: "2013-05-13T18:18:08+00:00",
-              url: "https://test.example.com/gitlab/gitlab/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428",
+              url: "https://test.example.com/gitlab/gitlab/-/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428",
               author: {
                 name: "Test User",
                 email: "test@example.com"

spec/frontend/monitoring/components/charts/time_series_spec.js

@@ -385,7 +385,7 @@ describe('Time series component', () => {
 
         describe('when tooltip is showing deployment data', () => {
           const mockSha = 'mockSha';
-          const commitUrl = `${mockProjectDir}/commit/${mockSha}`;
+          const commitUrl = `${mockProjectDir}/-/commit/${mockSha}`;
 
           beforeEach(done => {
             timeSeriesAreaChart.vm.tooltip.isDeployment = true;

spec/frontend/monitoring/mock_data.js

@@ -169,7 +169,7 @@ export const deploymentData = [
     iid: 3,
     sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187',
     commitUrl:
-      'http://test.host/frontend-fixtures/environments-project/commit/f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187',
+      'http://test.host/frontend-fixtures/environments-project/-/commit/f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187',
     ref: {
       name: 'master',
     },
@@ -183,7 +183,7 @@ export const deploymentData = [
     iid: 2,
     sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187',
     commitUrl:
-      'http://test.host/frontend-fixtures/environments-project/commit/f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187',
+      'http://test.host/frontend-fixtures/environments-project/-/commit/f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187',
     ref: {
       name: 'master',
     },
@@ -197,7 +197,7 @@ export const deploymentData = [
     iid: 1,
     sha: '6511e58faafaa7ad2228990ec57f19d66f7db7c2',
     commitUrl:
-      'http://test.host/frontend-fixtures/environments-project/commit/6511e58faafaa7ad2228990ec57f19d66f7db7c2',
+      'http://test.host/frontend-fixtures/environments-project/-/commit/6511e58faafaa7ad2228990ec57f19d66f7db7c2',
     ref: {
       name: 'update2-readme',
     },

spec/helpers/events_helper_spec.rb

@@ -98,7 +98,7 @@ describe EventsHelper do
     it 'returns a commit note url' do
       event.target = create(:note_on_commit, note: '+1 from me')
 
-      expect(subject).to eq("#{project_base_url}/commit/#{event.target.commit_id}#note_#{event.target.id}")
+      expect(subject).to eq("#{project_base_url}/-/commit/#{event.target.commit_id}#note_#{event.target.id}")
     end
 
     it 'returns a project snippet note url' do

spec/javascripts/ide/stores/modules/commit/actions_spec.js

@@ -131,7 +131,7 @@ describe('IDE commit module actions', () => {
         .dispatch('commit/setLastCommitMessage', { short_id: '123' })
         .then(() => {
           expect(store.state.lastCommitMsg).toContain(
-            'Your changes have been committed. Commit <a href="http://testing/commit/123" class="commit-sha">123</a>',
+            'Your changes have been committed. Commit <a href="http://testing/-/commit/123" class="commit-sha">123</a>',
           );
         })
         .then(done)
@@ -149,7 +149,7 @@ describe('IDE commit module actions', () => {
         })
         .then(() => {
           expect(store.state.lastCommitMsg).toBe(
-            'Your changes have been committed. Commit <a href="http://testing/commit/123" class="commit-sha">123</a> with 1 additions, 2 deletions.',
+            'Your changes have been committed. Commit <a href="http://testing/-/commit/123" class="commit-sha">123</a> with 1 additions, 2 deletions.',
           );
         })
         .then(done)
@@ -407,7 +407,7 @@ describe('IDE commit module actions', () => {
           .dispatch('commit/commitChanges')
           .then(() => {
             expect(store.state.lastCommitMsg).toBe(
-              'Your changes have been committed. Commit <a href="webUrl/commit/123" class="commit-sha">123</a> with 1 additions, 2 deletions.',
+              'Your changes have been committed. Commit <a href="webUrl/-/commit/123" class="commit-sha">123</a> with 1 additions, 2 deletions.',
             );
 
             done();

spec/lib/gitlab/etag_caching/router_spec.rb

@@ -59,7 +59,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'matches commit pipelines endpoint' do
     result = described_class.match(
-      '/my-group/my-project/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json'
+      '/my-group/my-project/-/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json'
     )
 
     expect(result).to be_present

spec/lib/gitlab/url_builder_spec.rb

@@ -10,7 +10,7 @@ describe Gitlab::UrlBuilder do
 
         url = described_class.build(commit)
 
-        expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.full_path}/commit/#{commit.id}"
+        expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.full_path}/-/commit/#{commit.id}"
       end
     end
 
@@ -86,7 +86,7 @@ describe Gitlab::UrlBuilder do
 
           url = described_class.build(note)
 
-          expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.full_path}/commit/#{note.commit_id}#note_#{note.id}"
+          expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}"
         end
       end
 
@@ -96,7 +96,7 @@ describe Gitlab::UrlBuilder do
 
           url = described_class.build(note)
 
-          expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.full_path}/commit/#{note.commit_id}#note_#{note.id}"
+          expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}"
         end
       end
 

spec/models/error_tracking/project_error_tracking_setting_spec.rb

@@ -267,7 +267,7 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
         end
 
         it { expect(result[:issue].gitlab_commit).to eq(commit_id) }
-        it { expect(result[:issue].gitlab_commit_path).to eq("/#{project.namespace.path}/#{project.path}/commit/#{commit_id}") }
+        it { expect(result[:issue].gitlab_commit_path).to eq("/#{project.namespace.path}/#{project.path}/-/commit/#{commit_id}") }
       end
     end
 

spec/models/project_services/jira_service_spec.rb

@@ -421,7 +421,7 @@ describe JiraService do
             GlobalID: 'GitLab',
             relationship: 'mentioned on',
             object: {
-              url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{commit_id}",
+              url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/#{commit_id}",
               title: "Solved by commit #{commit_id}.",
               icon: { title: 'GitLab', url16x16: favicon_path },
               status: { resolved: true }
@@ -464,7 +464,7 @@ describe JiraService do
         @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
         expect(WebMock).to have_requested(:post, @comment_url).with(
-          body: %r{#{custom_base_url}/#{project.full_path}/commit/#{commit_id}}
+          body: %r{#{custom_base_url}/#{project.full_path}/-/commit/#{commit_id}}
         ).once
       end
 
@@ -479,7 +479,7 @@ describe JiraService do
         @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
         expect(WebMock).to have_requested(:post, @comment_url).with(
-          body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{commit_id}}
+          body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/#{commit_id}}
         ).once
       end
 

spec/requests/api/releases_spec.rb

@@ -61,9 +61,9 @@ describe API::Releases do
       it 'returns rendered helper paths' do
         get api("/projects/#{project.id}/releases", maintainer)
 
-        expect(json_response.first['commit_path']).to eq("/#{release_2.project.full_path}/commit/#{release_2.commit.id}")
+        expect(json_response.first['commit_path']).to eq("/#{release_2.project.full_path}/-/commit/#{release_2.commit.id}")
         expect(json_response.first['tag_path']).to eq("/#{release_2.project.full_path}/-/tags/#{release_2.tag}")
-        expect(json_response.second['commit_path']).to eq("/#{release_1.project.full_path}/commit/#{release_1.commit.id}")
+        expect(json_response.second['commit_path']).to eq("/#{release_1.project.full_path}/-/commit/#{release_1.commit.id}")
         expect(json_response.second['tag_path']).to eq("/#{release_1.project.full_path}/-/tags/#{release_1.tag}")
       end
 
@@ -164,7 +164,7 @@ describe API::Releases do
 
           expect(response).to match_response_schema('public_api/v4/releases')
           expect(json_response.first['assets']['count']).to eq(release.links.count + release.sources.count)
-          expect(json_response.first['commit_path']).to eq("/#{release.project.full_path}/commit/#{release.commit.id}")
+          expect(json_response.first['commit_path']).to eq("/#{release.project.full_path}/-/commit/#{release.commit.id}")
           expect(json_response.first['tag_path']).to eq("/#{release.project.full_path}/-/tags/#{release.tag}")
         end
       end
@@ -214,7 +214,7 @@ describe API::Releases do
         expect(json_response['author']['name']).to eq(maintainer.name)
         expect(json_response['commit']['id']).to eq(commit.id)
         expect(json_response['assets']['count']).to eq(4)
-        expect(json_response['commit_path']).to eq("/#{release.project.full_path}/commit/#{release.commit.id}")
+        expect(json_response['commit_path']).to eq("/#{release.project.full_path}/-/commit/#{release.commit.id}")
         expect(json_response['tag_path']).to eq("/#{release.project.full_path}/-/tags/#{release.tag}")
       end
 

spec/routing/project_routing_spec.rb

@@ -445,10 +445,14 @@ describe 'project routing' do
   # project_commit GET    /:project_id/commit/:id(.:format) commit#show {id: /\h{7,40}/, project_id: /[^\/]+/}
   describe Projects::CommitController, 'routing' do
     it 'to #show' do
+      expect(get('/gitlab/gitlabhq/-/commit/4246fbd')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd')
+      expect(get('/gitlab/gitlabhq/-/commit/4246fbd.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'diff')
+      expect(get('/gitlab/gitlabhq/-/commit/4246fbd.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'patch')
+      expect(get('/gitlab/gitlabhq/-/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5')
+    end
+
+    it 'to #show unscoped routing' do
       expect(get('/gitlab/gitlabhq/commit/4246fbd')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd')
-      expect(get('/gitlab/gitlabhq/commit/4246fbd.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'diff')
-      expect(get('/gitlab/gitlabhq/commit/4246fbd.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'patch')
-      expect(get('/gitlab/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5')
     end
   end
 

spec/services/git/branch_hooks_service_spec.rb

@@ -69,6 +69,7 @@ describe Git::BranchHooksService do
               Gitlab.config.gitlab.url,
               project.namespace.to_param,
               project.to_param,
+              '-',
               'commit',
               commit.id
             ].join('/')

spec/services/git/branch_push_service_spec.rb

@@ -421,7 +421,7 @@ describe Git::BranchPushService, services: true do
         let(:message) { "this is some work.\n\ncloses JIRA-1" }
         let(:comment_body) do
           {
-            body: "Issue solved with [#{closing_commit.id}|http://#{Gitlab.config.gitlab.host}/#{project.full_path}/commit/#{closing_commit.id}]."
+            body: "Issue solved with [#{closing_commit.id}|http://#{Gitlab.config.gitlab.host}/#{project.full_path}/-/commit/#{closing_commit.id}]."
           }.to_json
         end
 

spec/services/git/tag_hooks_service_spec.rb

@@ -107,6 +107,7 @@ describe Git::TagHooksService, :service do
                Gitlab.config.gitlab.url,
                project.namespace.to_param,
                project.to_param,
+               '-',
                'commit',
                commit.id
               ].join('/')

spec/services/system_note_service_spec.rb

@@ -322,7 +322,7 @@ describe SystemNoteService do
       links = []
       if link_exists
         url = if type == 'commit'
-                "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/commit/#{commit.id}"
+                "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/-/commit/#{commit.id}"
               else
                 "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/-/merge_requests/#{merge_request.iid}"
               end
@@ -462,7 +462,7 @@ describe SystemNoteService do
     describe "existing reference" do
       before do
         allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
-        message = "[#{author.name}|http://localhost/#{author.username}] mentioned this issue in [a commit of #{project.full_path}|http://localhost/#{project.full_path}/commit/#{commit.id}]:\n'#{commit.title.chomp}'"
+        message = "[#{author.name}|http://localhost/#{author.username}] mentioned this issue in [a commit of #{project.full_path}|http://localhost/#{project.full_path}/-/commit/#{commit.id}]:\n'#{commit.title.chomp}'"
         allow_next_instance_of(JIRA::Resource::Issue) do |instance|
           allow(instance).to receive(:comments).and_return([OpenStruct.new(body: message)])
         end