Merge branch 'ac-review-app-changes-33418' into 'master'

Expose changed URLs in MR ci_environments_status

See merge request gitlab-org/gitlab-ce!22228

9 files changed, 258 insertions, 36 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 8bc3a81d771..f87337b67aa 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -198,43 +198,11 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def ci_environments_status
-    environments =
-      begin
-        @merge_request.environments_for(current_user).map do |environment|
-          project    = environment.project
-          deployment = environment.first_deployment_for(@merge_request.diff_head_sha)
-
-          stop_url =
-            if can?(current_user, :stop_environment, environment)
-              stop_project_environment_path(project, environment)
-            end
-
-          metrics_url =
-            if can?(current_user, :read_environment, environment) && environment.has_metrics?
-              metrics_project_environment_deployment_path(project, environment, deployment)
-            end
-
-          metrics_monitoring_url =
-            if can?(current_user, :read_environment, environment)
-              environment_metrics_path(environment)
-            end
-
-          {
-            id: environment.id,
-            name: environment.name,
-            url: project_environment_path(project, environment),
-            metrics_url: metrics_url,
-            metrics_monitoring_url: metrics_monitoring_url,
-            stop_url: stop_url,
-            external_url: environment.external_url,
-            external_url_formatted: environment.formatted_external_url,
-            deployed_at: deployment.try(:created_at),
-            deployed_at_formatted: deployment.try(:formatted_deployment_time)
-          }
-        end.compact
-      end
+    environments = @merge_request.environments_for(current_user).map do |environment|
+      EnvironmentStatus.new(environment, @merge_request)
+    end
 
-    render json: environments
+    render json: EnvironmentStatusSerializer.new(current_user: current_user).represent(environments)
   end
 
   def rebase
diff --git a/app/models/environment_status.rb b/app/models/environment_status.rb
new file mode 100644
index 00000000000..5ff3acc0e58
--- /dev/null
+++ b/app/models/environment_status.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+class EnvironmentStatus
+  include Gitlab::Utils::StrongMemoize
+
+  attr_reader :environment, :merge_request
+
+  delegate :id, to: :environment
+  delegate :name, to: :environment
+  delegate :project, to: :environment
+  delegate :deployed_at, to: :deployment, allow_nil: true
+
+  def initialize(environment, merge_request)
+    @environment = environment
+    @merge_request = merge_request
+  end
+
+  def deployment
+    strong_memoize(:deployment) do
+      environment.first_deployment_for(merge_request.diff_head_sha)
+    end
+  end
+
+  def deployed_at
+    deployment&.created_at
+  end
+
+  def changes
+    sha = merge_request.diff_head_sha
+    return [] if project.route_map_for(sha).nil?
+
+    changed_files.map { |file| build_change(file, sha) }.compact
+  end
+
+  def changed_files
+    merge_request.merge_request_diff
+      .merge_request_diff_files.where(deleted_file: false)
+  end
+
+  private
+
+  PAGE_EXTENSIONS = /\A\.(s?html?|php|asp|cgi|pl)\z/i.freeze
+
+  def build_change(file, sha)
+    public_path = project.public_path_for_source_path(file.new_path, sha)
+    return if public_path.nil?
+
+    ext = File.extname(public_path)
+    return if ext.present? && ext !~ PAGE_EXTENSIONS
+
+    {
+      path: public_path,
+      external_url: environment.external_url_for(file.new_path, sha)
+    }
+  end
+end
diff --git a/app/serializers/environment_status_entity.rb b/app/serializers/environment_status_entity.rb
new file mode 100644
index 00000000000..3dfa4f204c9
--- /dev/null
+++ b/app/serializers/environment_status_entity.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+class EnvironmentStatusEntity < Grape::Entity
+  include RequestAwareEntity
+
+  expose :id
+  expose :name
+
+  expose :url do |es|
+    project_environment_path(es.project, es.environment)
+  end
+
+  expose :metrics_url, if: ->(*) { can_read_environment? && environment.has_metrics? } do |es|
+    metrics_project_environment_deployment_path(es.project, es.environment, es.deployment)
+  end
+
+  expose :metrics_monitoring_url, if: ->(*) { can_read_environment? } do |es|
+    environment_metrics_path(es.environment)
+  end
+
+  expose :stop_url, if: ->(*) { can_stop_environment? } do |es|
+    stop_project_environment_path(es.project, es.environment)
+  end
+
+  expose :external_url do |es|
+    es.environment.external_url
+  end
+
+  expose :external_url_formatted do |es|
+    es.environment.formatted_external_url
+  end
+
+  expose :deployed_at
+
+  expose :deployed_at_formatted do |es|
+    es.deployment.try(:formatted_deployment_time)
+  end
+
+  expose :changes, if: ->(*) { Feature.enabled?(:ci_environments_status_changes, project) }
+
+  private
+
+  def environment
+    object.environment
+  end
+
+  def project
+    object.environment.project
+  end
+
+  def current_user
+    request.current_user
+  end
+
+  def can_read_environment?
+    can?(current_user, :read_environment, environment)
+  end
+
+  def can_stop_environment?
+    can?(current_user, :stop_environment, environment)
+  end
+end
diff --git a/app/serializers/environment_status_serializer.rb b/app/serializers/environment_status_serializer.rb
new file mode 100644
index 00000000000..f8d37934763
--- /dev/null
+++ b/app/serializers/environment_status_serializer.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class EnvironmentStatusSerializer < BaseSerializer
+  entity EnvironmentStatusEntity
+end
diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb
index cac56695319..90d6a338479 100644
--- a/spec/factories/deployments.rb
+++ b/spec/factories/deployments.rb
@@ -16,5 +16,10 @@ FactoryBot.define do
         allow(deployment.project.repository).to receive(:create_ref)
       end
     end
