From 15009be36e30e1f8101817ecbbc393dfa7f2deda Mon Sep 17 00:00:00 2001 From: jbat Date: Fri, 1 Nov 2024 15:04:28 +1100 Subject: [PATCH 1/9] NEP-18768 API Dashboard Report --- lib/superset/dashboard/datasets/list.rb | 10 ++ lib/superset/dataset/get.rb | 2 +- lib/superset/display.rb | 4 + lib/superset/services/dashboard_report.rb | 134 ++++++++++++++++++++++ 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 lib/superset/services/dashboard_report.rb diff --git a/lib/superset/dashboard/datasets/list.rb b/lib/superset/dashboard/datasets/list.rb index 8ebf9b9..b915d24 100644 --- a/lib/superset/dashboard/datasets/list.rb +++ b/lib/superset/dashboard/datasets/list.rb @@ -88,6 +88,16 @@ def rows end end + private + + def route + "dashboard/#{id}/datasets" + end + + def list_attributes + ['id', 'datasource_name', 'database_id', 'database_name', 'database_backend', 'schema'].map(&:to_sym) + end + # when displaying a list of datasets, show dashboard title as well def title @title ||= [id, dashboard.title].join(' ') diff --git a/lib/superset/dataset/get.rb b/lib/superset/dataset/get.rb index 9d042f5..e79628a 100644 --- a/lib/superset/dataset/get.rb +++ b/lib/superset/dataset/get.rb @@ -38,7 +38,7 @@ def database_id end def sql - ['sql'] + result['sql'] end private diff --git a/lib/superset/display.rb b/lib/superset/display.rb index 95f40c8..d7d24a9 100644 --- a/lib/superset/display.rb +++ b/lib/superset/display.rb @@ -18,6 +18,10 @@ def rows end end + def rows_hash + rows.map { |value| list_attributes.zip(value).to_h } + end + def title self.class.to_s end diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb new file mode 100644 index 0000000..bb40e38 --- /dev/null +++ b/lib/superset/services/dashboard_report.rb @@ -0,0 +1,134 @@ +# Creates a log report on a set of dashboards +# providing count of charts, datasets, and databases used in each dashboard +# as well as optional data sovereignty information + +module Superset + module Services + class DashboardReport + + attr_reader :dashboard_ids, :report_on_data_sovereignty_only + + def initialize(dashboard_ids: [], report_on_data_sovereignty_only: true) + @dashboard_ids = dashboard_ids.presence || all_dashboard_ids + @report_on_data_sovereignty_only = report_on_data_sovereignty_only + end + + def perform + create_report + + report_on_data_sovereignty_only ? display_data_sovereignty_report : @report + end + + def all_dashboard_ids + Superset::Dashboard::List.new().rows.map{|d| d['id']} + end + + def display_data_sovereignty_report + # filter by dashboards where + # 1. A filter dataset is not part of the dashboard datasets (might be ok for some cases) + # 2. There is more than one distinct dataset schema (never ok for embedded dashboards) + + puts "Data Sovereignty Report" + puts "-----------------------" + puts "Invalid Dashboards: #{data_sovereignty_issues.count}" + data_sovereignty_issues + end + + # possible data sovereignty issues + def data_sovereignty_issues + @report.map do |dashboard| + reasons = [] + chart_dataset_ids = dashboard[:datasets][:chart_datasets].map{|d| d[:id]} + + # invalid if any filters datasets are not part of the chart datasets + unknown_datasets = dashboard[:filters][:filter_dataset_ids] - chart_dataset_ids + if unknown_datasets.any? + reasons << "WARNING: One or more filter datasets is not included in chart datasets for " \ + "filter dataset ids: #{unknown_datasets.join(', ')}." + reasons << "DETAILS: #{unknown_dataset_details(unknown_datasets)}" + end + + # invalid if any filters datasets are not part of the chart datasets + chart_dataset_schemas = dashboard[:datasets][:chart_datasets].map{|d| d[:schema]}.uniq + if chart_dataset_schemas.count > 1 + reasons << "ERROR: Multiple distinct chart dataset schemas found. Expected 1. Found #{chart_dataset_schemas.count}. " \ + "schema names: #{chart_dataset_schemas.join(', ') }" + end + + { reasons: reasons, dashboard: dashboard } if reasons.any? + end.compact + end + + def unknown_dataset_details(unknown_datasets) + unknown_datasets.map do |dataset_id| + d = Superset::Dataset::Get.new(dataset_id) + d.result + { id: d.id, name: d.title } + rescue Happi::Error::NotFound => e + { id: dataset_id, name: '>>>> ERROR: DATASET DOES NOT EXIST <<<<' } + rescue => e + { id: dataset_id, name: '>>>> ERROR: #{e} <<<<' } + end + end + + def create_report + @report ||= begin + dashboard_ids.map do |dashboard_id| + dashboard = dashboard_result(dashboard_id) + # binding.pry + { + dashboard_id: dashboard_id, + dashboard_title: dashboard['dashboard_title'], + dashboard_url: dashboard['url'], + dashboard_client_tags: dashboard_client_tags(dashboard), + filters: filter_details(dashboard), + charts: chart_count(dashboard), + datasets: dataset_details(dashboard_id), + } + end + end + end + + def filter_details(dashboard) + { + filter_count: filter_count(dashboard), + filter_dataset_ids: filter_datasets(dashboard) + } + end + + def filter_count(dashboard) + dashboard['json_metadata']['native_filter_configuration'].count + end + + def filter_datasets(dashboard) + dashboard['json_metadata']['native_filter_configuration'].map do |filter| + filter['targets'].map{|d| d['datasetId']} if filter['type'] == 'NATIVE_FILTER' + end.flatten.compact.uniq + end + + def chart_count(dashboard) + dashboard['json_metadata']['chart_configuration'].count + end + + def dataset_details(dashboard_id) + datasets = Superset::Dashboard::Datasets::List.new(dashboard_id).rows_hash + { + dataset_count: datasets.count, + chart_datasets: datasets + } + end + + def dashboard_client_tags(dashboard) + dashboard['tags'].map { |t| t['name'] if t['name'].include?('client') }.compact.join(',') || 'None' + end + + def dashboard_result(dashboard_id) + # convert json_metadata within result to a hash + board = Superset::Dashboard::Get.new(dashboard_id) + board.result['json_metadata'] = JSON.parse(board.result['json_metadata']) + board.result['url'] = board.url # add full url to the dashboard result + board.result + end + end + end +end From 1f22dc0d0c9543a5daa5513895549b8df470ca63 Mon Sep 17 00:00:00 2001 From: jbat Date: Sat, 2 Nov 2024 16:34:09 +1100 Subject: [PATCH 2/9] update usage doc --- lib/superset/services/dashboard_report.rb | 52 +++++++++++++---------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb index bb40e38..1c49939 100644 --- a/lib/superset/services/dashboard_report.rb +++ b/lib/superset/services/dashboard_report.rb @@ -2,6 +2,12 @@ # providing count of charts, datasets, and databases used in each dashboard # as well as optional data sovereignty information +# Data Sovereignty in this context requires that all datasets used in a dashboard are from one database schema only. +# Primarily used to identify potential issues with embedded dashboards where data sovereignty is a concern. + +# Usage: +# Superset::Services::DashboardReport.new(dashboard_ids: [1,2,3]).perform + module Superset module Services class DashboardReport @@ -36,27 +42,29 @@ def display_data_sovereignty_report # possible data sovereignty issues def data_sovereignty_issues - @report.map do |dashboard| - reasons = [] - chart_dataset_ids = dashboard[:datasets][:chart_datasets].map{|d| d[:id]} - - # invalid if any filters datasets are not part of the chart datasets - unknown_datasets = dashboard[:filters][:filter_dataset_ids] - chart_dataset_ids - if unknown_datasets.any? - reasons << "WARNING: One or more filter datasets is not included in chart datasets for " \ - "filter dataset ids: #{unknown_datasets.join(', ')}." - reasons << "DETAILS: #{unknown_dataset_details(unknown_datasets)}" - end - - # invalid if any filters datasets are not part of the chart datasets - chart_dataset_schemas = dashboard[:datasets][:chart_datasets].map{|d| d[:schema]}.uniq - if chart_dataset_schemas.count > 1 - reasons << "ERROR: Multiple distinct chart dataset schemas found. Expected 1. Found #{chart_dataset_schemas.count}. " \ - "schema names: #{chart_dataset_schemas.join(', ') }" - end - - { reasons: reasons, dashboard: dashboard } if reasons.any? - end.compact + @data_sovereignty_issues ||= begin + @report.map do |dashboard| + reasons = [] + chart_dataset_ids = dashboard[:datasets][:chart_datasets].map{|d| d[:id]} + + # invalid if any filters datasets are not part of the chart datasets + unknown_datasets = dashboard[:filters][:filter_dataset_ids] - chart_dataset_ids + if unknown_datasets.any? + reasons << "WARNING: One or more filter datasets is not included in chart datasets for " \ + "filter dataset ids: #{unknown_datasets.join(', ')}." + reasons << "DETAILS: #{unknown_dataset_details(unknown_datasets)}" + end + + # invalid if any filters datasets are not part of the chart datasets + chart_dataset_schemas = dashboard[:datasets][:chart_datasets].map{|d| d[:schema]}.uniq + if chart_dataset_schemas.count > 1 + reasons << "ERROR: Multiple distinct chart dataset schemas found. Expected 1. Found #{chart_dataset_schemas.count}. " \ + "schema names: #{chart_dataset_schemas.join(', ') }" + end + + { reasons: reasons, dashboard: dashboard } if reasons.any? + end.compact + end end def unknown_dataset_details(unknown_datasets) @@ -66,8 +74,6 @@ def unknown_dataset_details(unknown_datasets) { id: d.id, name: d.title } rescue Happi::Error::NotFound => e { id: dataset_id, name: '>>>> ERROR: DATASET DOES NOT EXIST <<<<' } - rescue => e - { id: dataset_id, name: '>>>> ERROR: #{e} <<<<' } end end From 3c3c33db5463582046cafe9fb422adf6d4d077d9 Mon Sep 17 00:00:00 2001 From: jbat Date: Mon, 4 Nov 2024 08:42:01 +1100 Subject: [PATCH 3/9] add default order, update specs --- lib/superset/dashboard/list.rb | 2 +- lib/superset/dashboard/list_all.rb | 62 +++++++++++++ lib/superset/request.rb | 7 +- lib/superset/tag/add_to_object.rb | 6 ++ lib/superset/tag/delete_from_object.rb | 45 +++++++++ spec/superset/chart/list_spec.rb | 7 +- spec/superset/dashboard/list_spec.rb | 11 ++- spec/superset/database/list_spec.rb | 9 +- spec/superset/dataset/list_spec.rb | 12 +-- spec/superset/request_spec.rb | 6 ++ spec/superset/security/user/list_spec.rb | 25 +---- spec/superset/tag/delete_from_object_spec.rb | 96 ++++++++++++++++++++ 12 files changed, 247 insertions(+), 41 deletions(-) create mode 100644 lib/superset/dashboard/list_all.rb create mode 100644 lib/superset/tag/delete_from_object.rb create mode 100644 spec/superset/tag/delete_from_object_spec.rb diff --git a/lib/superset/dashboard/list.rb b/lib/superset/dashboard/list.rb index c105860..70d941a 100644 --- a/lib/superset/dashboard/list.rb +++ b/lib/superset/dashboard/list.rb @@ -7,7 +7,7 @@ module Dashboard class List < Superset::Request attr_reader :title_contains, :title_equals, :tags_equal, :ids_not_in, :include_filter_dataset_schemas - def initialize(page_num: 0, title_contains: '', title_equals: '', tags_equal: [], ids_not_in: [], include_filter_dataset_schemas: false) + def initialize(page_num: 0, title_contains: '', title_equals: '', tags_equal: [], ids_not_in: [], include_filter_dataset_schemas: false, **extra_args) @title_contains = title_contains @title_equals = title_equals @tags_equal = tags_equal diff --git a/lib/superset/dashboard/list_all.rb b/lib/superset/dashboard/list_all.rb new file mode 100644 index 0000000..a039684 --- /dev/null +++ b/lib/superset/dashboard/list_all.rb @@ -0,0 +1,62 @@ +# Superset has a 100 result limit for requests +# This is a wrapper for Superset::Dashboard::List to recursively list all dashboards + +# TODO - would be very handy to create a parent class for this +# to then be able to use the same pattern for other ::List classes + +module Superset + module Dashboard + class ListAll + include Display + + def initialize(**kwargs) + kwargs.each do |key, value| + instance_variable_set("@#{key}", value) + self.class.attr_reader key + end + end + + def constructor_args + instance_variables.each_with_object({}) do |var, hash| + hash[var.to_s.delete('@').to_sym] = instance_variable_get(var) + end + end + + def perform + page_num = 0 + boards = [] + boards << next_group = Dashboard::List.new(page_num: page_num, **constructor_args).result + while !next_group.empty? + boards << next_group = Dashboard::List.new(page_num: page_num += 1, **constructor_args).result + end + @result = boards.flatten + end + + def result + @result ||= [] + end + + def rows + result.map do |d| + list_attributes.map do |la| + la == :url ? "#{superset_host}#{d[la]}" : d[la] + end + end + end + + def ids + result.map { |d| d[:id] } + end + + private + + def list_attributes + [:id, :dashboard_title, :status, :url] + end + + def superset_host + ENV['SUPERSET_HOST'] + end + end + end +end diff --git a/lib/superset/request.rb b/lib/superset/request.rb index d9d53a5..0ae0f83 100644 --- a/lib/superset/request.rb +++ b/lib/superset/request.rb @@ -33,7 +33,12 @@ def superset_host end def query_params - [filters, pagination].join + [filters, pagination, order_by].join + end + + def order_by + # by default, we will order by changed_on as per the GUI + ",order_column:changed_on,order_direction:desc" end private diff --git a/lib/superset/tag/add_to_object.rb b/lib/superset/tag/add_to_object.rb index 1a0e5bb..61eb098 100644 --- a/lib/superset/tag/add_to_object.rb +++ b/lib/superset/tag/add_to_object.rb @@ -1,5 +1,11 @@ # frozen_string_literal: true +# Add tags to an object +# for object type options see ObjectType.to_a +# +# Usage: +# Superset::Tag::AddToObject.new(object_type_id: ObjectType::DASHBOARD, object_id: new_dashboard.id, tags: tags).perform + module Superset module Tag class AddToObject < Superset::Request diff --git a/lib/superset/tag/delete_from_object.rb b/lib/superset/tag/delete_from_object.rb new file mode 100644 index 0000000..b049754 --- /dev/null +++ b/lib/superset/tag/delete_from_object.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +# Delete tags from an object +# for object type options see ObjectType.to_a +# +# Usage: +# Superset::Tag::DeleteFromObject.new(object_type_id: ObjectType::DASHBOARD, object_id: 101, tag: 'test-tag').perform + +module Superset + module Tag + class DeleteFromObject < Superset::Request + + attr_reader :object_type_id, :object_id, :tag + + def initialize(object_type_id:, object_id:, tag:) + @object_type_id = object_type_id + @object_id = object_id + @tag = tag + end + + def perform + validate_constructor_args + + response + end + + def response + @response ||= client.delete(route) + end + + def validate_constructor_args + raise InvalidParameterError, "object_type_id integer is required" unless object_type_id.present? && object_type_id.is_a?(Integer) + raise InvalidParameterError, "object_type_id is not a known value" unless ObjectType.list.include?(object_type_id) + raise InvalidParameterError, "object_id integer is required" unless object_id.present? && object_id.is_a?(Integer) + raise InvalidParameterError, "tag string is required" unless tag.present? && tag.is_a?(String) + end + + private + + def route + "tag/#{object_type_id}/#{object_id}/#{tag}/" + end + end + end +end diff --git a/spec/superset/chart/list_spec.rb b/spec/superset/chart/list_spec.rb index d648001..8ad7d0b 100644 --- a/spec/superset/chart/list_spec.rb +++ b/spec/superset/chart/list_spec.rb @@ -74,6 +74,7 @@ "viz_type"=>"dist_bar" }] end + let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } before do allow(subject).to receive(:result).and_return(result) @@ -106,14 +107,14 @@ describe '#query_params' do specify 'with defaults' do - expect(subject.query_params).to eq("page:0,page_size:100") + expect(subject.query_params).to eq(default_query_params) end context 'with name_contains filters' do subject { described_class.new(name_contains: 'birth') } specify do - expect(subject.query_params).to eq("filters:!((col:slice_name,opr:ct,value:'birth')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:slice_name,opr:ct,value:'birth')),#{default_query_params}") end end @@ -125,7 +126,7 @@ "filters:!(" \ "(col:slice_name,opr:ct,value:'birth')," \ "(col:dashboards,opr:rel_m_m,value:3)" \ - "),page:0,page_size:100") + "),#{default_query_params}") end end end diff --git a/spec/superset/dashboard/list_spec.rb b/spec/superset/dashboard/list_spec.rb index 13dffca..9acddc0 100644 --- a/spec/superset/dashboard/list_spec.rb +++ b/spec/superset/dashboard/list_spec.rb @@ -19,6 +19,7 @@ } ] end + let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } before do allow(subject).to receive(:result).and_return(result) @@ -62,7 +63,7 @@ context 'for pagination' do context 'with defaults' do specify do - expect(subject.query_params).to eq("page:0,page_size:100") + expect(subject.query_params).to eq(default_query_params) end end @@ -70,7 +71,7 @@ subject { described_class.new(page_num: 5) } specify do - expect(subject.query_params).to eq("page:5,page_size:100") + expect(subject.query_params).to eq(default_query_params.gsub('page:0', 'page:5')) end end end @@ -79,7 +80,7 @@ subject { described_class.new(title_contains: 'acme') } specify do - expect(subject.query_params).to eq("filters:!((col:dashboard_title,opr:ct,value:'acme')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:dashboard_title,opr:ct,value:'acme')),#{default_query_params}") end end @@ -91,7 +92,7 @@ "filters:!(" \ "(col:dashboard_title,opr:ct,value:'birth')," \ "(col:tags,opr:dashboard_tags,value:'template')" \ - "),page:0,page_size:100") + "),#{default_query_params}") end end @@ -106,7 +107,7 @@ "(col:tags,opr:dashboard_tags,value:'template')," \ "(col:tags,opr:dashboard_tags,value:'client:acme')," \ "(col:tags,opr:dashboard_tags,value:'product:turbo-charged-feet')" \ - "),page:3,page_size:100") + "),#{default_query_params.gsub('page:0', 'page:3')}") end end end diff --git a/spec/superset/database/list_spec.rb b/spec/superset/database/list_spec.rb index 9c446ac..e2de2ea 100644 --- a/spec/superset/database/list_spec.rb +++ b/spec/superset/database/list_spec.rb @@ -20,6 +20,7 @@ ] } end + let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } describe '#rows' do before do @@ -40,7 +41,7 @@ context 'for pagination' do context 'with defaults' do specify do - expect(subject.query_params).to eq("page:0,page_size:100") + expect(subject.query_params).to eq(default_query_params) end end @@ -48,7 +49,7 @@ subject { described_class.new(page_num: 5) } specify do - expect(subject.query_params).to eq("page:5,page_size:100") + expect(subject.query_params).to eq(default_query_params.gsub('page:0', 'page:5')) end end end @@ -57,7 +58,7 @@ subject { described_class.new(title_contains: 'acme') } specify do - expect(subject.query_params).to eq("filters:!((col:database_name,opr:ct,value:'acme')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:database_name,opr:ct,value:'acme')),#{default_query_params}") end end @@ -65,7 +66,7 @@ subject { described_class.new(uuid_equals: '123') } specify do - expect(subject.query_params).to eq("filters:!((col:uuid,opr:eq,value:'123')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:uuid,opr:eq,value:'123')),#{default_query_params}") end end end diff --git a/spec/superset/dataset/list_spec.rb b/spec/superset/dataset/list_spec.rb index 992b30c..a602fc9 100644 --- a/spec/superset/dataset/list_spec.rb +++ b/spec/superset/dataset/list_spec.rb @@ -43,6 +43,8 @@ ] end + let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } + before do allow(subject).to receive(:result).and_return(result) allow(subject).to receive(:superset_host).and_return(superset_host) @@ -50,8 +52,6 @@ end describe '#rows' do - #before { stub_const("Superset::Request::PAGE_SIZE", "3") } - specify do expect(subject.rows).to eq( [ @@ -64,14 +64,14 @@ describe '#query_params' do specify 'with defaults' do - expect(subject.query_params).to eq("page:0,page_size:100") + expect(subject.query_params).to eq("#{default_query_params}") end context 'with title_contains filters' do subject { described_class.new(title_contains: 'birth') } specify do - expect(subject.query_params).to eq("filters:!((col:table_name,opr:ct,value:'birth')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:table_name,opr:ct,value:'birth')),#{default_query_params}") end end @@ -79,7 +79,7 @@ subject { described_class.new(title_equals: 'birth_days') } specify do - expect(subject.query_params).to eq("filters:!((col:table_name,opr:eq,value:'birth_days')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:table_name,opr:eq,value:'birth_days')),#{default_query_params}") end end @@ -88,7 +88,7 @@ subject { described_class.new(title_equals: 'birth_days', schema_equals: 'schema_one') } specify do - expect(subject.query_params).to eq("filters:!((col:table_name,opr:eq,value:'birth_days'),(col:schema,opr:eq,value:'schema_one')),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:table_name,opr:eq,value:'birth_days'),(col:schema,opr:eq,value:'schema_one')),#{default_query_params}") end end diff --git a/spec/superset/request_spec.rb b/spec/superset/request_spec.rb index 0f9cc32..4ff1b63 100644 --- a/spec/superset/request_spec.rb +++ b/spec/superset/request_spec.rb @@ -38,4 +38,10 @@ expect(subject.superset_host).to eq(host) end end + + describe '#query_params' do + it 'returns the default query params' do + expect(subject.send(:query_params)).to eq("page:0,page_size:100,order_column:changed_on,order_direction:desc") + end + end end diff --git a/spec/superset/security/user/list_spec.rb b/spec/superset/security/user/list_spec.rb index e6fa34c..db9133d 100644 --- a/spec/superset/security/user/list_spec.rb +++ b/spec/superset/security/user/list_spec.rb @@ -35,41 +35,24 @@ }] end + let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } + before do allow(subject).to receive(:result).and_return(result) allow(subject).to receive(:superset_host).and_return(superset_host) allow(subject).to receive(:response).and_return( { 'count': 45 } ) end - describe '#list' do - before { stub_const("Superset::Request::PAGE_SIZE", "3") } - - specify do - expect(subject.table.to_s).to eq( - "+----+------------+-----------+----------------------+--------+-------------+----------------------------+\n" + - "| 45 Matching Users for Host: https://test.superset.host.com |\n" + - "| 3 Users listed with: page:0,page_size:3 |\n" + - "+----+------------+-----------+----------------------+--------+-------------+----------------------------+\n" + - "| Id | First name | Last name | Email | Active | Login count | Last login |\n" + - "+----+------------+-----------+----------------------+--------+-------------+----------------------------+\n" + - "| 99 | Ben | Barrow | ben.barrow@mymail.io | true | 7 | 2023-11-07T01:20:52.690091 |\n" + - "| 44 | Em | Vier | em.vier@mymail.io | true | 2 | 2023-09-12T07:36:07.115849 |\n" + - "| 55 | Raf | Zar | raf.zar@mymail.io | true | 2 | 2023-10-27T03:32:44.185404 |\n" + - "+----+------------+-----------+----------------------+--------+-------------+----------------------------+" - ) - end - end - describe '#query_params' do specify 'with defaults' do - expect(subject.query_params).to eq("page:0,page_size:100") + expect(subject.query_params).to eq(default_query_params) end context 'with email filters' do subject { described_class.new(email_contains: 'mymail') } specify do - expect(subject.query_params).to eq("filters:!((col:email,opr:ct,value:mymail)),page:0,page_size:100") + expect(subject.query_params).to eq("filters:!((col:email,opr:ct,value:mymail)),#{default_query_params}") end end end diff --git a/spec/superset/tag/delete_from_object_spec.rb b/spec/superset/tag/delete_from_object_spec.rb new file mode 100644 index 0000000..2d0429b --- /dev/null +++ b/spec/superset/tag/delete_from_object_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' + +RSpec.describe Superset::Tag::DeleteFromObject do + subject { described_class.new(object_type_id: object_type_id, object_id: object_id, tag: tag) } + let(:dashboard_id) { 1 } + let(:response) { { "message"=>"OK" } } + + + describe 'perform' do + context 'with valid params' do + let(:object_type_id) { 1 } + let(:object_id) { 1 } + let(:tag) { 'tag1' } + + before do + allow(subject).to receive(:response).and_return(response) + end + + it 'returns the response' do + expect(subject.perform).to eq response + end + end + + context 'with invalid params' do + context 'when object_type_id is not an integer' do + let(:object_type_id) { 'q' } + let(:object_id) { 1 } + let(:tag) { ['tag1', 'tag2'] } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "object_type_id integer is required") + end + end + + context 'when object_type_id is not a known value' do + let(:object_type_id) { 5 } + let(:object_id) { 1 } + let(:tag) { ['tag1', 'tag2'] } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "object_type_id is not a known value") + end + end + + context 'when object_type_id is not present' do + let(:object_type_id) { nil } + let(:object_id) { 1 } + let(:tag) { ['tag1', 'tag2'] } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "object_type_id integer is required") + end + end + + context 'when object_id is not an integer' do + let(:object_type_id) { 1 } + let(:object_id) { 'q' } + let(:tag) { ['tag1', 'tag2'] } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "object_id integer is required") + end + end + + context 'when object_id is not present' do + let(:object_type_id) { 1 } + let(:object_id) { nil } + let(:tag) { ['tag1', 'tag2'] } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "object_id integer is required") + end + end + + context 'when tag is not an string' do + let(:object_type_id) { 1 } + let(:object_id) { 1 } + let(:tag) { 101 } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "tag string is required") + end + end + + context 'when tags array contains non-string values' do + let(:object_type_id) { 1 } + let(:object_id) { 1 } + let(:tag) { nil } + + it 'raises an error' do + expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "tag string is required") + end + end + end + end +end From 251552d90118440a90f8db77bf8feafda6897223 Mon Sep 17 00:00:00 2001 From: jbat Date: Mon, 4 Nov 2024 09:10:33 +1100 Subject: [PATCH 4/9] adjust order by parms to not have a default --- lib/superset/dashboard/list.rb | 4 ++++ lib/superset/request.rb | 12 +++++++----- spec/superset/chart/list_spec.rb | 4 +--- spec/superset/database/list_spec.rb | 2 +- spec/superset/dataset/list_spec.rb | 2 +- spec/superset/request_spec.rb | 2 +- spec/superset/security/user/list_spec.rb | 2 +- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/superset/dashboard/list.rb b/lib/superset/dashboard/list.rb index 70d941a..8aafd5e 100644 --- a/lib/superset/dashboard/list.rb +++ b/lib/superset/dashboard/list.rb @@ -87,6 +87,10 @@ def ids_not_in_filters ids_not_in.map {|id| "(col:id,opr:neq,value:'#{id}')"}.join(',') end + def order_by + ",order_column:changed_on,order_direction:desc" + end + def list_attributes [:id, :dashboard_title, :status, :url] end diff --git a/lib/superset/request.rb b/lib/superset/request.rb index 0ae0f83..3f1ee9d 100644 --- a/lib/superset/request.rb +++ b/lib/superset/request.rb @@ -36,11 +36,6 @@ def query_params [filters, pagination, order_by].join end - def order_by - # by default, we will order by changed_on as per the GUI - ",order_column:changed_on,order_direction:desc" - end - private def route @@ -59,6 +54,13 @@ def pagination "page:#{page_num},page_size:#{PAGE_SIZE}" end + def order_by + # order options are to not be consistant across all objects + # eg changed_on is NOT available on all objects .. requires customization in each ::List class + # + # Example only: ",order_column:changed_on,order_direction:desc" + end + def filters "" end diff --git a/spec/superset/chart/list_spec.rb b/spec/superset/chart/list_spec.rb index 8ad7d0b..f4e99a0 100644 --- a/spec/superset/chart/list_spec.rb +++ b/spec/superset/chart/list_spec.rb @@ -74,12 +74,10 @@ "viz_type"=>"dist_bar" }] end - let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } + let(:default_query_params) { "page:0,page_size:100" } before do allow(subject).to receive(:result).and_return(result) - #allow(subject).to receive(:superset_host).and_return(superset_host) - #allow(subject).to receive(:response).and_return( { 'count': 2 } ) end describe '#rows' do diff --git a/spec/superset/database/list_spec.rb b/spec/superset/database/list_spec.rb index e2de2ea..23ea52c 100644 --- a/spec/superset/database/list_spec.rb +++ b/spec/superset/database/list_spec.rb @@ -20,7 +20,7 @@ ] } end - let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } + let(:default_query_params) { "page:0,page_size:100" } describe '#rows' do before do diff --git a/spec/superset/dataset/list_spec.rb b/spec/superset/dataset/list_spec.rb index a602fc9..e806033 100644 --- a/spec/superset/dataset/list_spec.rb +++ b/spec/superset/dataset/list_spec.rb @@ -43,7 +43,7 @@ ] end - let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } + let(:default_query_params) { "page:0,page_size:100" } before do allow(subject).to receive(:result).and_return(result) diff --git a/spec/superset/request_spec.rb b/spec/superset/request_spec.rb index 4ff1b63..8122c4c 100644 --- a/spec/superset/request_spec.rb +++ b/spec/superset/request_spec.rb @@ -41,7 +41,7 @@ describe '#query_params' do it 'returns the default query params' do - expect(subject.send(:query_params)).to eq("page:0,page_size:100,order_column:changed_on,order_direction:desc") + expect(subject.send(:query_params)).to eq("page:0,page_size:100") end end end diff --git a/spec/superset/security/user/list_spec.rb b/spec/superset/security/user/list_spec.rb index db9133d..a9b9ac2 100644 --- a/spec/superset/security/user/list_spec.rb +++ b/spec/superset/security/user/list_spec.rb @@ -35,7 +35,7 @@ }] end - let(:default_query_params) { "page:0,page_size:100,order_column:changed_on,order_direction:desc" } + let(:default_query_params) { "page:0,page_size:100" } before do allow(subject).to receive(:result).and_return(result) From e74e49ac070e75a72e530edca5c2df8656ad26cc Mon Sep 17 00:00:00 2001 From: jbat Date: Tue, 5 Nov 2024 10:26:37 +1100 Subject: [PATCH 5/9] adjust notifications --- lib/superset/services/dashboard_report.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb index 1c49939..d8a1036 100644 --- a/lib/superset/services/dashboard_report.rb +++ b/lib/superset/services/dashboard_report.rb @@ -47,7 +47,7 @@ def data_sovereignty_issues reasons = [] chart_dataset_ids = dashboard[:datasets][:chart_datasets].map{|d| d[:id]} - # invalid if any filters datasets are not part of the chart datasets + # add WARNING msg if any filters datasets are not part of the chart datasets unknown_datasets = dashboard[:filters][:filter_dataset_ids] - chart_dataset_ids if unknown_datasets.any? reasons << "WARNING: One or more filter datasets is not included in chart datasets for " \ @@ -55,7 +55,7 @@ def data_sovereignty_issues reasons << "DETAILS: #{unknown_dataset_details(unknown_datasets)}" end - # invalid if any filters datasets are not part of the chart datasets + # add ERROR msg if multiple chart dataset schemas are found chart_dataset_schemas = dashboard[:datasets][:chart_datasets].map{|d| d[:schema]}.uniq if chart_dataset_schemas.count > 1 reasons << "ERROR: Multiple distinct chart dataset schemas found. Expected 1. Found #{chart_dataset_schemas.count}. " \ From 9209a211587c9d351373fc52859fb9991218911a Mon Sep 17 00:00:00 2001 From: jbat Date: Tue, 25 Mar 2025 09:32:39 +1100 Subject: [PATCH 6/9] updates report and spec --- lib/superset/services/dashboard_report.rb | 25 ++-- .../services/dashboard_report_spec.rb | 136 ++++++++++++++++++ 2 files changed, 148 insertions(+), 13 deletions(-) create mode 100644 spec/superset/services/dashboard_report_spec.rb diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb index d8a1036..90bb11b 100644 --- a/lib/superset/services/dashboard_report.rb +++ b/lib/superset/services/dashboard_report.rb @@ -15,33 +15,32 @@ class DashboardReport attr_reader :dashboard_ids, :report_on_data_sovereignty_only def initialize(dashboard_ids: [], report_on_data_sovereignty_only: true) - @dashboard_ids = dashboard_ids.presence || all_dashboard_ids + @dashboard_ids = dashboard_ids @report_on_data_sovereignty_only = report_on_data_sovereignty_only end def perform - create_report + create_dashboard_report + load_data_sovereignty_issues report_on_data_sovereignty_only ? display_data_sovereignty_report : @report end - def all_dashboard_ids - Superset::Dashboard::List.new().rows.map{|d| d['id']} - end + private def display_data_sovereignty_report # filter by dashboards where # 1. A filter dataset is not part of the dashboard datasets (might be ok for some cases) - # 2. There is more than one distinct dataset schema (never ok for embedded dashboards) + # 2. There is more than one distinct dataset schema (never ok for embedded dashboards where the expected schema num is only one) puts "Data Sovereignty Report" puts "-----------------------" - puts "Invalid Dashboards: #{data_sovereignty_issues.count}" - data_sovereignty_issues + puts "Possible Invalid Dashboards (CONFIRMATION REQUIRED): #{@data_sovereignty_issues.count}" + @data_sovereignty_issues end # possible data sovereignty issues - def data_sovereignty_issues + def load_data_sovereignty_issues @data_sovereignty_issues ||= begin @report.map do |dashboard| reasons = [] @@ -77,7 +76,7 @@ def unknown_dataset_details(unknown_datasets) end end - def create_report + def create_dashboard_report @report ||= begin dashboard_ids.map do |dashboard_id| dashboard = dashboard_result(dashboard_id) @@ -86,7 +85,7 @@ def create_report dashboard_id: dashboard_id, dashboard_title: dashboard['dashboard_title'], dashboard_url: dashboard['url'], - dashboard_client_tags: dashboard_client_tags(dashboard), + dashboard_tags: dashboard_tags(dashboard), filters: filter_details(dashboard), charts: chart_count(dashboard), datasets: dataset_details(dashboard_id), @@ -124,8 +123,8 @@ def dataset_details(dashboard_id) } end - def dashboard_client_tags(dashboard) - dashboard['tags'].map { |t| t['name'] if t['name'].include?('client') }.compact.join(',') || 'None' + def dashboard_tags(dashboard) + dashboard['tags'].map{|t| t['name']}.join('|') end def dashboard_result(dashboard_id) diff --git a/spec/superset/services/dashboard_report_spec.rb b/spec/superset/services/dashboard_report_spec.rb new file mode 100644 index 0000000..75c1311 --- /dev/null +++ b/spec/superset/services/dashboard_report_spec.rb @@ -0,0 +1,136 @@ +require 'spec_helper' + +RSpec.describe Superset::Services::DashboardReport do + let(:dashboard_ids) { [1, 2] } + let(:service) { described_class.new(dashboard_ids: dashboard_ids) } + + describe '#perform' do + let(:dashboard_response) do + { + 'dashboard_title' => 'Test Dashboard', + 'tags' => [{'name' => 'tag1'}, {'name' => 'tag2'}], + 'json_metadata' => { + 'native_filter_configuration' => [ + { + 'type' => 'NATIVE_FILTER', + 'targets' => [{'datasetId' => 100}] + } + ], + 'chart_configuration' => [{}, {}] # 2 charts + }.to_json + } + end + + let(:datasets_response) do + [ + {id: 1, schema: 'schema1', title: 'Dataset 1'}, + {id: 2, schema: 'schema1', title: 'Dataset 2'} + ] + end + + before do + allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response) + allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') + allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response) + end + + context 'when report_on_data_sovereignty_only is true' do + it 'returns data sovereignty issues only' do + result = service.perform + binding.pry + expect(result).to be_an(Array) + expect(result).to all(include(:reasons, :dashboard)) + end + end + + context 'when report_on_data_sovereignty_only is false' do + let(:service) { described_class.new(dashboard_ids: dashboard_ids, report_on_data_sovereignty_only: false) } + + it 'returns full dashboard report' do + result = service.perform + expect(result).to be_an(Array) + expect(result.first).to include( + :dashboard_id, + :dashboard_title, + :dashboard_url, + :dashboard_tags, + :filters, + :charts, + :datasets + ) + end + end + end + + describe '#data_sovereignty_issues' do + context 'when filter dataset is not in chart datasets' do + let(:dashboard_response) do + { + 'dashboard_title' => 'Test Dashboard', + 'tags' => [], + 'json_metadata' => { + 'native_filter_configuration' => [ + { + 'type' => 'NATIVE_FILTER', + 'targets' => [{'datasetId' => 999}] + } + ], + 'chart_configuration' => [] + }.to_json + } + end + + let(:datasets_response) do + [ + {id: 1, schema: 'schema1', title: 'Dataset 1'} + ] + end + + before do + allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response) + allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') + allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response) + allow_any_instance_of(Superset::Dataset::Get).to receive(:result) + allow_any_instance_of(Superset::Dataset::Get).to receive(:id).and_return(999) + allow_any_instance_of(Superset::Dataset::Get).to receive(:title).and_return('Unknown Dataset') + end + + it 'reports warning for unknown filter datasets' do + result = service.perform + expect(result.first[:reasons]).to include(a_string_matching(/WARNING: One or more filter datasets/)) + end + end + + context 'when multiple schemas are found' do + let(:datasets_response) do + [ + {id: 1, schema: 'schema1', title: 'Dataset 1'}, + {id: 2, schema: 'schema2', title: 'Dataset 2'} + ] + end + + before do + allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return( + { + 'dashboard_title' => 'Test Dashboard', + 'tags' => [], + 'json_metadata' => { + 'native_filter_configuration' => [], + 'chart_configuration' => [{}, {}] + }.to_json + } + ) + allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') + allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response) + end + + it 'reports error for multiple schemas' do + result = service.perform + expect(result.first[:reasons]).to include(a_string_matching(/ERROR: Multiple distinct chart dataset schemas/)) + end + end + end + + + +end From b50cc16cb290ce76809366ce2279863539674c7d Mon Sep 17 00:00:00 2001 From: jbat Date: Thu, 3 Apr 2025 08:47:27 +1100 Subject: [PATCH 7/9] update spec, fix compare bug --- lib/superset/dashboard/compare.rb | 10 ++- lib/superset/dashboard/datasets/list.rb | 61 ++++++++----------- lib/superset/dashboard/list.rb | 2 +- lib/superset/services/dashboard_report.rb | 21 +++---- .../services/dashboard_report_spec.rb | 27 ++++++-- 5 files changed, 65 insertions(+), 56 deletions(-) diff --git a/lib/superset/dashboard/compare.rb b/lib/superset/dashboard/compare.rb index cb7dfe6..cbae1bf 100644 --- a/lib/superset/dashboard/compare.rb +++ b/lib/superset/dashboard/compare.rb @@ -65,7 +65,13 @@ def list_cross_filters def native_filter_configuration(dashboard_result) rows = [] JSON.parse(dashboard_result['json_metadata'])['native_filter_configuration'].each do |filter| - filter['targets'].each {|t| rows << [ t['column']['name'], t['datasetId'] ] } + filter['targets'].each do |t| + if t['column'] + rows << [ filter['name'], t['column']['name'], t['datasetId'] ] + else + rows << [ filter['name'], '>NO DATASET LINKED<', t['datasetId'] ] # some filters don't have a dataset linked, ie date filter + end + end end rows end @@ -73,7 +79,7 @@ def native_filter_configuration(dashboard_result) def list_native_filters_for(dashboard_result) puts Terminal::Table.new( title: [dashboard_result['id'], dashboard_result['dashboard_title']].join(' - '), - headings: ['Filter Name', 'Dataset Id'], + headings: ['Filter Name', 'Dataset Column', 'Dataset Id'], rows: native_filter_configuration(dashboard_result) ) end diff --git a/lib/superset/dashboard/datasets/list.rb b/lib/superset/dashboard/datasets/list.rb index b915d24..a587697 100644 --- a/lib/superset/dashboard/datasets/list.rb +++ b/lib/superset/dashboard/datasets/list.rb @@ -9,8 +9,8 @@ module Datasets class List < Superset::Request attr_reader :id, :include_filter_datasets # id - dashboard id - def self.call(id) - self.new(id).list + def self.call(dashboard_id: id) + self.new(dashboard_id: id).list end def initialize(dashboard_id:, include_filter_datasets: false) @@ -48,6 +48,28 @@ def datasets_details chart_datasets + filter_datasets(filter_dataset_ids_not_used_in_charts) end + def rows + datasets_details.map do |d| + [ + d[:id], + d[:datasource_name], + d[:database][:id], + d[:database][:name], + d[:database][:backend], + d[:schema], + d[:filter_only] + ] + end + end + + def title + @title ||= [id, dashboard.title].join(' ') + end + + def dashboard + @dashboard ||= Superset::Dashboard::Get.new(id) + end + private def filter_dataset_ids @@ -73,40 +95,7 @@ def route def list_attributes ['id', 'datasource_name', 'database_id', 'database_name', 'database_backend', 'schema', 'filter_only'].map(&:to_sym) end - - def rows - datasets_details.map do |d| - [ - d[:id], - d[:datasource_name], - d[:database][:id], - d[:database][:name], - d[:database][:backend], - d[:schema], - d[:filter_only] - ] - end - end - - private - - def route - "dashboard/#{id}/datasets" - end - - def list_attributes - ['id', 'datasource_name', 'database_id', 'database_name', 'database_backend', 'schema'].map(&:to_sym) - end - - # when displaying a list of datasets, show dashboard title as well - def title - @title ||= [id, dashboard.title].join(' ') - end - - def dashboard - @dashboard ||= Superset::Dashboard::Get.new(id) - end end end end -end +end \ No newline at end of file diff --git a/lib/superset/dashboard/list.rb b/lib/superset/dashboard/list.rb index 8aafd5e..9171d33 100644 --- a/lib/superset/dashboard/list.rb +++ b/lib/superset/dashboard/list.rb @@ -7,7 +7,7 @@ module Dashboard class List < Superset::Request attr_reader :title_contains, :title_equals, :tags_equal, :ids_not_in, :include_filter_dataset_schemas - def initialize(page_num: 0, title_contains: '', title_equals: '', tags_equal: [], ids_not_in: [], include_filter_dataset_schemas: false, **extra_args) + def initialize(page_num: 0, title_contains: '', title_equals: '', tags_equal: [], ids_not_in: [], include_filter_dataset_schemas: false) @title_contains = title_contains @title_equals = title_equals @tags_equal = tags_equal diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb index 90bb11b..c4bddd3 100644 --- a/lib/superset/services/dashboard_report.rb +++ b/lib/superset/services/dashboard_report.rb @@ -30,12 +30,12 @@ def perform def display_data_sovereignty_report # filter by dashboards where - # 1. A filter dataset is not part of the dashboard datasets (might be ok for some cases) + # 1. A filter dataset is not part of the dashboard datasets (might be ok for some cases, ie a dummy dataset listing dates only) # 2. There is more than one distinct dataset schema (never ok for embedded dashboards where the expected schema num is only one) puts "Data Sovereignty Report" puts "-----------------------" - puts "Possible Invalid Dashboards (CONFIRMATION REQUIRED): #{@data_sovereignty_issues.count}" + puts "Possible Invalid Dashboards: #{@data_sovereignty_issues.count}" @data_sovereignty_issues end @@ -54,7 +54,7 @@ def load_data_sovereignty_issues reasons << "DETAILS: #{unknown_dataset_details(unknown_datasets)}" end - # add ERROR msg if multiple chart dataset schemas are found + # add ERROR msg if multiple chart dataset schemas are found, ie all datasets should be sourced from the same db schema chart_dataset_schemas = dashboard[:datasets][:chart_datasets].map{|d| d[:schema]}.uniq if chart_dataset_schemas.count > 1 reasons << "ERROR: Multiple distinct chart dataset schemas found. Expected 1. Found #{chart_dataset_schemas.count}. " \ @@ -80,7 +80,6 @@ def create_dashboard_report @report ||= begin dashboard_ids.map do |dashboard_id| dashboard = dashboard_result(dashboard_id) - # binding.pry { dashboard_id: dashboard_id, dashboard_title: dashboard['dashboard_title'], @@ -102,7 +101,7 @@ def filter_details(dashboard) end def filter_count(dashboard) - dashboard['json_metadata']['native_filter_configuration'].count + dashboard['json_metadata']['native_filter_configuration']&.count || 0 end def filter_datasets(dashboard) @@ -116,7 +115,7 @@ def chart_count(dashboard) end def dataset_details(dashboard_id) - datasets = Superset::Dashboard::Datasets::List.new(dashboard_id).rows_hash + datasets = Superset::Dashboard::Datasets::List.new(dashboard_id: dashboard_id).rows_hash { dataset_count: datasets.count, chart_datasets: datasets @@ -128,11 +127,11 @@ def dashboard_tags(dashboard) end def dashboard_result(dashboard_id) - # convert json_metadata within result to a hash - board = Superset::Dashboard::Get.new(dashboard_id) - board.result['json_metadata'] = JSON.parse(board.result['json_metadata']) - board.result['url'] = board.url # add full url to the dashboard result - board.result + # convert json_metadata within result to a hash + board = Superset::Dashboard::Get.new(dashboard_id) + board.result['json_metadata'] = JSON.parse(board.result['json_metadata']) + board.result['url'] = board.url # add full url to the dashboard result + board.result end end end diff --git a/spec/superset/services/dashboard_report_spec.rb b/spec/superset/services/dashboard_report_spec.rb index 75c1311..83c04f9 100644 --- a/spec/superset/services/dashboard_report_spec.rb +++ b/spec/superset/services/dashboard_report_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' RSpec.describe Superset::Services::DashboardReport do - let(:dashboard_ids) { [1, 2] } + let(:dashboard_ids) { [1] } let(:service) { described_class.new(dashboard_ids: dashboard_ids) } describe '#perform' do @@ -16,7 +16,7 @@ 'targets' => [{'datasetId' => 100}] } ], - 'chart_configuration' => [{}, {}] # 2 charts + 'chart_configuration' => [{}, {}] }.to_json } end @@ -29,15 +29,26 @@ end before do +# allow(ENV).to receive(:[]).with('SUPERSET_HOST').and_return('http://example.com/') allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response) allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') - allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response) + allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: 1).and_return( + instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response) + ) + + allow(Superset::Dataset::Get).to receive(:new).with(100).and_return( + instance_double(Superset::Dataset::Get, + result: { 'id' => 100, 'title' => "Dataset 1" }, + id: 100, + title: "Dataset 1" + ) + ) end context 'when report_on_data_sovereignty_only is true' do it 'returns data sovereignty issues only' do result = service.perform - binding.pry + expect(result).to be_an(Array) expect(result).to all(include(:reasons, :dashboard)) end @@ -89,7 +100,9 @@ before do allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response) allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') - allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response) + allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything).and_return( + instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response) + ) allow_any_instance_of(Superset::Dataset::Get).to receive(:result) allow_any_instance_of(Superset::Dataset::Get).to receive(:id).and_return(999) allow_any_instance_of(Superset::Dataset::Get).to receive(:title).and_return('Unknown Dataset') @@ -121,7 +134,9 @@ } ) allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') - allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response) + allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything).and_return( + instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response) + ) end it 'reports error for multiple schemas' do From 2c89619285374f99522f0fc823002841587a25c9 Mon Sep 17 00:00:00 2001 From: jbat Date: Thu, 3 Apr 2025 09:13:59 +1100 Subject: [PATCH 8/9] update report output --- lib/superset/dashboard/compare.rb | 14 +++++++------- lib/superset/request.rb | 1 + lib/superset/services/dashboard_report.rb | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/superset/dashboard/compare.rb b/lib/superset/dashboard/compare.rb index cbae1bf..a257657 100644 --- a/lib/superset/dashboard/compare.rb +++ b/lib/superset/dashboard/compare.rb @@ -36,29 +36,29 @@ def second_dashboard end def list_datasets - puts "\n ====== DASHBOARD DATASETS ====== " + puts "\n >>>>>>>>>>>>>>>>>>>>>>>>> DASHBOARD DATASETS <<<<<<<<<<<<<<<<<<<<<<<<<<<< " Superset::Dashboard::Datasets::List.new(dashboard_id: first_dashboard_id).list + puts "\n" Superset::Dashboard::Datasets::List.new(dashboard_id: second_dashboard_id).list end def list_charts - puts "\n ====== DASHBOARD CHARTS ====== " + puts "\n >>>>>>>>>>>>>>>>>>>>>>>>> DASHBOARD CHARTS <<<<<<<<<<<<<<<<<<<<<<<<<<<< " Superset::Dashboard::Charts::List.new(first_dashboard_id).list - puts '' + puts "\n" Superset::Dashboard::Charts::List.new(second_dashboard_id).list end def list_native_filters - puts "\n ====== DASHBOARD NATIVE FILTERS ====== " + puts "\n >>>>>>>>>>>>>>>>>>>>>>>>> DASHBOARD NATIVE FILTERS <<<<<<<<<<<<<<<<<<<<<<<<<<<< " list_native_filters_for(first_dashboard) - puts '' + puts "\n" list_native_filters_for(second_dashboard) end def list_cross_filters - puts "\n ====== DASHBOARD CROSS FILTERS ====== " + puts "\n >>>>>>>>>>>>>>>>>>>>>>>>> DASHBOARD CROSS FILTERS <<<<<<<<<<<<<<<<<<<<<<<<<<<< " list_cross_filters_for(first_dashboard) - puts '' list_cross_filters_for(second_dashboard) end diff --git a/lib/superset/request.rb b/lib/superset/request.rb index 3f1ee9d..4da4fc6 100644 --- a/lib/superset/request.rb +++ b/lib/superset/request.rb @@ -70,3 +70,4 @@ def logger end end end + diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb index c4bddd3..1171590 100644 --- a/lib/superset/services/dashboard_report.rb +++ b/lib/superset/services/dashboard_report.rb @@ -35,7 +35,7 @@ def display_data_sovereignty_report puts "Data Sovereignty Report" puts "-----------------------" - puts "Possible Invalid Dashboards: #{@data_sovereignty_issues.count}" + puts "Possible Invalid Dashboard Datasets: #{@data_sovereignty_issues.count}" @data_sovereignty_issues end From 8566f2be49fbe8576d67bb9116a6b7ec5af224f5 Mon Sep 17 00:00:00 2001 From: jbat Date: Fri, 4 Apr 2025 14:11:14 +1100 Subject: [PATCH 9/9] update report to includes dashboard dataset filters --- lib/superset/services/dashboard_report.rb | 2 +- spec/superset/services/dashboard_report_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/superset/services/dashboard_report.rb b/lib/superset/services/dashboard_report.rb index 1171590..bdb8fb3 100644 --- a/lib/superset/services/dashboard_report.rb +++ b/lib/superset/services/dashboard_report.rb @@ -115,7 +115,7 @@ def chart_count(dashboard) end def dataset_details(dashboard_id) - datasets = Superset::Dashboard::Datasets::List.new(dashboard_id: dashboard_id).rows_hash + datasets = Superset::Dashboard::Datasets::List.new(dashboard_id: dashboard_id, include_filter_datasets: true).rows_hash { dataset_count: datasets.count, chart_datasets: datasets diff --git a/spec/superset/services/dashboard_report_spec.rb b/spec/superset/services/dashboard_report_spec.rb index 83c04f9..2fc1d18 100644 --- a/spec/superset/services/dashboard_report_spec.rb +++ b/spec/superset/services/dashboard_report_spec.rb @@ -32,7 +32,7 @@ # allow(ENV).to receive(:[]).with('SUPERSET_HOST').and_return('http://example.com/') allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response) allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') - allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: 1).and_return( + allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: 1, include_filter_datasets: true).and_return( instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response) ) @@ -100,7 +100,7 @@ before do allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response) allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') - allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything).and_return( + allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything, include_filter_datasets: true).and_return( instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response) ) allow_any_instance_of(Superset::Dataset::Get).to receive(:result) @@ -134,7 +134,7 @@ } ) allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1') - allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything).and_return( + allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything, include_filter_datasets: true).and_return( instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response) ) end