Merge branch '57171-add-dashboard-settings' into 'master'

Add ProjectMetricsDashboardSetting model and table

See merge request gitlab-org/gitlab-ce!27550

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,

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

app/models/project_metrics_setting.rb

+# 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

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

changelogs/unreleased/57171-add-dashboard-settings.yml

+---
+title: Add backend support for a External Dashboard URL setting
+merge_request: 27550
+author:
+type: added

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."

db/migrate/20190422082247_create_project_metrics_settings.rb

+# 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

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

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:

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
 

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 = {})

spec/factories/project_metrics_settings.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :project_metrics_setting, class: ProjectMetricsSetting do
+    project
+    external_dashboard_url 'https://grafana.com'
+  end
+end

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

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

spec/models/project_metrics_setting_spec.rb

+# 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

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