+
+    trait :review_app do
+      sha { TestEnv::BRANCH_SHA['pages-deploy'] }
+      ref 'pages-deploy'
+    end
   end
 end
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 8094c43b065..2392bfc4a53 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -101,6 +101,20 @@ FactoryBot.define do
       end
     end
 
+    trait :deployed_review_app do
+      target_branch 'pages-deploy-target'
+
+      transient do
+        deployment { create(:deployment, :review_app) }
+      end
+
+      after(:build) do |merge_request, evaluator|
+        merge_request.source_branch = evaluator.deployment.ref
+        merge_request.source_project = evaluator.deployment.project
+        merge_request.target_project = evaluator.deployment.project
+      end
+    end
+
     after(:build) do |merge_request|
       target_project = merge_request.target_project
       source_project = merge_request.source_project
diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb
new file mode 100644
index 00000000000..f2eb263c98c
--- /dev/null
+++ b/spec/models/environment_status_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe EnvironmentStatus do
+  let(:deployment)    { create(:deployment, :review_app) }
+  let(:environment)   { deployment.environment}
+  let(:project)       { deployment.project }
+  let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) }
+
+  subject(:environment_status) { described_class.new(environment, merge_request) }
+
+  it { is_expected.to delegate_method(:id).to(:environment) }
+  it { is_expected.to delegate_method(:name).to(:environment) }
+  it { is_expected.to delegate_method(:project).to(:environment) }
+  it { is_expected.to delegate_method(:deployed_at).to(:deployment).as(:created_at) }
+
+  describe '#project' do
+    subject { environment_status.project }
+
+    it { is_expected.to eq(project) }
+  end
+
+  describe '#merge_request' do
+    subject { environment_status.merge_request }
+
+    it { is_expected.to eq(merge_request) }
+  end
+
+  describe '#deployment' do
+    subject { environment_status.deployment }
+
+    it { is_expected.to eq(deployment) }
+  end
+
+  # $ git diff --stat pages-deploy-target...pages-deploy
+  # .gitlab/route-map.yml              |  5 +++++
+  # files/html/500.html                | 13 -------------
+  # files/html/page.html               |  3 +++
+  # files/js/application.js            |  3 +++
+  # files/markdown/ruby-style-guide.md |  4 ++++
+  # pages-deploy.txt                   |  1 +
+  #
+  # $ cat .gitlab/route-map.yml
+  # - source: /files\/markdown\/(.+)\.md$/
+  #   public: '\1.html'
+  #
+  # - source: /files\/(.+)/
+  #   public: '\1'
+  describe '#changes' do
+    it 'contains only added and modified public pages' do
+      expect(environment_status.changes).to contain_exactly(
+        {
+          path: 'ruby-style-guide.html',
+          external_url: "#{environment.external_url}/ruby-style-guide.html"
+        }, {
+          path: 'html/page.html',
+          external_url: "#{environment.external_url}/html/page.html"
+        }
+      )
+    end
+  end
+end
diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb
new file mode 100644
index 00000000000..6894c65d639
--- /dev/null
+++ b/spec/serializers/environment_status_entity_spec.rb
@@ -0,0 +1,49 @@
+require 'spec_helper'
+
+describe EnvironmentStatusEntity do
+  let(:user)    { create(:user) }
+  let(:request) { double('request') }
+
+  let(:deployment)    { create(:deployment, :review_app) }
+  let(:environment)   { deployment.environment}
+  let(:project)       { deployment.project }
+  let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) }
+
+  let(:environment_status) { EnvironmentStatus.new(environment, merge_request) }
+  let(:entity)             { described_class.new(environment_status, request: request) }
+
+  subject { entity.as_json }
+
+  before do
+    allow(request).to receive(:current_user).and_return(user)
+  end
+
+  it { is_expected.to include(:id) }
+  it { is_expected.to include(:name) }
+  it { is_expected.to include(:url) }
+  it { is_expected.to include(:external_url) }
+  it { is_expected.to include(:external_url_formatted) }
+  it { is_expected.to include(:deployed_at) }
+  it { is_expected.to include(:deployed_at_formatted) }
+  it { is_expected.to include(:changes) }
+
+  it { is_expected.not_to include(:stop_url) }
+  it { is_expected.not_to include(:metrics_url) }
+  it { is_expected.not_to include(:metrics_monitoring_url) }
+
+  context 'when :ci_environments_status_changes feature flag is disabled' do
+    before do
+      stub_feature_flags(ci_environments_status_changes: false)
+    end
+
+    it { is_expected.not_to include(:changes) }
+  end
+
+  context 'when the user is project maintainer' do
+    before do
+      project.add_maintainer(user)
+    end
+
+    it { is_expected.to include(:stop_url) }
+  end
+end
diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb
index 97875669d0e..1a9aa252511 100644
--- a/spec/support/helpers/test_env.rb
+++ b/spec/support/helpers/test_env.rb
@@ -31,6 +31,8 @@ module TestEnv
     'symlink-expand-diff'                => '81e6355',
     'expand-collapse-files'              => '025db92',
     'expand-collapse-lines'              => '238e82d',
+    'pages-deploy'                       => '7897d5b',
+    'pages-deploy-target'                => '7975be0',
     'video'                              => '8879059',
     'add-balsamiq-file'                  => 'b89b56d',
     'crlf-diff'                          => '5938907',