Skip to content

Commit b50cc16

Browse files
jbatjbat
committed
update spec, fix compare bug
1 parent 9209a21 commit b50cc16

File tree

5 files changed

+65
-56
lines changed

5 files changed

+65
-56
lines changed

‎lib/superset/dashboard/compare.rb‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,21 @@ def list_cross_filters
6565
def native_filter_configuration(dashboard_result)
6666
rows = []
6767
JSON.parse(dashboard_result['json_metadata'])['native_filter_configuration'].each do |filter|
68-
filter['targets'].each {|t| rows << [ t['column']['name'], t['datasetId'] ] }
68+
filter['targets'].each do |t|
69+
if t['column']
70+
rows << [ filter['name'], t['column']['name'], t['datasetId'] ]
71+
else
72+
rows << [ filter['name'], '>NO DATASET LINKED<', t['datasetId'] ] # some filters don't have a dataset linked, ie date filter
73+
end
74+
end
6975
end
7076
rows
7177
end
7278

7379
def list_native_filters_for(dashboard_result)
7480
puts Terminal::Table.new(
7581
title: [dashboard_result['id'], dashboard_result['dashboard_title']].join(' - '),
76-
headings: ['Filter Name', 'Dataset Id'],
82+
headings: ['Filter Name', 'Dataset Column', 'Dataset Id'],
7783
rows: native_filter_configuration(dashboard_result)
7884
)
7985
end

‎lib/superset/dashboard/datasets/list.rb‎

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ module Datasets
99
class List < Superset::Request
1010
attr_reader :id, :include_filter_datasets # id - dashboard id
1111

12-
def self.call(id)
13-
self.new(id).list
12+
def self.call(dashboard_id: id)
13+
self.new(dashboard_id: id).list
1414
end
1515

1616
def initialize(dashboard_id:, include_filter_datasets: false)
@@ -48,6 +48,28 @@ def datasets_details
4848
chart_datasets + filter_datasets(filter_dataset_ids_not_used_in_charts)
4949
end
5050

51+
def rows
52+
datasets_details.map do |d|
53+
[
54+
d[:id],
55+
d[:datasource_name],
56+
d[:database][:id],
57+
d[:database][:name],
58+
d[:database][:backend],
59+
d[:schema],
60+
d[:filter_only]
61+
]
62+
end
63+
end
64+
65+
def title
66+
@title ||= [id, dashboard.title].join(' ')
67+
end
68+
69+
def dashboard
70+
@dashboard ||= Superset::Dashboard::Get.new(id)
71+
end
72+
5173
private
5274

5375
def filter_dataset_ids
@@ -73,40 +95,7 @@ def route
7395
def list_attributes
7496
['id', 'datasource_name', 'database_id', 'database_name', 'database_backend', 'schema', 'filter_only'].map(&:to_sym)
7597
end
76-
77-
def rows
78-
datasets_details.map do |d|
79-
[
80-
d[:id],
81-
d[:datasource_name],
82-
d[:database][:id],
83-
d[:database][:name],
84-
d[:database][:backend],
85-
d[:schema],
86-
d[:filter_only]
87-
]
88-
end
89-
end
90-
91-
private
92-
93-
def route
94-
"dashboard/#{id}/datasets"
95-
end
96-
97-
def list_attributes
98-
['id', 'datasource_name', 'database_id', 'database_name', 'database_backend', 'schema'].map(&:to_sym)
99-
end
100-
101-
# when displaying a list of datasets, show dashboard title as well
102-
def title
103-
@title ||= [id, dashboard.title].join(' ')
104-
end
105-
106-
def dashboard
107-
@dashboard ||= Superset::Dashboard::Get.new(id)
108-
end
10998
end
11099
end
111100
end
112-
end
101+
end

‎lib/superset/dashboard/list.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ module Dashboard
77
class List < Superset::Request
88
attr_reader :title_contains, :title_equals, :tags_equal, :ids_not_in, :include_filter_dataset_schemas
99

10-
def initialize(page_num: 0, title_contains: '', title_equals: '', tags_equal: [], ids_not_in: [], include_filter_dataset_schemas: false, **extra_args)
10+
def initialize(page_num: 0, title_contains: '', title_equals: '', tags_equal: [], ids_not_in: [], include_filter_dataset_schemas: false)
1111
@title_contains = title_contains
1212
@title_equals = title_equals
1313
@tags_equal = tags_equal

‎lib/superset/services/dashboard_report.rb‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ def perform
3030

3131
def display_data_sovereignty_report
3232
# filter by dashboards where
33-
# 1. A filter dataset is not part of the dashboard datasets (might be ok for some cases)
33+
# 1. A filter dataset is not part of the dashboard datasets (might be ok for some cases, ie a dummy dataset listing dates only)
3434
# 2. There is more than one distinct dataset schema (never ok for embedded dashboards where the expected schema num is only one)
3535

3636
puts "Data Sovereignty Report"
3737
puts "-----------------------"
38-
puts "Possible Invalid Dashboards (CONFIRMATION REQUIRED): #{@data_sovereignty_issues.count}"
38+
puts "Possible Invalid Dashboards: #{@data_sovereignty_issues.count}"
3939
@data_sovereignty_issues
4040
end
4141

