Merge pull request #14 from rdytech/NEP-17493-save-embedded-domain-in-duplicate-process

[NEP-17493] duplicate dashboard should also create embedded setting

Author: jbat

lib/superset/dashboard/embedded/put.rb

@@ -2,19 +2,22 @@ module Superset
   module Dashboard
     module Embedded
       class Put < Superset::Request
-        attr_reader :dashboard_id, :embedded_domain
+        attr_reader :dashboard_id, :allowed_domains
 
-        def initialize(dashboard_id: , embedded_domain: )
+        def initialize(dashboard_id: , allowed_domains: )
           @dashboard_id = dashboard_id
-          @embedded_domain = embedded_domain
+          @allowed_domains = allowed_domains
         end
 
         def response
+          raise  InvalidParameterError, 'dashboard_id integer is required' if dashboard_id.nil? || dashboard_id.class != Integer
+          raise  InvalidParameterError, 'allowed_domains array is required' if allowed_domains.nil? || allowed_domains.class != Array
+
           @response ||= client.put(route, params)
         end
 
         def params
-          { "allowed_domains": [embedded_domain] }
+          { "allowed_domains": allowed_domains }
         end
 
         def uuid

lib/superset/dataset/duplicate.rb

@@ -15,9 +15,9 @@ def initialize(source_dataset_id: :source_dataset_id, new_dataset_name: :new_dat
       def perform
         raise "Error: source_dataset_id integer is required" unless source_dataset_id.present? && source_dataset_id.is_a?(Integer)
         raise "Error: new_dataset_name string is required" unless new_dataset_name.present? && new_dataset_name.is_a?(String)
-        raise "Error: new_dataset_name already in use" if new_dataset_name_already_in_use?
+        raise "Error: new_dataset_name already in use in this schema: #{new_dataset_name}. Suggest you add (COPY) as a suffix to the name" if new_dataset_name_already_in_use?
 
-        logger.info("  Start Duplicate Source Dataset Id: #{source_dataset_id} to New Dataset Name: #{new_dataset_name}")
+        logger.info("  Duplicating Source Dataset in schema #{source_dataset.schema}. Source Dataset Id: #{source_dataset_id} to New Dataset Name: #{new_dataset_name}")
 
         new_dataset_id
       end
@@ -34,10 +34,14 @@ def params
       end
 
       private
-      
+
+      def source_dataset
+        @source_dataset ||= Dataset::Get.new(source_dataset_id).perform
+      end
+
+      # The API demands that the new_dataset_name be uniq within the schema it points to.
       def new_dataset_name_already_in_use?
-        existing_datasets = List.new(title_equals: new_dataset_name)
-        existing_datasets.result.any?
+        Dataset::List.new(title_equals: new_dataset_name, schema_equals: source_dataset.schema).result.any?
       end
 
       def new_dataset_id

lib/superset/dataset/get.rb

@@ -12,10 +12,19 @@ def self.call(id)
         self.new(id).list
       end
 
+      def perform
+        response
+        self
+      end
+
       def rows
         [ [title, schema, database_name, database_id] ]
       end
 
+      def schema
+        result['schema']
+      end
+
       private
 
       def route
@@ -39,9 +48,6 @@ def title
         result['name']
       end
 
-      def schema
-        result['schema']
-      end
 
       def sql
         ['sql']

lib/superset/dataset/list.rb

@@ -1,11 +1,12 @@
 module Superset
   module Dataset
     class List < Superset::Request
-      attr_reader :title_contains, :title_equals
+      attr_reader :title_contains, :title_equals, :schema_equals
 
-      def initialize(page_num: 0, title_contains: '', title_equals: '')
+      def initialize(page_num: 0, title_contains: '', title_equals: '', schema_equals: '')
         @title_contains = title_contains
         @title_equals = title_equals
+        @schema_equals = schema_equals
         super(page_num: page_num)
       end
 
@@ -20,10 +21,14 @@ def route
       end
 
       def filters
-        raise 'ERROR: only one filter supported currently' if  title_contains.present? && title_equals.present?
-
-        return "filters:!((col:table_name,opr:ct,value:'#{title_contains}'))," if title_contains.present?
-        return "filters:!((col:table_name,opr:eq,value:'#{title_equals}'))," if title_equals.present?
+        # TODO filtering across all list classes can be refactored to support multiple options in a more flexible way
+        filters = []
+        filters << "(col:table_name,opr:ct,value:'#{title_contains}')" if title_contains.present?
+        filters << "(col:table_name,opr:eq,value:'#{title_equals}')" if title_equals.present?
+        filters << "(col:schema,opr:eq,value:'#{schema_equals}')" if schema_equals.present?
+        unless filters.empty?
+          "filters:!(" + filters.join(',') + "),"
+        end
       end
 
       def list_attributes

lib/superset/dataset/update_schema.rb

@@ -2,12 +2,13 @@ module Superset
   module Dataset
     class UpdateSchema < Superset::Request
 
-      attr_reader :source_dataset_id, :target_database_id, :target_schema
+      attr_reader :source_dataset_id, :target_database_id, :target_schema, :remove_copy_suffix
 
-      def initialize(source_dataset_id: :source_dataset_id, target_database_id: :target_database_id, target_schema: :target_schema)
+      def initialize(source_dataset_id: :source_dataset_id, target_database_id: :target_database_id, target_schema: :target_schema, remove_copy_suffix: false)
         @source_dataset_id = source_dataset_id
         @target_database_id = target_database_id
         @target_schema = target_schema
+        @remove_copy_suffix = remove_copy_suffix
       end
 
       def perform
@@ -35,6 +36,7 @@ def params_updated
           new_params.merge!("database_id": target_database_id)  # add the target database id
           new_params['schema'] = target_schema
           new_params['owners'] = new_params['owners'].map {|o| o['id'] } # expects an array of user ids
+          new_params['table_name'] = new_params['table_name'].gsub(/ \(COPY\)/, '') if remove_copy_suffix
 
           # remove unwanted fields from metrics and columns arrays
           new_params['metrics'].each {|m| m.delete('changed_on') }

lib/superset/services/duplicate_dashboard.rb

@@ -7,30 +7,35 @@ module Superset
   module Services
     class DuplicateDashboard < Superset::Request
 
-      attr_reader :source_dashboard_id, :target_schema, :target_database_id
+      DUPLICATED_DATASET_SUFFIX = ' (COPY)'
 
-      def initialize(source_dashboard_id:, target_schema:, target_database_id: )
+      attr_reader :source_dashboard_id, :target_schema, :target_database_id, :allowed_domains
+
+      def initialize(source_dashboard_id:, target_schema:, target_database_id: , allowed_domains: nil)
         @source_dashboard_id = source_dashboard_id
         @target_schema = target_schema
         @target_database_id = target_database_id
+        @allowed_domains = allowed_domains
       end
 
       def perform
         validate_params
 
+        # Pull the Datasets for all charts on the source dashboard
+        source_dashboard_datasets
+
         # create a new_dashboard by copying the source_dashboard using with 'duplicate_slices: true' to get a new set of charts.
+        # The new_dashboard will have a copy of charts from the source_dashboard, but with the same datasets as the source_dashboard
         new_dashboard
 
-        # Pull the Datasets for all charts on the source dashboard  
-        # currently the new_dashboard charts(slices) all point to these same datasets from the orig source dashboard 
-        source_dashboard_datasets
-
         # Duplicate these Datasets to the new target schema and target database
         duplicate_source_dashboard_datasets
 
         # Update the Charts on the New Dashboard with the New Datasets
         update_charts_with_new_datasets
 
+        created_embedded_config
+
         end_log_message
 
         # return the new dashboard id and url
@@ -41,7 +46,16 @@ def perform
         raise e
       end
 
-      # private
+      private
+
+      def created_embedded_config
+        return unless allowed_domains.present?
+
+        result = Dashboard::Embedded::Put.new(dashboard_id: new_dashboard.id, allowed_domains: allowed_domains).result
+        logger.info "  Embedded Domain Added to New Dashboard #{new_dashboard.id}:"
+        logger.info "  Embedded Domain allowed_domains: #{result['allowed_domains']}"
+        logger.info "  Embedded Domain uuid: #{result['uuid']}"
+      end
 
       def dataset_duplication_tracker
         @dataset_duplication_tracker ||= []
@@ -68,7 +82,7 @@ def update_charts_with_new_datasets
       def duplicate_source_dashboard_datasets
         source_dashboard_datasets.each do |dataset|
           # duplicate the dataset
-          new_dataset_id = Superset::Dataset::Duplicate.new(source_dataset_id: dataset[:id], new_dataset_name: "#{dataset[:datasource_name]} #{target_schema} DUPLICATION").perform
+          new_dataset_id = Superset::Dataset::Duplicate.new(source_dataset_id: dataset[:id], new_dataset_name: "#{dataset[:datasource_name]}#{DUPLICATED_DATASET_SUFFIX}").perform
 
           # keep track of the previous dataset and the matching new dataset_id
           dataset_duplication_tracker <<  { source_dataset_id: dataset[:id], new_dataset_id: new_dataset_id }
@@ -111,6 +125,10 @@ 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?
+ 
+        # 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 target_database_available_schemas
@@ -121,10 +139,29 @@ def source_dashboard_has_more_than_one_schema?
         source_dashboard_schemas.count > 1
       end
 
+      # Pull the Datasets for all charts on the source dashboard
       def source_dashboard_schemas
         source_dashboard_datasets.map { |dataset| dataset[:schema] }.uniq
       end
 
+      def source_dashboard_dataset_names
+        source_dashboard_datasets.map { |dataset| dataset[:datasource_name] }.uniq
+      end
+
+      # identify any already existing datasets in the target schema that have the same name as the source dashboard datasets
+      # note this is prior to adding the (COPY) suffix
+      # here we will need to decide if we want to use the existing dataset or not see NEP-????
+      # for now we will exit with an error if we find any existing datasets of the same name
+      def target_schema_matching_dataset_names
+        source_dashboard_dataset_names.map do |source_dataset_name|
+          existing_names = Superset::Dataset::List.new(title_contains: source_dataset_name, schema_equals: target_schema).result.map{|t|t['table_name']}.uniq # contains match to cover with suffix as well
+          unless existing_names.flatten.empty?
+            logger.error "  HALTING PROCESS: Schema #{target_schema} already has Dataset called #{existing_names}"
+          end
+          existing_names
+        end.flatten.compact
+      end
+
       def logger
         @logger ||= Superset::Logger.new
       end

spec/superset/chart/update_dataset_spec.rb

@@ -19,7 +19,6 @@
       "id"=>chart_id,
       "is_managed_externally"=>false,
       "owners"=>[{"first_name"=>"Jay", "id"=>9, "last_name"=>"Bee"}],
-      "query_context"=>nil,
       "slice_name"=>"JRStg DoB per Year",
       "tags"=>[{"id"=>1, "name"=>"owner:9", "type"=>3}, {"id"=>28, "name"=>"type:chart", "type"=>2}],
       "thumbnail_url"=>"/api/v1/chart/54507/thumbnail/1595a10937091faff0aed5df628a1292/",

spec/superset/dashboard/embedded/put_spec.rb

@@ -1,17 +1,17 @@
 require 'spec_helper'
 
 RSpec.describe Superset::Dashboard::Embedded::Put, type: :service do
-  subject { described_class.new(dashboard_id: dashboard_id, embedded_domain: allowed_domain) }
+  subject { described_class.new(dashboard_id: dashboard_id, allowed_domains: allowed_domains) }
   let(:dashboard_id) { 1 }
 
   describe 'with a dashboard that has embedded settings, ie has a result' do
-    let(:allowed_domain) { ['http://test-domain.io/'] }
+    let(:allowed_domains) { ['http://test-domain.io/'] }
     let(:uuid) { '631bxxxx-xxxx-xxxx-xxxx-xxxxxxxxx247' }
     let(:response) do
       {
         'result' =>
           {
-            "allowed_domains" => allowed_domain,
+            "allowed_domains" => allowed_domains,
             "changed_by"=>{"first_name"=>"Jay", "id"=>9, "last_name"=>"Bee", "username"=>"4bf....3f5"},
             "changed_on"      => "2023-10-30T03:06:51.437527",
             "dashboard_id"    => "1",
@@ -20,13 +20,53 @@
       }.with_indifferent_access
     end
 
-    before do
-      allow(subject).to receive(:response).and_return(response)
+    context 'with params' do
+      before { allow(subject).to receive(:response).and_return(response) }
+
+      context 'that are valid' do
+        it 'returns uuid' do
+          expect(subject.uuid).to eq(uuid)
+        end
+      end
+    end
+
+    context 'where allowed_domains is not an array' do
+      let(:allowed_domains) { 'http://test-domain.io/' }
+
+      before { allow(subject).to receive(:response).and_call_original }
+
+      it 'raises error' do
+        expect { subject.response }.to raise_error(Superset::Request::InvalidParameterError, "allowed_domains array is required")
+      end
     end
 
-    describe '#uuid' do
-      it 'returns uuid' do
-        expect(subject.uuid).to eq(uuid)
+    context 'where allowed_domains is nil' do
+      let(:allowed_domains) { nil }
+
+      before { allow(subject).to receive(:response).and_call_original }
+
+      it 'raises error' do
+        expect { subject.response }.to raise_error(Superset::Request::InvalidParameterError, "allowed_domains array is required")
+      end
+    end
+
+    context 'where dashboard_id is nil' do
+      let(:dashboard_id) { nil }
+
+      before { allow(subject).to receive(:response).and_call_original }
+
+      it 'raises error' do
+        expect { subject.response }.to raise_error(Superset::Request::InvalidParameterError, "dashboard_id integer is required")
+      end
+    end
+
+    context 'where dashboard_id is not an int' do
+      let(:dashboard_id) { 'asdf' }
+
+      before { allow(subject).to receive(:response).and_call_original }
+
+      it 'raises error' do
+        expect { subject.response }.to raise_error(Superset::Request::InvalidParameterError, "dashboard_id integer is required")
       end
     end
   end

spec/superset/dataset/duplicate_spec.rb

@@ -15,6 +15,7 @@
   before do
     allow(subject).to receive(:response).and_return(response)
     allow(subject).to receive(:new_dataset_name_already_in_use?).and_return(false)
+    allow(subject).to receive(:source_dataset).and_return(double(schema: 'public'))
   end
 
   describe '#perform' do

spec/superset/dataset/list_spec.rb

@@ -83,5 +83,14 @@
       end
     end
 
+
+    context 'with title_contains and schema_contains' do
+      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")
+      end
+    end
+
   end
 end