Merge branch 'seek-1.15' into main

app/controllers/git_controller.rb

@@ -8,6 +8,7 @@ class GitController < ApplicationController
   before_action :fetch_git_version
   before_action :get_tree, only: [:tree]
   before_action :get_blob, only: [:blob, :download, :raw]
+  before_action :check_fetched, only: [:download, :raw]
 
   user_content_actions :raw
 
@@ -257,6 +258,12 @@ def log_event
     end
   end
 
+  def check_fetched
+    if @blob.remote? && !@blob.fetched?
+      render_git_error('This file is held externally.', status: 404)
+    end
+  end
+
   # # Rugged does not allow streaming blobs
   # def stream_blob(blob, filename)
   #   response.headers['Content-Disposition'] = "attachment; filename=#{filename}"

app/controllers/studies_controller.rb

@@ -202,7 +202,7 @@ def batch_create
     # e.g: Study.new(title: 'title', investigation: investigations(:metabolomics_investigation), policy: FactoryBot.create(:private_policy))
     # study.policy = Policy.create(name: 'default policy', access_type: 1)
     # render plain: params[:studies].inspect
-    metadata_types = ExtendedMetadataType.where(title: 'MIAPPE metadata', supported_type: 'Study').last
+    metadata_types = ExtendedMetadataType.where(title: ExtendedMetadataType::MIAPPE_TITLE, supported_type: 'Study').last
     studies_length = params[:studies][:title].length
     studies_uploaded = false
     data_file_uploaded = false

app/helpers/studies_helper.rb

@@ -24,6 +24,6 @@ def authorised_studies(projects = nil)
   end
 
   def show_batch_miappe_button?
-    ExtendedMetadataType.where(supported_type: 'Study', title: 'MIAPPE metadata v1.1').any?
+    ExtendedMetadataType.where(supported_type: 'Study', title: ExtendedMetadataType::MIAPPE_TITLE).any?
   end
 end

app/models/concerns/has_extended_metadata.rb

@@ -3,8 +3,10 @@ module HasExtendedMetadata
 
   included do
     def extended_metadata_attribute_values_for_search
-      extended_metadata ? extended_metadata.data.values.reject(&:blank?).uniq : []
+      return [] unless extended_metadata
+      extended_metadata.data.extract_all_values.uniq.compact
     end
+
   end
 
   class_methods do

app/models/extended_metadata.rb

@@ -22,4 +22,5 @@ def extended_metadata_type=(type)
   def attribute_class
     ExtendedMetadataAttribute
   end
+
 end

app/models/extended_metadata_type.rb

@@ -13,6 +13,9 @@ class ExtendedMetadataType < ApplicationRecord
   scope :enabled, ->{ where(enabled: true) }
   scope :disabled, ->{ where(enabled: false) }
 
+  # built in type
+  MIAPPE_TITLE = 'MIAPPE metadata v1.1'.freeze
+
   def attribute_by_title(title)
     extended_metadata_attributes.where(title: title).first
   end

app/models/sample.rb

@@ -145,8 +145,12 @@ def refresh_linking_samples
           sample['title'] = title if sample['id'] == id
         end
         metadata.values[p - 1] = item_linked_samples
-        s.json_metadata = metadata.to_json
-        s.save
+
+        # only update if changed, to prevent triggered jobs that could potentially create an infinate loop
+        if s.json_metadata != metadata.to_json
+          s.json_metadata = metadata.to_json
+          s.save
+        end
       end
     end
   end

app/models/study_batch_upload.rb

@@ -29,7 +29,7 @@ def self.extract_study_data_from_file(studies_file)
   def self.extract_studies_from_file(studies_file)
     studies = []
     parsed_sheet = Seek::Templates::StudiesReader.new(studies_file)
