Merge pull request #17 from rdytech/NEP-17492-validate-and-duplicate-filters

[NEP-17492] Validate and duplicate filters to new dashboard

Author: jbat

Gemfile.lock

@@ -14,7 +14,7 @@ GIT
 PATH
   remote: .
   specs:
-    superset (0.1.1)
+    superset (0.1.2)
       faraday (~> 1.0)
       faraday-multipart (~> 1.0)
       json (~> 2.6)
@@ -144,6 +144,7 @@ GEM
 PLATFORMS
   aarch64-linux
   arm64-darwin-22
+  arm64-darwin-23
 
 DEPENDENCIES
   happi!

lib/superset/chart/update_dataset.rb

@@ -37,8 +37,11 @@ def params_updated
           new_params.merge!("owners": chart.owner_ids )                  # expects an array of user ids
 
           new_params.merge!("params": updated_chart_params.to_json)      # updated to point to the new params
-          new_params.merge!("query_context": updated_chart_query_context.to_json) # update to point to the new query context
-          new_params.merge!("query_context_generation": true)            # new param set to true to regenerate the query context
+          query_context = updated_chart_query_context
+          if query_context
+            new_params.merge!("query_context": query_context.to_json) # update to point to the new query context
+            new_params.merge!("query_context_generation": true)            # new param set to true to regenerate the query context
+          end
 
           new_params
         end

lib/superset/dashboard/put.rb

@@ -0,0 +1,38 @@
+
+# frozen_string_literal: true
+
+module Superset
+  module Dashboard
+    class Put < Superset::Request
+
+      attr_reader :target_dashboard_id, :params
+
+      def initialize(target_dashboard_id:, params:)
+        @target_dashboard_id = target_dashboard_id
+        @params = params
+      end
+
+      def perform
+        raise "Error: target_dashboard_id integer is required" unless target_dashboard_id.present? && target_dashboard_id.is_a?(Integer)
+        raise "Error: params hash is required" unless params.present? && params.is_a?(Hash)
+
+        response
+        Superset::Dashboard::Get.new(id).perform
+      end
+
+      def response
+        @response ||= client.put(route, params)
+      end
+
+      def id
+        response["result"]["id"]
+      end
+
+      private
+
+      def route
+        "dashboard/#{target_dashboard_id}"
+      end
+    end
+  end
+end

lib/superset/services/duplicate_dashboard.rb

@@ -34,6 +34,9 @@ def perform
         # Update the Charts on the New Dashboard with the New Datasets
         update_charts_with_new_datasets
 
+        # Duplicate filters to the new target schema and target database
+        duplicate_source_dashboard_filters
+
         created_embedded_config
 
         end_log_message
@@ -63,7 +66,13 @@ def dataset_duplication_tracker
 
       def update_charts_with_new_datasets
         # get all chart ids for the new dashboard
-        chart_ids_list = Superset::Dashboard::Charts::List.new(new_dashboard.id).chart_ids
+        new_charts_list = Superset::Dashboard::Charts::List.new(new_dashboard.id).result
+        chart_ids_list = new_charts_list&.map { |r| r['id'] }&.compact
+        original_charts = Superset::Dashboard::Charts::List.new(source_dashboard_id).result.map { |r| [r['slice_name'], r['id']] }.to_h
+        new_charts = new_charts_list.map { |r| [r['id'], r['slice_name']] }.to_h
+        json_metadata = new_dashboard.result['json_metadata']
+
+        return unless chart_ids_list.any?
 
         # for each chart, update the charts current dataset_id with the new dataset_id
         chart_ids_list.each do |chart_id|
@@ -75,8 +84,14 @@ def update_charts_with_new_datasets
           new_dataset_id = dataset_duplication_tracker.find { |dataset| dataset[:source_dataset_id] == current_chart_dataset_id }&.fetch(:new_dataset_id, nil)
 
           # update the chart to target the new dataset_id
-          Superset::Chart::UpdateDataset.new(chart_id: chart_id, target_dataset_id: new_dataset_id).response
+          Superset::Chart::UpdateDataset.new(chart_id: chart_id, target_dataset_id: new_dataset_id).perform
+
+          # update json metadata
+          original_chart_id = original_charts[new_charts[chart_id]]
+          json_metadata.gsub!(original_chart_id.to_s, chart_id.to_s)
         end
