Fix validation and error handling for invalid sample ids

app/controllers/projects/samples_controller.rb

@@ -128,10 +128,10 @@ def destroy_multiple # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
 
       samples_to_delete = get_samples(destroy_multiple_params['sample_ids'])
       samples_to_delete_count = destroy_multiple_params['sample_ids'].count
-
+      deleted_samples = []
       samples_to_delete.each do |sample|
         ::Samples::DestroyService.new(sample, current_user).execute
-        samples_to_delete -= [sample] if sample.deleted?
+        deleted_samples += [sample] if sample.deleted?
       end
 
       @pagy, @samples = pagy(load_samples)
@@ -140,17 +140,24 @@ def destroy_multiple # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
         namespace: @project.namespace,
         show_fields: params[:q] && params[:q][:metadata].to_i == 1
       )
+
       # No selected samples deleted
-      if samples_to_delete.count.positive? && samples_to_delete.count == samples_to_delete_count
-        render status: :unprocessable_entity, locals: { message: nil, not_deleted_samples: samples_to_delete }
+      if deleted_samples.empty?
+        render status: :unprocessable_entity, locals: { type: :error, message: t('.no_deleted_samples') }
       # Partial sample deletion
-      elsif samples_to_delete.count.positive?
-        render status: :multi_status,
-               locals: { type: :success, message: t('.partial_success'), not_deleted_samples: samples_to_delete }
+      elsif deleted_samples.count.positive? && deleted_samples.count != samples_to_delete_count
+        messages = [
+          { type: :success,
+            message: t('.partial_success',
+                       deleted: "#{deleted_samples.count}/#{samples_to_delete_count}") },
+          { type: :error,
+            message: t('.partial_error',
+                       not_deleted: "#{samples_to_delete_count - deleted_samples.count}/#{samples_to_delete_count}") }
+        ]
+        render status: :multi_status, locals: { messages: }
       # All samples deleted successfully
       else
-        render status: :ok,
-               locals: { type: :success, message: t('.success'), not_deleted_samples: nil }
+        render status: :ok, locals: { type: :success, message: t('.success'), not_deleted_samples: nil }
       end
     end
 
@@ -216,7 +223,12 @@ def destroy_multiple_params
     end
 
     def get_samples(sample_ids)
-      sample_ids.map { |sample_id| Sample.find(sample_id) }
+      samples_to_delete = []
+      sample_ids.each do |sample_id|
+        sample = Sample.find_by(id: sample_id)
+        samples_to_delete << sample unless sample.nil?
+      end
+      samples_to_delete
     end
   end
 end

app/views/projects/samples/destroy_multiple.turbo_stream.erb

@@ -6,18 +6,16 @@
   },
 ) %>
 
-<% unless message.nil? %>
-  <%= turbo_stream.append "flashes" do %>
-    <%= viral_flash(type:, data: message) %>
-  <% end %>
-<% end %>
-
-<% unless not_deleted_samples.nil? %>
-  <% not_deleted_samples.each do |sample| %>
+<% if defined?(messages) %>
+  <% messages.each do |m| %>
     <%= turbo_stream.append "flashes" do %>
-      <%= viral_flash(type: :error, data: t(".error", sample_name: sample.name)) %>
+      <%= viral_flash(type: m[:type], data: m[:message]) %>
     <% end %>
   <% end %>
+<% else %>
+  <%= turbo_stream.append "flashes" do %>
+    <%= viral_flash(type:, data: message) %>
+  <% end %>
 <% end %>
 
 <%= turbo_stream.update "project_samples_list",

config/locales/en.yml

@@ -1305,8 +1305,9 @@ en:
           submit_button: Confirm
           title: Delete Samples
       destroy_multiple:
-        error: Sample %{sample_name} was not deleted
-        partial_success: Some samples were successfully deleted
+        no_deleted_samples: Selected samples could not be deleted
+        partial_error: "%{not_deleted} samples could not be deleted"
+        partial_success: "%{deleted} samples were successfully deleted"
         success: Samples were successfully deleted
     workflow_executions:
       cancel:

test/controllers/projects/samples_controller_test.rb

@@ -164,5 +164,23 @@ class SamplesControllerTest < ActionDispatch::IntegrationTest
 
       assert_response :success
     end
+
+    test 'test partially deleting multiple samples' do
+      sample2 = samples(:sample2)
+      sample30 = samples(:sample30)
+      delete destroy_multiple_namespace_project_samples_url(@namespace, @project), params: { multiple_deletion: {
+        sample_ids: [@sample1.id, sample2.id, sample30.id, 'invalid_sample_id']
+      } }, as: :turbo_stream
+
+      assert_response :multi_status
+    end
+
+    test 'test deleting no samples in destroy_multiple ' do
+      delete destroy_multiple_namespace_project_samples_url(@namespace, @project), params: { multiple_deletion: {
+        sample_ids: %w[invalid_sample_id_1 invalid_sample_id_2 invalid_sample_id_3]
+      } }, as: :turbo_stream
+
+      assert_response :unprocessable_entity
+    end
   end
 end