Merge branch 'deployment-merge-requests-widget' into 'master'

Use explicitly tracked deployments for MR widgets

See merge request gitlab-org/gitlab!20066

app/controllers/projects/merge_requests_controller.rb

@@ -218,11 +218,16 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def ci_environments_status
-    environments = if ci_environments_status_on_merge_result?
-                     EnvironmentStatus.after_merge_request(@merge_request, current_user)
-                   else
-                     EnvironmentStatus.for_merge_request(@merge_request, current_user)
-                   end
+    environments =
+      if ci_environments_status_on_merge_result?
+        if Feature.enabled?(:deployment_merge_requests_widget, @project)
+          EnvironmentStatus.for_deployed_merge_request(@merge_request, current_user)
+        else
+          EnvironmentStatus.after_merge_request(@merge_request, current_user)
+        end
+      else
+        EnvironmentStatus.for_merge_request(@merge_request, current_user)
+      end
 
     render json: EnvironmentStatusSerializer.new(current_user: current_user).represent(environments)
   end

app/models/environment_status.rb

@@ -20,6 +20,28 @@ class EnvironmentStatus
     build_environments_status(mr, user, mr.merge_pipeline)
   end
 
+  def self.for_deployed_merge_request(mr, user)
+    statuses = []
+
+    mr.recent_visible_deployments.each do |deploy|
+      env = deploy.environment
+
+      next unless Ability.allowed?(user, :read_environment, env)
+
+      statuses <<
+        EnvironmentStatus.new(deploy.project, env, mr, deploy.sha)
+    end
+
+    # Existing projects that used deployments prior to the introduction of
+    # explicitly linked merge requests won't have any data using this new
+    # approach, so we fall back to retrieving deployments based on CI pipelines.
+    if statuses.any?
+      statuses
+    else
+      after_merge_request(mr, user)
+    end
+  end
+
   def initialize(project, environment, merge_request, sha)
     @project = project
     @environment = environment

app/models/merge_request.rb

@@ -73,6 +73,14 @@ class MergeRequest < ApplicationRecord
   has_many :merge_request_assignees
   has_many :assignees, class_name: "User", through: :merge_request_assignees
 
