use InvalidParameterError for bulk errors, adjusts note

Author: jbat

lib/superset/chart/bulk_delete.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
-# TODO: the gui delete has a confirmation step, this does not.  Potentially add a confirm_delete parameter to the constructor
-# that would ensure that all charts belong to an expected dashboard before deleting.  ( not sure if this is a good idea )
-
+# TODO: the gui delete has a confirmation step, this API call does not.
+# Potentially we could add a confirm_delete parameter to the constructor that would ensure that all charts either
+#   1 belong to only an expected dashboard before deleting
+#   2 or do not belong to any dashboards
+#  ( not sure if this needed at this point )
 
 module Superset
   module Chart

lib/superset/chart/delete.rb

@@ -10,7 +10,7 @@ def initialize(chart_id: )
       end
 
       def perform
-        raise "Error: chart_id integer is required" unless chart_id.present? && chart_id.is_a?(Integer)
+        raise InvalidParameterError, "chart_id integer is required" unless chart_id.present? && chart_id.is_a?(Integer)
 
         logger.info("Attempting to delete chart with id: #{chart_id}")
         response

lib/superset/dashboard/bulk_delete.rb

@@ -1,6 +1,10 @@
 # frozen_string_literal: true
-# TODO: the gui delete has a confirmation step, this does not.  Potentially add a confirm_delete parameter to the constructor
-# that would ensure that no charts belong to a dashboard before deleting
+
+# TODO: the gui delete has a confirmation step, this API call does not.
+# Potentially we could add a confirm_delete parameter to the constructor that would ensure that all dashboards either
+#   1 have an expected set of charts or filters before deleting
+#   2 or do not have any charts or filters linked to them
+#  ( not sure if this needed at this point )
 
 module Superset
   module Dashboard

lib/superset/dashboard/delete.rb

@@ -12,7 +12,7 @@ def initialize(dashboard_id: , confirm_zero_charts: true)
       end
 
       def perform
-        raise "Error: dashboard_id integer is required" unless dashboard_id.present? && dashboard_id.is_a?(Integer)
+        raise InvalidParameterError, "dashboard_id integer is required" unless dashboard_id.present? && dashboard_id.is_a?(Integer)
 
         confirm_zero_charts_on_dashboard if confirm_zero_charts
 

lib/superset/dataset/bulk_delete.rb

@@ -1,6 +1,10 @@
 # frozen_string_literal: true
-# TODO: the gui delete has a confirmation step, this does not.  Potentially add a confirm_delete parameter to the constructor
-# that would ensure that no charts point to a dataset before deleting
+
+# TODO: the gui delete has a confirmation step, this API call does not.
+# Potentially we could add a confirm_delete parameter to the constructor that would ensure that all datasets either
+#   1 belong to only an expected charts or filters before deleting
+#   2 or do not belong to any charts or filters
+#  ( not sure if this needed at this point )
 
 module Superset
   module Dataset

lib/superset/dataset/delete.rb

@@ -10,7 +10,7 @@ def initialize(dataset_id: )
       end
 
       def perform
-        raise "Error: dataset_id integer is required" unless dataset_id.present? && dataset_id.is_a?(Integer)
+        raise InvalidParameterError, "dataset_id integer is required" unless dataset_id.present? && dataset_id.is_a?(Integer)
 
         logger.info("Attempting to delete dataset with id: #{dataset_id}")
         response

spec/superset/chart/delete_spec.rb

@@ -14,15 +14,15 @@
       let(:chart_id) { nil }
 
       it 'raises an error' do
-        expect { subject.perform }.to raise_error(RuntimeError, "Error: chart_id integer is required")
+        expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "chart_id integer is required")
       end
     end
 
     context 'when chart_id is not an integer' do
       let(:chart_id) { 'string' }
 
       it 'raises an error' do
-        expect { subject.perform }.to raise_error(RuntimeError, "Error: chart_id integer is required")
+        expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "chart_id integer is required")
       end
     end
 

spec/superset/dashboard/delete_spec.rb

@@ -14,15 +14,15 @@
       let(:dashboard_id) { nil }
 
       it 'raises an error' do
-        expect { subject.perform }.to raise_error(RuntimeError, "Error: dashboard_id integer is required")
+        expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "dashboard_id integer is required")
       end
     end
 
     context 'when dashboard_id is not an integer' do
       let(:dashboard_id) { 'string' }
 
       it 'raises an error' do
-        expect { subject.perform }.to raise_error(RuntimeError, "Error: dashboard_id integer is required")
+        expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "dashboard_id integer is required")
       end
     end
 

spec/superset/dataset/delete_spec.rb

@@ -14,15 +14,15 @@
       let(:dataset_id) { nil }
 
       it 'raises an error' do
-        expect { subject.perform }.to raise_error(RuntimeError, "Error: dataset_id integer is required")
+        expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "dataset_id integer is required")
       end
     end
 
     context 'when dataset_id is not an integer' do
       let(:dataset_id) { 'string' }
 
       it 'raises an error' do
-        expect { subject.perform }.to raise_error(RuntimeError, "Error: dataset_id integer is required")
+        expect { subject.perform }.to raise_error(Superset::Request::InvalidParameterError, "dataset_id integer is required")
       end
     end