Allow external resources to be replaced

Gemfile.lock

@@ -1,6 +1,6 @@
 GIT
   remote: https://github.com/ElixirTeSS/TeSS_RDF_Extractors
-  revision: f2e44f0eeeb299f0bc10f725496b39d6246938f0
+  revision: c89fcad2a46ead316af14b270d26c6ba924646b6
   branch: master
   specs:
     tess_rdf_extractors (1.1.0)
@@ -115,8 +115,8 @@ GEM
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
       tzinfo (~> 2.0)
-    addressable (2.8.5)
-      public_suffix (>= 2.0.2, < 6.0)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
     aes_key_wrap (1.1.0)
     ahoy_matey (4.2.1)
       activesupport (>= 5.2)
@@ -136,6 +136,7 @@ GEM
       erubi (>= 1.0.0)
       rack (>= 0.9.0)
       rouge (>= 1.0.0)
+    bigdecimal (3.1.8)
     bindata (2.5.0)
     bindex (0.8.1)
     binding_of_caller (1.0.0)
@@ -162,7 +163,7 @@ GEM
       json_schema (~> 0.14, >= 0.14.3)
       openapi_parser (~> 1.0)
       rack (>= 1.5)
-    concurrent-ruby (1.3.3)
+    concurrent-ruby (1.3.4)
     connection_pool (2.4.1)
     countries (5.6.0)
       unaccent (~> 0.3)
@@ -215,7 +216,7 @@ GEM
     globalid (1.2.1)
       activesupport (>= 6.1)
     gravtastic (3.2.6)
-    haml (6.2.3)
+    haml (6.3.0)
       temple (>= 0.8.2)
       thor
       tilt
@@ -264,13 +265,14 @@ GEM
       bindata
       faraday (~> 2.0)
       faraday-follow_redirects
-    json-ld (3.3.1)
+    json-ld (3.3.2)
       htmlentities (~> 4.3)
       json-canonicalization (~> 1.0)
       link_header (~> 0.0, >= 0.0.8)
       multi_json (~> 1.15)
       rack (>= 2.2, < 4)
       rdf (~> 3.3)
+      rexml (~> 3.2)
     json-ld-preloaded (3.3.0)
       json-ld (~> 3.3)
       rdf (~> 3.3)
@@ -333,7 +335,7 @@ GEM
     listen (3.8.0)
       rb-fsevent (~> 0.10, >= 0.10.3)
       rb-inotify (~> 0.9, >= 0.9.10)
-    logger (1.6.0)
+    logger (1.6.1)
     loofah (2.22.0)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
@@ -370,7 +372,7 @@ GEM
     multi_xml (0.6.0)
     multipart-post (2.4.0)
     nested_form (0.3.2)
-    net-http-persistent (4.0.2)
+    net-http-persistent (4.0.4)
       connection_pool (~> 2.2)
     net-imap (0.4.14)
       date
@@ -383,7 +385,7 @@ GEM
       net-protocol
     netrc (0.11.0)
     nio4r (2.7.3)
-    nokogiri (1.16.6)
+    nokogiri (1.16.7)
       mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     numerizer (0.1.1)
@@ -425,19 +427,19 @@ GEM
     pry-byebug (3.10.1)
       byebug (~> 11.0)
       pry (>= 0.13, < 0.15)
-    psych (5.1.1.1)
+    psych (5.1.2)
       stringio
     public_activity (2.0.2)
       actionpack (>= 5.0.0)
       activerecord (>= 5.0)
       i18n (>= 0.5.0)
       railties (>= 5.0.0)
-    public_suffix (5.0.4)
+    public_suffix (6.0.1)
     puma (6.4.3)
       nio4r (~> 2.0)
     pundit (2.3.1)
       activesupport (>= 3.0.0)
-    racc (1.8.0)
+    racc (1.8.1)
     rack (2.2.9)
     rack-cors (2.0.2)
       rack (>= 2.0.0)
@@ -498,8 +500,9 @@ GEM
     rb-fsevent (0.11.2)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
-    rdf (3.3.1)
+    rdf (3.3.2)
       bcp47_spec (~> 0.2)
