[Scripts] Handle configuration definition error from script service G…

…raphQL

lib/project_types/script/layers/domain/script_config.rb

@@ -4,14 +4,15 @@ module Script
   module Layers
     module Domain
       class ScriptConfig
-        attr_reader :content, :version, :title, :description, :configuration_ui, :configuration
+        attr_reader :content, :version, :title, :description, :configuration_ui, :configuration, :filename
 
         REQUIRED_FIELDS = %w(version title)
 
-        def initialize(content:)
+        def initialize(content:, filename:)
           validate_content!(content)
 
           @content = content
+          @filename = filename
           @version = @content["version"].to_s
           @title = @content["title"]
           @description = @content["description"]

lib/project_types/script/layers/infrastructure/errors.rb

@@ -5,6 +5,15 @@ module Layers
     module Infrastructure
       module Errors
         class BuildError < ScriptProjectError; end
+
+        class ScriptConfigurationDefinitionError < ScriptProjectError
+          attr_reader :filename
+          def initialize(message:, filename:)
+            @filename = filename
+            super(message)
+          end
+        end
+
         class ScriptConfigSyntaxError < ScriptProjectError; end
 
         class ScriptConfigMissingKeysError < ScriptProjectError

lib/project_types/script/layers/infrastructure/script_project_repository.rb

@@ -204,7 +204,7 @@ def update_hash(hash:, title:)
           end
 
           def from_h(hash)
-            Domain::ScriptConfig.new(content: hash)
+            Domain::ScriptConfig.new(content: hash, filename: filename)
           rescue Domain::Errors::MissingScriptConfigFieldError => e
             raise missing_field_error, e.field
           end

lib/project_types/script/layers/infrastructure/script_service.rb

@@ -46,6 +46,11 @@ def set_app_script(
 
           if user_errors.any? { |e| e["tag"] == "already_exists_error" }
             raise Errors::ScriptRepushError, uuid
+          elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_error" })
+            raise Errors::ScriptConfigurationDefinitionError.new(
+              message: e["message"],
+              filename: script_config.filename,
+            )
           elsif (e = user_errors.any? { |err| err["tag"] == "configuration_definition_syntax_error" })
             raise Errors::ScriptConfigSyntaxError
           elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_missing_keys_error" })

lib/project_types/script/messages/messages.rb

@@ -65,6 +65,9 @@ module Messages
           app_not_connected_cause: "Script is not connected to an app.",
           app_not_connected_help: "Run shopify connect or enter fields for api-key and api-secret.",
 
+          configuration_error_cause: "There was a problem with the configuration defined in %{filename}. %{message}",
+          configuration_error_help: "Fix the error and try again.",
+
           configuration_syntax_error_cause: "The script.json is not formatted properly.",
           configuration_syntax_error_help: "Fix the errors and try again.",
 

lib/project_types/script/ui/error_handler.rb

@@ -138,6 +138,15 @@ def self.error_messages(e)
             cause_of_error: ShopifyCLI::Context.message("script.error.no_script_config_yml_file_cause"),
             help_suggestion: ShopifyCLI::Context.message("script.error.no_script_config_yml_file_help"),
           }
+        when Layers::Infrastructure::Errors::ScriptConfigurationDefinitionError
+          {
+            cause_of_error: ShopifyCLI::Context.message(
+              "script.error.configuration_error_cause",
+              message: e.message,
+              filename: e.filename,
+            ),
+            help_suggestion: ShopifyCLI::Context.message("script.error.configuration_error_help"),
+          }
         when Layers::Infrastructure::Errors::ScriptConfigSyntaxError
           {
             cause_of_error: ShopifyCLI::Context.message("script.error.configuration_syntax_error_cause"),

test/minitest_ext.rb

@@ -93,6 +93,14 @@ def to_s # :nodoc:
       end.join("\n")
     end
 
+    def assert_raises_and_validate(exception_class, field_expectation_proc)
+      yield
+    rescue exception_class => e
+      field_expectation_proc.call(e)
+    else
+      flunk("Expected a #{exception_class} to be raised but it was not")
+    end
+
     private
 
     def stub_prompt_for_cli_updates

test/project_types/script/layers/domain/script_config_test.rb

@@ -12,8 +12,9 @@
       "configuration" => {},
     }
   end