@@ -54,7 +54,7 @@ def load_data_sovereignty_issues
5454
reasons << "DETAILS: #{unknown_dataset_details(unknown_datasets)}"
5555
end
5656

57-
# add ERROR msg if multiple chart dataset schemas are found
57+
# add ERROR msg if multiple chart dataset schemas are found, ie all datasets should be sourced from the same db schema
5858
chart_dataset_schemas = dashboard[:datasets][:chart_datasets].map{|d| d[:schema]}.uniq
5959
if chart_dataset_schemas.count > 1
6060
reasons << "ERROR: Multiple distinct chart dataset schemas found. Expected 1. Found #{chart_dataset_schemas.count}. " \
@@ -80,7 +80,6 @@ def create_dashboard_report
8080
@report ||= begin
8181
dashboard_ids.map do |dashboard_id|
8282
dashboard = dashboard_result(dashboard_id)
83-
# binding.pry
8483
{
8584
dashboard_id: dashboard_id,
8685
dashboard_title: dashboard['dashboard_title'],
@@ -102,7 +101,7 @@ def filter_details(dashboard)
102101
end
103102

104103
def filter_count(dashboard)
105-
dashboard['json_metadata']['native_filter_configuration'].count
104+
dashboard['json_metadata']['native_filter_configuration']&.count || 0
106105
end
107106

108107
def filter_datasets(dashboard)
@@ -116,7 +115,7 @@ def chart_count(dashboard)
116115
end
117116

118117
def dataset_details(dashboard_id)
119-
datasets = Superset::Dashboard::Datasets::List.new(dashboard_id).rows_hash
118+
datasets = Superset::Dashboard::Datasets::List.new(dashboard_id: dashboard_id).rows_hash
120119
{
121120
dataset_count: datasets.count,
122121
chart_datasets: datasets
@@ -128,11 +127,11 @@ def dashboard_tags(dashboard)
128127
end
129128

130129
def dashboard_result(dashboard_id)
131-
# convert json_metadata within result to a hash
132-
board = Superset::Dashboard::Get.new(dashboard_id)
133-
board.result['json_metadata'] = JSON.parse(board.result['json_metadata'])
134-
board.result['url'] = board.url # add full url to the dashboard result
135-
board.result
130+
# convert json_metadata within result to a hash
131+
board = Superset::Dashboard::Get.new(dashboard_id)
132+
board.result['json_metadata'] = JSON.parse(board.result['json_metadata'])
133+
board.result['url'] = board.url # add full url to the dashboard result
134+
board.result
136135
end
137136
end
138137
end

‎spec/superset/services/dashboard_report_spec.rb‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
require 'spec_helper'
22

33
RSpec.describe Superset::Services::DashboardReport do
4-
let(:dashboard_ids) { [1, 2] }
4+
let(:dashboard_ids) { [1] }
55
let(:service) { described_class.new(dashboard_ids: dashboard_ids) }
66

77
describe '#perform' do
@@ -16,7 +16,7 @@
1616
'targets' => [{'datasetId' => 100}]
1717
}
1818
],
19-
'chart_configuration' => [{}, {}] # 2 charts
19+
'chart_configuration' => [{}, {}]
2020
}.to_json
2121
}
2222
end
@@ -29,15 +29,26 @@
2929
end
3030

3131
before do
32+
# allow(ENV).to receive(:[]).with('SUPERSET_HOST').and_return('http://example.com/')
3233
allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response)
3334
allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1')
34-
allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response)
35+
allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: 1).and_return(
36+
instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response)
37+
)
38+
39+
allow(Superset::Dataset::Get).to receive(:new).with(100).and_return(
40+
instance_double(Superset::Dataset::Get,
41+
result: { 'id' => 100, 'title' => "Dataset 1" },
42+
id: 100,
43+
title: "Dataset 1"
44+
)
45+
)
3546
end
3647

3748
context 'when report_on_data_sovereignty_only is true' do
3849
it 'returns data sovereignty issues only' do
3950
result = service.perform
40-
binding.pry
51+
4152
expect(result).to be_an(Array)
4253
expect(result).to all(include(:reasons, :dashboard))
4354
end
@@ -89,7 +100,9 @@
89100
before do
90101
allow_any_instance_of(Superset::Dashboard::Get).to receive(:result).and_return(dashboard_response)
91102
allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1')
92-
allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response)
103+
allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything).and_return(
104+
instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response)
105+
)
93106
allow_any_instance_of(Superset::Dataset::Get).to receive(:result)
94107
allow_any_instance_of(Superset::Dataset::Get).to receive(:id).and_return(999)
95108
allow_any_instance_of(Superset::Dataset::Get).to receive(:title).and_return('Unknown Dataset')
@@ -121,7 +134,9 @@
121134
}
122135
)
123136
allow_any_instance_of(Superset::Dashboard::Get).to receive(:url).and_return('http://example.com/dashboard/1')
124-
allow_any_instance_of(Superset::Dashboard::Datasets::List).to receive(:rows_hash).and_return(datasets_response)
137+
allow(Superset::Dashboard::Datasets::List).to receive(:new).with(dashboard_id: anything).and_return(
138+
instance_double(Superset::Dashboard::Datasets::List, rows_hash: datasets_response)
139+
)
125140
end
126141

127142
it 'reports error for multiple schemas' do

0 commit comments

Comments
 (0)