-    metadata_type = ExtendedMetadataType.where(title: 'MIAPPE metadata', supported_type: 'Study').last
+    metadata_type = ExtendedMetadataType.where(title: ExtendedMetadataType::MIAPPE_TITLE, supported_type: 'Study').last
     columns = [2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
     study_start_row_index = 4
     parsed_sheet.each_record(3, columns) do |index, data|

app/views/git/_blob.html.erb

@@ -8,8 +8,13 @@
 
 <div>
   <div class="pull-right">
-    <%= button_link_to('Download', 'download', polymorphic_path([@parent_resource, :git_download], version: @git_version.version, path: @blob.path)) %>
-    <%= button_link_to('Raw', 'markup', polymorphic_path([@parent_resource, :git_raw], version: @git_version.version, path: @blob.path)) %>
+    <% if @blob.fetched? %>
+      <%= button_link_to('Download', 'download', polymorphic_path([@parent_resource, :git_download], version: @git_version.version, path: @blob.path)) %>
+      <%= button_link_to('Raw', 'markup', polymorphic_path([@parent_resource, :git_raw], version: @git_version.version, path: @blob.path)) %>
+    <% end %>
+    <% if @blob.remote? %>
+      <%= button_link_to('External Link', 'external_link', @blob.url, target: :_blank) %>
+    <% end %>
     <% if @git_version.can_edit? %>
       <% if @git_version.mutable? %>
         <%= button_link_to('Move/rename', 'move', '#', { 'data-toggle' => 'modal', 'data-target' => '#git-move-modal' }) %>
@@ -25,7 +30,9 @@
   </div>
 
   <strong>Path: </strong> <%= @blob.path -%><br/>
-  <strong>Size: </strong> <%= number_to_human_size @blob.size -%><br/>
+  <% if @blob.fetched? %>
+    <strong>Size: </strong> <%= number_to_human_size @blob.size -%><br/>
+  <% end %>
   <div class="hidden">
     <strong>SHA: </strong> <code><%= @blob.oid %></code><br/>
   </div>

lib/seek/config.rb

@@ -365,7 +365,7 @@ def omniauth_elixir_aai_config
           name: :elixir_aai,
           scope: [:openid, :email],
           response_type: 'code',
-          issuer: 'https://proxy.aai.lifescience-ri.eu/',
+          issuer: 'https://login.aai.lifescience-ri.eu/oidc/',
           discovery: true,
           client_options: {
             identifier: omniauth_elixir_aai_client_id,

lib/seek/json_metadata/data.rb

@@ -34,6 +34,17 @@ def mass_assign(hash, pre_process: true)
         end
       end
 
+      # extracts all values from the data as a list, including from within nested structures
+      def extract_all_values(data = self)
+        data.values.collect do |value|
+          if value.is_a?(Seek::JSONMetadata::Data)
+            extract_all_values(value)
+          else
+            value
+          end
+        end.flatten
+      end
+
       private
 
       def validate_hash(hash)

lib/seek/renderers/text_renderer.rb

@@ -6,7 +6,13 @@ def can_render?
       end
 
       def render_content
-        "<pre>#{h(blob.read)}</pre>"
+        content = blob.read
+        if content.empty?
+          '<span class="subtle">No content to display</span>'
+        else
+          "<pre>#{h(content)}</pre>"
+        end
+
       end
 
       def render_standalone

test/factories/extended_metadata_types.rb

@@ -142,7 +142,7 @@
   end
 
   factory(:study_extended_metadata_type_for_MIAPPE, class: ExtendedMetadataType) do
-    title { 'MIAPPE metadata' }
+    title { ExtendedMetadataType::MIAPPE_TITLE }
     supported_type { 'Study' }
     after(:build) do |a|
       a.extended_metadata_attributes << FactoryBot.create(:name_extended_metadata_attribute, title:'id')

test/functional/git_controller_test.rb

@@ -210,6 +210,17 @@ def setup
     assert_select 'img.git-image-preview[src=?]', workflow_git_raw_path(@workflow, version: @git_version.version, path: 'test.svg')
   end
 
+  test 'get remote blob' do
+    @git_version.add_remote_file('something', 'https://example.com/something', fetch: false)
+    @git_version.save!
+    get :blob, params: { workflow_id: @workflow.id, version: @git_version.version, path: 'something' }
+
+    assert_response :success
+    assert_select 'a.btn[href=?]', workflow_git_remove_file_path(@workflow, version: @git_version.version, path: 'something')
+    assert_select 'a.btn[href=?]', 'https://example.com/something'
+    assert_select 'img.git-image-preview[src=?]', workflow_git_raw_path(@workflow, version: @git_version.version, path: 'something'), count: 0
+  end
+
   test 'get raw binary file' do
     get :raw, params: { workflow_id: @workflow.id, version: @git_version.version, path: 'diagram.png' }
 
@@ -224,6 +235,24 @@ def setup
     assert @response.header['Content-Disposition'].include?('attachment')
   end
 
+  test 'attempting to download remote blob throws error' do
+    @git_version.add_remote_file('something', 'https://example.com/something', fetch: false)
+    @git_version.save!
+    get :download, params: { workflow_id: @workflow.id, version: @git_version.version, path: 'something' }
+
+    assert_redirected_to workflow_path(@workflow, tab: 'files')
+    assert flash[:error].include?('held externally')
+  end
+
+  test 'attempting to get raw remote blob throws error' do
+    @git_version.add_remote_file('something', 'https://example.com/something', fetch: false)
+    @git_version.save!
+    get :raw, params: { workflow_id: @workflow.id, version: @git_version.version, path: 'something' }
+
+    assert_redirected_to workflow_path(@workflow, tab: 'files')
+    assert flash[:error].include?('held externally')
+  end
+
   test 'getting non-existent blob throws error' do
     get :blob, params: { workflow_id: @workflow.id, version: @git_version.version, path: 'doesnotexist' }
 

test/unit/config_test.rb

@@ -619,7 +619,7 @@ class ConfigTest < ActiveSupport::TestCase
         refute Seek::Config.omniauth_elixir_aai_legacy_mode
         assert_equal '/seeks/seek1/auth/elixir_aai/callback', config[:callback_path]
         assert_equal 'http://localhost/seeks/seek1/auth/elixir_aai/callback', config[:client_options][:redirect_uri]
-        assert_equal 'https://proxy.aai.lifescience-ri.eu/',config[:issuer]
+        assert_equal 'https://login.aai.lifescience-ri.eu/oidc/',config[:issuer]
         with_config_value(:omniauth_elixir_aai_legacy_mode, true) do
           assert Seek::Config.omniauth_elixir_aai_legacy_mode
           config = Seek::Config.omniauth_elixir_aai_config

test/unit/investigation_test.rb

@@ -221,6 +221,10 @@ class InvestigationTest < ActiveSupport::TestCase
                    )
     )
     assert_equal ['James','25'].sort, item.extended_metadata_attribute_values_for_search.map(&:to_s).sort