+  let(:filename) { "script.config.yml" }
 
-  subject { Script::Layers::Domain::ScriptConfig.new(content: content) }
+  subject { Script::Layers::Domain::ScriptConfig.new(content: content, filename: filename) }
 
   describe "#initialize" do
     it "constructs a ScriptConfig" do
@@ -22,6 +23,7 @@
       assert_equal("Some Description", subject.description)
       assert(subject.configuration_ui)
       assert_equal({}, subject.configuration)
+      assert_equal(filename, subject.filename)
     end
   end
 

test/project_types/script/layers/domain/script_project_test.rb

@@ -8,7 +8,13 @@
   let(:extension_point_type) { "discount" }
   let(:script_name) { "foo_script" }
   let(:language) { "assemblyscript" }
-  let(:script_config) { Script::Layers::Domain::ScriptConfig.new(content: script_config_content) }
+  let(:script_config_filename) { "script.config.yml" }
+  let(:script_config) do
+    Script::Layers::Domain::ScriptConfig.new(
+      content: script_config_content,
+      filename: script_config_filename,
+    )
+  end
   let(:script_config_content) do
     {
       "version" => "1",

test/project_types/script/layers/infrastructure/script_service_test.rb

@@ -11,8 +11,12 @@
   let(:schema_major_version) { "1" }
   let(:schema_minor_version) { "0" }
   let(:use_msgpack) { true }
+  let(:script_config_filename) { "script.config.yml" }
   let(:script_config) do
-    Script::Layers::Domain::ScriptConfig.new(content: expected_script_config_content)
+    Script::Layers::Domain::ScriptConfig.new(
+      content: expected_script_config_content,
+      filename: script_config_filename,
+    )
   end
   let(:script_name) { "script name" }
   let(:script_config_version) { "1" }
@@ -175,6 +179,28 @@
         end
       end
 
+      describe "when configuration is invalid" do
+        let(:response) do
+          {
+            "data" => {
+              "appScriptSet" => {
+                "userErrors" => [{ "message" => "error", "tag" => "configuration_definition_error" }],
+              },
+            },
+          }
+        end
+
+        it "should raise ScriptConfigurationDefinitionError" do
+          assert_raises_and_validate(
+            Script::Layers::Infrastructure::Errors::ScriptConfigurationDefinitionError,
+            proc do |e|
+              assert_equal("error", e.message)
+              assert_equal(script_config_filename, e.filename)
+            end,
+          ) { subject }
+        end
+      end
+
       describe "when response is empty" do
         let(:response) { nil }
 

test/project_types/script/test_helpers/fake_script_project_repository.rb

@@ -69,20 +69,26 @@ def initialize
       end
 
       def create(content)
-        @cache = Script::Layers::Domain::ScriptConfig.new(content: content)
+        @cache = from_h(content)
       end
 
       def update!(title:)
         hash = get!.content
         hash["title"] = title
 
-        @cache = Script::Layers::Domain::ScriptConfig.new(content: hash)
+        @cache = from_h(hash)
       end
 
       def get!
         raise Script::Layers::Infrastructure::Errors::NoScriptConfigFileError if @cache.nil?
         @cache
       end
+
+      private
+
+      def from_h(hash)
+        Script::Layers::Domain::ScriptConfig.new(content: hash, filename: "script.config.yml")
+      end
     end
   end
 end

test/project_types/script/ui/error_handler_test.rb

@@ -233,6 +233,18 @@ def should_call_display_and_raise
         end
       end
 
+      describe "when ScriptConfigurationDefinitionError" do
+        let(:err) do
+          Script::Layers::Infrastructure::Errors::ScriptConfigurationDefinitionError.new(
+            message: "message",
+            filename: "filename",
+          )
+        end
+        it "should call display_and_raise" do
+          should_call_display_and_raise
+        end
+      end
+
       describe "when ScriptConfigSyntaxError" do
         let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigSyntaxError.new }
         it "should call display_and_raise" do