+  has_many :deployment_merge_requests
+
+  # These are deployments created after the merge request has been merged, and
+  # the merge request was tracked explicitly (instead of implicitly using a CI
+  # build).
+  has_many :deployments,
+    through: :deployment_merge_requests
+
   KNOWN_MERGE_PARAMS = [
     :auto_merge_strategy,
     :should_remove_source_branch,
@@ -1475,6 +1483,10 @@ class MergeRequest < ApplicationRecord
     true
   end
 
+  def recent_visible_deployments
+    deployments.visible.includes(:environment).order(id: :desc).limit(10)
+  end
+
   private
 
   def with_rebase_lock

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -1280,6 +1280,28 @@ describe Projects::MergeRequestsController do
       end
     end
 
+    it 'uses the explicitly linked deployments' do
+      expect(EnvironmentStatus)
+        .to receive(:for_deployed_merge_request)
+        .with(merge_request, user)
+        .and_call_original
+
+      get_ci_environments_status(environment_target: 'merge_commit')
+    end
+
+    context 'when the deployment_merge_requests_widget feature flag is disabled' do
+      it 'uses the deployments retrieved using CI builds' do
+        stub_feature_flags(deployment_merge_requests_widget: false)
+
+        expect(EnvironmentStatus)
+          .to receive(:after_merge_request)
+          .with(merge_request, user)
+          .and_call_original
+
+        get_ci_environments_status(environment_target: 'merge_commit')
+      end
+    end
+
     def get_ci_environments_status(extra_params = {})
       params = {
         namespace_id: merge_request.project.namespace.to_param,

spec/lib/gitlab/import_export/all_models.yml

@@ -139,6 +139,8 @@ merge_requests:
 - blocking_merge_requests
 - blocked_merge_requests
 - description_versions
+- deployment_merge_requests
+- deployments
 external_pull_requests:
 - project
 merge_request_diff:

spec/models/environment_status_spec.rb

@@ -92,6 +92,84 @@ describe EnvironmentStatus do
     end
   end
 
+  describe '.for_deployed_merge_request' do
+    context 'when a merge request has no explicitly linked deployments' do
+      it 'returns the statuses based on the CI pipelines' do
+        mr = create(:merge_request, :merged)
+
+        expect(described_class)
+          .to receive(:after_merge_request)
+          .with(mr, mr.author)
+          .and_return([])
+
+        statuses = described_class.for_deployed_merge_request(mr, mr.author)
+
+        expect(statuses).to eq([])
+      end
+    end
+
+    context 'when a merge request has explicitly linked deployments' do
+      let(:merge_request) { create(:merge_request, :merged) }
+
+      let(:environment) do
+        create(:environment, project: merge_request.target_project)
+      end
+
+      it 'returns the statuses based on the linked deployments' do
+        deploy = create(
+          :deployment,
+          :success,
+          project: merge_request.target_project,
+          environment: environment,
+          deployable: nil
+        )
+
+        deploy.link_merge_requests(merge_request.target_project.merge_requests)
+
+        statuses = described_class
+          .for_deployed_merge_request(merge_request, merge_request.author)
+
+        expect(statuses.length).to eq(1)
+        expect(statuses[0].environment).to eq(environment)
+        expect(statuses[0].merge_request).to eq(merge_request)
+      end
+
+      it 'excludes environments the user can not see' do
+        deploy = create(
+          :deployment,
+          :success,
+          project: merge_request.target_project,
+          environment: environment,
+          deployable: nil
+        )
+
+        deploy.link_merge_requests(merge_request.target_project.merge_requests)
+
+        statuses = described_class
+          .for_deployed_merge_request(merge_request, create(:user))
+
+        expect(statuses).to be_empty
+      end
+
+      it 'excludes deployments that have the status "created"' do
+        deploy = create(
+          :deployment,
+          :created,
+          project: merge_request.target_project,
+          environment: environment,
+          deployable: nil
+        )
+
+        deploy.link_merge_requests(merge_request.target_project.merge_requests)
+
+        statuses = described_class
+          .for_deployed_merge_request(merge_request, merge_request.author)
+
+        expect(statuses).to be_empty
+      end
+    end
+  end
+
   describe '.build_environments_status' do
     subject { described_class.send(:build_environments_status, merge_request, user, pipeline) }
 

spec/models/merge_request_spec.rb

@@ -3391,4 +3391,56 @@ describe MergeRequest do
       ])
     end
   end
+
+  describe '#recent_visible_deployments' do
+    let(:merge_request) { create(:merge_request) }
+
+    let(:environment) do
+      create(:environment, project: merge_request.target_project)
+    end
+
+    it 'returns visible deployments' do
+      created = create(
+        :deployment,
+        :created,
+        project: merge_request.target_project,
+        environment: environment
+      )
+
+      success = create(
+        :deployment,
+        :success,
+        project: merge_request.target_project,
+        environment: environment
+      )
+
+      failed = create(
+        :deployment,
+        :failed,
+        project: merge_request.target_project,
+        environment: environment
+      )
+
+      merge_request.deployment_merge_requests.create!(deployment: created)
+      merge_request.deployment_merge_requests.create!(deployment: success)
+      merge_request.deployment_merge_requests.create!(deployment: failed)
+
+      expect(merge_request.recent_visible_deployments).to eq([failed, success])
+    end
+
+    it 'only returns a limited number of deployments' do
+      20.times do
+        deploy = create(
+          :deployment,
+          :success,
+          project: merge_request.target_project,
+          environment: environment
+        )
+
+        merge_request.deployment_merge_requests.create!(deployment: deploy)
+      end
+
+      expect(merge_request.recent_visible_deployments.count).to eq(10)
+    end
+  end
 end