diff options
author | Reuben Pereira <rpereira@gitlab.com> | 2019-04-26 20:23:26 +0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2019-04-26 20:23:26 +0300 |
commit | 4376167a04aaeed084ad93aab2e65550109235c6 (patch) | |
tree | 99fa950932bf6ea5c0d453c9d4494a0adb0c16f3 | |
parent | bb6908cd5bcb14d70dfadbc2a16d7310bfa65445 (diff) |
Add ProjectMetricsDashboardSetting model and table
This new table will be used to store the external_dashboard_url which
allows users to add a link to their external dashboards (ex Grafana)
to the Metrics dashboard.
-rw-r--r-- | app/controllers/projects/settings/operations_controller.rb | 2 | ||||
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/models/project_metrics_setting.rb | 9 | ||||
-rw-r--r-- | app/services/projects/operations/update_service.rb | 11 | ||||
-rw-r--r-- | changelogs/unreleased/57171-add-dashboard-settings.yml | 5 | ||||
-rw-r--r-- | config/locales/en.yml | 2 | ||||
-rw-r--r-- | db/migrate/20190422082247_create_project_metrics_settings.rb | 14 | ||||
-rw-r--r-- | db/schema.rb | 7 | ||||
-rw-r--r-- | lib/gitlab/import_export/import_export.yml | 1 | ||||
-rw-r--r-- | lib/gitlab/import_export/relation_factory.rb | 3 | ||||
-rw-r--r-- | spec/controllers/projects/settings/operations_controller_spec.rb | 14 | ||||
-rw-r--r-- | spec/factories/project_metrics_settings.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/safe_model_attributes.yml | 6 | ||||
-rw-r--r-- | spec/models/project_metrics_setting_spec.rb | 55 | ||||
-rw-r--r-- | spec/services/projects/operations/update_service_spec.rb | 50 |
16 files changed, 188 insertions, 4 deletions
diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 1fafc33e917..5cfb0ac307d 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -56,6 +56,8 @@ module Projects # overridden in EE def permitted_project_params { + metrics_setting_attributes: [:external_dashboard_url], + error_tracking_setting_attributes: [ :enabled, :api_host, diff --git a/app/models/project.rb b/app/models/project.rb index 626ff9e1389..a29100405f9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -188,6 +188,7 @@ class Project < ApplicationRecord has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_one :project_repository, inverse_of: :project has_one :error_tracking_setting, inverse_of: :project, class_name: 'ErrorTracking::ProjectErrorTrackingSetting' + has_one :metrics_setting, inverse_of: :project, class_name: 'ProjectMetricsSetting' # Merge Requests for target project should be removed with it has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project @@ -297,6 +298,7 @@ class Project < ApplicationRecord reject_if: ->(attrs) { attrs[:id].blank? && attrs[:url].blank? } accepts_nested_attributes_for :error_tracking_setting, update_only: true + accepts_nested_attributes_for :metrics_setting, update_only: true, allow_destroy: true delegate :name, to: :owner, allow_nil: true, prefix: true delegate :members, to: :team, prefix: true diff --git a/app/models/project_metrics_setting.rb b/app/models/project_metrics_setting.rb new file mode 100644 index 00000000000..a2a7dc571a4 --- /dev/null +++ b/app/models/project_metrics_setting.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class ProjectMetricsSetting < ApplicationRecord + belongs_to :project + + validates :external_dashboard_url, + length: { maximum: 255 }, + addressable_url: { enforce_sanitization: true, ascii_only: true } +end diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index aedf79c86d7..48eddb0e8d0 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -12,7 +12,16 @@ module Projects private def project_update_params - error_tracking_params + error_tracking_params.merge(metrics_setting_params) + end + + def metrics_setting_params + attribs = params[:metrics_setting_attributes] + return {} unless attribs + + destroy = attribs[:external_dashboard_url].blank? + + { metrics_setting_attributes: attribs.merge(_destroy: destroy) } end def error_tracking_params diff --git a/changelogs/unreleased/57171-add-dashboard-settings.yml b/changelogs/unreleased/57171-add-dashboard-settings.yml new file mode 100644 index 00000000000..f235872b35c --- /dev/null +++ b/changelogs/unreleased/57171-add-dashboard-settings.yml @@ -0,0 +1,5 @@ +--- +title: Add backend support for a External Dashboard URL setting +merge_request: 27550 +author: +type: added diff --git a/config/locales/en.yml b/config/locales/en.yml index eb3b7771968..a3dceb2fb62 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -14,6 +14,8 @@ en: token: "Auth Token" project: "Project" api_url: "Sentry API URL" + project/metrics_setting: + external_dashboard_url: "External dashboard URL" errors: messages: label_already_exists_at_group_level: "already exists at group level for %{group}. Please choose another one." diff --git a/db/migrate/20190422082247_create_project_metrics_settings.rb b/db/migrate/20190422082247_create_project_metrics_settings.rb new file mode 100644 index 00000000000..3e21dd0a934 --- /dev/null +++ b/db/migrate/20190422082247_create_project_metrics_settings.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateProjectMetricsSettings < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :project_metrics_settings, id: :int, primary_key: :project_id, default: nil do |t| + t.string :external_dashboard_url, null: false + t.foreign_key :projects, column: :project_id, on_delete: :cascade + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 3a5d567ac57..23566a31a9d 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: 20190408163745) do +ActiveRecord::Schema.define(version: 20190422082247) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1681,6 +1681,10 @@ ActiveRecord::Schema.define(version: 20190408163745) do t.index ["project_id"], name: "index_project_import_data_on_project_id", using: :btree end + create_table "project_metrics_settings", primary_key: "project_id", id: :integer, default: nil, force: :cascade do |t| + t.string "external_dashboard_url", null: false + end + create_table "project_mirror_data", id: :serial, force: :cascade do |t| t.integer "project_id", null: false t.string "status" @@ -2529,6 +2533,7 @@ ActiveRecord::Schema.define(version: 20190408163745) do 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_metrics_settings", "projects", on_delete: :cascade add_foreign_key "project_mirror_data", "projects", on_delete: :cascade add_foreign_key "project_repositories", "projects", on_delete: :cascade add_foreign_key "project_repositories", "shards", on_delete: :restrict diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index ce268793128..c6d4fda4af5 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -75,6 +75,7 @@ project_tree: - :project_badges - :ci_cd_settings - :error_tracking_setting + - :metrics_setting # Only include the following attributes for the models specified. included_attributes: diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 61a1aa6da5a..e1e70a008d9 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -25,7 +25,8 @@ module Gitlab metrics: 'MergeRequest::Metrics', ci_cd_settings: 'ProjectCiCdSetting', error_tracking_setting: 'ErrorTracking::ProjectErrorTrackingSetting', - links: 'Releases::Link' }.freeze + links: 'Releases::Link', + metrics_setting: 'ProjectMetricsSetting' }.freeze USER_REFERENCES = %w[author_id assignee_id updated_by_id merged_by_id latest_closed_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id closed_by_id].freeze diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 0dd73972098..aa9cd41ed19 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -166,6 +166,20 @@ describe Projects::Settings::OperationsController do end end + context 'metrics dashboard setting' do + describe 'PATCH #update' do + let(:params) do + { + metrics_setting_attributes: { + external_dashboard_url: 'https://gitlab.com' + } + } + end + + it_behaves_like 'PATCHable' + end + end + private def project_params(project, params = {}) diff --git a/spec/factories/project_metrics_settings.rb b/spec/factories/project_metrics_settings.rb new file mode 100644 index 00000000000..234753f9b87 --- /dev/null +++ b/spec/factories/project_metrics_settings.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :project_metrics_setting, class: ProjectMetricsSetting do + project + external_dashboard_url 'https://grafana.com' + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 54369ff75f4..482e9c05da8 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -322,6 +322,7 @@ project: - pool_repository - kubernetes_namespaces - error_tracking_setting +- metrics_setting award_emoji: - awardable - user @@ -360,3 +361,5 @@ error_tracking_setting: - project suggestions: - note +metrics_setting: +- project diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index ebb62124cb1..591a3d296c2 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -606,7 +606,6 @@ ResourceLabelEvent: - user_id - created_at ErrorTracking::ProjectErrorTrackingSetting: -- id - api_url - project_id - project_name @@ -626,3 +625,8 @@ MergeRequestAssignee: - id - user_id - merge_request_id +ProjectMetricsSetting: +- project_id +- external_dashboard_url +- created_at +- updated_at diff --git a/spec/models/project_metrics_setting_spec.rb b/spec/models/project_metrics_setting_spec.rb new file mode 100644 index 00000000000..7df01625ba1 --- /dev/null +++ b/spec/models/project_metrics_setting_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ProjectMetricsSetting do + describe 'Associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'Validations' do + context 'when external_dashboard_url is over 255 chars' do + before do + subject.external_dashboard_url = 'https://' + 'a' * 250 + end + + it 'fails validation' do + expect(subject).not_to be_valid + expect(subject.errors.messages[:external_dashboard_url]) + .to include('is too long (maximum is 255 characters)') + end + end + + context 'with unsafe url' do + before do + subject.external_dashboard_url = %{https://replaceme.com/'><script>alert(document.cookie)</script>} + end + + it { is_expected.to be_invalid } + end + + context 'non ascii chars in external_dashboard_url' do + before do + subject.external_dashboard_url = 'http://gitlab.com/api/0/projects/project1/something€' + end + + it { is_expected.to be_invalid } + end + + context 'internal url in external_dashboard_url' do + before do + subject.external_dashboard_url = 'http://192.168.1.1' + end + + it { is_expected.to be_valid } + end + + context 'external_dashboard_url is blank' do + before do + subject.external_dashboard_url = '' + end + + it { is_expected.to be_invalid } + end + end +end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 86b1ec83f50..7e765659b9d 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -11,6 +11,56 @@ describe Projects::Operations::UpdateService do subject { described_class.new(project, user, params) } describe '#execute' do + context 'metrics dashboard setting' do + let(:params) do + { + metrics_setting_attributes: { + external_dashboard_url: 'http://gitlab.com' + } + } + end + + context 'without existing metrics dashboard setting' do + it 'creates a setting' do + expect(result[:status]).to eq(:success) + + expect(project.reload.metrics_setting.external_dashboard_url).to eq( + 'http://gitlab.com' + ) + end + end + + context 'with existing metrics dashboard setting' do + before do + create(:project_metrics_setting, project: project) + end + + it 'updates the settings' do + expect(result[:status]).to eq(:success) + + expect(project.reload.metrics_setting.external_dashboard_url).to eq( + 'http://gitlab.com' + ) + end + + context 'with blank external_dashboard_url in params' do + let(:params) do + { + metrics_setting_attributes: { + external_dashboard_url: '' + } + } + end + + it 'destroys the metrics_setting entry in DB' do + expect(result[:status]).to eq(:success) + + expect(project.reload.metrics_setting).to be_nil + end + end + end + end + context 'error tracking' do context 'with existing error tracking setting' do let(:params) do |