diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 2953db764a0c7348041efd1b717b60535b16a47b..15e565d5742d23ba8645bb6978e2b97d071343bb 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -3,7 +3,6 @@ module Projects module Settings class OperationsController < Projects::ApplicationController - before_action :check_license before_action :authorize_update_environment! helper_method :error_tracking_setting @@ -65,10 +64,6 @@ def permitted_project_params ] } end - - def check_license - render_404 unless helpers.settings_operations_available? - end end end end diff --git a/ee/app/controllers/ee/projects/settings/operations_controller.rb b/ee/app/controllers/ee/projects/settings/operations_controller.rb index d7d58c5872c1c27d706b7f7cc52c04ee356922a8..aaceb3819a6c7047b580d079fc6645ec5b9aa7b5 100644 --- a/ee/app/controllers/ee/projects/settings/operations_controller.rb +++ b/ee/app/controllers/ee/projects/settings/operations_controller.rb @@ -25,7 +25,7 @@ def reset_alerting_token end end - helper_method :tracing_setting + helper_method :tracing_setting, :incident_management_available? private @@ -45,11 +45,37 @@ def alerting_params def tracing_setting @tracing_setting ||= project.tracing_setting || project.build_tracing_setting end + + def has_tracing_license? + project.feature_available?(:tracing, current_user) + end + + def has_incident_management_license? + project.feature_available?(:incident_management, current_user) + end + + def incident_management_feature_enabled? + ::Feature.enabled?(:incident_management) + end + + def incident_management_available? + incident_management_feature_enabled? && has_incident_management_license? + end end override :permitted_project_params def permitted_project_params - super.merge(tracing_setting_attributes: [:external_url]) + permitted_params = super + + if has_tracing_license? + permitted_params[:tracing_setting_attributes] = [:external_url] + end + + if incident_management_available? + permitted_params[:incident_management_setting_attributes] = [:create_issue, :send_email, :issue_template_key] + end + + permitted_params end override :render_update_response diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb index 9989d9a290b4912cfd3d23152e5d944e7a98749b..61da461b258bb5d76880d1d9c834cb5103cfabb4 100644 --- a/ee/app/helpers/ee/projects_helper.rb +++ b/ee/app/helpers/ee/projects_helper.rb @@ -214,5 +214,10 @@ def settings_operations_available? @project.feature_available?(:tracing, current_user) && can?(current_user, :read_environment, @project) end + + def project_incident_management_setting + @project_incident_management_setting ||= @project.incident_management_setting || + @project.build_incident_management_setting + end end end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 0fc6bd7a04a14ce04698d48c83120dbfe2b11502..734b0baca9f7327596b70966795bac7296c2dc83 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -123,6 +123,7 @@ module Project accepts_nested_attributes_for :tracing_setting, update_only: true, allow_destroy: true accepts_nested_attributes_for :alerting_setting, update_only: true + accepts_nested_attributes_for :incident_management_setting, update_only: true alias_attribute :fallback_approvals_required, :approvals_before_merge end diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb index 0104ea9becd661f3b56bec8f7c723fceab2626e4..c072c6144ea19f2e92e5ec7dc3e314c7806a9054 100644 --- a/ee/app/models/license.rb +++ b/ee/app/models/license.rb @@ -95,6 +95,7 @@ class License < ActiveRecord::Base tracing insights web_ide_terminal + incident_management ] EEU_FEATURES.freeze diff --git a/ee/app/services/ee/projects/operations/update_service.rb b/ee/app/services/ee/projects/operations/update_service.rb index f8d67c71eb852bf873a7c428f6cf405ba74d1bfd..ef09c57d51aaeda6dba9e12ab0f6ef82b0e10929 100644 --- a/ee/app/services/ee/projects/operations/update_service.rb +++ b/ee/app/services/ee/projects/operations/update_service.rb @@ -12,6 +12,7 @@ def project_update_params super .merge(tracing_setting_params) .merge(alerting_setting_params) + .merge(incident_management_setting_params) end private @@ -41,6 +42,10 @@ def alerting_setting_params { alerting_setting_attributes: attr } end + + def incident_management_setting_params + params.slice(:incident_management_setting_attributes) + end end end end diff --git a/ee/changelogs/unreleased/4925-use-send-email-setting.yml b/ee/changelogs/unreleased/4925-use-send-email-setting.yml new file mode 100644 index 0000000000000000000000000000000000000000..8d7c04ba769cae4eb5c3f2c7547988957863d67e --- /dev/null +++ b/ee/changelogs/unreleased/4925-use-send-email-setting.yml @@ -0,0 +1,6 @@ +--- +title: Add an incident management settings form and create issues from alertmanager + alerts +merge_request: 9773 +author: +type: added diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb index 003889797e26b6200234ad2abf4f51fc7b772970..4cd403895eb2a3538d4e7affabc03350e5710316 100644 --- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb @@ -10,19 +10,17 @@ end describe 'GET show' do - shared_examples 'user without read access' do |project_visibility| + shared_examples 'user without read access' do |project_visibility, project_role| let(:project) { create(:project, project_visibility) } - %w[guest reporter developer].each do |role| - before do - project.public_send("add_#{role}", user) - end + before do + project.add_role(user, project_role) + end - it 'returns 404' do - get :show, params: project_params(project) + it 'returns 404' do + get :show, params: project_params(project) - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end @@ -53,7 +51,7 @@ context 'with a license' do before do - stub_licensed_features(tracing: true) + stub_licensed_features(tracing: true, incident_management: true) end context 'with maintainer role' do @@ -63,9 +61,11 @@ end context 'without maintainer role' do - it_behaves_like 'user without read access', :public - it_behaves_like 'user without read access', :private - it_behaves_like 'user without read access', :internal + %w[guest reporter developer].each do |role| + it_behaves_like 'user without read access', :public, role + it_behaves_like 'user without read access', :private, role + it_behaves_like 'user without read access', :internal, role + end end context 'when user not logged in' do @@ -73,7 +73,7 @@ sign_out(user) end - it_behaves_like 'user without read access', :public + it_behaves_like 'user without read access', :public, :maintainer it_behaves_like 'user needs to login', :private it_behaves_like 'user needs to login', :internal @@ -82,12 +82,12 @@ context 'without license' do before do - stub_licensed_features(tracing: false) + stub_licensed_features(tracing: false, incident_management: false) end - it_behaves_like 'user without read access', :public - it_behaves_like 'user without read access', :private - it_behaves_like 'user without read access', :internal + it_behaves_like 'user with read access', :public + it_behaves_like 'user with read access', :private + it_behaves_like 'user with read access', :internal end end @@ -96,24 +96,74 @@ let(:private_project) { create(:project, :private) } let(:internal_project) { create(:project, :internal) } + let(:incident_management_settings) do + { + create_issue: true, + send_email: true, + issue_template_key: 'some-key' + } + end + before do public_project.add_maintainer(user) private_project.add_maintainer(user) internal_project.add_maintainer(user) end - shared_examples 'user without write access' do |project_visibility| + shared_examples 'user without write access' do |project_visibility, project_role| let(:project) { create(:project, project_visibility) } - %w[guest reporter developer].each do |role| - before do - project.public_send("add_#{role}", user) + before do + project.add_role(user, project_role) + end + + it 'does not create tracing_setting' do + update_project( + project, + tracing_params: { external_url: 'https://gitlab.com' } + ) + + expect(project.tracing_setting).to be_nil + end + + it 'does not create incident_management_setting' do + update_project( + project, + incident_management_params: incident_management_settings + ) + + expect(project.incident_management_setting).to be_nil + end + + context 'with existing incident_management_setting' do + let(:new_incident_management_settings) do + { + create_issue: false, + send_email: false, + issue_template_key: 'some-other-template' + } end - it 'does not update tracing external_url' do - update_project(project, external_url: 'https://gitlab.com') + let!(:incident_management_setting) do + create(:project_incident_management_setting, + project: project, + **incident_management_settings) + end - expect(project.tracing_setting).to be_nil + it 'does not update incident_management_setting' do + update_project(project, + incident_management_params: new_incident_management_settings) + + setting = project.incident_management_setting + expect(setting.create_issue).to( + eq(incident_management_settings[:create_issue]) + ) + expect(setting.send_email).to( + eq(incident_management_settings[:send_email]) + ) + expect(setting.issue_template_key).to( + eq(incident_management_settings[:issue_template_key]) + ) end end end @@ -134,7 +184,7 @@ end it 'shows a notice' do - update_project(project, { external_url: 'http://gitlab.com' }) + update_project(project, tracing_params: { external_url: 'http://gitlab.com' }) expect(response).to redirect_to(project_settings_operations_url(project)) expect(flash[:notice]).to eq _('Your changes have been saved') @@ -147,7 +197,7 @@ end it 'renders show page' do - update_project(project, { external_url: 'http://gitlab.com' }) + update_project(project, tracing_params: { external_url: 'http://gitlab.com' }) expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:show) @@ -157,33 +207,77 @@ context 'with a license' do before do - stub_licensed_features(tracing: true) + stub_licensed_features(tracing: true, incident_management: true) end - shared_examples 'user with write access' do |project_visibility, value_to_set, value_to_check| + shared_examples 'user with write access' do |project_visibility| let(:project) { create(:project, project_visibility) } + let(:tracing_url) { 'https://gitlab.com' } before do project.add_maintainer(user) end - it 'updates tracing external_url' do - update_project(project, external_url: value_to_set) + it 'creates tracing setting' do + update_project( + project, + tracing_params: { external_url: tracing_url } + ) + + expect(project.tracing_setting.external_url).to eq(tracing_url) + end + + it 'creates incident management settings' do + update_project( + project, + incident_management_params: incident_management_settings + ) + + expect(project.incident_management_setting.create_issue).to( + eq(incident_management_settings.dig(:create_issue)) + ) + expect(project.incident_management_setting.send_email).to( + eq(incident_management_settings.dig(:send_email)) + ) + expect(project.incident_management_setting.issue_template_key).to( + eq(incident_management_settings.dig(:issue_template_key)) + ) + end - expect(project.tracing_setting.external_url).to eq(value_to_check) + it 'creates tracing and incident management settings' do + update_project( + project, + tracing_params: { external_url: tracing_url }, + incident_management_params: incident_management_settings + ) + + expect(project.tracing_setting.external_url).to eq(tracing_url) + expect(project.incident_management_setting.create_issue).to( + eq(incident_management_settings.dig(:create_issue)) + ) + expect(project.incident_management_setting.send_email).to( + eq(incident_management_settings.dig(:send_email)) + ) + expect(project.incident_management_setting.issue_template_key).to( + eq(incident_management_settings.dig(:issue_template_key)) + ) end end context 'with maintainer role' do - it_behaves_like 'user with write access', :public, 'https://gitlab.com', 'https://gitlab.com' - it_behaves_like 'user with write access', :private, 'https://gitlab.com', 'https://gitlab.com' - it_behaves_like 'user with write access', :internal, 'https://gitlab.com', 'https://gitlab.com' + it_behaves_like 'user with write access', :public + it_behaves_like 'user with write access', :private + it_behaves_like 'user with write access', :internal end context 'with non maintainer roles' do - it_behaves_like 'user without write access', :public - it_behaves_like 'user without write access', :private - it_behaves_like 'user without write access', :internal + %w[guest reporter developer].each do |role| + context "with #{role} role" do + it_behaves_like 'user without write access', :public, role + it_behaves_like 'user without write access', :private, role + it_behaves_like 'user without write access', :internal, role + end + end end context 'with anonymous user' do @@ -191,9 +285,9 @@ sign_out(user) end - it_behaves_like 'user without write access', :public - it_behaves_like 'user without write access', :private - it_behaves_like 'user without write access', :internal + it_behaves_like 'user without write access', :public, :maintainer + it_behaves_like 'user without write access', :private, :maintainer + it_behaves_like 'user without write access', :internal, :maintainer end context 'with existing tracing_setting' do @@ -205,26 +299,26 @@ end it 'unsets external_url with nil' do - update_project(project, external_url: nil) + update_project(project, tracing_params: { external_url: nil }) expect(project.tracing_setting).to be_nil end it 'unsets external_url with empty string' do - update_project(project, external_url: '') + update_project(project, tracing_params: { external_url: '' }) expect(project.tracing_setting).to be_nil end it 'fails validation with invalid url' do expect do - update_project(project, external_url: "invalid") + update_project(project, tracing_params: { external_url: "invalid" }) end.not_to change(project.tracing_setting, :external_url) end it 'does not set external_url if not present in params' do expect do - update_project(project, some_param: 'some_value') + update_project(project, tracing_params: { some_param: 'some_value' }) end.not_to change(project.tracing_setting, :external_url) end end @@ -237,33 +331,85 @@ end it 'fails validation with invalid url' do - update_project(project, external_url: "invalid") + update_project(project, tracing_params: { external_url: "invalid" }) expect(project.tracing_setting).to be_nil end it 'does not set external_url if not present in params' do - update_project(project, some_param: 'some_value') + update_project(project, tracing_params: { some_param: 'some_value' }) expect(project.tracing_setting).to be_nil end end + + context 'with existing incident management setting' do + let(:project) { create(:project) } + + let(:new_incident_management_settings) do + { + create_issue: false, + send_email: false, + issue_template_key: 'some-other-template' + } + end + + let!(:incident_management_setting) do + create(:project_incident_management_setting, + project: project, + **incident_management_settings) + end + + before do + project.add_maintainer(user) + end + + it 'updates incident management setting' do + update_project(project, + incident_management_params: new_incident_management_settings) + + setting = project.incident_management_setting + expect(setting.create_issue).to( + eq(new_incident_management_settings[:create_issue]) + ) + expect(setting.send_email).to( + eq(new_incident_management_settings[:send_email]) + ) + expect(setting.issue_template_key).to( + eq(new_incident_management_settings[:issue_template_key]) + ) + end + end end context 'without a license' do before do - stub_licensed_features(tracing: false) + stub_licensed_features(tracing: false, incident_management: false) + end + + it_behaves_like 'user without write access', :public, :maintainer + it_behaves_like 'user without write access', :private, :maintainer + it_behaves_like 'user without write access', :internal, :maintainer + end + + context 'with incident_management feature flag disabled' do + before do + stub_feature_flags(incident_management: false) end - it_behaves_like 'user without write access', :public - it_behaves_like 'user without write access', :private - it_behaves_like 'user without write access', :internal + it_behaves_like 'user without write access', :public, :maintainer + it_behaves_like 'user without write access', :private, :maintainer + it_behaves_like 'user without write access', :internal, :maintainer end private - def update_project(project, params) - patch :update, params: project_params(project, params) + def update_project(project, tracing_params: nil, incident_management_params: nil) + patch :update, params: project_params( + project, + tracing_params: tracing_params, + incident_management_params: incident_management_params + ) project.reload end @@ -372,12 +518,13 @@ def reset_alerting_token private - def project_params(project, params = {}) + def project_params(project, tracing_params: nil, incident_management_params: nil) { namespace_id: project.namespace, project_id: project, project: { - tracing_setting_attributes: params + tracing_setting_attributes: tracing_params, + incident_management_setting_attributes: incident_management_params } } end diff --git a/ee/spec/helpers/ee/projects_helper_spec.rb b/ee/spec/helpers/ee/projects_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..168b3d99fe855fe0c2b601037cc559f4964a6093 --- /dev/null +++ b/ee/spec/helpers/ee/projects_helper_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ProjectsHelper do + describe '#project_incident_management_setting' do + set(:project) { create(:project) } + + before do + helper.instance_variable_set(:@project, project) + end + + context 'when incident_management_setting exists' do + let(:project_incident_management_setting) do + create(:project_incident_management_setting, project: project) + end + + it 'return project_incident_management_setting' do + expect(helper.project_incident_management_setting).to( + eq(project_incident_management_setting) + ) + end + end + + context 'when incident_management_setting does not exist' do + it 'builds incident_management_setting' do + expect(helper.project_incident_management_setting.persisted?).to be(false) + + expect(helper.project_incident_management_setting.send_email).to be(true) + expect(helper.project_incident_management_setting.create_issue).to be(false) + expect(helper.project_incident_management_setting.issue_template_key).to be(nil) + end + end + end +end