Batch sample import activities (#971)

* add metadata parsing

* rework empty metadata handling

* activities for projects and groups

* Fix group sample activity

* fix wrong usage of include_activity in workflow completion service

* update keys

* fix duplication in activity component

* removed unneeded parent class

* try updating sapporo build instructions

* undo sapporo change

* add tests

* fix test name clash

* update activity counts

* fix modules

app/components/activities/groups/sample_activity_component.html.erb

@@ -0,0 +1,15 @@
+<span>
+  <% if import_samples_action %>
+    <%= t(
+      "#{@activity[:key]}",
+      user: @activity[:user],
+      href:
+          link_to(
+            @activity[:imported_samples_count],
+            "#",
+            class: "text-slate-800 dark:text-slate-300 font-medium cursor-not-allowed",
+            disabled: true,
+          ),
+    ) %>
+  <% end %>
+</span>

app/components/activities/groups/sample_activity_component.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Activities
+  module Groups
+    # Component for rendering an group sample activity
+    class SampleActivityComponent < Component
+      def initialize(activity: nil)
+        @activity = activity
+      end
+
+      def import_samples_action
+        @activity[:action] == 'import_samples'
+      end
+    end
+  end
+end

app/components/activities/list_item_component.html.erb

@@ -34,6 +34,8 @@
         ) %>
       <% elsif group_link_action %>
         <%= render Activities::NamespaceGroupLinkActivityComponent.new(activity: activity) %>
+      <% elsif sample_action %>
+        <%= render Activities::Groups::SampleActivityComponent.new(activity: activity) %>
       <% else %>
         <%= render Activities::GroupActivityComponent.new(activity: activity) %>
       <% end %>

app/components/activities/list_item_component.rb

@@ -36,7 +36,7 @@ def sample_clone_action
 
     def sample_action
       %w[sample_create sample_update attachment_create attachment_destroy
-         metadata_update sample_destroy sample_destroy_multiple].include?(@activity[:action])
+         metadata_update sample_destroy sample_destroy_multiple import_samples].include?(@activity[:action])
     end
 
     def sample_transfer_action

app/components/activities/projects/sample_activity_component.html.erb

@@ -6,6 +6,14 @@
             class: "text-slate-800 dark:text-slate-300 font-medium cursor-not-allowed",
             disabled: true
           ) %>
+ <% elsif import_samples_action %>
+    <% href = link_to(
+          @activity[:imported_samples_count],
+          "#",
+          class:
+            "text-slate-800 dark:text-slate-300 font-medium cursor-not-allowed",
+          disabled: true,
+        ) %>
   <% else %>
     <% if sample_exists(@activity[:sample]) %>
       <% url =

app/components/activities/projects/sample_activity_component.rb

@@ -18,6 +18,10 @@ def sample_exists(sample)
       def samples_tab
         @activity[:action] == 'metadata_update' ? 'metadata' : ''
       end
+
+      def import_samples_action
+        @activity[:action] == 'import_samples'
+      end
     end
   end
 end

app/models/concerns/track_activity.rb

@@ -208,6 +208,7 @@ def transfer_activity_parameters(params, activity) # rubocop:disable Metrics/Abc
   def namespace_project_sample_activity_parameters(params, activity)
     params = add_sample_activity_params(params, activity)
     params = add_metadata_template_params(params, activity)
+    params = add_samples_import_params(params, activity)
     add_bulk_sample_params(params, activity)
   end
 
@@ -234,6 +235,14 @@ def add_metadata_template_params(params, activity)
     )
   end
 
+  def add_samples_import_params(params, activity)
+    return params unless activity.parameters[:action] == 'import_samples'
+
+    params.merge(
+      imported_samples_count: activity.parameters[:imported_samples_count]
+    )
+  end
+
   def add_bulk_sample_params(params, activity)
     return params unless activity.parameters[:action] == 'sample_destroy_multiple'
 
@@ -249,6 +258,7 @@ def format_created_at(created_at)
 
   def additional_group_activity_params(params, activity) # rubocop:disable Metrics/AbcSize
     params = add_metadata_template_params(params, activity)
+    params = add_samples_import_params(params, activity)
 
     if activity.parameters[:action] == 'group_subgroup_destroy'
       params.merge!({ removed_group_puid: activity.parameters[:removed_group_puid] })

app/services/samples/batch_file_import_service.rb

@@ -10,6 +10,8 @@ def initialize(namespace, user = nil, blob_id = nil, params = {})
       @project_puid_column = params[:project_puid_column]
       @sample_description_column = params[:sample_description_column]
       required_headers = [@sample_name_column, @project_puid_column]
+      @project_samples_count = {}
+      @project_puid_map = {}
       super(namespace, user, blob_id, required_headers, 0, params)
     end
 
