better error handling and success reporting of submitted JSON #2134

seek4science · Feb 11, 2025 · 5c92a00 · 5c92a00
1 parent e7610be
commit 5c92a00
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 17 deletions.
diff --git a/app/controllers/extended_metadata_types_controller.rb b/app/controllers/extended_metadata_types_controller.rb
@@ -69,12 +69,29 @@ def create_from_ttl
 
   def submit_jsons
     jsons = params['emt_jsons']
+    failures = []
+    successes = []
     jsons.each do |json|
-      io = StringIO.new(json)
-      extended_metadata_type = Seek::ExtendedMetadataType::ExtendedMetadataTypeExtractor.extract_extended_metadata_type(io)
-      extended_metadata_type.save!
+      begin
+        extended_metadata_type = Seek::ExtendedMetadataType::ExtendedMetadataTypeExtractor.extract_extended_metadata_type(StringIO.new(json))
+        if extended_metadata_type.save
+          successes << "#{extended_metadata_type.title}(#{extended_metadata_type.supported_type})"
+        else
+          failures << "#{extended_metadata_type.title}(#{extended_metadata_type.supported_type}) - #{extended_metadata_type.errors.full_messages.join(', ')}"
+        end
+      rescue JSON::ParserError
+        failures << "Failed to parse JSON"
+      rescue StandardError => e
+        failures << e.message
+      end
+    end
+    if successes.any?
+      flash[:notice] = "#{successes.count} #{t('extended_metadata_type').pluralize(successes.count)} successfully created for: #{successes.join(', ')}."
     end
-    flash[:notice] = "#{jsons.length} #{t('extended_metadata_type').pluralize} were successfully created."
+    if failures.any?
+      flash[:error] = "#{failures.count} #{t('extended_metadata_type').pluralize(failures.count)} failed to be created: #{failures.join(', ')}."
+    end
+
     redirect_to administer_extended_metadata_types_path
   end
 

diff --git a/lib/seek/extended_metadata_type/extended_metadata_type_extractor.rb b/lib/seek/extended_metadata_type/extended_metadata_type_extractor.rb
@@ -3,6 +3,7 @@
 module Seek
   module ExtendedMetadataType
     module ExtendedMetadataTypeExtractor
+      class ValidationError < StandardError; end;
 
       SCHEMA_PATH = Rails.root.join('lib', 'seek', 'extended_metadata_type', 'extended_metadata_type_schema.json').freeze
 
@@ -11,15 +12,10 @@ def self.extract_extended_metadata_type(file)
         begin
           data_hash = JSON.parse(file.read)
         rescue JSON::ParserError => e
-          raise StandardError, "Failed to parse JSON file: #{e}"
+          raise JSON::ParserError, "Failed to parse JSON file: #{e.message}"
         end
 
-
-        begin
-          valid_emt_json?(data_hash)
-        rescue StandardError => e
-          raise StandardError,  e
-        end
+        valid_emt_json?(data_hash)
 
         create_extended_metadata_type_from_json(data_hash)
 
@@ -33,13 +29,13 @@ def self.valid_emt_json?(json)
         schema = JSON.parse(File.read(SCHEMA_PATH))
         errors = JSON::Validator.fully_validate(schema, json)
 
-        raise StandardError, "Invalid JSON file: #{errors.join(', ')}" if errors.present?
+        raise ValidationError, "Invalid JSON file: #{errors.join(', ')}" if errors.present?
 
       end
 
       def self.validate_attribute_type(type)
         unless SampleAttributeType.where(title: type).present?
-          raise StandardError, "The attribute type '#{type}' does not exist."
+          raise ValidationError, "The attribute type '#{type}' does not exist."
         end
       end
 

diff --git a/test/functional/extended_metadata_types_controller_test.rb b/test/functional/extended_metadata_types_controller_test.rb
@@ -319,7 +319,35 @@ class ExtendedMetadataTypesControllerTest < ActionController::TestCase
       post :submit_jsons, params: { emt_jsons: [json1, json2] }
     end
     assert_redirected_to administer_extended_metadata_types_path
-    assert_equal '2 Extended Metadata Types were successfully created.', flash[:notice]
+    assert_equal '2 Extended Metadata Types successfully created for: person(ExtendedMetadata), family(Investigation).', flash[:notice]
+    assert_nil flash[:error]
+  end
+
+  test 'submit jsons - invalid resulting EMT' do
+    json1 = file_fixture('extended_metadata_type/invalid_supported_type_emt.json').read
+    json2 = file_fixture('extended_metadata_type/valid_simple_emt.json').read
+    person = FactoryBot.create(:admin)
+    login_as(person)
+    assert_difference('ExtendedMetadataType.count', 1) do
+      post :submit_jsons, params: { emt_jsons: [json1, json2] }
+    end
+    assert_redirected_to administer_extended_metadata_types_path
+    assert_equal '1 Extended Metadata Type successfully created for: person(ExtendedMetadata).', flash[:notice]
+    assert_equal "1 Extended Metadata Type failed to be created: publication(Journal) - Supported type 'Journal' is not a valid support type!.", flash[:error]
+  end
+
+  test 'submit jsons - invalid JSON' do
+    json1 = file_fixture('extended_metadata_type/invalid_json.json').read
+    json2 = file_fixture('extended_metadata_type/invalid_emt_with_wrong_type.json').read
+    json3 = file_fixture('extended_metadata_type/valid_simple_emt.json').read
+    person = FactoryBot.create(:admin)
+    login_as(person)
+    assert_difference('ExtendedMetadataType.count', 1) do
+      post :submit_jsons, params: { emt_jsons: [json1, json2, json3] }
+    end
+    assert_redirected_to administer_extended_metadata_types_path
+    assert_equal '1 Extended Metadata Type successfully created for: person(ExtendedMetadata).', flash[:notice]
+    assert_equal "2 Extended Metadata Types failed to be created: Failed to parse JSON, The attribute type 'String1' does not exist..", flash[:error]
   end
 
 end
diff --git a/test/unit/extended_metadatas/emt_extractor_test.rb b/test/unit/extended_metadatas/emt_extractor_test.rb
@@ -101,7 +101,7 @@ class EmtExtractorTest < ActiveSupport::TestCase
 
     assert_no_difference('ExtendedMetadataType.count') do
 
-      error = assert_raises(StandardError) do
+      error = assert_raises(JSON::ParserError) do
         Seek::ExtendedMetadataType::ExtendedMetadataTypeExtractor.extract_extended_metadata_type(invalid_emt_file)
       end
 
@@ -115,7 +115,7 @@ class EmtExtractorTest < ActiveSupport::TestCase
 
     assert_no_difference('ExtendedMetadataType.count') do
 
-      error = assert_raises(StandardError) do
+      error = assert_raises(Seek::ExtendedMetadataType::ExtendedMetadataTypeExtractor::ValidationError) do
         Seek::ExtendedMetadataType::ExtendedMetadataTypeExtractor.extract_extended_metadata_type(invalid_emt_file)
       end
 
@@ -128,7 +128,7 @@ class EmtExtractorTest < ActiveSupport::TestCase
 
     assert_no_difference('ExtendedMetadataType.count') do
 
-      error = assert_raises(StandardError) do
+      error = assert_raises(ActiveRecord::RecordNotFound) do
         Seek::ExtendedMetadataType::ExtendedMetadataTypeExtractor.extract_extended_metadata_type(invalid_emt_file)
       end