diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index f2dfb87dbda..fa782c6fbb7 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -22,7 +22,7 @@ class ChatNotificationService < Service end def can_test? - super && valid? + valid? end def self.supported_events diff --git a/app/models/service.rb b/app/models/service.rb index cbb75186206..c71a7d169ec 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -132,7 +132,7 @@ class Service < ActiveRecord::Base end def can_test? - !project.empty_repo? + true end # reason why service cannot be tested diff --git a/changelogs/unreleased/31138-improve-test-settings-for-services-in-empty-projects.yml b/changelogs/unreleased/31138-improve-test-settings-for-services-in-empty-projects.yml new file mode 100644 index 00000000000..cb1de425d66 --- /dev/null +++ b/changelogs/unreleased/31138-improve-test-settings-for-services-in-empty-projects.yml @@ -0,0 +1,4 @@ +--- +title: Improves test settings for chat notification services for empty projects +merge_request: 10886 +author: diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index d76aa38f741..1ff34553f0a 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -41,7 +41,7 @@ module Gitlab type = Gitlab::Git.tag_ref?(ref) ? 'tag_push' : 'push' # Hash to be passed as post_receive_data - data = { + { object_kind: type, event_name: type, before: oldrev, @@ -61,16 +61,15 @@ module Gitlab repository: project.hook_attrs.slice(:name, :url, :description, :homepage, :git_http_url, :git_ssh_url, :visibility_level) } - - data end # This method provide a sample data generated with # existing project and commits to test webhooks def build_sample(project, user) - commits = project.repository.commits(project.default_branch, limit: 3) ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}" - build(project, user, commits.last.id, commits.first.id, ref, commits) + commits = project.repository.commits(project.default_branch.to_s, limit: 3) rescue [] + + build(project, user, commits.last&.id, commits.first&.id, ref, commits) end def checkout_sha(repository, newrev, ref) diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 16365642a34..2d892f4a2b7 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -8,6 +8,7 @@ describe Projects::ServicesController do before do sign_in(user) project.team << [user, :master] + controller.instance_variable_set(:@project, project) controller.instance_variable_set(:@service, service) end @@ -18,20 +19,60 @@ describe Projects::ServicesController do end describe "#test" do - context 'success' do - it "redirects and show success message" do - expect(service).to receive(:test).and_return({ success: true, result: 'done' }) + context 'when can_test? returns false' do + it 'renders 404' do + allow_any_instance_of(Service).to receive(:can_test?).and_return(false) + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html - expect(response.status).to redirect_to('/') + + expect(response).to have_http_status(404) + end + end + + context 'success' do + context 'with empty project' do + let(:project) { create(:empty_project) } + + context 'with chat notification service' do + let(:service) { project.create_microsoft_teams_service(webhook: 'http://webhook.com') } + + it 'redirects and show success message' do + allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true) + + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('We sent a request to the provided URL') + end + end + + it 'redirects and show success message' do + expect(service).to receive(:test).and_return(success: true, result: 'done') + + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('We sent a request to the provided URL') + end + end + + it "redirects and show success message" do + expect(service).to receive(:test).and_return(success: true, result: 'done') + + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + + expect(response).to redirect_to(root_path) expect(flash[:notice]).to eq('We sent a request to the provided URL') end end context 'failure' do it "redirects and show failure message" do - expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' }) + expect(service).to receive(:test).and_return(success: false, result: 'Bad test') + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html - expect(response.status).to redirect_to('/') + + expect(response).to redirect_to(root_path) expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test') end end diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/project_services/chat_notification_service_spec.rb index 592c90cda36..8fbe42248ae 100644 --- a/spec/models/project_services/chat_notification_service_spec.rb +++ b/spec/models/project_services/chat_notification_service_spec.rb @@ -11,10 +11,10 @@ describe ChatNotificationService, models: true do describe '#can_test?' do context 'with empty repository' do - it 'returns false' do + it 'returns true' do subject.project = create(:empty_project, :empty_repo) - expect(subject.can_test?).to be false + expect(subject.can_test?).to be true end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 4d0181ca297..134882648b9 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -11,43 +11,48 @@ describe Service, models: true do end describe "Test Button" do - before do - @service = Service.new - end + describe '#can_test?' do + let(:service) { create(:service, project: project) } - describe "Testable" do - let(:project) { create(:project, :repository) } + context 'when repository is not empty' do + let(:project) { create(:project, :repository) } - before do - allow(@service).to receive(:project).and_return(project) - @testable = @service.can_test? + it 'returns true' do + expect(service.can_test?).to be true + end end - describe '#can_test?' do - it { expect(@testable).to eq(true) } - end + context 'when repository is empty' do + let(:project) { create(:empty_project) } - describe '#test' do - let(:data) { 'test' } - - it 'test runs execute' do - expect(@service).to receive(:execute).with(data) - - @service.test(data) + it 'returns true' do + expect(service.can_test?).to be true end end end - describe "With commits" do - let(:project) { create(:project, :repository) } + describe '#test' do + let(:data) { 'test' } + let(:service) { create(:service, project: project) } - before do - allow(@service).to receive(:project).and_return(project) - @testable = @service.can_test? + context 'when repository is not empty' do + let(:project) { create(:project, :repository) } + + it 'test runs execute' do + expect(service).to receive(:execute).with(data) + + service.test(data) + end end - describe '#can_test?' do - it { expect(@testable).to eq(true) } + context 'when repository is empty' do + let(:project) { create(:empty_project) } + + it 'test runs execute' do + expect(service).to receive(:execute).with(data) + + service.test(data) + end end end end