diff --git a/ee/app/services/ee/protected_branches/create_service.rb b/ee/app/services/ee/protected_branches/create_service.rb index 17c9ac21b64683a5dba46d89e069a767c49441ab..83fa94dce819d59b39bf5672b84f65631ea10824 100644 --- a/ee/app/services/ee/protected_branches/create_service.rb +++ b/ee/app/services/ee/protected_branches/create_service.rb @@ -23,6 +23,7 @@ def save_protected_branch super sync_code_owner_approval_rules if project.feature_available?(:code_owners) + track_onboarding_progress protected_branch end @@ -58,6 +59,12 @@ def merge_requests_for_branch .preload_source_project .select(&:source_project) end + + def track_onboarding_progress + return unless protected_branch.code_owner_approval_required + + OnboardingProgressService.new(project.namespace).execute(action: :code_owners_enabled) + end end end end diff --git a/ee/spec/services/ee/protected_branches/create_service_spec.rb b/ee/spec/services/ee/protected_branches/create_service_spec.rb index 3d1d37c4acbd978234027fbfe9e686856c46bd07..a1ddfe61d262c2201056c497bc1311675efe64cb 100644 --- a/ee/spec/services/ee/protected_branches/create_service_spec.rb +++ b/ee/spec/services/ee/protected_branches/create_service_spec.rb @@ -33,7 +33,7 @@ params[:code_owner_approval_required] = true end - it "ignores incoming params and sets code_owner_approval_required to false" do + it "ignores incoming params and sets code_owner_approval_required to false", :aggregate_failures do expect { service.execute }.to change(ProtectedBranch, :count).by(1) expect(ProtectedBranch.last.code_owner_approval_required).to be_falsy end @@ -42,20 +42,33 @@ context "when available" do before do stub_licensed_features(code_owner_approval_required: true) + params[:code_owner_approval_required] = code_owner_approval_required end - it "sets code_owner_approval_required to true when param is true" do - params[:code_owner_approval_required] = true + context "when code_owner_approval_required param is true" do + let(:code_owner_approval_required) { true } - expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(ProtectedBranch.last.code_owner_approval_required).to be_truthy + it "sets code_owner_approval_required to true", :aggregate_failures do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(ProtectedBranch.last.code_owner_approval_required).to be_truthy + end + + it_behaves_like 'records an onboarding progress action', :code_owners_enabled do + let(:namespace) { target_project.namespace } + + subject { service.execute } + end end - it "sets code_owner_approval_required to false when param is false" do - params[:code_owner_approval_required] = false + context "when code_owner_approval_required param is false" do + let(:code_owner_approval_required) { false } - expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(ProtectedBranch.last.code_owner_approval_required).to be_falsy + it "sets code_owner_approval_required to false", :aggregate_failures do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(ProtectedBranch.last.code_owner_approval_required).to be_falsy + end + + it_behaves_like 'does not record an onboarding progress action' end end end @@ -69,7 +82,7 @@ ) end - it "calls MergeRequest::SyncCodeOwnerApprovalRules to update open MRs" do + it "calls MergeRequest::SyncCodeOwnerApprovalRules to update open MRs", :aggregate_failures do expect(::MergeRequests::SyncCodeOwnerApprovalRules).to receive(:new).with(merge_request).and_call_original expect { service.execute }.to change(ProtectedBranch, :count).by(1) end @@ -80,7 +93,7 @@ params[:name] = wildcard end - it "calls MergeRequest::SyncCodeOwnerApprovalRules to update open MRs for #{wildcard}" do + it "calls MergeRequest::SyncCodeOwnerApprovalRules to update open MRs for #{wildcard}", :aggregate_failures do expect(::MergeRequests::SyncCodeOwnerApprovalRules).to receive(:new).with(merge_request).and_call_original expect { service.execute }.to change(ProtectedBranch, :count).by(1) end