feat(upload): Handle archived users in upload #2754

aminedhobb · 2025-04-01T01:05:34Z

Contexte

Cette PR regroupe les résolutions de plusieurs issues avant le déploiement de la nouvelle fonctionnalité d'upload.
J'explicite ci-dessous les différents sujets qu'elle traite. Elle se lit bien commit par commit aussi.

Usagers archivés et dossiers traités

On affiche clairement dans l'interface les usagers archivés. Au niveau du département, on considère qu'on est archivé si on l'usager est archivé dans au moins une des orgas
On déselectionne par défaut les usagers archivés et les usagers dont le suivi a été fermé et on l'affiche dans la tooltip
Je fais en sorte que les usagers archivés soient désarchivés s'ils sont finalement sélectionnés et que les dossiers fermés soient rouverts

Sauvegarde de la sélection des dossiers

Je reprends les travaux initiés dans #2697 .

Je renomme les colonnes marked_for_user_save et marked_for_invitation en selected_for_user_save et selected_for_invitation qui sont plus parlant
On set maintenant la valeur selected_for_user_save au moment de la création de la user_row, et dépend de nos règles métiers pour savoir si on sélectionne par défaut ou non
Lorsque l'on check/uncheck les cases dans l'interface on fait une requête au serveur pour modifier la valeur en db
On ajoute un endpoint batch_update qui permet d'updater plusieurs rows d'un coup lorsqu'on sélectionne/déselectionne tout
On set la valeur de selected_for_invitation lorsque la sauvegarde de l'usager a réussi en fonction de nos règles métiers
Je garde l'info de si la case "tout sélectionner/tout déselectionner" est cochée dans les cookies pour que la case reste cochée/décochée au reload de la page

Bug sur les matching users

Il y avait un bug sur le matching des users au début du fait qu'on ne formattait les attributs de nos rows au moment de requêter les matchs potentiels: b961f7a

Ajout des features tests

Je rajoute plusieurs features tests pour prendre en compte nos nouvelles règles métiers et vérifier que les interactions fonctionnent correctement sur l'UI: 2eb7977

Michaelvilleneuve · 2025-04-01T11:03:02Z

app/controllers/user_list_uploads/user_rows_controller.rb

