Merge pull request #2142 from broadinstitute/jb-anndata-cluster-ext-link

Allow providing external link for AnnData clusters (SCP-5816)

app/controllers/api/v1/study_files_controller.rb

@@ -749,7 +749,8 @@ def study_file_params
           heatmap_file_info_attributes: [:id, :_destroy, :custom_scaling, :color_min, :color_max, :legend_label],
           cluster_form_info_attributes: [
             :_id, :name, :obsm_key_name, :description, :x_axis_label, :y_axis_label, :x_axis_min, :x_axis_max,
-            :y_axis_min, :y_axis_max, :z_axis_min, :z_axis_max, spatial_cluster_associations: []
+            :y_axis_min, :y_axis_max, :z_axis_min, :z_axis_max, :external_link_url, :external_link_title,
+            :external_link_description, spatial_cluster_associations: []
           ],
           metadata_form_info_attributes: [:_id, :use_metadata_convention, :description],
           extra_expression_form_info_attributes: [:_id, :taxon_id, :description, :y_axis_label],

app/controllers/api/v1/visualization/clusters_controller.rb

@@ -256,11 +256,7 @@ def self.get_cluster_viz_data(study, cluster, url_params)
             customColors: custom_annotation_colors,
             clusterFileId: cluster.study_file_id.to_s,
             isSplitLabelArrays: is_split_label_arrays,
-            externalLink: {
-              url: cluster.study_file[:external_link_url],
-              title: cluster.study_file[:external_link_title],
-              description: cluster.study_file[:external_link_description]
-            }
+            externalLink: get_cluster_external_link(cluster, cluster.study_file)
           }
         end
 
@@ -276,6 +272,19 @@ def self.get_selected_subsample_threshold(param, cluster)
           end
           subsample
         end
+
+        def self.get_cluster_external_link(cluster, study_file)
+          if study_file.is_viz_anndata?
+            data = study_file.ann_data_file_info.find_fragment(data_type: :cluster, name: cluster.name)
+          else
+            data = study_file.attributes
+          end
+          {
+            url: data[:external_link_url],
+            title: data[:external_link_title],
+            description: data[:external_link_description]
+          }
+        end
       end
     end
   end

app/models/ann_data_file_info.rb

@@ -15,9 +15,10 @@ class AnnDataFileInfo
   DATA_FRAGMENT_PARAMS = {
     cluster: %i[
       _id data_type name description obsm_key_name x_axis_label y_axis_label x_axis_min x_axis_max y_axis_min
-      y_axis_max z_axis_min z_axis_max taxon_id parse_status spatial_cluster_associations
+      y_axis_max z_axis_min z_axis_max external_link_url external_link_title external_link_description
+      parse_status spatial_cluster_associations
     ],
-    expression: %i[_id data_type taxon_id description expression_file_info]
+    expression: %i[_id data_type taxon_id description expression_file_info y_axis_label]
   }.freeze
 
   # required keys for data_fragments, by type
@@ -73,21 +74,16 @@ def merge_form_data(form_data)
       fragment_form = merged_data[form_segment_name]
       next if fragment_form.blank? || fragment_form.empty?
 
+      allowed_params = DATA_FRAGMENT_PARAMS[key]&.reject {|k, _| k == :data_type }
       case key
       when :metadata
         merged_data[:use_metadata_convention] = fragment_form[:use_metadata_convention]
       when :cluster
-        fragments << extract_form_fragment(
-          fragment_form, key,
-          :_id, :name, :description, :obsm_key_name, :x_axis_label, :y_axis_label, :x_axis_min, :x_axis_max,
-          :y_axis_min, :y_axis_max, :z_axis_min, :z_axis_max, :spatial_cluster_associations
-        )
+        fragments << extract_form_fragment(fragment_form, key, *allowed_params)
       when :expression
         merged_data[:taxon_id] = fragment_form[:taxon_id]
         merged_exp_fragment = fragment_form.merge(expression_file_info: merged_data[:expression_file_info_attributes])
-        fragments << extract_form_fragment(
-          merged_exp_fragment, key, :_id, :description, :y_axis_label, :taxon_id, :expression_file_info
-        )
+        fragments << extract_form_fragment(merged_exp_fragment, key, *allowed_params)
       end
       # remove from form data once processed to allow normal save of nested form data
       merged_data.delete(form_segment_name)

test/api/visualization/clusters_controller_test.rb

@@ -20,7 +20,10 @@ class ClustersControllerTest < ActionDispatch::IntegrationTest
                                                      cells: ['A', 'B', 'C']
                                                   },
                                                   annotation_input: [{name: 'foo', type: 'group', values: ['bar', 'bar', 'baz']},
-                                                                     {name: 'intensity', type: 'numeric', values: [1, 2, 5]}])
+                                                                     {name: 'intensity', type: 'numeric', values: [1, 2, 5]}],
+                                                  external_link_url: 'https://example.com',
+                                                  external_link_title: 'Example link',
+                                                  external_link_description: 'This is a link')
 
     @basic_study_metadata_file = FactoryBot.create(:metadata_file,
                                                    name: 'metadata.txt',
@@ -101,7 +104,7 @@ class ClustersControllerTest < ActionDispatch::IntegrationTest
       "subsample"=>"all",
       "isSplitLabelArrays"=>false,
       "consensus"=>nil,
-      "externalLink"=>{"url"=>nil, "title"=>nil, "description"=>nil}
+      "externalLink"=>{"url"=>"https://example.com", "title"=>"Example link", "description"=>"This is a link"}
     }, json)
   end
 