+      bigdecimal (~> 3.1, >= 3.1.5)
       link_header (~> 0.0, >= 0.0.8)
     rdf-aggregate-repo (3.3.0)
       rdf (~> 3.3)
@@ -557,7 +560,7 @@ GEM
     rdf-turtle (3.3.0)
       ebnf (~> 2.4)
       rdf (~> 3.3)
-    rdf-vocab (3.3.0)
+    rdf-vocab (3.3.1)
       rdf (~> 3.3)
     rdf-xsd (3.3.0)
       rdf (~> 3.3)
@@ -579,8 +582,7 @@ GEM
       netrc (~> 0.8)
     reverse_markdown (2.1.1)
       nokogiri
-    rexml (3.3.6)
-      strscan
+    rexml (3.3.8)
     rouge (4.1.3)
     rsolr (2.5.0)
       builder (>= 2.1.2)
@@ -697,8 +699,7 @@ GEM
       actionpack (>= 5.2)
       activesupport (>= 5.2)
       sprockets (>= 3.0.0)
-    stringio (3.1.0)
-    strscan (3.1.0)
+    stringio (3.1.1)
     swd (2.0.2)
       activesupport (>= 3)
       attr_required (>= 0.0.5)
@@ -712,8 +713,8 @@ GEM
       climate_control (>= 0.0.3, < 1.0)
     terser (1.1.17)
       execjs (>= 0.3.0, < 3)
-    thor (1.3.1)
-    tilt (2.3.0)
+    thor (1.3.2)
+    tilt (2.4.0)
     timeout (0.4.1)
     turbo-rails (1.5.0)
       actionpack (>= 6.0.0)
@@ -733,7 +734,7 @@ GEM
       unf_ext
     unf_ext (0.0.8.2)
     unicode-display_width (2.4.2)
-    unicode-types (1.9.0)
+    unicode-types (1.10.0)
     validate_email (0.1.6)
       activemodel (>= 3.0)
       mail (>= 2.2.5)

app/controllers/events_controller.rb

@@ -235,7 +235,8 @@ def event_params
                                   { node_names: [] }, { target_audience: [] }, { eligibility: [] }, :visible,
                                   { host_institutions: [] }, :capacity, :contact, :recognition, :learning_objectives,
                                   :prerequisites, :tech_requirements, :cost_basis, :cost_value, :cost_currency, :language,
-                                  external_resources_attributes: %i[id url title _destroy], material_ids: [],
+                                  external_resources_attributes: %i[id url title _destroy],
+                                  external_resources: [:url, :title], material_ids: [],
                                   llm_interaction_attributes: %i[id scrape_or_process model prompt input output needs_processing _destroy],
                                   locked_fields: [])
   end

app/controllers/materials_controller.rb

@@ -174,6 +174,7 @@ def material_params
                                      { operation_names: [] }, { operation_uris: [] },
                                      { node_ids: [] }, { node_names: [] }, { fields: [] },
                                      external_resources_attributes: [:id, :url, :title, :_destroy],
+                                     external_resources: [:url, :title],
                                      event_ids: [], locked_fields: [])
   end
 

app/models/concerns/has_external_resources.rb

@@ -33,4 +33,20 @@ def remove_duplicate_external_resources
     resources = external_resources.to_a.sort_by { |x| x.created_at || 1.year.from_now }
     (resources - resources.uniq { |r| [r.url, r.title] }).each(&:mark_for_destruction)
   end
+
+  # Allow setting of external resources from an array of hashes/params.
+  # Unlike `external_resources_attributes=`, this will clear any redundant external resources, and will preserve any
+  # existing external resources with the same title & URL.
+  def external_resources=(ers)
+    existing = external_resources
+    objs = ers.map do |er|
+      if er.is_a?(ExternalResource)
+        er
+      else
+        existing.detect { |ex| ex.url == er[:url] && ex.title == er[:title] } || external_resources.build(er)
+      end
+    end
+
+    super(objs)
+  end
 end

test/models/material_test.rb