@@ -52,6 +54,9 @@ def perform_file_import # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
         response[sample_name] = process_sample_row(sample_name, project, description, metadata)
       end
       cleanup_files
+
+      create_activities unless @project_samples_count.empty?
+
       response
     end
 
@@ -105,20 +110,21 @@ def process_metadata_row(data)
     end
 
     def process_sample_row(name, project, description, metadata)
-      sample_params = { name:, description: }
+      sample_params = { name:, description:, include_activity: false }
       sample = Samples::CreateService.new(current_user, project, sample_params).execute
 
       if sample.persisted?
         Metadata::UpdateService.new(
-          sample.project, sample, current_user, { 'metadata' => metadata }
+          sample.project, sample, current_user, { 'metadata' => metadata, include_activity: false }
         ).execute
 
+        increment_sample_count(project)
+
         sample
       else
         sample.errors.map do |error|
           {
-            path: ['sample', error.attribute.to_s.camelize(:lower)],
-            message: error.message
+            path: ['sample', error.attribute.to_s.camelize(:lower)], message: error.message
           }
         end
       end
@@ -132,5 +138,41 @@ def group_namespace_projects
         }
       )
     end
+
+    def create_activities # rubocop:disable Metrics/MethodLength
+      total_sample_count = 0
+      @project_samples_count.each do |project_puid, sample_count|
+        @project_puid_map[project_puid].namespace.create_activity(
+          key: 'namespaces_project_namespace.import_samples.create',
+          owner: current_user,
+          parameters:
+          {
+            imported_samples_count: sample_count,
+            action: 'import_samples'
+          }
+        )
+        total_sample_count += sample_count
+      end
+
+      return unless @namespace.group_namespace? && total_sample_count.positive?
+
+      @namespace.create_activity key: 'group.import_samples.create',
+                                 owner: current_user,
+                                 parameters:
+                                 {
+                                   imported_samples_count: total_sample_count,
+                                   action: 'import_samples'
+                                 }
+    end
+
+    def increment_sample_count(project)
+      project_puid = project.puid
+      @project_samples_count[project_puid] = if @project_samples_count.key?(project_puid)
+                                               @project_samples_count[project_puid] + 1
+                                             else
+                                               @project_puid_map[project_puid] = project
+                                               1
+                                             end
+    end
   end
 end

app/services/samples/create_service.rb

@@ -8,21 +8,22 @@ class CreateService < BaseService
     def initialize(user = nil, project = nil, params = {})
       super(user, params)
       @project = project
-      @sample = Sample.new(params.merge(project_id: project&.id))
+      @include_activity = params.key?(:include_activity) ? params[:include_activity] : true
+      @sample = Sample.new(params.merge(project_id: project&.id).except(:include_activity))
     end
 
     def execute
       authorize! @project, to: :create_sample? unless @project.nil?
 
-      if sample.save
+      if sample.save && @include_activity
         @project.namespace.create_activity key: 'namespaces_project_namespace.samples.create',
                                            owner: current_user,
                                            parameters:
-                                            {
-                                              sample_id: sample.id,
-                                              sample_puid: sample.puid,
-                                              action: 'sample_create'
-                                            }
+                                             {
+                                               sample_id: sample.id,
+                                               sample_puid: sample.puid,
+                                               action: 'sample_create'
+                                             }
       end
 
       update_samples_count if @project.parent.type == 'Group'

app/services/samples/metadata/update_service.rb

@@ -15,7 +15,7 @@ def initialize(project, sample, user = nil, params = {})
         @metadata = params['metadata']
         @analysis_id = params['analysis_id']
         @metadata_changes = { added: [], updated: [], deleted: [], not_updated: [], unchanged: [] }
-        @include_activity = params.key?('include_activity') ? params['include_activity'] : true
+        @include_activity = params.key?(:include_activity) ? params[:include_activity] : true
       end
 
       def execute # rubocop:disable Metrics/MethodLength, Metrics/AbcSize

app/services/workflow_executions/completion_service.rb

