[ENHC0010020] Delete multiple samples from project samples table (#629)

* start delete multiple samples

* Add tests

* Fix validation and error handling for invalid sample ids

* cleanup

* Fix confirmation dialog

* add fr translations

* remove print statement

* fix turbo_stream updates for destroy samples

* remove unnecessary view

* Add single remove functionality to remove selected samples

* start implementing infinite scroll controller to delete multiple sample dialog

* cleanup

* change destructive button using tailwind.css file to actual tailwind classes

* cleanup single delete controller

* move single delete controller logic to use selection controller

* Fix files deletion

* Add infinite scroll description using singular/plural forms for selection count

* Add and fix tests

* refactor destroy_multiple to remove rubocop disables

* revert to tailwind.css classes for action-link button

* Improve sample query

* update turbo stream replace in destroy.turbo_stream view

* Add additional tests

* add fr translations

* Change testing logic to potentially prevent future flaky tests

* further fix tests, add additional asserts

* start changing over multi delete to different controller, add multi delete service

* Continue refactoring multiple delete

* test routes

* test logic fix

* Test different turbo streams for destroyu

* Revert "Test different turbo streams for destroyu"

This reverts commit b862f12.

* Revert "test logic fix"

This reverts commit 5b773b1.

* Revert "test routes"

This reverts commit 0df6153.

* Revert "Continue refactoring multiple delete"

This reverts commit cf31823.

* Revert "start changing over multi delete to different controller, add multi delete service"

This reverts commit 3eaf059.

* Add multiple delete service for samples

* fix test

* Revert "fix test"

This reverts commit e1b7068.

* Revert "Add multiple delete service for samples"

This reverts commit 51911c2.

* Revert "Revert "start changing over multi delete to different controller, add multi delete service""

This reverts commit 9469e1b.

* Revert "Revert "Continue refactoring multiple delete""

This reverts commit 8646591.

* Revert "Revert "test routes""

This reverts commit 5fd6a2c.

* Revert "Revert "test logic fix""

This reverts commit 37f026f.

* Revert "Revert "Test different turbo streams for destroyu""

This reverts commit 9dfb191.

* revert back to new controller, remove all old code from old controller/views

* attempt to fix from show page

* revert work prior to show page work

* Further revert changes

* Fix rubocop unhappiness

* cleanup and add fr translations

* fix up deletions controller test

* Cleanup, add tests for multiple destroy service

* Fix translations in samples testg

* update multiple service test to use multiple samples for metadata summary test

* move assert to prevent flaky testing

* add before_action to get partial name for new action

* change return if for def sample, fix def destroy block

* fix test associated with previous commit

* move multiple destroy service into original destroy service

* cleanup, fix tests

* Change destroy service to use params to specify between single sample and multiple sample ids

* remove @ in destroy service

* remove return from destroy_single

* test removing unnecessary assertion to fix flaky test

* test fixing flaky test

* fix tests

* fix test

app/components/samples/table_component.html.erb

@@ -185,10 +185,9 @@
                 <% if @row_actions[:destroy] %>
                   <%= link_to(
                     t(:"projects.samples.index.remove_button"),
-                    project_sample_path(sample.project, sample),
+                    new_namespace_project_samples_deletion_path(sample_id: sample.id, deletion_type: 'single'),
                     data: {
-                      turbo_method: :delete,
-                      turbo_confirm: t(:"projects.samples.index.remove_button_confirmation"),
+                      "turbo-prefetch": false,
                     },
                     class:
                       "font-medium text-blue-600 underline dark:text-blue-500 hover:no-underline cursor-pointer",

app/controllers/projects/samples/attachments_controller.rb

@@ -5,6 +5,7 @@ module Samples
     # Controller actions for Project Samples Attachments
     class AttachmentsController < Projects::Samples::ApplicationController
       before_action :attachment, only: %i[destroy]
+      before_action :new_destroy_params, only: %i[new_destroy]
 
       def new
         authorize! @project, to: :update_sample?
@@ -38,6 +39,15 @@ def create
         end
       end
 
+      def new_destroy
+        authorize! @sample, to: :destroy_attachment?
+        render turbo_stream: turbo_stream.update('sample_modal',
+                                                 partial: 'delete_attachment_modal',
+                                                 locals: {
+                                                   open: true
+                                                 }), status: :ok
+      end
+
       def destroy # rubocop:disable Metrics/MethodLength
         authorize! @sample, to: :destroy_attachment?
 
@@ -70,6 +80,11 @@ def attachment
         @attachment = @sample.attachments.find_by(id: params[:id]) || not_found
       end
 
+      def new_destroy_params
+        @attachment = Attachment.find_by(id: params[:attachment_id])
+        @sample = @attachment.attachable
+      end
+
       def destroy_status(attachment, count)
         return count == 2 ? :ok : :multi_status if attachment.associated_attachment
 

app/controllers/projects/samples/deletions_controller.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+module Projects
+  module Samples
+    # Controller actions for Project Samples Deletions
+    class DeletionsController < Projects::ApplicationController
+      before_action :sample, only: %i[new destroy]
+      before_action :new_dialog_partial, only: :new
+
+      def new
+        authorize! @project, to: :destroy_sample?
+        render turbo_stream: turbo_stream.update('samples_dialog',
+                                                 partial: @partial,
+                                                 locals: {
+                                                   open: true
+                                                 }), status: :ok
+      end
+
+      def destroy # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
+        ::Samples::DestroyService.new(@project, current_user, { sample: @sample }).execute
+
+        respond_to do |format|
+          if @sample.deleted?
+            format.html do
+              flash[:success] = t('.success', sample_name: @sample.name, project_name: @project.namespace.human_name)
+              redirect_to namespace_project_samples_path(format: :html)
+            end
+            format.turbo_stream do
+              render status: :ok, locals: { type: 'success',
+                                            message: t('.success', sample_name: @sample.name,
+                                                                   project_name: @project.namespace.human_name) }
+            end
+          else
+            format.turbo_stream do
+              render status: :unprocessable_entity,
+                     locals: { type: 'alert', message: @sample.errors.full_messages.first }
+            end
+          end
+        end
+      end
+
+      def destroy_multiple
+        samples_to_delete_count = destroy_multiple_params['sample_ids'].count
+
+        deleted_samples_count = ::Samples::DestroyService.new(@project, current_user, destroy_multiple_params).execute
+
+        # No selected samples deleted
+        if deleted_samples_count.zero?
+          render status: :unprocessable_entity, locals: { type: :error, message: t('.no_deleted_samples') }
+        # Partial sample deletion
+        elsif deleted_samples_count.positive? && deleted_samples_count != samples_to_delete_count
+          render status: :multi_status,
+                 locals: { messages: get_multi_status_destroy_multiple_message(deleted_samples_count,
+                                                                               samples_to_delete_count) }
+        # All samples deleted successfully
+        else
+          render status: :ok, locals: { type: :success, message: t('.success') }
+        end
+      end
+
+      private
+
+      def sample
+        # Necessary return for new when deletion_type = 'multiple', as has no params[:sample_id] defined
+        return if params[:deletion_type] == 'multiple'
+
+        @sample = Sample.find_by(id: params[:id] || params[:sample_id], project_id: project.id) || not_found
+      end
+
+      def new_dialog_partial
+        @partial = params['deletion_type'] == 'single' ? 'new_deletion_dialog' : 'new_multiple_deletions_dialog'
+      end
+
+      def destroy_multiple_params
+        params.require(:multiple_deletion).permit(sample_ids: [])
+      end
+
+      def get_multi_status_destroy_multiple_message(deleted_samples_count, samples_to_delete_count)
+        [
+          { 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}") }
+        ]
+      end
+    end
+  end
+end

app/controllers/projects/samples_controller.rb

@@ -5,9 +5,9 @@ module Projects
   class SamplesController < Projects::ApplicationController # rubocop:disable Metrics/ClassLength
     include Metadata
 
-    before_action :sample, only: %i[show edit update destroy view_history_version]
+    before_action :sample, only: %i[show edit update view_history_version]
     before_action :current_page
-    before_action :set_search_params, only: %i[index destroy]
+    before_action :set_search_params, only: %i[index]
     before_action :set_metadata_fields, only: :index
 
     def index
@@ -78,37 +78,6 @@ def update
       end
     end
 
-    def destroy # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
-      ::Samples::DestroyService.new(@sample, current_user).execute
-      @pagy, @samples = pagy(load_samples)
-      @q = load_samples.ransack(params[:q])
-
-      if @sample.deleted?
-        respond_to do |format|
-          format.html do
-            flash[:success] = t('.success', sample_name: @sample.name, project_name: @project.namespace.human_name)
-            redirect_to namespace_project_samples_path(format: :html)
-          end
-          format.turbo_stream do
-            fields_for_namespace(
-              namespace: @project.namespace,
-              show_fields: params[:q] && params[:q][:metadata].to_i == 1
-            )
-            render status: :ok, locals: { type: 'success',
-                                          message: t('.success', sample_name: @sample.name,
-                                                                 project_name: @project.namespace.human_name) }
-          end
-        end
-      else
-        respond_to do |format|
-          format.turbo_stream do
-            render status: :unprocessable_entity,
-                   locals: { type: 'alert', message: @sample.errors.full_messages.first }
-          end
-        end
-      end
-    end
-
     def select
       authorize! @project, to: :sample_listing?
       @samples = []

app/javascript/controllers/infinite_scroll_controller.js

@@ -6,7 +6,9 @@ export default class extends Controller {
   static targets = ["all", "pageForm", "pageFormContent", "scrollable", "summary"];
   static values = {
     fieldName: String,
-    pagedFieldName: String
+    pagedFieldName: String,
+    singular: String,
+    plural: String
   }
 
   #page = 1;
@@ -27,12 +29,18 @@ export default class extends Controller {
     }
   }
 
-  #replaceCountPlaceholder(){
-    const summary = this.summaryTarget;
-    summary.textContent = summary.textContent.replace(
-      "COUNT_PLACEHOLDER",
-      this.selectionOutlet.getNumSelected(),
-    );
+  #replaceCountPlaceholder() {
+    const numSelected = this.selectionOutlet.getNumSelected()
+    let summary = this.summaryTarget;
+
+    if (numSelected == 1) {
+      summary.textContent = this.singularValue
+    } else {
+      summary.textContent = this.pluralValue.replace(
+        "COUNT_PLACEHOLDER",
+        numSelected
+      )
+    }
   }
 
   #makePagedHiddenInputs() {
@@ -41,7 +49,7 @@ export default class extends Controller {
     const end = this.#page * itemsPerPage;
     const ids = this.allIds.slice(start, end);
 
-    if(ids && ids.length){
+    if (ids && ids.length) {
       const fragment = document.createDocumentFragment();
       for (const id of ids) {
         fragment.appendChild(
@@ -69,7 +77,7 @@ export default class extends Controller {
     this.allTarget.appendChild(fragment);
   }
 
-  clear(){
+  clear() {
     this.selectionOutlet.clear();
   }
 }

app/javascript/controllers/selection_controller.js

@@ -45,7 +45,6 @@ export default class extends Controller {
   }
 
   remove({ params: { id } }) {
-    id = JSON.stringify(id).replaceAll(",", ", ");
     this.#addOrRemove(false, id);
   }
 

app/services/samples/destroy_service.rb

@@ -3,18 +3,43 @@
 module Samples
   # Service used to Delete Samples
   class DestroyService < BaseService
-    attr_accessor :sample
+    attr_accessor :sample, :sample_ids, :project
 
-    def initialize(sample, user = nil, params = {})
-      super(user, params.except(:sample, :id))
-      @sample = sample
+    def initialize(project, user = nil, params = {})
+      super(user, params)
+      @project = project
+      @sample = params[:sample] if params[:sample]
+      @sample_ids = params[:sample_ids] if params[:sample_ids]
     end
 
     def execute
-      authorize! sample.project, to: :destroy_sample?
+      authorize! project, to: :destroy_sample?
 
+      sample.nil? ? destroy_multiple : destroy_single
+    end
+
+    private
+
+    def destroy_single
       sample.destroy
 
+      update_metadata_summary(sample)
+    end
+
+    def destroy_multiple
+      samples = Sample.where(id: sample_ids).where(project_id: project.id)
+      samples_to_delete_count = samples.count
+
+      samples = samples.destroy_all
+
+      samples.each do |sample|
+        update_metadata_summary(sample)
+      end
+
+      samples_to_delete_count
+    end
+
+    def update_metadata_summary(sample)
       sample.project.namespace.update_metadata_summary_by_sample_deletion(sample) if sample.deleted?
     end
   end

app/views/projects/samples/attachments/_attachment.html.erb

@@ -101,22 +101,12 @@
     <% if allowed_to?(:destroy_attachment?, @sample) %>
       <%= link_to(
         t(".delete"),
-        namespace_project_sample_attachment_path(
+        namespace_project_sample_attachment_new_destroy_path(
           sample_id: @sample.id,
-          id: attachment.id
+          attachment_id: attachment.id
         ),
         data: {
-          turbo_method: :delete,
-          action: "turbo:submit-end->selection#remove",
-          "selection-id-param":
-            (
-              if attachment.associated_attachment
-                [attachment.id, attachment.associated_attachment.id]
-              else
-                attachment.id
-              end
-            ),
-          turbo_confirm: t(".delete_confirm")
+          "turbo-prefetch": false,
         },
         class:
           "font-medium text-blue-600 underline dark:text-blue-500 hover:no-underline cursor-pointer"

app/views/projects/samples/attachments/_delete_attachment_modal.html.erb

@@ -0,0 +1,42 @@
+<%= viral_dialog(open: open, size: :large) do |dialog| %>
+  <%= dialog.with_header(title: t(".title")) %>
+  <%= dialog.with_section do %>
+
+    <%= turbo_frame_tag("deletion-alert") %>
+
+    <div
+      data-controller="selection"
+      data-selection-delete-id-param="<%= @attachment.id %>"
+      data-selection-storage-key-value='<%="files-#{@sample.id}" %>'
+      class="mb-4 font-normal text-slate-500 dark:text-slate-400 overflow-x-visible">
+      <p class="mb-4">
+        <%= t(".description") %>
+      </p>
+      <%= form_for(:deletion, url: namespace_project_sample_attachment_path(id: @attachment.id), method: :delete) do |form| %>
+        <%= form.submit t(".submit_button"),
+                    class: "
+                      button
+                      text-sm
+                      px-5
+                      py-2.5
+                      text-white
+                      bg-red-700
+                      border-red-800
+                      focus:outline-none
+                      hover:bg-red-800
+                      focus:ring-red-300
+                      dark:focus:ring-red-700
+                      dark:bg-red-600
+                      dark:text-white
+                      dark:border-red-600
+                      dark:hover:bg-red-700",
+                    data: {
+                      turbo_frame: "_top",
+                      action: "click->selection#remove",
+                      "selection-id-param": @attachment.id
+                    } %>
+        </div>
+      <% end %>
+    </div>
+  <% end %>
+<% end %>