+
+        Superset::Dashboard::Put.new(target_dashboard_id: new_dashboard.id, params: { "json_metadata" => json_metadata }).perform
       end
 
       def duplicate_source_dashboard_datasets
@@ -88,8 +103,23 @@ def duplicate_source_dashboard_datasets
           dataset_duplication_tracker <<  { source_dataset_id: dataset[:id], new_dataset_id: new_dataset_id }
 
           # update the new dataset with the target schema and target database
-          Superset::Dataset::UpdateSchema.new(source_dataset_id: new_dataset_id, target_database_id: target_database_id, target_schema: target_schema).response
+          Superset::Dataset::UpdateSchema.new(source_dataset_id: new_dataset_id, target_database_id: target_database_id, target_schema: target_schema).perform
+        end
+      end
+
+      def duplicate_source_dashboard_filters
+        return unless source_dashboard_filters.length.positive?
+
+        json_metadata = Superset::Dashboard::Get.new(new_dashboard.id).json_metadata
+        configuration = json_metadata['native_filter_configuration']&.map do |filter_config|
+          targets = filter_config['targets']
+          target_filter_dataset_id = dataset_duplication_tracker.find { |d| d[:source_dataset_id] == targets.first["datasetId"] }&.fetch(:new_dataset_id, nil)
+          filter_config['targets'] = [targets.first.merge({ "datasetId"=> target_filter_dataset_id })]
+          filter_config
         end
+
+        json_metadata['native_filter_configuration'] = configuration || []
+        Superset::Dashboard::Put.new(target_dashboard_id: new_dashboard.id, params: { "json_metadata" => json_metadata.to_json }).perform
       end
 
       def new_dashboard
@@ -125,10 +155,14 @@ def validate_params
         # schema validations
         raise ValidationError, "Schema #{target_schema} does not exist in target database: #{target_database_id}" unless target_database_available_schemas.include?(target_schema)
         raise ValidationError, "The source_dashboard_id #{source_dashboard_id} datasets are required to point to one schema only. Actual schema list is #{source_dashboard_schemas.join(',')}" if source_dashboard_has_more_than_one_schema?
+        raise ValidationError, "The source_dashboard_id #{source_dashboard_id} filters point to more than one schema." if any_unpermitted_filters?
 
         # new dataset validations
         raise ValidationError, "DATASET NAME CONFLICT: The Target Schema #{target_schema} already has existing datasets named: #{target_schema_matching_dataset_names.join(',')}" unless target_schema_matching_dataset_names.empty?
+      end
 
+      def source_dashboard
+        @source_dashboard ||= Superset::Dashboard::Get.new(source_dashboard_id)
       end
 
       def target_database_available_schemas
@@ -162,6 +196,27 @@ def target_schema_matching_dataset_names
         end.flatten.compact
       end
 
+      def allowed_filters(dashboard_id)
+        Superset::Dashboard::Datasets::List.new(dashboard_id).result
+      end
+
+      def source_allowed_filters
+        @source_allowed_filters ||= Superset::Dashboard::Datasets::List.new(source_dashboard_id).result
+      end
+
+      def source_dashboard_filters
+        filters_configuration = JSON.parse(source_dashboard.result['json_metadata'])['native_filter_configuration'] || []
+        return Array.new unless filters_configuration && filters_configuration.any?
+
+        filters = filters_configuration.map { |c| c['targets'] }.flatten.compact
+      end
+
+      def any_unpermitted_filters?
+        source_allowed_filter_ids = source_allowed_filters.map { |r| r["id"] }.compact.uniq
+        source_dashboard_filter_ids = source_dashboard_filters.map { |t| t["datasetId"] }.compact.uniq
+        (source_dashboard_filter_ids - source_allowed_filter_ids).any?
+      end
+
       def logger
         @logger ||= Superset::Logger.new
       end
@@ -179,4 +234,4 @@ def end_log_message
       end
     end
   end
-end
+end

spec/superset/dashboard/put_spec.rb