@@ -138,7 +138,7 @@ def merge_metadata_onto_samples
         params = {
           'metadata' => swe.metadata,
           'analysis_id' => @workflow_execution.id,
-          'include_activity' => false
+          include_activity: false
         }
         Samples::Metadata::UpdateService.new(
           swe.sample.project, swe.sample, current_user, params

config/locales/en.yml

@@ -247,6 +247,8 @@ en:
     group:
       create_html: "%{user} created group"
       destroy_html: "%{user} removed group"
+      import_samples:
+        create_html: "%{user} imported %{href} samples to projects within this group"
       member:
         create_html: "%{user} added member %{href} to group"
         destroy_html: "%{user} removed member %{href} from group"
@@ -278,6 +280,8 @@ en:
     namespaces_project_namespace:
       create_html: "%{user} created project"
       destroy_html: "%{user} removed project"
+      import_samples:
+        create_html: "%{user} imported %{href} samples to project"
       member:
         create_html: "%{user} added member %{href} to project"
         destroy_html: "%{user} removed member %{href} from project"

config/locales/fr.yml

@@ -247,6 +247,8 @@ fr:
     group:
       create_html: "%{user} created group"
       destroy_html: "%{user} removed group"
+      import_samples:
+        create_html: "%{user} imported %{href} samples to projects within this group"
       member:
         create_html: "%{user} added member %{href} to group"
         destroy_html: "%{user} removed member %{href} from group"
@@ -278,6 +280,8 @@ fr:
     namespaces_project_namespace:
       create_html: "%{user} created project"
       destroy_html: "%{user} removed project"
+      import_samples:
+        create_html: "%{user} imported %{href} samples to project"
       member:
         create_html: "%{user} added member %{href} to project"
         destroy_html: "%{user} removed member %{href} from project"

test/components/activity_component_test.rb

@@ -13,9 +13,9 @@ class ActivityComponentTest < ViewComponentTestCase
     ).reverse
 
     # Limit set to 10 per page
-    @pagy = Pagy.new(count: 12, page: 1, limit: 10)
+    @pagy = Pagy.new(count: 13, page: 1, limit: 10)
 
-    assert_equal 12, @activities.length
+    assert_equal 13, @activities.length
     assert_equal 3, @project2_activities.length
 
     assert_equal(1, @activities.count { |activity| activity[:key].include?('project_namespace.create') })
@@ -64,9 +64,9 @@ class ActivityComponentTest < ViewComponentTestCase
     @activities = group.human_readable_activity(group.retrieve_group_activity).reverse
 
     # Limit set to 10 per page
-    @pagy = Pagy.new(count: 8, page: 1, limit: 10)
+    @pagy = Pagy.new(count: 9, page: 1, limit: 10)
 
-    assert_equal 8, @activities.length
+    assert_equal 9, @activities.length
 
     assert_equal(1, @activities.count { |activity| activity[:key].include?('group.create') })
     assert_equal(1, @activities.count { |activity| activity[:key].include?('group.subgroups.create') })
@@ -75,6 +75,7 @@ class ActivityComponentTest < ViewComponentTestCase
     assert_equal(1, @activities.count { |activity| activity[:key].include?('group.projects.transfer_in') })
     assert_equal(1, @activities.count { |activity| activity[:key].include?('group.member.create') })
     assert_equal(1, @activities.count { |activity| activity[:key].include?('group.namespace_group_link.create') })
+    assert_equal(1, @activities.count { |activity| activity[:key].include?('group.import_samples.create') })
 
     render_inline ActivityComponent.new(activities: @activities, pagy: @pagy)
 

test/components/groups/sample_activity_component_test.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'view_component_test_case'
+
+module Groups
+  class SampleActivityComponentTest < ViewComponentTestCase
+    include ActionView::Helpers::SanitizeHelper
+
+    setup do
+      @user = users(:john_doe)
+    end
+
+    test 'batch sample import actvity' do
+      group = groups(:group_one)
+      activities = group.human_readable_activity(group.retrieve_group_activity).reverse
+
+      assert_equal(1, activities.count do |activity|
+        activity[:key].include?('group.import_samples.create')
+      end)
+
+      activity_to_render = activities.find do |a|
+        a[:key] == 'activity.group.import_samples.create_html'
+      end
+
+      render_inline Activities::Groups::SampleActivityComponent.new(activity: activity_to_render)
+
+      assert_text strip_tags(
+        I18n.t('activity.group.import_samples.create_html',
+               user: 'System',
+               href: 2)
+      )
+      assert_selector 'a',
+                      text: 2
+    end
+  end
+end

test/components/projects/sample_activity_component_test.rb

@@ -250,5 +250,28 @@ class SampleActivityComponentTest < ViewComponentTestCase
       assert_selector 'a[disabled="disabled"]',
                       text: sample.puid
     end
+
+    test 'batch sample import actvity' do
+      project_namespace = namespaces_project_namespaces(:project1_namespace)
+      activities = project_namespace.human_readable_activity(project_namespace.retrieve_project_activity).reverse
+
+      assert_equal(1, activities.count do |activity|
+        activity[:key].include?('project_namespace.import_samples.create')
+      end)
+
+      activity_to_render = activities.find do |a|
+        a[:key] == 'activity.namespaces_project_namespace.import_samples.create_html'
+      end
+
+      render_inline Activities::Projects::SampleActivityComponent.new(activity: activity_to_render)
+
+      assert_text strip_tags(
+        I18n.t('activity.namespaces_project_namespace.import_samples.create_html',
+               user: 'System',
+               href: 2)
+      )
+      assert_selector 'a',
+                      text: 2
+    end
   end
 end