@@ -391,6 +391,72 @@ class MaterialTest < ActiveSupport::TestCase
     assert_not_includes material.external_resources, new_resource, 'Should preserve oldest external resource'
   end
 
+  test 'should set external resources from params' do
+    material = materials(:good_material)
+    assert_empty material.external_resources
+
+    assert_difference('ExternalResource.count', 2) do
+      material.update!(external_resources: [{ title: 'Cool Website!', url: 'https://tess.elixir-uk.org/' },
+                                            { title: 'Cooler Website!', url: 'https://tess.elixir-europe.org/' }])
+    end
+
+    er = material.reload.external_resources
+    assert_equal 2, er.count
+    assert_equal 'Cool Website!', er[0].title
+    assert_equal 'https://tess.elixir-uk.org/', er[0].url
+    assert_equal 'Cooler Website!', er[1].title
+    assert_equal 'https://tess.elixir-europe.org/', er[1].url
+  end
+
+  test 'should remove redundant external resources and preserve IDs of retained ones' do
+    material = materials(:material_with_external_resource)
+    original_resources = material.external_resources.to_a
+    assert_equal 3, original_resources.length
+
+    # [
+    #   { url: "https://tess.elixir-uk.org/", title: "TeSS" },
+    #   { url: "https://bio.tools/tool/SR-Tesseler", title: "SR-Tesseler" },
+    #   { url: "https://fairsharing.org/bsg-p123456", title: "Share Fairing" }
+    # ]
+
+    assert_difference('ExternalResource.count', -1) do
+      material.update!(external_resources: [{ title: 'TeSS', url: 'https://tess.elixir-uk.org/' },
+                                            { title: 'Changed title', url: 'https://bio.tools/tool/SR-Tesseler' }])
+    end
+
+    er = material.reload.external_resources
+    assert_equal 2, er.count
+    assert_equal 'TeSS', er[0].title
+    assert_equal 'https://tess.elixir-uk.org/', er[0].url
+    assert_equal original_resources[0].id, er[0].id, 'Should have preserved original ExternalResource'
+    assert_equal 'Changed title', er[1].title
+    assert_equal 'https://bio.tools/tool/SR-Tesseler', er[1].url
+    assert_not_equal original_resources[1].id, er[1].id, 'Should have replaced modified ExternalResource'
+  end
+
+  test 'can set external resources using objects or params' do
+    material = materials(:material_with_external_resource)
+    original_resources = material.external_resources.to_a
+    assert_equal 3, original_resources.length
+
+    assert_no_difference('ExternalResource.count') do
+      material.update!(external_resources: original_resources.first(2) +
+        [{ title: 'Zombocom', url: 'https://zombo.com' }])
+    end
+
+    er = material.reload.external_resources
+    assert_equal 3, er.count
+    assert_equal 'TeSS', er[0].title
+    assert_equal 'https://tess.elixir-uk.org/', er[0].url
+    assert_equal original_resources[0].id, er[0].id, 'Should have preserved first ExternalResource'
+    assert_equal 'SR-Tesseler', er[1].title
+    assert_equal 'https://bio.tools/tool/SR-Tesseler', er[1].url
+    assert_equal original_resources[1].id, er[1].id, 'Should have preserved second ExternalResource'
+    assert_equal 'Zombocom', er[2].title
+    assert_equal 'https://zombo.com', er[2].url
+    assert_not_equal original_resources[2].id, er[2].id, 'Should have replaced third ExternalResource'
+  end
+
   test 'verified users scope' do
     bad_user = users(:unverified_user)
     bad_material = bad_user.materials.build(title: 'bla', url: 'http://example.com/spam', description: 'vvv',

test/workers/source_test_worker_test.rb

@@ -105,7 +105,7 @@ class SourceTestWorkerTest < ActiveSupport::TestCase
     assert sample
     assert results[:run_time] > 0
     assert results[:finished_at] > 1.day.ago
-    ext_res = sample[:external_resources_attributes]
+    ext_res = sample[:external_resources]
     assert_equal 1, ext_res.length
     res = ext_res.first
     assert res.is_a?(Hash)