Merge pull request #8308 from colinux/fix-instructeurs-notification

fix(instructeurs): behavior of sort by notifications checkbox

app/components/dossiers/notified_toggle_component.rb

@@ -12,15 +12,15 @@ class Dossiers::NotifiedToggleComponent < ApplicationComponent
   end
 
   def active?
-    sorted_by_notifications? && order_asc?
+    sorted_by_notifications? && order_desc?
   end
 
   def icon_class_name
     active? ? 'fr-fi-checkbox' : 'fr-fi-checkbox-blank'
   end
 
-  def order_asc?
-    current_order == 'asc'
+  def order_desc?
+    current_order == 'desc'
   end
 
   def current_order

app/models/procedure_presentation.rb

@@ -293,13 +293,15 @@ class ProcedurePresentation < ApplicationRecord
     update!(sort: {
       TABLE => table,
       COLUMN => column,
-      ORDER => opposite_order_for(table, column)
+      ORDER => order.presence || opposite_order_for(table, column)
     })
   end
 
   def opposite_order_for(table, column)
     if sort.values_at(TABLE, COLUMN) == [table, column]
       sort['order'] == 'asc' ? 'desc' : 'asc'
+    elsif [table, column] == ["notifications", "notifications"]
+      'desc' # default order for notifications
     else
       'asc'
     end

spec/system/instructeurs/procedure_sort_spec.rb

@@ -1,43 +1,63 @@
-describe "procedure sort" do
+describe "procedure sort", js: true do
   let(:instructeur) { create(:instructeur) }
   let(:procedure) { create(:procedure, :published, :with_type_de_champ, instructeurs: [instructeur]) }
   let!(:new_unfollow_dossier) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_instruction)) }
   let!(:followed_dossier) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_instruction)) }
-  let!(:new_unfollow_dossier_2) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_instruction)) }
+  let!(:followed_dossier_2) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_instruction)) }
 
   before do
     instructeur.follow(followed_dossier)
-    followed_dossier.champs_public.first.update(value: '123')
+    instructeur.follow(followed_dossier_2)
+    followed_dossier.champs_public.first.update(value: '123') # touch the dossier
 
     login_as(instructeur.user, scope: :user)
-    visit instructeur_procedure_path(procedure)
+    visit instructeur_procedure_path(procedure, statut: "suivis")
   end
 
   scenario "should be able to sort with header" do
-    all(".dossiers-table tbody tr:nth-child(1) .number-col a", text: new_unfollow_dossier_2.id)
-    all(".dossiers-table tbody tr:nth-child(2) .number-col a", text: followed_dossier.id)
-    all(".dossiers-table tbody tr:nth-child(3) .number-col a", text: new_unfollow_dossier.id)
+    # sorted by notifications (updated_at desc) by default, filtered by followed
+    expect(all(".dossiers-table tbody tr").count).to eq(2)
+    expect(find(".dossiers-table tbody tr:nth-child(1) .number-col a").text).to eq(followed_dossier.id.to_s)
+    expect(find(".dossiers-table tbody tr:nth-child(2) .number-col a").text).to eq(followed_dossier_2.id.to_s)
 
-    find("thead .number-col a").click # reverse id filter
+    find("thead .number-col a").click # sort by id asc
 
-    all(".dossiers-table tbody tr:nth-child(1) .number-col a", text: new_unfollow_dossier.id)
-    all(".dossiers-table tbody tr:nth-child(2) .number-col a", text: followed_dossier.id)
-    all(".dossiers-table tbody tr:nth-child(3) .number-col a", text: new_unfollow_dossier_2.id)
+    expect(find(".dossiers-table tbody tr:nth-child(1) .number-col a").text).to eq(followed_dossier.id.to_s)
+    expect(find(".dossiers-table tbody tr:nth-child(2) .number-col a").text).to eq(followed_dossier_2.id.to_s)
+
+    find("thead .number-col a").click # reverse order - sort by id desc
+
+    expect(find(".dossiers-table tbody tr:nth-child(1) .number-col a").text).to eq(followed_dossier_2.id.to_s)
+    expect(find(".dossiers-table tbody tr:nth-child(2) .number-col a").text).to eq(followed_dossier.id.to_s)
   end
 
-  scenario "should be able to sort with direct link to notificaiton filter" do
-    # dossier sorted by id
-    check "Remonter les dossiers avec une notification"
+  scenario "should be able to sort with direct link to notification sort" do
+    # the real input checkbox is hidden - DSFR set a fake checkbox with a label, so we can't use "check/uncheck" methods
+    # but we can assert on the hidden checkbox state
+    expect(page).to have_checked_field("Remonter les dossiers avec une notification")
 
-    # sort by notification
-    all(".dossiers-table tbody tr:nth-child(1) .number-col a", text: followed_dossier.id)
-    all(".dossiers-table tbody tr:nth-child(2) .number-col a", text: new_unfollow_dossier.id)
-    all(".dossiers-table tbody tr:nth-child(3) .number-col a", text: new_unfollow_dossier_2.id)
+    find("label", text: "Remonter les dossiers avec une notification").click # reverse order - sort by updated_at asc
 
-    uncheck "Remonter les dossiers avec une notification"
+    expect(page).not_to have_checked_field("Remonter les dossiers avec une notification")
+    expect(find(".dossiers-table tbody tr:nth-child(1) .number-col a").text).to eq(followed_dossier_2.id.to_s)
+    expect(find(".dossiers-table tbody tr:nth-child(2) .number-col a").text).to eq(followed_dossier.id.to_s)
 
-    all(".dossiers-table tbody tr:nth-child(1) .number-col a", text: new_unfollow_dossier_2.id)
-    all(".dossiers-table tbody tr:nth-child(2) .number-col a", text: followed_dossier.id)
-    all(".dossiers-table tbody tr:nth-child(3) .number-col a", text: new_unfollow_dossier.id)
+    find("label", text: "Remonter les dossiers avec une notification").click # set order back - sort by updated_at desc
+
+    expect(page).to have_checked_field("Remonter les dossiers avec une notification")
+    expect(find(".dossiers-table tbody tr:nth-child(1) .number-col a").text).to eq(followed_dossier.id.to_s)
+    expect(find(".dossiers-table tbody tr:nth-child(2) .number-col a").text).to eq(followed_dossier_2.id.to_s)
+  end
+
+  scenario "should be able to sort back by notification filter after any other sort" do
+    find("thead .number-col a").click # sort by id asc
+
+    expect(page).not_to have_checked_field("Remonter les dossiers avec une notification")
+
+    find("label", text: "Remonter les dossiers avec une notification").click # sort by updated_at desc
+    expect(page).to have_checked_field("Remonter les dossiers avec une notification")
+
+    expect(find(".dossiers-table tbody tr:nth-child(1) .number-col a").text).to eq(followed_dossier.id.to_s)
+    expect(find(".dossiers-table tbody tr:nth-child(2) .number-col a").text).to eq(followed_dossier_2.id.to_s)
   end
 end