corrections from Sim review

app/components/dossiers/batch_select_more_component.rb

@@ -3,4 +3,33 @@ class Dossiers::BatchSelectMoreComponent < ApplicationComponent
     @dossiers_count = dossiers_count
     @filtered_sorted_ids = filtered_sorted_ids
   end
+
+  def not_selected_button_data
+    {
+      action: "batch-operation#onSelectMore",
+      dossiers: @filtered_sorted_ids.first(Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT).join(',')
+    }
+  end
+
+  def selected_button_data
+    {
+      action: "batch-operation#onDeleteSelection"
+    }
+  end
+
+  def not_selected_text
+    if @dossiers_count <= Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT
+      t(".select_all", dossiers_count: @dossiers_count)
+    else
+      t(".select_all_limit", dossiers_count: @dossiers_count, limit: Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT)
+    end
+  end
+
+  def selected_text
+    if @dossiers_count <= Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT
+      t(".selected_all", dossiers_count: @dossiers_count)
+    else
+      t(".selected_all_limit", limit: Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT)
+    end
+  end
 end

app/components/dossiers/batch_select_more_component/batch_select_more_component.en.yml

@@ -1,7 +1,7 @@
 en:
-  selected_html: All <span id='dynamic_number'>n</span> files from this page are selected.
+  pagination_files_selected_html: All <span id='dynamic_number'>n</span> files from this page are selected.
   select_all: "Select all %{dossiers_count} files."
   select_all_limit: "Select first %{limit} files on %{dossiers_count}"
-  selected_all: "%{dossiers_count} files are selected."
+  selected_all: "All %{dossiers_count} files are selected."
   selected_all_limit: "%{limit} files are selected."
   delete_selection: "Delete selection"

app/components/dossiers/batch_select_more_component/batch_select_more_component.fr.yml

@@ -1,7 +1,7 @@
 fr:
-  selected_html: Les <span id='dynamic_number'>n</span> dossiers de cette page sont sélectionnés.
+  pagination_files_selected_html: Les <span id='dynamic_number'>n</span> dossiers de cette page sont sélectionnés.
-  select_all: "Sélectionner les %{dossiers_count} dossiers."
+  select_all: "Sélectionner tous les %{dossiers_count} dossiers."
   select_all_limit: "Sélectionner les %{limit} premiers dossiers sur les %{dossiers_count}"
-  selected_all: "%{dossiers_count} dossiers sont sélectionnés."
+  selected_all: "Tous les %{dossiers_count} dossiers sont sélectionnés."
   selected_all_limit: "%{limit} dossiers sont sélectionnés."
   delete_selection: "Effacer la sélection"

app/components/dossiers/batch_select_more_component/batch_select_more_component.html.haml

@@ -2,20 +2,14 @@
   %td.fr-py-2w.text-center{ colspan: 100 }
     #not_selected
       %p
-        = t('.selected_html')
+        = t('.pagination_files_selected_html')
-        %button.fr-btn.fr-btn--sm.fr-btn--tertiary-no-outline{ data: { action: "batch-operation#onSelectMore", dossiers: @filtered_sorted_ids.first(Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT).join(',') } }
+        %button.fr-btn.fr-btn--sm.fr-btn--tertiary-no-outline{ data: not_selected_button_data }
-          - if @dossiers_count <= Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT
+          = not_selected_text
-            = t(".select_all", dossiers_count: @dossiers_count)
-          - else
-            = t(".select_all_limit", dossiers_count: @dossiers_count, limit: Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT)
 
     #selected.hidden
       %p
-        - if @dossiers_count <= Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT
+        = selected_text
-          = t(".selected_all", dossiers_count: @dossiers_count)
+        %button.fr-btn.fr-btn--sm.fr-btn--tertiary-no-outline{ data: selected_button_data }
-        - else
-          = t(".selected_all_limit", limit: Instructeurs::ProceduresController::BATCH_SELECTION_LIMIT)
-        %button.fr-btn.fr-btn--sm.fr-btn--tertiary-no-outline{ data: { action: "batch-operation#onDeleteSelection" } }
           = t(".delete_selection")
 
       = hidden_field_tag :"batch_operation[dossier_ids][]", "", form: dom_id(BatchOperation.new), id: dom_id(BatchOperation.new, "input_multiple_ids")

app/controllers/instructeurs/batch_operations_controller.rb

@@ -4,8 +4,7 @@ module Instructeurs
     before_action :ensure_ownership!
 
     def create
-      dossier_ids = batch_operation_params[:dossier_ids].join(',').split(',').uniq
+      batch = BatchOperation.safe_create!(batch_operation_params)
-      batch = BatchOperation.safe_create!(batch_operation_params.merge(dossier_ids: dossier_ids))
       flash[:alert] = "Le traitement de masse n'a pas été lancé. Vérifiez que l'action demandée est possible pour les dossiers sélectionnés" if batch.blank?
       redirect_back(fallback_location: instructeur_procedure_url(@procedure.id))
     end
@@ -15,6 +14,7 @@ module Instructeurs
     def batch_operation_params
       params.require(:batch_operation)
         .permit(:operation, :motivation, :justificatif_motivation, dossier_ids: [])
+        .merge(dossier_ids: params['batch_operation']['dossier_ids'].join(',').split(',').uniq)
         .merge(instructeur: current_instructeur)
     end
 

app/javascript/controllers/batch_operation_controller.ts

@@ -20,7 +20,7 @@ export class BatchOperationController extends ApplicationController {
     this.inputTargets.forEach((e) => (e.checked = target.checked));
     this.toggleSubmitButtonWhenNeeded();
 
-    const pagination = document.querySelector('.pagination');
+    const pagination = document.querySelector('tfoot .pagination');
     if (pagination) {
       displayNotice(this.inputTargets);
     }

spec/system/instructeurs/batch_operation_spec.rb

@@ -92,10 +92,10 @@ describe 'BatchOperation a dossier:', js: true do
       # click on check_all make the notice appear
       find("##{dom_id(BatchOperation.new, :checkbox_all)}").check
       expect(page).to have_selector('#js_batch_select_more')
-      expect(page).to have_content('Les 2 dossiers de cette page sont sélectionnés. Sélectionner les 3 dossiers.')
+      expect(page).to have_content('Les 2 dossiers de cette page sont sélectionnés. Sélectionner tous les 3 dossiers.')
 
       # click on selection link fill checkbox value with dossier_ids
-      click_on("Sélectionner les 3 dossiers")
+      click_on("Sélectionner tous les 3 dossiers")
       expect(page).to have_content('3 dossiers sont sélectionnés. Effacer la sélection ')
       expect(find_field("batch_operation[dossier_ids][]", type: :hidden).value).to eq "#{dossier_3.id},#{dossier_2.id},#{dossier_1.id}"
 
@@ -107,7 +107,7 @@ describe 'BatchOperation a dossier:', js: true do
 
       # click on check_all + notice link and submit
       find("##{dom_id(BatchOperation.new, :checkbox_all)}").check
-      click_on("Sélectionner les 3 dossiers")
+      click_on("Sélectionner tous les 3 dossiers")
 
       expect { click_on "Suivre les dossiers" }
         .to change { BatchOperation.count }