@@ -0,0 +1,68 @@
+require 'spec_helper'
+
+RSpec.describe Superset::Dashboard::Put do
+  subject { described_class.new(target_dashboard_id: dashboard_id, params: params) }
+  let(:dashboard_id) { 1 }
+  let(:params) {{
+    "json_metadata" => { 
+      "key1" => "value1", 
+      "positions" => { "key2" => "value2" }
+    }.to_json 
+  }}
+
+  let(:new_dashboard_id) { 2 }
+  let(:new_dashboard_instance) { instance_double("Superset::Dashboard::Get") }
+
+  before do
+    allow(subject).to receive(:response).and_return( { "result" => 
+      {"id"=>new_dashboard_id, "last_modified_time"=>1708484547.0} }
+    )
+  end
+
+  describe 'perform' do
+    context 'with valid params' do
+      before do
+        allow(Superset::Dashboard::Get).to receive(:new).with(new_dashboard_id).and_return(new_dashboard_instance)
+        allow(new_dashboard_instance).to receive(:perform).and_return(new_dashboard_instance)
+      end
+
+      it 'returns the new dashboard object' do
+        expect(subject.perform).to be new_dashboard_instance
+      end
+    end
+
+    context 'with invalid params' do
+      context 'when source_dashboard_id is not an integer' do
+        let(:dashboard_id) { 'q' }
+        
+        it 'raises an error' do
+          expect { subject.perform }.to raise_error("Error: target_dashboard_id integer is required")
+        end
+      end
+
+      context 'when source_dashboard_id is not present' do
+        let(:dashboard_id) { nil }
+        
+        it 'raises an error' do
+          expect { subject.perform }.to raise_error("Error: target_dashboard_id integer is required")
+        end
+      end
+
+      context 'when params is not a hash' do
+        let(:params) { 'q' }
+        
+        it 'raises an error' do
+          expect { subject.perform }.to raise_error("Error: params hash is required")
+        end
+      end
+
+      context 'when params is nil' do
+        let(:params) { nil }
+        
+        it 'raises an error' do
+          expect { subject.perform }.to raise_error("Error: params hash is required")
+        end
+      end
+    end
+  end
+end

spec/superset/services/duplicate_dashboard_spec.rb

@@ -11,16 +11,20 @@
   let(:target_schema) { 'schema_one' }
   let(:target_database_id) { 6 }
   let(:target_database_available_schemas) { ['schema_one', 'schema_two', 'schema_three'] }
-
   let(:source_dataset_1) { 101 }
   let(:source_dataset_2) { 102 }
   let(:source_dashboard_datasets) {[
     {id: source_dataset_1, datasource_name: "Dataset 1", schema: "schema1", database: {id: 9, name: "db_9", backend: "postgresql"}},
     {id: source_dataset_2, datasource_name: "Dataset 2", schema: "schema1", database: {id: 9, name: "db_9", backend: "postgresql"}}
   ]}
+  let(:json_metadata) { { 'native_filter_configuration' => [{ 'targets' => [{ 'datasetId' => 201 }]}]} }
+  let(:new_json_metadata) { { 'native_filter_configuration' => [{ 'targets' => [{ 'datasetId' => 201 }]}]} }
+  let(:source_allowed_filters) { [{'id' => 201}] }
+  let(:source_dashboard_filters) {[{ 'datasetId' => 201 }]}
+  let(:dataset_duplication_tracker) { [{ source_dataset_id: 201, new_dataset_id: 201 }] }
 
   let(:new_dashboard_id) { 2 }
-  let(:new_dashboard) { double('new_dashboard', id: new_dashboard_id, url: "http://superset-host.com/superset/dashboard/#{new_dashboard_id}") }
+  let(:new_dashboard) { double('new_dashboard', id: new_dashboard_id, url: "http://superset-host.com/superset/dashboard/#{new_dashboard_id}", json_metadata: json_metadata) }
 
   let(:new_dataset_1) { 201 }
   let(:new_dataset_2) { 202 }
@@ -35,30 +39,40 @@
     allow(subject).to receive(:new_dashboard).and_return(new_dashboard)
     allow(subject).to receive(:source_dashboard_datasets).and_return(source_dashboard_datasets)
     allow(subject).to receive(:target_schema_matching_dataset_names).and_return(existing_target_datasets_list)
+    allow(subject).to receive(:source_allowed_filters).and_return(source_allowed_filters)
+    allow(subject).to receive(:source_dashboard_filters).and_return(source_dashboard_filters)
+    allow(subject).to receive(:dataset_duplication_tracker).and_return(dataset_duplication_tracker)
+    allow(new_dashboard).to receive(:result).and_return({ 'json_metadata' => json_metadata.to_json })
   end
 
   describe '#perform' do
     context 'with valid params' do
