diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-07 00:06:18 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-07 00:06:18 +0000 |
commit | ee6b185429b62857a5e490526095a7699e8539f3 (patch) | |
tree | de54f53b76c5da2bf05cbd333e1910bbe655d653 | |
parent | cf85de264d049f1f8ff14b23f38f8331ae4c60fa (diff) | |
download | gitlab-ce-ee6b185429b62857a5e490526095a7699e8539f3.tar.gz |
Add latest changes from gitlab-org/gitlab@master
23 files changed, 684 insertions, 11 deletions
diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 698988e9295..4a38912db9b 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -10,6 +10,10 @@ class Deployment < ApplicationRecord belongs_to :cluster, class_name: 'Clusters::Cluster', optional: true belongs_to :user belongs_to :deployable, polymorphic: true, optional: true # rubocop:disable Cop/PolymorphicAssociations + has_many :deployment_merge_requests + + has_many :merge_requests, + through: :deployment_merge_requests has_internal_id :iid, scope: :project, init: ->(s) do Deployment.where(project: s.project).maximum(:iid) if s&.project @@ -149,6 +153,18 @@ class Deployment < ApplicationRecord project.deployments.joins(:environment) .where(environments: { name: self.environment.name }, ref: self.ref) .where.not(id: self.id) + .order(id: :desc) + .take + end + + def previous_environment_deployment + project + .deployments + .success + .joins(:environment) + .where(environments: { name: environment.name }) + .where.not(id: self.id) + .order(id: :desc) .take end @@ -181,6 +197,18 @@ class Deployment < ApplicationRecord deployable&.user || user end + def link_merge_requests(relation) + select = relation.select(['merge_requests.id', id]).to_sql + + # We don't use `Gitlab::Database.bulk_insert` here so that we don't need to + # first pluck lots of IDs into memory. + DeploymentMergeRequest.connection.execute(<<~SQL) + INSERT INTO #{DeploymentMergeRequest.table_name} + (merge_request_id, deployment_id) + #{select} + SQL + end + private def ref_path diff --git a/app/models/deployment_merge_request.rb b/app/models/deployment_merge_request.rb new file mode 100644 index 00000000000..ff4d9f66202 --- /dev/null +++ b/app/models/deployment_merge_request.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class DeploymentMergeRequest < ApplicationRecord + belongs_to :deployment, optional: false + belongs_to :merge_request, optional: false +end diff --git a/app/models/environment.rb b/app/models/environment.rb index 602dce89c4a..aace408b5ed 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -10,7 +10,11 @@ class Environment < ApplicationRecord has_many :successful_deployments, -> { success }, class_name: 'Deployment' has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment' + has_one :last_deployable, through: :last_deployment, source: 'deployable', source_type: 'CommitStatus' + has_one :last_pipeline, through: :last_deployable, source: 'pipeline' has_one :last_visible_deployment, -> { visible.distinct_on_environment }, class_name: 'Deployment' + has_one :last_visible_deployable, through: :last_visible_deployment, source: 'deployable', source_type: 'CommitStatus' + has_one :last_visible_pipeline, through: :last_visible_deployable, source: 'pipeline' before_validation :nullify_external_url before_validation :generate_slug, if: ->(env) { env.slug.blank? } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1f1c2db56a3..5e3cef0a52f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -205,6 +205,9 @@ class MergeRequest < ApplicationRecord scope :by_commit_sha, ->(sha) do where('EXISTS (?)', MergeRequestDiff.select(1).where('merge_requests.latest_merge_request_diff_id = merge_request_diffs.id').by_commit_sha(sha)).reorder(nil) end + scope :by_merge_commit_sha, -> (sha) do + where(merge_commit_sha: sha) + end scope :join_project, -> { joins(:target_project) } scope :references_project, -> { references(:target_project) } scope :with_api_entity_associations, -> { diff --git a/app/services/deployments/after_create_service.rb b/app/services/deployments/after_create_service.rb index 2572802e6a1..e0a4e5419cc 100644 --- a/app/services/deployments/after_create_service.rb +++ b/app/services/deployments/after_create_service.rb @@ -33,12 +33,21 @@ module Deployments if environment.save && !environment.stopped? deployment.update_merge_request_metrics! + link_merge_requests(deployment) end end end private + def link_merge_requests(deployment) + unless Feature.enabled?(:deployment_merge_requests, deployment.project) + return + end + + LinkMergeRequestsService.new(deployment).execute + end + def environment_options options&.dig(:environment) || {} end diff --git a/app/services/deployments/link_merge_requests_service.rb b/app/services/deployments/link_merge_requests_service.rb new file mode 100644 index 00000000000..71186659290 --- /dev/null +++ b/app/services/deployments/link_merge_requests_service.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Deployments + # Service class for linking merge requests to deployments. + class LinkMergeRequestsService + attr_reader :deployment + + # The number of commits per query for which to find merge requests. + COMMITS_PER_QUERY = 5_000 + + def initialize(deployment) + @deployment = deployment + end + + def execute + return unless deployment.success? + + if (prev = deployment.previous_environment_deployment) + link_merge_requests_for_range(prev.sha, deployment.sha) + else + # When no previous deployment is found we fall back to linking all merge + # requests merged into the deployed branch. This will not always be + # accurate, but it's better than having no data. + # + # We can't use the first commit in the repository as a base to compare + # to, as this will not scale to large repositories. For example, GitLab + # itself has over 150 000 commits. + link_all_merged_merge_requests + end + end + + def link_merge_requests_for_range(from, to) + commits = project + .repository + .commits_between(from, to) + .map(&:id) + + # For some projects the list of commits to deploy may be very large. To + # ensure we do not end up running SQL queries with thousands of WHERE IN + # values, we run one query per a certain number of commits. + # + # In most cases this translates to only a single query. For very large + # deployment we may end up running a handful of queries to get and insert + # the data. + commits.each_slice(COMMITS_PER_QUERY) do |slice| + merge_requests = + project.merge_requests.merged.by_merge_commit_sha(slice) + + deployment.link_merge_requests(merge_requests) + end + end + + def link_all_merged_merge_requests + merge_requests = + project.merge_requests.merged.by_target_branch(deployment.ref) + + deployment.link_merge_requests(merge_requests) + end + + private + + def project + deployment.project + end + end +end diff --git a/app/services/deployments/update_service.rb b/app/services/deployments/update_service.rb index 7c8215d28f2..97b233f16a7 100644 --- a/app/services/deployments/update_service.rb +++ b/app/services/deployments/update_service.rb @@ -10,7 +10,22 @@ module Deployments end def execute - deployment.update(status: params[:status]) + # A regular update() does not trigger the state machine transitions, which + # we need to ensure merge requests are linked when changing the status to + # success. To work around this we use this case statment, using the right + # event methods to trigger the transition hooks. + case params[:status] + when 'running' + deployment.run + when 'success' + deployment.succeed + when 'failed' + deployment.drop + when 'canceled' + deployment.cancel + else + false + end end end end diff --git a/changelogs/unreleased/deployment-commit-tracking.yml b/changelogs/unreleased/deployment-commit-tracking.yml new file mode 100644 index 00000000000..357bf6af60f --- /dev/null +++ b/changelogs/unreleased/deployment-commit-tracking.yml @@ -0,0 +1,5 @@ +--- +title: Add deployment_merge_requests table +merge_request: 18755 +author: +type: other diff --git a/changelogs/unreleased/jc-add-internal-socket-dir-to-setup.yml b/changelogs/unreleased/jc-add-internal-socket-dir-to-setup.yml new file mode 100644 index 00000000000..119e836b1ee --- /dev/null +++ b/changelogs/unreleased/jc-add-internal-socket-dir-to-setup.yml @@ -0,0 +1,5 @@ +--- +title: Add internal_socket_dir to gitaly config in setup helper +merge_request: 19170 +author: +type: other diff --git a/db/migrate/20191017134513_add_deployment_merge_requests.rb b/db/migrate/20191017134513_add_deployment_merge_requests.rb new file mode 100644 index 00000000000..dbe09463d22 --- /dev/null +++ b/db/migrate/20191017134513_add_deployment_merge_requests.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class AddDeploymentMergeRequests < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :deployment_merge_requests, id: false do |t| + t.references( + :deployment, + foreign_key: { on_delete: :cascade }, + type: :integer, + index: false, + null: false + ) + + t.references( + :merge_request, + foreign_key: { on_delete: :cascade }, + type: :integer, + index: true, + null: false + ) + + t.index( + [:deployment_id, :merge_request_id], + unique: true, + name: 'idx_deployment_merge_requests_unique_index' + ) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6fbfd45a11a..babe53623f6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1257,6 +1257,13 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do t.index ["token_encrypted"], name: "index_deploy_tokens_on_token_encrypted", unique: true end + create_table "deployment_merge_requests", id: false, force: :cascade do |t| + t.integer "deployment_id", null: false + t.integer "merge_request_id", null: false + t.index ["deployment_id", "merge_request_id"], name: "idx_deployment_merge_requests_unique_index", unique: true + t.index ["merge_request_id"], name: "index_deployment_merge_requests_on_merge_request_id" + end + create_table "deployments", id: :serial, force: :cascade do |t| t.integer "iid", null: false t.integer "project_id", null: false @@ -4191,6 +4198,8 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do add_foreign_key "dependency_proxy_blobs", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "dependency_proxy_group_settings", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade + add_foreign_key "deployment_merge_requests", "deployments", on_delete: :cascade + add_foreign_key "deployment_merge_requests", "merge_requests", on_delete: :cascade add_foreign_key "deployments", "clusters", name: "fk_289bba3222", on_delete: :nullify add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade add_foreign_key "description_versions", "epics", on_delete: :cascade diff --git a/lib/gitlab/data_builder/deployment.rb b/lib/gitlab/data_builder/deployment.rb index f11e032ab84..70587b3132a 100644 --- a/lib/gitlab/data_builder/deployment.rb +++ b/lib/gitlab/data_builder/deployment.rb @@ -6,11 +6,17 @@ module Gitlab extend self def build(deployment) + # Deployments will not have a deployable when created using the API. + deployable_url = + if deployment.deployable + Gitlab::UrlBuilder.build(deployment.deployable) + end + { object_kind: 'deployment', status: deployment.status, deployable_id: deployment.deployable_id, - deployable_url: Gitlab::UrlBuilder.build(deployment.deployable), + deployable_url: deployable_url, environment: deployment.environment.name, project: deployment.project.hook_attrs, short_sha: deployment.short_sha, diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb index 0d3e78c0a66..c449c6879bc 100644 --- a/lib/gitlab/setup_helper.rb +++ b/lib/gitlab/setup_helper.rb @@ -40,6 +40,11 @@ module Gitlab config = { socket_path: address.sub(/\Aunix:/, ''), storage: storages } config[:auth] = { token: 'secret' } if Rails.env.test? + + internal_socket_dir = File.join(gitaly_dir, 'internal_sockets') + FileUtils.mkdir(internal_socket_dir) unless File.exist?(internal_socket_dir) + config[:internal_socket_dir] = internal_socket_dir + config[:'gitaly-ruby'] = { dir: File.join(gitaly_dir, 'ruby') } if gitaly_ruby config[:'gitlab-shell'] = { dir: Gitlab.config.gitlab_shell.path } config[:bin_dir] = Gitlab.config.gitaly.client_path diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index f4da206990c..f8738d28d83 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -51,6 +51,10 @@ FactoryBot.define do finished_at { Time.now } end + trait :created do + status { :created } + end + # This trait hooks the state maechine's events trait :succeed do after(:create) do |deployment, evaluator| diff --git a/spec/lib/gitlab/data_builder/deployment_spec.rb b/spec/lib/gitlab/data_builder/deployment_spec.rb index 0a6e2302b09..42d7329494d 100644 --- a/spec/lib/gitlab/data_builder/deployment_spec.rb +++ b/spec/lib/gitlab/data_builder/deployment_spec.rb @@ -35,5 +35,12 @@ describe Gitlab::DataBuilder::Deployment do expect(data[:commit_url]).to eq(expected_commit_url) expect(data[:commit_title]).to eq(commit.title) end + + it 'does not include the deployable URL when there is no deployable' do + deployment = create(:deployment, status: :failed, deployable: nil) + data = described_class.build(deployment) + + expect(data[:deployable_url]).to be_nil + end end end diff --git a/spec/models/deployment_merge_request_spec.rb b/spec/models/deployment_merge_request_spec.rb new file mode 100644 index 00000000000..fd5be52d47c --- /dev/null +++ b/spec/models/deployment_merge_request_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DeploymentMergeRequest do + let(:mr) { create(:merge_request, :merged) } + let(:deployment) { create(:deployment, :success, project: project) } + let(:project) { mr.project } + + subject { described_class.new(deployment: deployment, merge_request: mr) } + + it { is_expected.to belong_to(:deployment).required } + it { is_expected.to belong_to(:merge_request).required } +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 3a0b3c46ad0..52c19d4814c 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -10,6 +10,8 @@ describe Deployment do it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster') } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:deployable) } + it { is_expected.to have_many(:deployment_merge_requests) } + it { is_expected.to have_many(:merge_requests).through(:deployment_merge_requests) } it { is_expected.to delegate_method(:name).to(:environment).with_prefix } it { is_expected.to delegate_method(:commit).to(:project) } @@ -361,4 +363,82 @@ describe Deployment do .to raise_error(ActiveRecord::RecordNotFound) end end + + describe '#previous_deployment' do + it 'returns the previous deployment' do + deploy1 = create(:deployment) + deploy2 = create( + :deployment, + project: deploy1.project, + environment: deploy1.environment + ) + + expect(deploy2.previous_deployment).to eq(deploy1) + end + end + + describe '#link_merge_requests' do + it 'links merge requests with a deployment' do + deploy = create(:deployment) + mr1 = create( + :merge_request, + :merged, + target_project: deploy.project, + source_project: deploy.project + ) + + mr2 = create( + :merge_request, + :merged, + target_project: deploy.project, + source_project: deploy.project + ) + + deploy.link_merge_requests(deploy.project.merge_requests) + + expect(deploy.merge_requests).to include(mr1, mr2) + end + end + + describe '#previous_environment_deployment' do + it 'returns the previous deployment of the same environment' do + deploy1 = create(:deployment, :success, ref: 'v1.0.0') + deploy2 = create( + :deployment, + :success, + project: deploy1.project, + environment: deploy1.environment, + ref: 'v1.0.1' + ) + + expect(deploy2.previous_environment_deployment).to eq(deploy1) + end + + it 'ignores deployments that were not successful' do + deploy1 = create(:deployment, :failed, ref: 'v1.0.0') + deploy2 = create( + :deployment, + :success, + project: deploy1.project, + environment: deploy1.environment, + ref: 'v1.0.1' + ) + + expect(deploy2.previous_environment_deployment).to be_nil + end + + it 'ignores deployments for different environments' do + deploy1 = create(:deployment, :success, ref: 'v1.0.0') + preprod = create(:environment, project: deploy1.project, name: 'preprod') + deploy2 = create( + :deployment, + :success, + project: deploy1.project, + environment: preprod, + ref: 'v1.0.1' + ) + + expect(deploy2.previous_environment_deployment).to be_nil + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 4566045bffd..8f3f45e159d 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe Environment, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers using RSpec::Parameterized::TableSyntax + include RepoHelpers let(:project) { create(:project, :stubbed_repository) } subject(:environment) { create(:environment, project: project) } @@ -505,6 +506,14 @@ describe Environment, :use_clean_rails_memory_store_caching do end end + context 'when there is a deployment record with failed status' do + let!(:deployment) { create(:deployment, :failed, environment: environment) } + + it 'returns the previous deployment' do + is_expected.to eq(previous_deployment) + end + end + context 'when there is a deployment record with success status' do let!(:deployment) { create(:deployment, :success, environment: environment) } @@ -557,6 +566,89 @@ describe Environment, :use_clean_rails_memory_store_caching do end end + describe '#last_visible_pipeline' do + let(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + let(:environment) { create(:environment, project: project) } + let(:commit) { project.commit } + + let(:success_pipeline) do + create(:ci_pipeline, :success, project: project, user: user, sha: commit.sha) + end + + let(:failed_pipeline) do + create(:ci_pipeline, :failed, project: project, user: user, sha: commit.sha) + end + + it 'uses the last deployment even if it failed' do + pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha) + ci_build = create(:ci_build, project: project, pipeline: pipeline) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build, sha: commit.sha) + + last_pipeline = environment.last_visible_pipeline + + expect(last_pipeline).to eq(pipeline) + end + + it 'returns nil if there is no deployment' do + create(:ci_build, project: project, pipeline: success_pipeline) + + expect(environment.last_visible_pipeline).to be_nil + end + + it 'does not return an invisible pipeline' do + failed_pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha) + ci_build_a = create(:ci_build, project: project, pipeline: failed_pipeline) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_a, sha: commit.sha) + pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha) + ci_build_b = create(:ci_build, project: project, pipeline: pipeline) + create(:deployment, :created, project: project, environment: environment, deployable: ci_build_b, sha: commit.sha) + + last_pipeline = environment.last_visible_pipeline + + expect(last_pipeline).to eq(failed_pipeline) + end + + context 'for the environment' do + it 'returns the last pipeline' do + pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha) + ci_build = create(:ci_build, project: project, pipeline: pipeline) + create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha) + + last_pipeline = environment.last_visible_pipeline + + expect(last_pipeline).to eq(pipeline) + end + + context 'with multiple deployments' do + it 'returns the last pipeline' do + pipeline_a = create(:ci_pipeline, project: project, user: user) + pipeline_b = create(:ci_pipeline, project: project, user: user) + ci_build_a = create(:ci_build, project: project, pipeline: pipeline_a) + ci_build_b = create(:ci_build, project: project, pipeline: pipeline_b) + create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a) + create(:deployment, :success, project: project, environment: environment, deployable: ci_build_b) + + last_pipeline = environment.last_visible_pipeline + + expect(last_pipeline).to eq(pipeline_b) + end + end + + context 'with multiple pipelines' do + it 'returns the last pipeline' do + create(:ci_build, project: project, pipeline: success_pipeline) + ci_build_b = create(:ci_build, project: project, pipeline: failed_pipeline) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b, sha: commit.sha) + + last_pipeline = environment.last_visible_pipeline + + expect(last_pipeline).to eq(failed_pipeline) + end + end + end + end + describe '#has_terminals?' do subject { environment.has_terminals? } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 62f73c3867b..d4137d2ada4 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -283,6 +283,16 @@ describe MergeRequest do end end + describe '.by_merge_commit_sha' do + it 'returns merge requests that match the given merge commit' do + mr = create(:merge_request, :merged, merge_commit_sha: '123abc') + + create(:merge_request, :merged, merge_commit_sha: '123def') + + expect(described_class.by_merge_commit_sha('123abc')).to eq([mr]) + end + end + describe '.in_projects' do it 'returns the merge requests for a set of projects' do expect(described_class.in_projects(Project.all)).to eq([subject]) diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index ad7be531979..21c073280f7 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -137,14 +137,42 @@ describe API::Deployments do expect(response).to have_gitlab_http_status(500) end + + it 'links any merged merge requests to the deployment' do + mr = create( + :merge_request, + :merged, + target_project: project, + source_project: project, + target_branch: 'master', + source_branch: 'foo' + ) + + post( + api("/projects/#{project.id}/deployments", user), + params: { + environment: 'production', + sha: sha, + ref: 'master', + tag: false, + status: 'success' + } + ) + + deploy = project.deployments.last + + expect(deploy.merge_requests).to eq([mr]) + end end context 'as a developer' do - it 'creates a new deployment' do - developer = create(:user) + let(:developer) { create(:user) } + before do project.add_developer(developer) + end + it 'creates a new deployment' do post( api("/projects/#{project.id}/deployments", developer), params: { @@ -161,6 +189,32 @@ describe API::Deployments do expect(json_response['sha']).to eq(sha) expect(json_response['ref']).to eq('master') end + + it 'links any merged merge requests to the deployment' do + mr = create( + :merge_request, + :merged, + target_project: project, + source_project: project, + target_branch: 'master', + source_branch: 'foo' + ) + + post( + api("/projects/#{project.id}/deployments", developer), + params: { + environment: 'production', + sha: sha, + ref: 'master', + tag: false, + status: 'success' + } + ) + + deploy = project.deployments.last + + expect(deploy.merge_requests).to eq([mr]) + end end context 'as non member' do @@ -182,7 +236,7 @@ describe API::Deployments do end describe 'PUT /projects/:id/deployments/:deployment_id' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:build) { create(:ci_build, :failed, project: project) } let(:environment) { create(:environment, project: project) } let(:deploy) do @@ -191,7 +245,8 @@ describe API::Deployments do :failed, project: project, environment: environment, - deployable: nil + deployable: nil, + sha: project.commit.sha ) end @@ -216,6 +271,26 @@ describe API::Deployments do expect(response).to have_gitlab_http_status(200) expect(json_response['status']).to eq('success') end + + it 'links merge requests when the deployment status changes to success', :sidekiq_inline do + mr = create( + :merge_request, + :merged, + target_project: project, + source_project: project, + target_branch: 'master', + source_branch: 'foo' + ) + + put( + api("/projects/#{project.id}/deployments/#{deploy.id}", user), + params: { status: 'success' } + ) + + deploy = project.deployments.last + + expect(deploy.merge_requests).to eq([mr]) + end end context 'as a developer' do diff --git a/spec/services/deployments/after_create_service_spec.rb b/spec/services/deployments/after_create_service_spec.rb index b34483ea85b..94532ed81ae 100644 --- a/spec/services/deployments/after_create_service_spec.rb +++ b/spec/services/deployments/after_create_service_spec.rb @@ -53,6 +53,14 @@ describe Deployments::AfterCreateService do service.execute end + it 'links merge requests to deployment' do + expect_next_instance_of(Deployments::LinkMergeRequestsService, deployment) do |link_mr_service| + expect(link_mr_service).to receive(:execute) + end + + service.execute + end + it 'returns the deployment' do expect(subject.execute).to eq(deployment) end @@ -237,4 +245,30 @@ describe Deployments::AfterCreateService do end end end + + describe '#update_environment' do + it 'links the merge requests' do + double = instance_double(Deployments::LinkMergeRequestsService) + + allow(Deployments::LinkMergeRequestsService) + .to receive(:new) + .with(deployment) + .and_return(double) + + expect(double).to receive(:execute) + + service.update_environment(deployment) + end + + context 'when the tracking of merge requests is disabled' do + it 'does nothing' do + stub_feature_flags(deployment_merge_requests: false) + + expect(Deployments::LinkMergeRequestsService) + .not_to receive(:new) + + service.update_environment(deployment) + end + end + end end diff --git a/spec/services/deployments/link_merge_requests_service_spec.rb b/spec/services/deployments/link_merge_requests_service_spec.rb new file mode 100644 index 00000000000..ba069658dfd --- /dev/null +++ b/spec/services/deployments/link_merge_requests_service_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Deployments::LinkMergeRequestsService do + describe '#execute' do + context 'when the deployment did not succeed' do + it 'does nothing' do + deploy = create(:deployment, :failed) + + expect(deploy).not_to receive(:link_merge_requests) + + described_class.new(deploy).execute + end + end + + context 'when there is a previous deployment' do + it 'links all merge requests merged since the previous deployment' do + deploy1 = create(:deployment, :success, sha: 'foo') + deploy2 = create( + :deployment, + :success, + sha: 'bar', + project: deploy1.project, + environment: deploy1.environment + ) + + service = described_class.new(deploy2) + + expect(service) + .to receive(:link_merge_requests_for_range) + .with('foo', 'bar') + + service.execute + end + end + + context 'when there are no previous deployments' do + it 'links all merged merge requests' do + deploy = create(:deployment, :success) + service = described_class.new(deploy) + + expect(service).to receive(:link_all_merged_merge_requests) + + service.execute + end + end + end + + describe '#link_merge_requests_for_range' do + it 'links merge requests' do + project = create(:project, :repository) + environment = create(:environment, project: project) + deploy = + create(:deployment, :success, project: project, environment: environment) + + mr1 = create( + :merge_request, + :merged, + merge_commit_sha: '1e292f8fedd741b75372e19097c76d327140c312', + source_project: project, + target_project: project + ) + + mr2 = create( + :merge_request, + :merged, + merge_commit_sha: '2d1db523e11e777e49377cfb22d368deec3f0793', + source_project: project, + target_project: project + ) + + described_class.new(deploy).link_merge_requests_for_range( + '7975be0116940bf2ad4321f79d02a55c5f7779aa', + 'ddd0f15ae83993f5cb66a927a28673882e99100b' + ) + + expect(deploy.merge_requests).to include(mr1, mr2) + end + end + + describe '#link_all_merged_merge_requests' do + it 'links all merged merge requests targeting the deployed branch' do + project = create(:project, :repository) + environment = create(:environment, project: project) + deploy = + create(:deployment, :success, project: project, environment: environment) + + mr1 = create( + :merge_request, + :merged, + source_project: project, + target_project: project, + source_branch: 'source1', + target_branch: deploy.ref + ) + + mr2 = create( + :merge_request, + :merged, + source_project: project, + target_project: project, + source_branch: 'source2', + target_branch: deploy.ref + ) + + mr3 = create( + :merge_request, + :merged, + source_project: project, + target_project: project, + target_branch: 'foo' + ) + + described_class.new(deploy).link_all_merged_merge_requests + + expect(deploy.merge_requests).to include(mr1, mr2) + expect(deploy.merge_requests).not_to include(mr3) + end + end +end diff --git a/spec/services/deployments/update_service_spec.rb b/spec/services/deployments/update_service_spec.rb index a923099b82c..8a918d28ffd 100644 --- a/spec/services/deployments/update_service_spec.rb +++ b/spec/services/deployments/update_service_spec.rb @@ -3,13 +3,55 @@ require 'spec_helper' describe Deployments::UpdateService do - let(:deploy) { create(:deployment, :running) } - let(:service) { described_class.new(deploy, status: 'success') } + let(:deploy) { create(:deployment) } describe '#execute' do - it 'updates the status of a deployment' do - expect(service.execute).to eq(true) - expect(deploy.status).to eq('success') + it 'can update the status to running' do + expect(described_class.new(deploy, status: 'running').execute) + .to be_truthy + + expect(deploy).to be_running + end + + it 'can update the status to success' do + expect(described_class.new(deploy, status: 'success').execute) + .to be_truthy + + expect(deploy).to be_success + end + + it 'can update the status to failed' do + expect(described_class.new(deploy, status: 'failed').execute) + .to be_truthy + + expect(deploy).to be_failed + end + + it 'can update the status to canceled' do + expect(described_class.new(deploy, status: 'canceled').execute) + .to be_truthy + + expect(deploy).to be_canceled + end + + it 'returns false when the status is not supported' do + expect(described_class.new(deploy, status: 'kittens').execute) + .to be_falsey + end + + it 'links merge requests when changing the status to success', :sidekiq_inline do + mr = create( + :merge_request, + :merged, + target_project: deploy.project, + source_project: deploy.project, + target_branch: 'master', + source_branch: 'foo' + ) + + described_class.new(deploy, status: 'success').execute + + expect(deploy.merge_requests).to eq([mr]) end end end |