Merge branch 'issue_444915' into 'master'

Add YAML errors field to Dashboard objects on GraphQL

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148058



Merged-by: Max Woolf <mwoolf@gitlab.com>
Approved-by: Max Woolf <mwoolf@gitlab.com>
Reviewed-by: Ezekiel Kigbo <3397881-ekigbo@users.noreply.gitlab.com>
Reviewed-by: Max Woolf <mwoolf@gitlab.com>
Co-authored-by: Felipe Artur <felipefac@gmail.com>

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

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

ee/app/models/concerns/product_analytics/schema_validator.rb

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

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

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'],

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)

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": {

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 }

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

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