Merge branch 'dz-redirect-unscoped-pipelines-routes' into 'master'

Move pipelines routes under /-/ scope

See merge request gitlab-org/gitlab!30730

app/models/project_services/chat_message/pipeline_message.rb

@@ -183,7 +183,7 @@ module ChatMessage
       if ref_type == 'tag'
         "#{project_url}/-/tags/#{ref}"
       else
-        "#{project_url}/commits/#{ref}"
+        "#{project_url}/-/commits/#{ref}"
       end
     end
 
@@ -200,14 +200,14 @@ module ChatMessage
     end
 
     def pipeline_failed_jobs_url
-      "#{project_url}/pipelines/#{pipeline_id}/failures"
+      "#{project_url}/-/pipelines/#{pipeline_id}/failures"
     end
 
     def pipeline_url
       if failed_jobs.any?
         pipeline_failed_jobs_url
       else
-        "#{project_url}/pipelines/#{pipeline_id}"
+        "#{project_url}/-/pipelines/#{pipeline_id}"
       end
     end
 

app/models/project_services/hipchat_service.rb

@@ -252,8 +252,8 @@ class HipchatService < Service
     status = pipeline_attributes[:status]
     duration = pipeline_attributes[:duration]
 
-    branch_link = "<a href=\"#{project_url}/commits/#{CGI.escape(ref)}\">#{ref}</a>"
-    pipeline_url = "<a href=\"#{project_url}/pipelines/#{pipeline_id}\">##{pipeline_id}</a>"
+    branch_link = "<a href=\"#{project_url}/-/commits/#{CGI.escape(ref)}\">#{ref}</a>"
+    pipeline_url = "<a href=\"#{project_url}/-/pipelines/#{pipeline_id}\">##{pipeline_id}</a>"
 
     "#{project_link}: Pipeline #{pipeline_url} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status(status)} in #{duration} second(s)"
   end

changelogs/unreleased/dz-redirect-unscoped-pipelines-routes.yml

+---
+title: Move pipelines routing under /-/ scope
+merge_request: 30730
+author:
+type: changed

config/routes/project.rb

@@ -315,6 +315,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
         draw :issues
         draw :merge_requests
+        draw :pipelines
 
         # The wiki and repository routing contains wildcard characters so
         # its preferable to keep it below all other project routes
@@ -380,17 +381,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       post 'alerts/notify', to: 'alerting/notifications#create'
 
-      # Unscoped route. It will be replaced with redirect to /-/pipelines/
-      # Issue https://gitlab.com/gitlab-org/gitlab/issues/118849
-      draw :pipelines
-
-      # To ensure an old unscoped routing is used for the UI we need to
-      # add prefix 'as' to the scope routing and place it below original routing.
-      # Issue https://gitlab.com/gitlab-org/gitlab/issues/118849
-      scope '-', as: 'scoped' do
-        draw :pipelines
-      end
-
       draw :legacy_builds
 
       resources :hooks, only: [:index, :create, :edit, :update, :destroy], constraints: { id: /\d+/ } do # rubocop: disable Cop/PutProjectRoutesUnderScope
@@ -486,6 +476,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       # Deprecated unscoped routing.
       # Issue https://gitlab.com/gitlab-org/gitlab/issues/118849
       scope as: 'deprecated' do
+        draw :pipelines
         draw :repository
       end
 

ee/app/assets/javascripts/onboarding/tour_parts.js

