Move expanded_environment_url to CreateDeploymentService

Because that's the only place we need it.

4 files changed, 57 insertions, 58 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 9e38a916a2e..4454057e418 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -138,17 +138,6 @@ module Ci
       ExpandVariables.expand(environment, simple_variables) if environment
     end
 
-    def expanded_environment_url
-      return @expanded_environment_url if defined?(@expanded_environment_url)
-
-      @expanded_environment_url =
-        if unexpanded_url = environment_url
-          ExpandVariables.expand(unexpanded_url, simple_variables)
-        else
-          persisted_environment&.external_url
-        end
-    end
-
     def has_environment?
       environment.present?
     end
@@ -482,15 +471,10 @@ module Ci
 
       variables = persisted_environment.predefined_variables
 
-      if url = environment_url
-        # Note that CI_ENVIRONMENT_URL should be the last variable, because
-        # here we're passing unexpanded environment_url for runner to expand,
-        # and the runner would expand in order. In order to make sure that
-        # CI_ENVIRONMENT_URL has everything available, such as variables
-        # from Environment#predefined_variables, we need to make sure it's
-        # the last variable.
-        variables << { key: 'CI_ENVIRONMENT_URL', value: url, public: true }
-      end
+        # Here we're passing unexpanded environment_url for runner to expand,
+        # and we need to make sure that CI_ENVIRONMENT_NAME and
+        # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
+      variables << { key: 'CI_ENVIRONMENT_URL', value: environment_url, public: true } if environment_url
 
       variables
     end
@@ -514,7 +498,11 @@ module Ci
     end
 
     def environment_url
-      options&.dig(:environment, :url)
+      return @environment_url if defined?(@environment_url)
+
+      @environment_url =
+        options&.dig(:environment, :url) ||
+          persisted_environment&.external_url
     end
 
     def build_attributes_from_config
diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb
index fd8781aa48f..85500d6c9c9 100644
--- a/app/services/create_deployment_service.rb
+++ b/app/services/create_deployment_service.rb
@@ -2,7 +2,7 @@ class CreateDeploymentService
   attr_reader :job
 
   delegate :expanded_environment_name,
-           :expanded_environment_url,
+           :simple_variables,
            :project,
            to: :job
 
@@ -50,6 +50,17 @@ class CreateDeploymentService
     @environment_options ||= job.options&.dig(:environment) || {}
   end
 
+  def expanded_environment_url
+    return @expanded_environment_url if defined?(@expanded_environment_url)
+
+    @expanded_environment_url =
+      if unexpanded_url = environment_options[:url]
+        ExpandVariables.expand(unexpanded_url, simple_variables)
+      else
+        environment&.external_url
+      end
+  end
+
   def on_stop
     environment_options[:on_stop]
   end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index ea6a5fc48c5..77ab9a5c5ab 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -451,42 +451,6 @@ describe Ci::Build, :models do
       end
     end
 
-    describe '#expanded_environment_url' do
-      subject { job.expanded_environment_url }
-
-      context 'when yaml environment uses $CI_COMMIT_REF_NAME' do
-        let(:job) do
-          create(:ci_build,
-                 ref: 'master',
-                 options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } })
-        end
-
-        it { is_expected.to eq('http://review/master') }
-      end
-
-      context 'when yaml environment uses yaml_variables containing symbol keys' do
-        let(:job) do
-          create(:ci_build,
-                 yaml_variables: [{ key: :APP_HOST, value: 'host' }],
-                 options: { environment: { url: 'http://review/$APP_HOST' } })
-        end
-
-        it { is_expected.to eq('http://review/host') }
-      end
-
-      context 'when yaml environment does not have url' do
-        let(:job) { create(:ci_build, environment: 'staging') }
-
-        let!(:environment) do
-          create(:environment, project: job.project, name: job.environment)
-        end
-
-        it 'returns the external_url from persisted environment' do
-          is_expected.to eq(environment.external_url)
-        end
-      end
-    end
-
     describe '#starts_environment?' do
       subject { build.starts_environment? }
 
diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb
index 6cf4342ad4c..790713cf2e4 100644
--- a/spec/services/create_deployment_service_spec.rb
+++ b/spec/services/create_deployment_service_spec.rb
@@ -122,6 +122,42 @@ describe CreateDeploymentService, services: true do
     end
   end
 
+  describe '#expanded_environment_url' do
+    subject { service.send(:expanded_environment_url) }
+
+    context 'when yaml environment uses $CI_COMMIT_REF_NAME' do
+      let(:job) do
+        create(:ci_build,
+               ref: 'master',
+               options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } })
+      end
+
+      it { is_expected.to eq('http://review/master') }
+    end
+
+    context 'when yaml environment uses yaml_variables containing symbol keys' do
+      let(:job) do
+        create(:ci_build,
+               yaml_variables: [{ key: :APP_HOST, value: 'host' }],
+               options: { environment: { url: 'http://review/$APP_HOST' } })
+      end
+
+      it { is_expected.to eq('http://review/host') }
+    end
+
+    context 'when yaml environment does not have url' do
+      let(:job) { create(:ci_build, environment: 'staging') }
+
+      let!(:environment) do
+        create(:environment, project: job.project, name: job.environment)
+      end
+
+      it 'returns the external_url from persisted environment' do
+        is_expected.to eq(environment.external_url)
+      end
+    end
+  end
+
   describe 'processing of builds' do
     shared_examples 'does not create deployment' do
       it 'does not create a new deployment' do