diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index aa116d77c1fb2e8f7ee8ce2cc865ad29aa3a8802..4fdc2b46286360825d30a03ea46edcfd8495c09e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -18415,6 +18415,7 @@ Represents a product analytics dashboard. | <a id="customizabledashboardcategory"></a>`category` | [`CustomizableDashboardCategory!`](#customizabledashboardcategory) | Category of dashboard. | | <a id="customizabledashboardconfigurationproject"></a>`configurationProject` | [`Project`](#project) | Project which contains the dashboard definition. | | <a id="customizabledashboarddescription"></a>`description` | [`String`](#string) | Description of the dashboard. | +| <a id="customizabledashboarderrors"></a>`errors` | [`[String!]`](#string) | Errors on yaml definition. | | <a id="customizabledashboardpanels"></a>`panels` | [`CustomizableDashboardPanelConnection!`](#customizabledashboardpanelconnection) | Panels shown on the dashboard. (see [Connections](#connections)) | | <a id="customizabledashboardslug"></a>`slug` | [`String!`](#string) | Slug of the dashboard. | | <a id="customizabledashboardtitle"></a>`title` | [`String!`](#string) | Title of the dashboard. | diff --git a/ee/app/graphql/types/product_analytics/dashboard_type.rb b/ee/app/graphql/types/product_analytics/dashboard_type.rb index 7d5fc8746bb56a04a35e12acd3d20459a161b99b..9d79bbc0233b7e158d077dded757e78c55980410 100644 --- a/ee/app/graphql/types/product_analytics/dashboard_type.rb +++ b/ee/app/graphql/types/product_analytics/dashboard_type.rb @@ -42,6 +42,11 @@ class DashboardType < BaseObject method: :config_project, null: true, description: 'Project which contains the dashboard definition.' + + field :errors, + type: [GraphQL::Types::String], + null: true, + description: 'Errors on yaml definition.' end end end diff --git a/ee/app/models/concerns/product_analytics/schema_validator.rb b/ee/app/models/concerns/product_analytics/schema_validator.rb new file mode 100644 index 0000000000000000000000000000000000000000..5b8fc4d161d007b6c701685fd1f6af428e8c946a --- /dev/null +++ b/ee/app/models/concerns/product_analytics/schema_validator.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module ProductAnalytics + module SchemaValidator + def schema_errors_for(yaml) + validator = JSONSchemer.schema(Pathname.new(self.class::SCHEMA_PATH)) + validator_errors = validator.validate(yaml) + validator_errors.map { |e| JSONSchemer::Errors.pretty(e) } if validator_errors.any? + end + end +end diff --git a/ee/app/models/product_analytics/dashboard.rb b/ee/app/models/product_analytics/dashboard.rb index 82b3e79c941d412d487bfe636c505ecbaa210082..1cd1035530954a691e2d66dac412c48bbc1f1ae3 100644 --- a/ee/app/models/product_analytics/dashboard.rb +++ b/ee/app/models/product_analytics/dashboard.rb @@ -2,13 +2,16 @@ module ProductAnalytics class Dashboard + include SchemaValidator + attr_reader :title, :description, :schema_version, :panels, :container, - :config_project, :slug, :path, :user_defined, :category + :config_project, :slug, :path, :user_defined, :category, :errors DASHBOARD_ROOT_LOCATION = '.gitlab/analytics/dashboards' PRODUCT_ANALYTICS_DASHBOARDS_LIST = %w[audience behavior].freeze - VALUE_STREAM_DASHBOARD_LIST = %w[value_streams_dashboard].freeze + VALUE_STREAM_DASHBOARD_NAME = 'value_streams_dashboard' + SCHEMA_PATH = 'ee/app/validators/json_schemas/analytics_dashboard.json' def self.for(container:, user:) unless container.is_a?(Group) || container.is_a?(Project) @@ -25,26 +28,36 @@ def self.for(container:, user:) root_trees = config_project&.repository&.tree(:head, DASHBOARD_ROOT_LOCATION) dashboards << builtin_dashboards(container, config_project, user) - dashboards << local_dashboards(container, config_project, root_trees.trees) if root_trees&.trees + dashboards << customized_dashboards(container, config_project, root_trees.trees) if root_trees&.trees - dashboards.flatten + dashboards.flatten.compact end - def initialize( - title:, description:, schema_version:, panels:, container:, slug:, user_defined:, - config_project:) - @title = title - @description = description - @schema_version = schema_version - @panels = panels - @container = container - @config_project = config_project - @slug = slug - @user_defined = user_defined + def initialize(**args) + @container = args[:container] + @config_project = args[:config_project] + @slug = args[:slug] + @user_defined = args[:user_defined] + + @yaml_definition = args[:config] + @title = @yaml_definition['title'] + @description = @yaml_definition['description'] + @schema_version = @yaml_definition['version'] + @panels = ProductAnalytics::Panel.from_data(@yaml_definition['panels'], config_project) @category = 'analytics' + + @errors = schema_errors_for(@yaml_definition) end - def self.local_dashboards(container, config_project, trees) + def ==(other) + slug == other.slug + end + + private + + attr_reader :yaml_definition + + def self.customized_dashboards(container, config_project, trees) trees.delete_if { |tree| tree.name == 'visualizations' }.map do |tree| config_data = config_project.repository.blob_data_at(config_project.repository.root_ref_sha, @@ -55,14 +68,11 @@ def self.local_dashboards(container, config_project, trees) config = YAML.safe_load(config_data) new( - container: container, - title: config['title'], slug: tree.name, - description: config['description'], - schema_version: config['version'], - panels: ProductAnalytics::Panel.from_data(config['panels'], config_project), - user_defined: true, - config_project: config_project + container: container, + config: config, + config_project: config_project, + user_defined: true ) end end @@ -83,51 +93,44 @@ def self.product_analytics_dashboards(container, config_project, user) config = load_yaml_dashboard_config(name, 'ee/lib/gitlab/analytics/product_analytics/dashboards') new( - container: container, - title: config['title'], slug: name, - description: config['description'], - schema_version: config['version'], - panels: ProductAnalytics::Panel.from_data(config['panels'], config_project), - user_defined: false, - config_project: config_project + container: container, + config: config, + config_project: config_project, + user_defined: false ) end end def self.value_stream_dashboard(container, config_project) - return [] unless container.value_streams_dashboard_available? - - VALUE_STREAM_DASHBOARD_LIST.map do |name| - config = load_yaml_dashboard_config(name, 'ee/lib/gitlab/analytics/value_stream_dashboard/dashboards') + return unless container.value_streams_dashboard_available? - new( - container: container, - title: config['title'], - slug: name, - description: config['description'], - schema_version: config['version'], - panels: ProductAnalytics::Panel.from_data(config['panels'], config_project), - user_defined: false, - config_project: config_project + config = + load_yaml_dashboard_config( + VALUE_STREAM_DASHBOARD_NAME, + 'ee/lib/gitlab/analytics/value_stream_dashboard/dashboards' ) - end + + new( + slug: VALUE_STREAM_DASHBOARD_NAME, + container: container, + config: config, + config_project: config_project, + user_defined: false + ) end def self.ai_impact_dashboard(container, config_project) - return [] unless container.ai_impact_dashboard_available? + return unless container.ai_impact_dashboard_available? config = load_yaml_dashboard_config('dashboard', 'ee/lib/gitlab/analytics/ai_impact_dashboard') new( - container: container, - title: config['title'], slug: 'ai_impact', - description: config['description'], - schema_version: config['version'], - panels: ProductAnalytics::Panel.from_data(config['panels'], config_project), - user_defined: false, - config_project: config_project + container: container, + config: config, + config_project: config_project, + user_defined: false ) end @@ -140,9 +143,5 @@ def self.builtin_dashboards(container, config_project, user) builtin.flatten end - - def ==(other) - slug == other.slug - end end end diff --git a/ee/app/models/product_analytics/panel.rb b/ee/app/models/product_analytics/panel.rb index 5a3823d85aebc487d0d1b8d5e280a19b53d88ed1..1878bb34e1902e84b618e88751c3da86a926ec4d 100644 --- a/ee/app/models/product_analytics/panel.rb +++ b/ee/app/models/product_analytics/panel.rb @@ -5,6 +5,8 @@ class Panel attr_reader :title, :grid_attributes, :visualization, :project, :query_overrides def self.from_data(panel_yaml, project) + return if panel_yaml.nil? + panel_yaml.map do |panel| new( title: panel['title'], diff --git a/ee/app/models/product_analytics/visualization.rb b/ee/app/models/product_analytics/visualization.rb index 6596b6140319d91a5ef1dc6c98df6625c4240baa..763d70d85a33cc33c67189fad5e72f98472eb255 100644 --- a/ee/app/models/product_analytics/visualization.rb +++ b/ee/app/models/product_analytics/visualization.rb @@ -2,6 +2,8 @@ module ProductAnalytics class Visualization + include SchemaValidator + attr_reader :type, :container, :data, :options, :config, :slug, :errors VISUALIZATIONS_ROOT_LOCATION = '.gitlab/analytics/dashboards/visualizations' @@ -120,13 +122,7 @@ def initialize(config:, slug:, init_error: nil) @errors = [e.message] end @slug = slug&.parameterize&.underscore - validate - end - - def validate - validator = JSONSchemer.schema(Pathname.new(SCHEMA_PATH)) - validator_errors = validator.validate(@config) - @errors = validator_errors.map { |e| JSONSchemer::Errors.pretty(e) } if validator_errors.any? + @errors = schema_errors_for(@config) end def self.visualization_config_path(data) diff --git a/ee/app/validators/json_schemas/analytics_dashboard.json b/ee/app/validators/json_schemas/analytics_dashboard.json index 0f006841f45a5a2c475a05609ad26cf1cec52cbd..e26ab3b09057d86fa8281c72596ae40d4be5e143 100644 --- a/ee/app/validators/json_schemas/analytics_dashboard.json +++ b/ee/app/validators/json_schemas/analytics_dashboard.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-06/schema#", "$ref": "#/definitions/AnalyticsDashboard", "definitions": { - "Welcome6": { + "AnalyticsDashboard": { "type": "object", "additionalProperties": false, "properties": { diff --git a/ee/spec/graphql/types/product_analytics/dashboard_type_spec.rb b/ee/spec/graphql/types/product_analytics/dashboard_type_spec.rb index 7866f85683f8fbffb248bf24ec6fc3c8ec3f1ad3..ccd5e2f240d0a810da8f2f0d4e59d3aef098a32c 100644 --- a/ee/spec/graphql/types/product_analytics/dashboard_type_spec.rb +++ b/ee/spec/graphql/types/product_analytics/dashboard_type_spec.rb @@ -4,7 +4,7 @@ RSpec.describe GitlabSchema.types['CustomizableDashboard'], feature_category: :product_analytics_data_management do let(:expected_fields) do - %i[title slug description panels user_defined configuration_project category] + %i[title slug description panels user_defined configuration_project category errors] end subject { described_class } diff --git a/ee/spec/models/product_analytics/dashboard_spec.rb b/ee/spec/models/product_analytics/dashboard_spec.rb index 909ed0a43cf85386b023b40ed26600d752f83f64..72f4ce338b05ad178336e5274724833e8822aef9 100644 --- a/ee/spec/models/product_analytics/dashboard_spec.rb +++ b/ee/spec/models/product_analytics/dashboard_spec.rb @@ -23,6 +23,42 @@ ) end + describe '#errors' do + let(:dashboard) do + described_class.new( + container: group, + config: YAML.safe_load(config_yaml), + slug: 'test2', + user_defined: true, + config_project: project + ) + end + + context 'when yaml is valid' do + let(:config_yaml) do + File.open(Rails.root.join('ee/spec/fixtures/product_analytics/dashboard_example_1.yaml')).read + end + + it 'returns nil' do + expect(dashboard.errors).to be_nil + end + end + + context 'when yaml is faulty' do + let(:config_yaml) do + <<-YAML +--- +title: not good yaml +description: with missing properties + YAML + end + + it 'returns schema errors' do + expect(dashboard.errors).to eq(["root is missing required keys: version, panels"]) + end + end + end + describe '.for' do context 'when resource is a project' do let(:resource_parent) { project } @@ -60,11 +96,13 @@ expect(subject.last.slug).to eq('dashboard_example_1') expect(subject.last.description).to eq('North Star Metrics across all departments for the last 3 quarters.') expect(subject.last.schema_version).to eq('1') + expect(subject.last.errors).to be_nil end end context 'when the dashboard file does not exist in the directory' do before do + # Invalid dashboard - should not be included project.repository.create_file( project.creator, '.gitlab/analytics/dashboards/dashboard_example_1/project_dashboard_example_wrongly_named.yaml', @@ -72,10 +110,22 @@ message: 'test', branch_name: 'master' ) + + # Valid dashboard - should be included + project.repository.create_file( + project.creator, + '.gitlab/analytics/dashboards/dashboard_example_2/dashboard_example_2.yaml', + File.open(Rails.root.join('ee/spec/fixtures/product_analytics/dashboard_example_1.yaml')).read, + message: 'test', + branch_name: 'master' + ) end it 'excludes the dashboard from the list' do - expect(subject.size).to eq(5) + expected_dashboards = + ["Audience", "Behavior", "Value Streams Dashboard", "AI impact analytics", "Dashboard Example 1"] + + expect(subject.map(&:title)).to eq(expected_dashboards) end end @@ -176,12 +226,13 @@ describe '#==' do let(:dashboard_1) { described_class.for(container: project, user: user).first } let(:dashboard_2) do + config_yaml = + File.open(Rails.root.join('ee/spec/fixtures/product_analytics/dashboard_example_1.yaml')).read + config_yaml = YAML.safe_load(config_yaml) + described_class.new( - title: 'a', - description: 'b', - schema_version: '1', - panels: [], container: project, + config: config_yaml, slug: 'test2', user_defined: true, config_project: project @@ -197,7 +248,7 @@ subject { described_class.value_stream_dashboard(project, config_project) } it 'returns the value stream dashboard' do - dashboard = subject.first + dashboard = subject expect(dashboard).to be_a(described_class) expect(dashboard.title).to eq('Value Streams Dashboard') expect(dashboard.slug).to eq('value_streams_dashboard') @@ -214,16 +265,16 @@ end context 'for projects' do - it 'returns an empty array' do + it 'returns nil' do dashboard = described_class.value_stream_dashboard(project, config_project) - expect(dashboard).to match_array([]) + expect(dashboard).to be_nil end end context 'for groups' do it 'returns the value streams dashboard' do - dashboard = described_class.value_stream_dashboard(group, config_project).first + dashboard = described_class.value_stream_dashboard(group, config_project) expect(dashboard).to be_a(described_class) expect(dashboard.title).to eq('Value Streams Dashboard') @@ -248,9 +299,7 @@ stub_feature_flags(ai_impact_analytics_dashboard: false) end - it 'returns an empty array' do - expect(subject).to match_array([]) - end + it { is_expected.to be_nil } end end @@ -268,9 +317,7 @@ stub_feature_flags(ai_impact_analytics_dashboard: false) end - it 'returns an empty array' do - expect(subject).to match_array([]) - end + it { is_expected.to be_nil } end end end diff --git a/ee/spec/models/product_analytics/panel_spec.rb b/ee/spec/models/product_analytics/panel_spec.rb index fc706db593c65bfde7839f13f2add25035dc5b33..1f019656a4cd07db6f2ac9aaff61d547090d2e85 100644 --- a/ee/spec/models/product_analytics/panel_spec.rb +++ b/ee/spec/models/product_analytics/panel_spec.rb @@ -25,4 +25,10 @@ .to eq({ 'xAxis' => { 'name' => 'Time', 'type' => 'time' }, 'yAxis' => { 'name' => 'Counts' } }) expect(subject.data['type']).to eq('Cube') end + + describe '.from_data' do + it 'returns nil when yaml is missing' do # instead of raising a 500 + expect(described_class.from_data(nil, project)).to be_nil + end + end end