@@ -333,7 +333,7 @@ const GUIDED_GITLAB_TOUR = [
     },
   },
   {
-    forUrl: ({ projectFullPath }) => new RegExp(`${projectFullPath}/pipelines/[0-9]+$`, ''),
+    forUrl: ({ projectFullPath }) => new RegExp(`${projectFullPath}/-/pipelines/[0-9]+$`, ''),
     getHelpContent: () => [
       {
         text: sprintf(
@@ -385,7 +385,7 @@ const GUIDED_GITLAB_TOUR = [
 
 const CREATE_PROJECT_TOUR = [
   {
-    forUrl: ({ projectFullPath }) => new RegExp(`${projectFullPath}/pipelines/[0-9]+$`, ''),
+    forUrl: ({ projectFullPath }) => new RegExp(`${projectFullPath}/-/pipelines/[0-9]+$`, ''),
     getHelpContent: null,
     actionPopover: {
       selector: '#js-onboarding-new-project-link',

ee/spec/services/ci/expire_pipeline_cache_service_spec.rb

@@ -12,7 +12,7 @@ describe Ci::ExpirePipelineCacheService do
       let(:source) { create(:ci_sources_pipeline, pipeline: pipeline) }
 
       it 'updates the cache of dependent pipeline' do
-        dependent_pipeline_path = "/#{source.source_project.full_path}/pipelines/#{source.source_pipeline.id}.json"
+        dependent_pipeline_path = "/#{source.source_project.full_path}/-/pipelines/#{source.source_pipeline.id}.json"
 
         allow_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch)
         expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(dependent_pipeline_path)
@@ -26,7 +26,7 @@ describe Ci::ExpirePipelineCacheService do
       let(:source) { create(:ci_sources_pipeline, source_job: build) }
 
       it 'updates the cache of dependent pipeline' do
-        dependent_pipeline_path = "/#{source.project.full_path}/pipelines/#{source.pipeline.id}.json"
+        dependent_pipeline_path = "/#{source.project.full_path}/-/pipelines/#{source.pipeline.id}.json"
 
         allow_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch)
         expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(dependent_pipeline_path)

spec/lib/gitlab/ci/status/stage/play_manual_spec.rb

@@ -31,7 +31,7 @@ describe Gitlab::Ci::Status::Stage::PlayManual do
 
     subject { play_manual.action_path }
 
-    it { is_expected.to eq("/#{pipeline.project.full_path}/pipelines/#{pipeline.id}/stages/#{stage.name}/play_manual") }
+    it { is_expected.to eq("/#{pipeline.project.full_path}/-/pipelines/#{pipeline.id}/stages/#{stage.name}/play_manual") }
   end
 
   describe '#action_method' do

spec/lib/gitlab/etag_caching/router_spec.rb

@@ -23,7 +23,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'matches issue title endpoint' do
     result = described_class.match(
-      '/my-group/my-project/issues/123/realtime_changes'
+      '/my-group/my-project/-/issues/123/realtime_changes'
     )
 
     expect(result).to be_present
@@ -32,7 +32,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'matches with a project name that includes a suffix of create' do
     result = described_class.match(
-      '/group/test-create/issues/123/realtime_changes'
+      '/group/test-create/-/issues/123/realtime_changes'
     )
 
     expect(result).to be_present
@@ -41,7 +41,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'matches with a project name that includes a prefix of create' do
     result = described_class.match(
-      '/group/create-test/issues/123/realtime_changes'
+      '/group/create-test/-/issues/123/realtime_changes'
     )
 
     expect(result).to be_present
@@ -50,7 +50,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'matches project pipelines endpoint' do
     result = described_class.match(
-      '/my-group/my-project/pipelines.json'
+      '/my-group/my-project/-/pipelines.json'
     )
 
     expect(result).to be_present
@@ -95,7 +95,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'does not match blob with confusing name' do
     result = described_class.match(
-      '/my-group/my-project/blob/master/pipelines.json'
+      '/my-group/my-project/-/blob/master/pipelines.json'
     )
 
     expect(result).to be_blank
@@ -121,7 +121,7 @@ describe Gitlab::EtagCaching::Router do
 
   it 'matches pipeline#show endpoint' do
     result = described_class.match(
-      '/my-group/my-project/pipelines/2.json'
+      '/my-group/my-project/-/pipelines/2.json'
     )
 
     expect(result).to be_present

spec/lib/gitlab/patch/action_dispatch_journey_formatter_spec.rb

@@ -7,7 +7,7 @@ describe Gitlab::Patch::ActionDispatchJourneyFormatter do
   let(:project) { create(:project, namespace: group) }
   let(:pipeline) { create(:ci_empty_pipeline, project: project) }
   let(:url) { Gitlab::Routing.url_helpers.project_pipeline_url(project, pipeline) }
-  let(:expected_path) { "#{project.full_path}/pipelines/#{pipeline.id}" }
+  let(:expected_path) { "#{project.full_path}/-/pipelines/#{pipeline.id}" }
 
   context 'custom implementation of #missing_keys' do
     before do

spec/models/ci/build_spec.rb

@@ -2286,7 +2286,7 @@ describe Ci::Build do
       let(:predefined_variables) do
         [
           { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true, masked: false },
-          { key: 'CI_PIPELINE_URL', value: project.web_url + "/pipelines/#{pipeline.id}", public: true, masked: false },
+          { key: 'CI_PIPELINE_URL', value: project.web_url + "/-/pipelines/#{pipeline.id}", public: true, masked: false },
           { key: 'CI_JOB_ID', value: build.id.to_s, public: true, masked: false },
           { key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true, masked: false },
           { key: 'CI_JOB_TOKEN', value: 'my-token', public: false, masked: true },

spec/models/project_services/chat_message/pipeline_message_spec.rb

@@ -61,8 +61,8 @@ describe ChatMessage::PipelineMessage do
 
   it "returns the pipeline summary in the activity's title" do
     expect(subject.activity[:title]).to eq(
-      "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \
-        " of branch [develop](http://example.gitlab.com/commits/develop)" \
+      "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \
+        " of branch [develop](http://example.gitlab.com/-/commits/develop)" \
         " by The Hacker (hacker) has passed"
     )
   end
@@ -74,8 +74,8 @@ describe ChatMessage::PipelineMessage do
 
     it "returns the summary with a 'failed' status" do
       expect(subject.activity[:title]).to eq(
-        "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \
-          " of branch [develop](http://example.gitlab.com/commits/develop)" \
+        "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \
+          " of branch [develop](http://example.gitlab.com/-/commits/develop)" \
           " by The Hacker (hacker) has failed"
       )
     end
@@ -88,8 +88,8 @@ describe ChatMessage::PipelineMessage do
 
     it "returns the summary with a 'passed with warnings' status" do
       expect(subject.activity[:title]).to eq(
-        "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \
-          " of branch [develop](http://example.gitlab.com/commits/develop)" \
+        "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \
+          " of branch [develop](http://example.gitlab.com/-/commits/develop)" \
           " by The Hacker (hacker) has passed with warnings"
       )
     end
@@ -102,8 +102,8 @@ describe ChatMessage::PipelineMessage do
 
     it "returns the summary with 'API' as the username" do
       expect(subject.activity[:title]).to eq(
-        "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \
-          " of branch [develop](http://example.gitlab.com/commits/develop)" \
+        "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \
+          " of branch [develop](http://example.gitlab.com/-/commits/develop)" \
           " by API has passed"
       )
     end
@@ -134,8 +134,8 @@ describe ChatMessage::PipelineMessage do
   it "returns the pipeline summary as the attachment's fallback property" do
     expect(subject.attachments.first[:fallback]).to eq(
       "<http://example.gitlab.com|project_name>:" \
-        " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \
-        " of branch <http://example.gitlab.com/commits/develop|develop>" \
+        " Pipeline <http://example.gitlab.com/-/pipelines/123|#123>" \
+        " of branch <http://example.gitlab.com/-/commits/develop|develop>" \
         " by The Hacker (hacker) has passed in 02:00:10"
     )
   end
@@ -199,7 +199,7 @@ describe ChatMessage::PipelineMessage do
   end
 
   it "returns the pipeline URL as the attachment's title_link property" do
-    expect(subject.attachments.first[:title_link]).to eq("http://example.gitlab.com/pipelines/123")
+    expect(subject.attachments.first[:title_link]).to eq("http://example.gitlab.com/-/pipelines/123")
   end
 
   it "returns two attachment fields" do
@@ -209,7 +209,7 @@ describe ChatMessage::PipelineMessage do
   it "returns the commit message as the attachment's second field property" do
     expect(subject.attachments.first[:fields][0]).to eq({
       title: "Branch",
-      value: "<http://example.gitlab.com/commits/develop|develop>",
+      value: "<http://example.gitlab.com/-/commits/develop|develop>",
       short: true
     })
   end
@@ -237,7 +237,7 @@ describe ChatMessage::PipelineMessage do
     it "returns the stage name and link to the 'Failed jobs' tab on the pipeline's page as the attachment's third field property" do
       expect(subject.attachments.first[:fields][2]).to eq({
         title: "Failed stage",
-        value: "<http://example.gitlab.com/pipelines/123/failures|test>",
+        value: "<http://example.gitlab.com/-/pipelines/123/failures|test>",
         short: true
       })
     end
@@ -261,7 +261,7 @@ describe ChatMessage::PipelineMessage do
     it "returns the stage names and links to the 'Failed jobs' tab on the pipeline's page as the attachment's third field property" do
       expect(subject.attachments.first[:fields][2]).to eq({
         title: "Failed stages",
-        value: "<http://example.gitlab.com/pipelines/123/failures|stage-2>, <http://example.gitlab.com/pipelines/123/failures|stage-1>, <http://example.gitlab.com/pipelines/123/failures|stage-3>",
+        value: "<http://example.gitlab.com/-/pipelines/123/failures|stage-2>, <http://example.gitlab.com/-/pipelines/123/failures|stage-1>, <http://example.gitlab.com/-/pipelines/123/failures|stage-3>",
         short: true
       })
     end
@@ -271,7 +271,7 @@ describe ChatMessage::PipelineMessage do
         "<http://example.gitlab.com/-/jobs/#{i}|job-#{i}>"
       end
 
-      expected_jobs << "and <http://example.gitlab.com/pipelines/123/failures|15 more>"
+      expected_jobs << "and <http://example.gitlab.com/-/pipelines/123/failures|15 more>"
 
       expect(subject.attachments.first[:fields][3]).to eq({
         title: "Failed jobs",
@@ -369,8 +369,8 @@ describe ChatMessage::PipelineMessage do
     it 'returns the pipeline summary as the attachments in markdown format' do
       expect(subject.attachments).to eq(
         "[project_name](http://example.gitlab.com):" \
-          " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \
-          " of branch [develop](http://example.gitlab.com/commits/develop)" \
+          " Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \
+          " of branch [develop](http://example.gitlab.com/-/commits/develop)" \
           " by The Hacker (hacker) has passed in 02:00:10"
       )
     end

spec/models/project_services/hipchat_service_spec.rb

@@ -327,8 +327,8 @@ describe HipchatService do
           user_name = data[:user][:name]
 
           expect(message).to eq("<a href=\"#{project_url}\">#{project_name}</a>: " \
-            "Pipeline <a href=\"#{project_url}/pipelines/#{pipeline.id}\">##{pipeline.id}</a> " \
-            "of <a href=\"#{project_url}/commits/#{ref}\">#{ref}</a> #{ref_type} " \
+            "Pipeline <a href=\"#{project_url}/-/pipelines/#{pipeline.id}\">##{pipeline.id}</a> " \
+            "of <a href=\"#{project_url}/-/commits/#{ref}\">#{ref}</a> #{ref_type} " \
             "by #{user_name} failed in #{duration} second(s)")
         end
       end

spec/serializers/merge_request_widget_entity_spec.rb

@@ -178,7 +178,7 @@ describe MergeRequestWidgetEntity do
     project.add_maintainer(user)
 
     expect(subject[:new_project_pipeline_path])
-      .to eq("/#{resource.project.full_path}/pipelines/new")
+      .to eq("/#{resource.project.full_path}/-/pipelines/new")
   end
 
   describe 'when source project is deleted' do

spec/services/ci/expire_pipeline_cache_service_spec.rb

@@ -10,9 +10,9 @@ describe Ci::ExpirePipelineCacheService do
 
   describe '#execute' do
     it 'invalidates Etag caching for project pipelines path' do
-      pipelines_path = "/#{project.full_path}/pipelines.json"
+      pipelines_path = "/#{project.full_path}/-/pipelines.json"
       new_mr_pipelines_path = "/#{project.full_path}/-/merge_requests/new.json"
-      pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json"
+      pipeline_path = "/#{project.full_path}/-/pipelines/#{pipeline.id}.json"
 
       expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
       expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)

spec/workers/expire_job_cache_worker_spec.rb

@@ -13,7 +13,7 @@ describe ExpireJobCacheWorker do
 
       include_examples 'an idempotent worker' do
         it 'invalidates Etag caching for the job path' do
-          pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json"
+          pipeline_path = "/#{project.full_path}/-/pipelines/#{pipeline.id}.json"
           job_path = "/#{project.full_path}/builds/#{job.id}.json"
 
           spy_store = Gitlab::EtagCaching::Store.new