diff --git a/CHANGELOG b/CHANGELOG index 353ab284ccb..34edeafd4d9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -24,6 +24,7 @@ v 8.12.0 (unreleased) - Cycle analytics (first iteration) !5986 - Remove vendor prefixes for linear-gradient CSS (ClemMakesApps) - Move pushes_since_gc from the database to Redis + - Limit number of shown environments on Merge Request: show only environments for target_branch, source_branch and tags - Add font color contrast to external label in admin area (ClemMakesApps) - Change logo animation to CSS (ClemMakesApps) - Instructions for enabling Git packfile bitmaps !6104 diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 5b71113feb9..2ab0bac95bc 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -70,6 +70,10 @@ module GitlabRoutingHelper namespace_project_runner_path(@project.namespace, @project, runner, *args) end + def environment_path(environment, *args) + namespace_project_environment_path(environment.project.namespace, environment.project, environment, *args) + end + def issue_path(entity, *args) namespace_project_issue_path(entity.project.namespace, entity.project, entity, *args) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 616efaf3c42..4c51c979599 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -670,9 +670,12 @@ class MergeRequest < ActiveRecord::Base def environments return [] unless diff_head_commit - target_project.environments.select do |environment| - environment.includes_commit?(diff_head_commit) - end + environments = source_project.environments_for( + source_branch, diff_head_commit) + environments += target_project.environments_for( + target_branch, diff_head_commit, with_tags: true) + + environments.uniq end def state_human_name diff --git a/app/models/project.rb b/app/models/project.rb index d7f20070be0..7265cb55594 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1293,6 +1293,22 @@ class Project < ActiveRecord::Base Gitlab::Redis.with { |redis| redis.del(pushes_since_gc_redis_key) } end + def environments_for(ref, commit, with_tags: false) + environment_ids = deployments.group(:environment_id). + select(:environment_id) + + environment_ids = + if with_tags + environment_ids.where('ref=? OR tag IS TRUE', ref) + else + environment_ids.where(ref: ref) + end + + environments.where(id: environment_ids).select do |environment| + environment.includes_commit?(commit) + end + end + private def pushes_since_gc_redis_key diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 494695a03a5..44e645a7e81 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -43,15 +43,16 @@ = icon("times-circle") Could not connect to the CI server. Please check your settings and try again. -- @merge_request.environments.each do |environment| - .mr-widget-heading - .ci_widget.ci-success - = ci_icon_for_status("success") - %span.hidden-sm - 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 external_url, target: '_blank' do - = icon('external-link', text: "View on #{external_url.gsub(/\A.*?:\/\//, '')}", right: true) +- @merge_request.environments.sort_by(&:name).each do |environment| + - if can?(current_user, :read_environment, environment) + .mr-widget-heading + .ci_widget.ci-success + = ci_icon_for_status("success") + %span.hidden-sm + Deployed to + = succeed '.' do + = link_to environment.name, environment_path(environment), class: 'environment' + - external_url = environment.external_url + - if external_url + = link_to external_url, target: '_blank' do + = icon('external-link', text: "View on #{external_url.gsub(/\A.*?:\/\//, '')}", right: true) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 06feeb1bbba..7cb97113c04 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -701,16 +701,57 @@ describe MergeRequest, models: true do end end - describe "#environments" do + describe '#environments' do let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project) } - it 'selects deployed environments' do - environments = create_list(:environment, 3, project: project) - create(:deployment, environment: environments.first, sha: project.commit('master').id) - create(:deployment, environment: environments.second, sha: project.commit('feature').id) + context 'with multiple environments' do + let(:environments) { create_list(:environment, 3, project: project) } - expect(merge_request.environments).to eq [environments.first] + before do + create(:deployment, environment: environments.first, ref: 'master', sha: project.commit('master').id) + create(:deployment, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(environments.first) + end + end + + context 'with environments on source project' do + let(:source_project) do + create(:project) do |fork_project| + fork_project.create_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id) + end + end + + let(:merge_request) do + create(:merge_request, + source_project: source_project, source_branch: 'feature', + target_project: project) + end + + let(:source_environment) { create(:environment, project: source_project) } + + before do + create(:deployment, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(source_environment) + end + + context 'with environments on target project' do + let(:target_environment) { create(:environment, project: project) } + + before do + create(:deployment, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(source_environment, target_environment) + end + end end context 'without a diff_head_commit' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a388ff703a6..83f61f0af0a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1647,6 +1647,47 @@ describe Project, models: true do end end + describe '#environments_for' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + context 'tagged deployment' do + before do + create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) + end + + it 'returns environment when with_tags is set' do + expect(project.environments_for('master', project.commit, with_tags: true)).to contain_exactly(environment) + end + + it 'does not return environment when no with_tags is set' do + expect(project.environments_for('master', project.commit)).to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(project.environments_for('master', project.commit('feature'))).to be_empty + end + end + + context 'branch deployment' do + before do + create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + end + + it 'returns environment when ref is set' do + expect(project.environments_for('master', project.commit)).to contain_exactly(environment) + end + + it 'does not environment when ref is different' do + expect(project.environments_for('feature', project.commit)).to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(project.environments_for('master', project.commit('feature'))).to be_empty + end + end + end + def enable_lfs allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) 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 733b2dfa7ff..21f49d396e7 100644 --- a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb @@ -15,6 +15,8 @@ describe 'projects/merge_requests/widget/_heading' do assign(:merge_request, merge_request) assign(:project, project) + allow(view).to receive(:can?).and_return(true) + render end