From 502d8caaa1deb0e711096e80e764237154ede424 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Thu, 14 Feb 2019 22:06:22 +1100 Subject: [PATCH 1/5] Add Incidents section to Operations settings --- .../operations/_error_tracking.html.haml | 4 ++-- .../settings/operations/show.html.haml | 1 + .../settings/operations/_incidents.html.haml | 22 +++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 ee/app/views/projects/settings/operations/_incidents.html.haml diff --git a/app/views/projects/settings/operations/_error_tracking.html.haml b/app/views/projects/settings/operations/_error_tracking.html.haml index 4911e8d37706d6..17c7525ed41605 100644 --- a/app/views/projects/settings/operations/_error_tracking.html.haml +++ b/app/views/projects/settings/operations/_error_tracking.html.haml @@ -2,8 +2,8 @@ - setting = error_tracking_setting -%section.settings.expanded.border-0.no-animate - .settings-header +%section.settings.expanded.no-animate + .settings-header.border-top %h4 = _('Error Tracking') %p diff --git a/app/views/projects/settings/operations/show.html.haml b/app/views/projects/settings/operations/show.html.haml index b36fa9a5f5145f..79b01ad933cd78 100644 --- a/app/views/projects/settings/operations/show.html.haml +++ b/app/views/projects/settings/operations/show.html.haml @@ -1,5 +1,6 @@ - @content_class = 'limit-container-width' unless fluid_layout - page_title _('Operations') += render_if_exists 'projects/settings/operations/incidents' = render 'projects/settings/operations/error_tracking', expanded: true = render_if_exists 'projects/settings/operations/tracing' diff --git a/ee/app/views/projects/settings/operations/_incidents.html.haml b/ee/app/views/projects/settings/operations/_incidents.html.haml new file mode 100644 index 00000000000000..68979b71c56b56 --- /dev/null +++ b/ee/app/views/projects/settings/operations/_incidents.html.haml @@ -0,0 +1,22 @@ +- templates = [] + +%section.settings.expanded.border-0.no-animate + .settings-header + %h4= _("Incidents") + %p + = _("Action to take when receiving an alert.") + = link_to help_page_path('administration/monitoring/prometheus/index', anchor: 'create-issues-from-alerts') do + = _("More information") + .settings-content + = form_for @project, url: project_settings_operations_path(@project), method: :patch do |f| + = form_errors(@project) + .form-group + = f.check_box :archived # TODO + = f.label :archived, _('Create an issue. Issues are created for each alert triggered.'), class: 'form-check-label' + .form-group + = f.label :first_day_of_week, _('Issue template') + = f.select :archived, templates, {}, class: 'form-control' + .form-group + = f.check_box :archived # TODO + = f.label :archived, _('Send an email notification to Developers.'), class: 'form-check-label' + = f.submit _('Save changes'), class: 'btn btn-success' -- GitLab From 1612d2abd896c55cbc401aaf52d9c900b02e4f58 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Tue, 26 Feb 2019 18:51:19 +0100 Subject: [PATCH 2/5] DB: Add table for incident management settings --- db/schema.rb | 9 ++++++++- ...reate_project_incident_management_settings.rb | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 ee/db/migrate/20190226154144_create_project_incident_management_settings.rb diff --git a/db/schema.rb b/db/schema.rb index 39a7f9d7e4d74f..0ffd7a2cb0cf11 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190204115450) do +ActiveRecord::Schema.define(version: 20190226154144) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -2272,6 +2272,12 @@ t.index ["project_id"], name: "index_project_import_data_on_project_id", using: :btree end + create_table "project_incident_management_settings", primary_key: "project_id", id: :integer, force: :cascade do |t| + t.boolean "create_issue", default: false, null: false + t.boolean "send_email", default: false, null: false + t.text "issue_template_key" + end + create_table "project_mirror_data", force: :cascade do |t| t.integer "project_id", null: false t.integer "retry_count", default: 0, null: false @@ -3495,6 +3501,7 @@ add_foreign_key "project_features", "projects", name: "fk_18513d9b92", on_delete: :cascade add_foreign_key "project_group_links", "projects", name: "fk_daa8cee94c", on_delete: :cascade add_foreign_key "project_import_data", "projects", name: "fk_ffb9ee3a10", on_delete: :cascade + add_foreign_key "project_incident_management_settings", "projects", on_delete: :cascade add_foreign_key "project_mirror_data", "projects", name: "fk_d1aad367d7", on_delete: :cascade add_foreign_key "project_repositories", "projects", on_delete: :cascade add_foreign_key "project_repositories", "shards", on_delete: :restrict diff --git a/ee/db/migrate/20190226154144_create_project_incident_management_settings.rb b/ee/db/migrate/20190226154144_create_project_incident_management_settings.rb new file mode 100644 index 00000000000000..9441552edb3e12 --- /dev/null +++ b/ee/db/migrate/20190226154144_create_project_incident_management_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class CreateProjectIncidentManagementSettings < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :project_incident_management_settings, id: :int, primary_key: :project_id do |t| + t.boolean :create_issue, default: false, null: false + t.boolean :send_email, default: false, null: false + t.text :issue_template_key + t.foreign_key :projects, column: :project_id, on_delete: :cascade + end + end +end -- GitLab From 4c510e514dd37d9b6e88dd4cfd5a115de36b21c8 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Tue, 26 Feb 2019 18:51:56 +0100 Subject: [PATCH 3/5] Add model ProjectIncidentManagementSetting --- ee/app/models/ee/project.rb | 2 ++ .../project_incident_management_setting.rb | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 ee/app/models/project_incident_management_setting.rb diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index dd1333427a1ad3..3d4489778798f3 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -41,6 +41,7 @@ module Project has_one :gitlab_slack_application_service has_one :tracing_setting, class_name: 'ProjectTracingSetting' has_one :alerting_setting, inverse_of: :project, class_name: 'Alerting::ProjectAlertingSetting' + has_one :incident_management_setting, inverse_of: :project, class_name: 'ProjectIncidentManagementSetting' has_one :feature_usage, class_name: 'ProjectFeatureUsage' has_many :reviews, inverse_of: :project @@ -120,6 +121,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/project_incident_management_setting.rb b/ee/app/models/project_incident_management_setting.rb new file mode 100644 index 00000000000000..c1bde661625e89 --- /dev/null +++ b/ee/app/models/project_incident_management_setting.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class ProjectIncidentManagementSetting < ApplicationRecord + belongs_to :project + + validate :validate_issue_template_path, if: :create_issue? + + def available_issue_templates + Gitlab::Template::IssueTemplate.all(project) + end + + private + + def validate_issue_template_path + return unless issue_template_key_changed? + + Gitlab::Template::IssueTemplate.find(issue_template_key, project) + rescue Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError + errors.add(:issue_template_key, 'not found') + end +end -- GitLab From c5ed6f0c41c3e4af8a1897b87fcc6af62c40267e Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Tue, 26 Feb 2019 18:52:22 +0100 Subject: [PATCH 4/5] Expose and manage incident management settings --- .../settings/operations_controller.rb | 19 ++++++++++--- .../ee/projects/operations/update_service.rb | 5 ++++ .../settings/operations/_incidents.html.haml | 27 ++++++++++--------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/ee/app/controllers/ee/projects/settings/operations_controller.rb b/ee/app/controllers/ee/projects/settings/operations_controller.rb index 92cb22a134c8d2..96637b4d5db608 100644 --- a/ee/app/controllers/ee/projects/settings/operations_controller.rb +++ b/ee/app/controllers/ee/projects/settings/operations_controller.rb @@ -8,6 +8,8 @@ module OperationsController extend ActiveSupport::Concern prepended do + include ::Gitlab::Utils::StrongMemoize + before_action :authorize_read_prometheus_alerts!, only: [:reset_alerting_token] @@ -25,7 +27,7 @@ def reset_alerting_token end end - helper_method :tracing_setting + helper_method :tracing_setting, :incident_management_setting private @@ -34,13 +36,24 @@ def alerting_params end def tracing_setting - @tracing_setting ||= project.tracing_setting || project.build_tracing_setting + strong_memoize(:tracing_setting) do + project.tracing_setting || project.build_tracing_setting + end + end + + def incident_management_setting + strong_memoize(:incident_management_setting) do + project.incident_management_setting || project.build_incident_management_setting + end end end override :permitted_project_params def permitted_project_params - super.merge(tracing_setting_attributes: [:external_url]) + super.merge( + tracing_setting_attributes: [:external_url], + incident_management_setting_attributes: [:create_issue, :send_email, :issue_template_key] + ) end end end diff --git a/ee/app/services/ee/projects/operations/update_service.rb b/ee/app/services/ee/projects/operations/update_service.rb index f8d67c71eb852b..ef09c57d51aaed 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/app/views/projects/settings/operations/_incidents.html.haml b/ee/app/views/projects/settings/operations/_incidents.html.haml index 68979b71c56b56..02a75665752518 100644 --- a/ee/app/views/projects/settings/operations/_incidents.html.haml +++ b/ee/app/views/projects/settings/operations/_incidents.html.haml @@ -1,22 +1,25 @@ -- templates = [] +- setting = incident_management_setting +- templates = [''] + setting.available_issue_templates.map { |t| [t.name, t.key] } %section.settings.expanded.border-0.no-animate .settings-header - %h4= _("Incidents") + %h4= _('Incidents') %p - = _("Action to take when receiving an alert.") + = _('Action to take when receiving an alert.') = link_to help_page_path('administration/monitoring/prometheus/index', anchor: 'create-issues-from-alerts') do - = _("More information") + = _('More information') .settings-content = form_for @project, url: project_settings_operations_path(@project), method: :patch do |f| = form_errors(@project) .form-group - = f.check_box :archived # TODO - = f.label :archived, _('Create an issue. Issues are created for each alert triggered.'), class: 'form-check-label' - .form-group - = f.label :first_day_of_week, _('Issue template') - = f.select :archived, templates, {}, class: 'form-control' - .form-group - = f.check_box :archived # TODO - = f.label :archived, _('Send an email notification to Developers.'), class: 'form-check-label' + = f.fields_for :incident_management_setting_attributes, setting do |form| + .form-check.form-group + = form.check_box :create_issue + = form.label :create_issue, _('Create an issue. Issues are created for each alert triggered.'), class: 'form-check-label' + .form-group + = form.label :issue_template_key, _('Issue template') + = form.select :issue_template_key, templates, {}, class: 'form-control' + .form-check.form-group + = form.check_box :send_email + = form.label :send_email, _('Send an email notification to Developers.'), class: 'form-check-label' = f.submit _('Save changes'), class: 'btn btn-success' -- GitLab From 68a8695e96365b5f45de8c2be95faec40671d235 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Thu, 28 Feb 2019 00:10:32 +1100 Subject: [PATCH 5/5] WIP: add some feature specs for settings page --- app/assets/stylesheets/pages/settings.scss | 5 +- .../operations/_error_tracking.html.haml | 2 +- .../settings/operations/_incidents.html.haml | 5 +- .../settings/operations_settings_spec.rb | 75 +++++++++++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 ee/spec/features/projects/settings/operations_settings_spec.rb diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 6a58072b0eb7d4..4872cf1442bbcf 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -23,7 +23,10 @@ } .settings { - border-bottom: 1px solid $gray-darker; + // border-top for each item except the top one + + .settings { + border-top: 1px solid $border-color; + } &:first-of-type { margin-top: 10px; diff --git a/app/views/projects/settings/operations/_error_tracking.html.haml b/app/views/projects/settings/operations/_error_tracking.html.haml index 17c7525ed41605..61a3423bd4ad9e 100644 --- a/app/views/projects/settings/operations/_error_tracking.html.haml +++ b/app/views/projects/settings/operations/_error_tracking.html.haml @@ -3,7 +3,7 @@ - setting = error_tracking_setting %section.settings.expanded.no-animate - .settings-header.border-top + .settings-header %h4 = _('Error Tracking') %p diff --git a/ee/app/views/projects/settings/operations/_incidents.html.haml b/ee/app/views/projects/settings/operations/_incidents.html.haml index 02a75665752518..410bba028e74ff 100644 --- a/ee/app/views/projects/settings/operations/_incidents.html.haml +++ b/ee/app/views/projects/settings/operations/_incidents.html.haml @@ -1,7 +1,10 @@ +- return unless @project.feature_available?(:incident_management, current_user) + +- templates = [] - setting = incident_management_setting - templates = [''] + setting.available_issue_templates.map { |t| [t.name, t.key] } -%section.settings.expanded.border-0.no-animate +%section.settings.expanded.no-animate .settings-header %h4= _('Incidents') %p diff --git a/ee/spec/features/projects/settings/operations_settings_spec.rb b/ee/spec/features/projects/settings/operations_settings_spec.rb new file mode 100644 index 00000000000000..3e432b1393cd1b --- /dev/null +++ b/ee/spec/features/projects/settings/operations_settings_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Projects > Settings' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:role) { :maintainer } + let(:create_issue) { 'Create an issue. Issues are created for each alert triggered.' } + let(:send_email) { 'Send an email notification to Developers.' } + let(:issue_template_1) { 'Feature:\nReason:' } + let(:issue_template_2) { 'Bug Description:\nExpected behavior:\n' } + + before do + sign_in(user) + project.add_role(user, role) + end + + # test + # licensed shows section + describe 'Incidents' do + context 'with license' do + before do + # stub_licensed_features(incident_alerts: true) + visit project_settings_operations_path(project) + end + + it 'renders form for incident management' do + expect(page).to have_selector('h4', text: 'Incidents') + end + + it 'sets correct default values' do + expect(find_field(create_issue)).not_to be_checked + expect(find_field(send_email)).to be_checked + end + + it 'updates form values' do + check(create_issue) + template_select = find('select[name="project[issue_template_key]"]') + template_select.find(:xpath, 'option[1]').select_option + check(send_email) + save_form + + expect(find_field(create_issue)).to be_checked + expect(template_select.selected).to eq(issue_template_2) + expect(find_field(send_email)).not_to be_checked + end + + # it 'updates issue template' do + # save_form + # end + + # it 'updates email notification' do + # save_form + # end + + def save_form + page.within "#incident_management_edit_project_#{project.id}" do + click_on 'Save changes' + end + end + end + + context 'without license' do + before do + # stub_licensed_features(incident_alerts: false) + visit project_settings_operations_path(project) + end + + it 'renders form for incident management' do + expect(page).not_to have_selector('h4', text: 'Incidents') + end + end + end +end \ No newline at end of file -- GitLab