From 07fc2f852a0b4136b6d97c1d9773819c47e7e8e7 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" <zegerjan@gitlab.com> Date: Tue, 9 Aug 2016 15:11:14 +0200 Subject: [PATCH] Method names changed to #includes_commit? --- .../stylesheets/pages/merge_requests.scss | 5 ++- app/models/deployment.rb | 3 +- app/models/environment.rb | 4 +-- app/models/merge_request.rb | 4 ++- .../merge_requests/widget/_heading.html.haml | 9 ++++-- db/schema.rb | 2 +- spec/models/deployment_spec.rb | 13 +++++--- spec/models/environment_spec.rb | 31 ++++++++++++++++--- spec/models/merge_request_spec.rb | 11 +++---- .../merge_requests/_heading.html.haml_spec.rb | 7 +++-- 10 files changed, 60 insertions(+), 29 deletions(-) diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 0a661e529f0..b4636269518 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -69,6 +69,10 @@ &.ci-success { color: $gl-success; + + a.environment { + color: inherit; + } } &.ci-success_with_warnings { @@ -126,7 +130,6 @@ &.has-conflicts .fa-exclamation-triangle { color: $gl-warning; } - } p:last-child { diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 19b08f49d96..1e338889714 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -37,8 +37,7 @@ class Deployment < ActiveRecord::Base deployable.try(:other_actions) end - def deployed_to?(ref) - commit = project.commit(ref) + def includes_commit?(commit) return false unless commit project.repository.is_ancestor?(commit.id, sha) diff --git a/app/models/environment.rb b/app/models/environment.rb index 7247125f8a0..75e6f869786 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -26,9 +26,9 @@ class Environment < ActiveRecord::Base self.external_url = nil if self.external_url.blank? end - def deployed_from?(ref) + def includes_commit?(commit) return false unless last_deployment - last_deployment.deployed_to?(ref) + last_deployment.includes_commit?(commit) end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 945b0d76505..491ee2792ec 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -591,8 +591,10 @@ class MergeRequest < ActiveRecord::Base end def environments + return unless diff_head_commit + target_project.environments.select do |environment| - environment.deployed_from?(ref_path) + environment.includes_commit?(diff_head_commit) end end diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 16e923b831c..494695a03a5 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -45,10 +45,13 @@ - @merge_request.environments.each do |environment| .mr-widget-heading - .ci_widget{ class: "ci-success" } + .ci_widget.ci-success = ci_icon_for_status("success") %span.hidden-sm - Released to #{environment.name}. + Deployed to + = succeed '.' do + = link_to environment.name, namespace_project_environment_path(@project.namespace, @project, environment), class: 'environment' - external_url = environment.external_url - if external_url - = link_to icon('external-link', text: "View on #{external_url.gsub(/\A.*?:\/\//, '')}"), external_url + = link_to external_url, target: '_blank' do + = icon('external-link', text: "View on #{external_url.gsub(/\A.*?:\/\//, '')}", right: true) diff --git a/db/schema.rb b/db/schema.rb index 6c85e1e9dba..1aa4e8a73d0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -589,12 +589,12 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.datetime "locked_at" t.integer "updated_by_id" t.string "merge_error" - t.text "merge_params" t.boolean "merge_when_build_succeeds", default: false, null: false t.integer "merge_user_id" t.string "merge_commit_sha" t.datetime "deleted_at" t.string "in_progress_merge_commit_sha" + t.text "merge_params" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index d7cb142dd32..bfff639ad78 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -16,23 +16,26 @@ 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 + describe '#includes_commit?' do let(:project) { create(:project) } let(:environment) { create(:environment, project: project) } let(:deployment) do - create(:deployment, environment: environment, - sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') + create(:deployment, environment: environment, sha: project.commit.id) end context 'when there is no project commit' do it 'returns false' do - expect(deployment.deployed_to?('random-branch')).to be false + commit = project.commit('feature') + + expect(deployment.includes_commit?(commit)).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 + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be true end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e65b4f82eff..c881897926e 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -31,12 +31,35 @@ describe Environment, models: true do end end - describe '#deployed_from?' do - let(:environment) { create(:environment) } - + describe '#includes_commit?' do context 'without a last deployment' do it "returns false" do - expect(environment.deployed_from?('HEAD')).to be false + expect(environment.includes_commit?('HEAD')).to be false + end + end + + context 'with a last deployment' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + context 'in the same branch' do + it 'returns true' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be true + end + end + + context 'not in the same branch' do + before do + deployment.update(sha: project.commit('feature').id) + end + + it 'returns false' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be false + end end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e605720a2dd..35a4418ebb3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -676,18 +676,15 @@ describe MergeRequest, models: true do describe "#environments" do let(:project) { create(:project) } - - let!(:deployment) do - create(:deployment, environment: environment, - sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') - end - let!(:environment) { create(:environment, project: project) } let!(:environment1) { create(:environment, project: project) } - + let!(:environment2) { create(:environment, project: project) } let(:merge_request) { create(:merge_request, source_project: project) } it 'selects deployed environments' do + create(:deployment, environment: environment, sha: project.commit('master').id) + create(:deployment, environment: environment1, sha: project.commit('feature').id) + expect(merge_request.environments).to eq [environment] end end diff --git a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb index 0302f14e660..733b2dfa7ff 100644 --- a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb @@ -8,18 +8,19 @@ describe 'projects/merge_requests/widget/_heading' do let(:merge_request) { create(:merge_request, :merged) } let(:environment) { create(:environment, project: project) } let!(:deployment) do - create(:deployment, environment: environment, - sha: 'a5391128b0ef5d21df5dd23d98557f4ef12fae20') + create(:deployment, environment: environment, sha: project.commit('master').id) end before do assign(:merge_request, merge_request) + assign(:project, project) render end it 'displays that the environment is deployed' do - expect(rendered).to match("Released to #{environment.name}") + expect(rendered).to match("Deployed to") + expect(rendered).to match("#{environment.name}") end end end -- GitLab