+
+    #nested
+    item = FactoryBot.create(:investigation, extended_metadata: FactoryBot.create(:role_multiple_extended_metadata))
+    assert_equal ['[email protected]','0012345','liddell','alice','wonder','land'].sort, item.extended_metadata_attribute_values_for_search.map(&:to_s).sort
   end
 
   test 'related sop ids' do

test/unit/jobs/linking_samples_update_job_test.rb

@@ -2,6 +2,7 @@
 
 class LinkingSamplesUpdateJobTest < ActiveSupport::TestCase
   def setup
+    @person = FactoryBot.create(:person)
     create_linked_samples
   end
 
@@ -17,9 +18,31 @@ def setup
     end
   end
 
+  test 'only trigger further jobs if the metadata changes' do
+    sample = Sample.first
+    sample.set_attribute_value('full name', 'Ali Mohammadi')
+    disable_authorization_checks { sample.save! }
+    assert_enqueued_jobs 2, only: LinkingSamplesUpdateJob do
+      LinkingSamplesUpdateJob.perform_now(sample)
+    end
+
+    sample.set_attribute_value('full name', 'Ali Mohammadi')
+    disable_authorization_checks { sample.save! }
+    assert_enqueued_jobs 0, only: LinkingSamplesUpdateJob do
+      LinkingSamplesUpdateJob.perform_now(sample)
+    end
+
+    sample.set_attribute_value('full name', 'Fred Flintstone')
+    disable_authorization_checks { sample.save! }
+    assert_enqueued_jobs 2, only: LinkingSamplesUpdateJob do
+      LinkingSamplesUpdateJob.perform_now(sample)
+    end
+  end
+
+
   def create_linked_samples
-    person = FactoryBot.create(:person)
-    project = person.projects.first
+
+    project = @person.projects.first
 
     main_sample = FactoryBot.create(:patient_sample)
     sample_type = main_sample.sample_type
@@ -38,4 +61,5 @@ def create_linked_samples
     linked_sample2.set_attribute_value(:patient, [main_sample.id])
     disable_authorization_checks { linked_sample2.save! }
   end
+
 end

test/unit/renderers_test.rb

@@ -363,6 +363,12 @@ class RenderersTest < ActiveSupport::TestCase
     blob = FactoryBot.create(:image_content_blob, asset: @asset)
     renderer = Seek::Renderers::TextRenderer.new(blob)
     refute renderer.can_render?
+
+    @git.add_remote_file('empty', 'https://example.com/file', fetch: false)
+    git_blob = @git.get_blob('empty')
+    renderer = Seek::Renderers::TextRenderer.new(git_blob)
+    assert renderer.can_render?
+    assert_equal '<span class="subtle">No content to display</span>', renderer.render
   end
 
   test 'image renderer' do

test/unit/studies/studies_extractor_test.rb

@@ -39,7 +39,8 @@ class StudiesExtractorTest < ActiveSupport::TestCase
 
   test 'extract study correctly' do
     user_uuid = 'user_uuid'
-    FactoryBot.create(:study_extended_metadata_type_for_MIAPPE)
+    extended_metadata_type = FactoryBot.create(:study_extended_metadata_type_for_MIAPPE)
+    assert_equal 'MIAPPE metadata v1.1', extended_metadata_type.title, 'must match the seed data title'
     studies_file = ContentBlob.new
     studies_file.tmp_io_object = File.open("#{Rails.root}/tmp/#{user_uuid}_studies_upload/#{@studies.first.name}")
     studies_file.original_filename = @studies.first.name.to_s