-
       before do
         # duplicating the current datasets
         expect(Superset::Dataset::Duplicate).to receive(:new).with(source_dataset_id: source_dataset_1, new_dataset_name: "Dataset 1 (COPY)").and_return(double(perform: new_dataset_1))
         expect(Superset::Dataset::Duplicate).to receive(:new).with(source_dataset_id: source_dataset_2, new_dataset_name: "Dataset 2 (COPY)").and_return(double(perform: new_dataset_2))
 
         # updating the new datasets to point to the target schema and target database
-        expect(Superset::Dataset::UpdateSchema).to receive(:new).with(source_dataset_id: new_dataset_1, target_database_id: target_database_id, target_schema: target_schema).and_return(double(response: true))
-        expect(Superset::Dataset::UpdateSchema).to receive(:new).with(source_dataset_id: new_dataset_2, target_database_id: target_database_id, target_schema: target_schema).and_return(double(response: true))
+        expect(Superset::Dataset::UpdateSchema).to receive(:new).with(source_dataset_id: new_dataset_1, target_database_id: target_database_id, target_schema: target_schema).and_return(double(perform: new_dataset_1))
+        expect(Superset::Dataset::UpdateSchema).to receive(:new).with(source_dataset_id: new_dataset_2, target_database_id: target_database_id, target_schema: target_schema).and_return(double(perform: new_dataset_2))
 
         # getting the list of charts for the source dashboard
-        allow(Superset::Dashboard::Charts::List).to receive(:new).with(new_dashboard_id).and_return(double(chart_ids: [new_chart_1, new_chart_2]))
+        allow(Superset::Dashboard::Charts::List).to receive(:new).with(source_dashboard_id).and_return(double(result: [{ 'slice_name' => "test", "id" => 3001}, { 'slice_name' => "test", "id" => 3002}], chart_ids: [new_chart_1, new_chart_2]))
+        allow(Superset::Dashboard::Charts::List).to receive(:new).with(new_dashboard_id).and_return(double(result: [{ 'slice_name' => "test", "id" => 3001}, { 'slice_name' => "test", "id" => 3002}]))
 
         # getting the current dataset_id for the new charts .. still pointing to the old datasets
         expect(Superset::Chart::Get).to receive(:new).with(3001).and_return(double(datasource_id: source_dataset_1))
         expect(Superset::Chart::Get).to receive(:new).with(3002).and_return(double(datasource_id: source_dataset_2))
 
         # updating the new charts to point to the new datasets
-        expect(Superset::Chart::UpdateDataset).to receive(:new).with(chart_id: new_chart_1, target_dataset_id: new_dataset_1).and_return(double(response: true))
-        expect(Superset::Chart::UpdateDataset).to receive(:new).with(chart_id: new_chart_2, target_dataset_id: new_dataset_2).and_return(double(response: true))
+        expect(Superset::Chart::UpdateDataset).to receive(:new).with(chart_id: new_chart_1, target_dataset_id: new_dataset_1).and_return(double(perform: true))
+        expect(Superset::Chart::UpdateDataset).to receive(:new).with(chart_id: new_chart_2, target_dataset_id: new_dataset_2).and_return(double(perform: true))
+
+        # get json metadata
+        expect(Superset::Dashboard::Get).to receive(:new).with(new_dashboard_id).and_return(double(json_metadata: json_metadata))
+
+        # update dashboard json metadata
+        expect(Superset::Dashboard::Put).to receive(:new).twice.with(target_dashboard_id: new_dashboard_id, params: { 'json_metadata' => new_json_metadata.to_json }).and_return(double(perform: true))
       end
 
       specify do
@@ -99,6 +113,15 @@
         end
       end
 
+      context 'filters set is outside source schema' do
+        let(:target_schema) { 'schema_one' }
+        let(:source_dashboard_filters) { [{ 'datasetId' => 201 }, { 'datasetId' => 203 }] }
+
+        specify do
+          expect { subject.perform }.to raise_error(Superset::Request::ValidationError, "The source_dashboard_id #{source_dashboard_id} filters point to more than one schema.")
+        end
+      end
+
       context 'source dashboard datasets use multiple schemas' do
         before do
           allow(subject).to receive(:source_dashboard_schemas).and_return(['schema_one', 'schema_five'])