@@ -197,4 +200,37 @@ class ClustersControllerTest < ActionDispatch::IntegrationTest
     ))
     assert_response :bad_request
   end
+
+  test 'should get external links' do
+    sign_in_and_update @user
+    study = FactoryBot.create(:detached_study,
+                              name_prefix: 'AnnData Cluster Link Study',
+                              user: @user,
+                              test_array: @@studies_to_clean)
+    study_file = FactoryBot.create(:ann_data_file,
+                                   name: 'data.h5ad',
+                                   study:,
+                                   reference_file: false,
+                                   cell_input: %w[A B C D],
+                                   annotation_input: [
+                                     { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] }
+                                   ],
+                                   coordinate_input: [
+                                     { tsne: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
+                                   ])
+    study_file.reload
+    fragment = study_file.ann_data_file_info.find_fragment(data_type: :cluster, name: 'tsne')
+    fragment[:external_link_url] = 'https://foo.com'
+    fragment[:external_link_title] = 'Foo'
+    fragment[:external_link_description] = 'This is foo'
+    frag_idx = study_file.ann_data_file_info.fragment_index_of(fragment)
+    study_file.ann_data_file_info.data_fragments[frag_idx] = fragment
+    study_file.save!
+    execute_http_request(:get, api_v1_study_cluster_path(study, 'tsne'))
+    assert_response :success
+    link = json.with_indifferent_access[:externalLink]
+    assert_equal 'https://foo.com', link[:url]
+    assert_equal 'Foo', link[:title]
+    assert_equal 'This is foo', link[:description]
+  end
 end

test/models/ann_data_file_info_test.rb

@@ -59,7 +59,10 @@ def generate_id
           x_axis_min: '-10',
           x_axis_max: '10',
           y_axis_min: '-10',
-          y_axis_max: '10'
+          y_axis_max: '10',
+          external_link_url: 'https://example.com',
+          external_link_title: 'Example Link',
+          external_link_description: 'This is an external link'
       }
     }
     merged_data = AnnDataFileInfo.new.merge_form_data(form_params)
@@ -71,6 +74,9 @@ def generate_id
     assert_equal 'x axis', cluster_fragment[:x_axis_label]
     assert_equal '10', cluster_fragment[:x_axis_max]
     assert_equal 'cluster description', cluster_fragment[:description]
+    assert_equal 'https://example.com', cluster_fragment[:external_link_url]
+    assert_equal 'Example Link', cluster_fragment[:external_link_title]
+    assert_equal 'This is an external link', cluster_fragment[:external_link_description]
     expr_fragment = merged_data.dig(root_form_key, :data_fragments).detect { |f| f[:data_type] == :expression }
     assert_equal 'expression description', expr_fragment[:description]
     assert_equal 'log(TPM) expression', expr_fragment[:y_axis_label]

test/models/delete_queue_job_test.rb

@@ -188,15 +188,15 @@ class DeleteQueueJobTest < ActiveSupport::TestCase
                                      { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] }
                                    ],
                                    coordinate_input: [
-                                     { x_tsne: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
+                                     { tsne: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
                                    ])
-    study.update(default_options: { cluster: 'x_tsne', annotation: 'disease--group--study' })
+    study.update(default_options: { cluster: 'tsne', annotation: 'disease--group--study' })
     study.reload
     assert_equal 1, study.cluster_groups.size
     assert_equal 1, study.cell_metadata.size
     assert_equal %w[A B C D], study.all_cells_array
     assert_equal study_file, study.metadata_file
-    assert_equal 'x_tsne', study.default_cluster.name
+    assert_equal 'tsne', study.default_cluster.name
     mock = Minitest::Mock.new
     mock.expect(:get_workspace_files, [], [String, Hash])
     ApplicationController.stub :firecloud_client, mock do

test/models/ingest_job_test.rb

@@ -225,7 +225,7 @@ class IngestJobTest < ActiveSupport::TestCase
                                         { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] }
                                       ],
                                       coordinate_input: [
-                                        { X_umap: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
+                                        { umap: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
                                       ],
                                       expression_input: {
                                         'phex' => [['A', 0.3], ['B', 1.0], ['C', 0.5], ['D', 0.1]]
@@ -774,7 +774,7 @@ class IngestJobTest < ActiveSupport::TestCase
                                      { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] }
                                    ],
                                    coordinate_input: [
-                                     { x_tsne: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
+                                     { tsne: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
                                    ],
                                    expression_input: {
                                      'phex' => [['A', 0.3], ['B', 1.0], ['C', 0.5], ['D', 0.1]]
@@ -815,7 +815,7 @@ class IngestJobTest < ActiveSupport::TestCase
                                      { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] }
                                    ],
                                    coordinate_input: [
-                                     { X_umap: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
+                                     { umap: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } }
                                    ])
     annotation_file = 'gs://test_bucket/anndata/h5ad_frag.metadata.tsv'
     cluster_file = 'gs://test_bucket/anndata/h5ad_frag.cluster.X_umap.tsv'

test/services/cluster_viz_service_test.rb

@@ -245,7 +245,7 @@ class ClusterVizServiceTest < ActiveSupport::TestCase
   end
 
   test 'should retrieve clustering information for AnnData files' do
-    cluster_name = 'x_tsne'
+    cluster_name = 'tsne'
     study = FactoryBot.create(:detached_study,
                               name_prefix: 'AnnData Cluster Test',
                               user: @user,