+
+    def batch_update_params
+      params.expect(
+        user_rows: [[:id, :selected_for_user_save, :selected_for_invitation]]


Pourquoi [[]] ?

c'est comme ça qu'on définit qu'on attend un array maintenant: https://medium.com/@rzvirfan/rails-8-introducing-params-expect-f99c4f1fec10

Michaelvilleneuve · 2025-04-01T12:16:11Z

app/helpers/user_list_upload/user_list_upload_helper.rb

  end

  def show_row_attribute?(attribute_name, user_list_upload)
    user_list_upload.restricted_user_attributes.exclude?(attribute_name.to_sym)
  end
+
+  def checkbox_to_select_all_checked?(attribute_name, user_list_upload_id)
+    cookies["checkbox_to_select_all_#{attribute_name}_checked_#{user_list_upload_id}"] != "false"


Je suis partagé sur le fait d'utiliser les cookies pour partager de l'info de cette façon, ça marche mais que se passe t-il si la personne arrête de toucher son navigateur pendant les 10 minutes ? La requête suivante va échouer non ?
J'ai peur qu'on se retrouve dans des state un peu bizarres, aussi toujours un peu peur d'overflow les cookies

Non ça n'échoue pas, c'est juste que la checkbox va être désélectionnée par défaut.
J'avoue c'est peut-être pas la meilleure utilisation mais pour moi c'était logique que cet état de "préférences" soit lié au navigateur.
J'ai augmenté leur durée à 1h et explicité un peu mieux le mécanisme ici: c573f65

Si on voit que ça crée des soucis on pourra changer et avoir ça en back-end dans le cache

Michaelvilleneuve · 2025-04-01T12:18:35Z

app/javascript/controllers/select_user_rows_controller.js

+  static targets = ["checkbox", "invitationFormatOption"]
+
+  connect() {
+    /* eslint-disable prefer-destructuring */


Faudrait peut-être changer la règle car j'avoue que destructurer pour mettre sur this me semble douteux ^^

Oui y a pas de moyen de disable la règle juste pour ces cas malheureusement.

Michaelvilleneuve · 2025-04-01T12:36:03Z

app/javascript/controllers/select_user_rows_controller.js

+    const form = document.createElement("form")
+    form.method = "post"
+    form.action = url
+    form.innerHTML = DOMPurify.sanitize(`
+      <input type="hidden" name="_method" value="patch">
+      <input type="hidden" name="authenticity_token" value="${document.querySelector("meta[name='csrf-token']").content}">


Ne pourrait-on pas créer un object Javascript pour servir de helper, il commence à y avoir pas mal de cas où on doit faire ça manuellement.
Je pense que cette méthode pourrait se réduire à

Server.fetch(url, { method: "patch", payload: { [`user_row[${attribute}]`]: value })

Bonne idée, on peut faire un ticket pour faire ça quand on aura un peu plus de temps.

app/javascript/stylesheets/_variables.scss

app/models/application_record.rb

Michaelvilleneuve · 2025-04-01T12:50:32Z

spec/features/agent_can_upload_user_list_spec.rb

+      ## Both rows are checked by default
+      expect(find("input[type='checkbox'][data-user-row-id='#{hernan_row.id}']")).to be_checked
+      expect(find("input[type='checkbox'][data-user-row-id='#{christian_row.id}']")).to be_checked
+
+      expect(hernan_row.reload.selected_for_user_save).to be_truthy
+      expect(christian_row.reload.selected_for_user_save).to be_truthy
+      expect(page).to have_content("2 usagers sélectionnés")
+
+      # Uncheck all rows
+      check_all_button = find("input[type='checkbox'][data-action='click->select-user-rows#toggleAll']")
+      expect(check_all_button).to be_checked
+      check_all_button.click
+
+      expect(page).to have_content("Aucun usager sélectionné", wait: 5)
+      expect(christian_row.reload.selected_for_user_save).to be_falsey
+      expect(hernan_row.reload.selected_for_user_save).to be_falsey
+      expect(page).to have_css("input[type='checkbox'][data-user-row-id='#{christian_row.id}']:not(:checked)")
+      expect(page).to have_css("input[type='checkbox'][data-user-row-id='#{hernan_row.id}']:not(:checked)")
+
+      # Check all rows
+      check_all_button.click
+
+      expect(page).to have_content("2 usagers sélectionnés", wait: 5)
+      expect(christian_row.reload.selected_for_user_save).to be_truthy
+      expect(hernan_row.reload.selected_for_user_save).to be_truthy
+      expect(page).to have_css("input[type='checkbox'][data-user-row-id='#{hernan_row.id}']:checked")
+      expect(page).to have_css("input[type='checkbox'][data-user-row-id='#{christian_row.id}']:checked")
+


Ça me va en l'état mais peut-être qu'il serait plus sain de faire ça en plusieurs tests, ça affinerait un peu et donnerait des erreurs un peu plus parlante en cas d'échec

Oui je comprends mais j'ai pensé que c'était bien aussi d'avoir un test e2e où on fait toutes les actions sur la même session.
Pour les comportements spécifiques j'ai fait des tests séparés.

Michaelvilleneuve

Juste quelques petits retours mais globalement ça me va 🙌

aminedhobb · 2025-04-01T13:56:19Z

Juste quelques petits retours mais globalement ça me va 🙌

Merci @Michaelvilleneuve , j'ai répondu à tes commentaires et appliqué des suggestions !

Michaelvilleneuve

Merci pour les petits changements 👍

aminedhobb added 9 commits March 20, 2025 16:56

feat(upload): Handle archived users and closed follow ups

c96b0a2

disable first and last override when arguments are passed

12d441a

feat(app): Save user rows selection

0f32e70

Merge branch 'staging' into feat/handle-archived-users-in-upload

744b722

fix bug when retrieving matching user

b961f7a

spell user_row method correctly

266dd68

Update specs

2eb7977

linter fix

05cc831

remove useless comments

1382a84

aminedhobb marked this pull request as ready for review April 1, 2025 01:19

aminedhobb self-assigned this Apr 1, 2025

aminedhobb requested review from Holist and Michaelvilleneuve April 1, 2025 08:46

Merge branch 'staging' into feat/handle-archived-users-in-upload

0d24818

Michaelvilleneuve reviewed Apr 1, 2025

View reviewed changes

app/javascript/stylesheets/_variables.scss Outdated Show resolved Hide resolved

Michaelvilleneuve reviewed Apr 1, 2025

View reviewed changes

app/models/application_record.rb Show resolved Hide resolved

Michaelvilleneuve reviewed Apr 1, 2025

View reviewed changes

Michaelvilleneuve previously approved these changes Apr 1, 2025

View reviewed changes

aminedhobb added 2 commits April 1, 2025 15:36

Update wording on user_list_uploads#show page

d6a5d70

Clarify cookie mechanism

c573f65

aminedhobb dismissed Michaelvilleneuve’s stale review via c573f65 April 1, 2025 13:37

fix linter warnings

72ec8d9

aminedhobb force-pushed the feat/handle-archived-users-in-upload branch from 594b2bd to 72ec8d9 Compare April 1, 2025 13:46

fix grammar

4ce913f

Michaelvilleneuve mentioned this pull request Apr 1, 2025

[Tech] Refacto les appels HTTP depuis le JS #2755

Open

Michaelvilleneuve approved these changes Apr 1, 2025

View reviewed changes

aminedhobb merged commit a0532e5 into staging Apr 1, 2025
6 checks passed

aminedhobb deleted the feat/handle-archived-users-in-upload branch April 1, 2025 14:19

aminedhobb mentioned this pull request Apr 2, 2025

feat(upload): save selected user row info #2697

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(upload): Handle archived users in upload #2754

feat(upload): Handle archived users in upload #2754

aminedhobb commented Apr 1, 2025 •

edited

Loading

Michaelvilleneuve Apr 1, 2025

aminedhobb Apr 1, 2025

Michaelvilleneuve Apr 1, 2025 •

edited

Loading

aminedhobb Apr 1, 2025

Michaelvilleneuve Apr 1, 2025

aminedhobb Apr 1, 2025

Michaelvilleneuve Apr 1, 2025

aminedhobb Apr 1, 2025

Michaelvilleneuve Apr 1, 2025

Michaelvilleneuve Apr 1, 2025

aminedhobb Apr 1, 2025

Michaelvilleneuve left a comment

aminedhobb commented Apr 1, 2025

Michaelvilleneuve left a comment

feat(upload): Handle archived users in upload #2754

feat(upload): Handle archived users in upload #2754

Conversation

aminedhobb commented Apr 1, 2025 • edited Loading

Contexte

Usagers archivés et dossiers traités

Sauvegarde de la sélection des dossiers

Bug sur les matching users

Ajout des features tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michaelvilleneuve Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michaelvilleneuve left a comment

Choose a reason for hiding this comment

aminedhobb commented Apr 1, 2025

Michaelvilleneuve left a comment

Choose a reason for hiding this comment

aminedhobb commented Apr 1, 2025 •

edited

Loading

Michaelvilleneuve Apr 1, 2025 •

edited

Loading