Incorporate feedback

CHANGELOG

@@ -37,7 +37,7 @@ v 8.11.0 (unreleased)
   - Update `timeago` plugin to use multiple string/locale settings
   - Remove unused images (ClemMakesApps)
   - Limit git rev-list output count to one in forced push check
-  - Show deployment status on merge requests
+  - Show deployment status on merge requests with external URLs
   - Clean up unused routes (Josef Strzibny)
   - Fix issue on empty project to allow developers to only push to protected branches if given permission
   - Add green outline to New Branch button. !5447 (winniehell)

app/models/deployment.rb

@@ -37,10 +37,10 @@ class Deployment < ActiveRecord::Base
     deployable.try(:other_actions)
   end
 
-  def deployed_to(ref)
+  def deployed_to?(ref)
     commit = project.commit(ref)
     return false unless commit
 
-    project.repository.merge_base(commit.id, sha) == commit.id
+    project.repository.is_ancestor?(commit.id, sha)
   end
 end

app/models/environment.rb

@@ -26,9 +26,9 @@ class Environment < ActiveRecord::Base
     self.external_url = nil if self.external_url.blank?
   end
 
-  def deployed_to?(ref)
-    return unless last_deployment
+  def deployed_from?(ref)
+    return false unless last_deployment
 
-    last_deployment.deployed_to(ref)
+    last_deployment.deployed_to?(ref)
   end
 end

app/models/merge_request.rb

@@ -592,7 +592,7 @@ class MergeRequest < ActiveRecord::Base
 
   def environments
     target_project.environments.select do |environment|
-      environment.deployed_to?(ref_path)
+      environment.deployed_from?(ref_path)
     end
   end
 

spec/models/deployment_spec.rb

@@ -15,4 +15,24 @@ describe Deployment, models: true do
 
   it { is_expected.to validate_presence_of(:ref) }
   it { is_expected.to validate_presence_of(:sha) }
+
+  describe '#deployed_to?' do
+    let(:project)     { create(:project) }
+    let(:environment) { create(:environment, project: project) }
+    let(:deployment) do
+      create(:deployment, environment: environment, sha: '5f923865dde3436854e9ceb9cdb7815618d4e849')
+    end
+
+    context 'when there is no project commit' do
+      it 'returns false' do
+        expect(deployment.deployed_to?('random-branch')).to be false
+      end
+    end
+
+    context 'when they share the same tree branch' do
+      it 'returns true' do
+        expect(deployment.deployed_to?('HEAD')).to be true
+      end
+    end
+  end
 end

spec/models/environment_spec.rb

@@ -30,4 +30,14 @@ describe Environment, models: true do
       expect(env.external_url).to be_nil
     end
   end
+
+  describe '#deployed_from?' do
+    let(:environment) { create(:environment) }
+
+    context 'without a last deployment' do
+      it "returns false" do
+        expect(environment.deployed_from?('HEAD')).to be false
+      end
+    end
+  end
 end

spec/models/merge_request_spec.rb

@@ -674,6 +674,21 @@ describe MergeRequest, models: true do
     end
   end
 
+  describe "#environments" do
+    let(:project)       { create(:project) }
+
+    let!(:deployment)   { create(:deployment, environment: environment, sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') }
+
+    let!(:environment)  { create(:environment, project: project) }
+    let!(:environment1) { create(:environment, project: project) }
+
+    let(:merge_request) { create(:merge_request, source_project: project) }
+
+    it 'selects deployed environments' do
+      expect(merge_request.environments).to eq [environment]
+    end
+  end
+
   describe "#reload_diff" do
     let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) }
 

spec/views/projects/merge_requests/_heading.html.haml_spec.rb

@@ -7,8 +7,10 @@ describe 'projects/merge_requests/widget/_heading' do
     let(:project)       { merge_request.target_project }
     let(:merge_request) { create(:merge_request, :merged) }
     let(:environment)   { create(:environment, project: project) }
-    let!(:deployment)   { create(:deployment, environment: environment,
-                                    sha: 'a5391128b0ef5d21df5dd23d98557f4ef12fae20') }
+    let!(:deployment)   do
+      create(:deployment, environment: environment,
+                          sha: 'a5391128b0ef5d21df5dd23d98557f4ef12fae20')
+    end
 
     before do
       assign(:merge_request, merge_request)