Unifier les créations de comptes pour les mairies et les comptes Saas #4871

victormours · 2024-12-05T10:44:30Z

Avant

Formulaire pour les mairies	Formulaire pour les comptes Saas

Après

Un formulaire unique, avec une checkbox à activer pour les mairies

Contexte

Cette PR est une première amélioration pour un point de douleur remonté par l'équipe déploiement pendant le séminaire : le fait que toutes les mairies soient dans le même territoire complique les choses.

C'est notamment impossible de permettre à une mairie qui utilise RDV SP de créer une organisation pour son CCAS en autonomie.

Ça limite aussi beaucoup les usages avancés du produit via l'espace admin, parce que les mairies n'ont pas accès à l'espace admin.

Ça complique le suivi de dossier pour l'équipe déploiement, parce qu'il n'y a pas le fonctionnement 1 territoire = 1 compte.

Solution

Cette PR est une première étape pour unifier les créations de comptes Saas et Mairies, et de faire que la connexion à l'ants soit une option supplémentaire, et pas un fonctionnement complètement différent.

On ajoute la colonne organisations.ants_connectable, et on pourra ensuite sortir individuellement des mairies du territoire commun avec le script fait pour cet usage (scripts/split_territory.rb).

francois-ferrandis

Encore un quick win qui vaut son pesant d'or ! 🚀

francois-ferrandis · 2024-12-05T14:31:13Z

app/controllers/api/ants/editor_controller.rb

-      .where(organisations: { territory_id: Territory.mairies&.id })
+      .where(organisations: { ants_connectable: true })


C'est beau ! 🤩

app/controllers/super_admins/comptes_controller.rb

francois-ferrandis · 2024-12-05T14:40:44Z

db/migrate/20241204093704_add_ants_connectable_to_organisations.rb

+    up_only do
+      mairies_territory = Territory.find_by(name: "Mairies")
+      if mairies_territory # La migration tourne aussi sur l'instance historique qui n'a pas ce territoire
+        Organisation.where(territory_id: mairies_territory.id).update_all(ants_connectable: true)
+      end
+    end


Si mon cerveau est encore en vie, il me semble qu'on aura aucune interruption puisque la migration s'exécute dans une transaction, et que le l'ancien code est compatible avec le nouveau schéma. 🚀

francois-ferrandis · 2024-12-05T14:42:58Z

spec/features/users/online_booking/on_rdv_mairie_spec.rb

-      expect(page).to have_link("modifier", href: prendre_rdv_path(departement: "MA", public_link_organisation_id: organisation.id, duration: 50))
-      expect(page).to have_link("modifier",
-                                href: prendre_rdv_path(departement: "MA", motif_name_with_location_type: passport_motif.name_with_location_type, public_link_organisation_id: organisation.id,
-                                                       duration: 50))
-      expect(page).to have_link("modifier",
-                                href: prendre_rdv_path(departement: "MA", lieu_id: lieu.id, motif_name_with_location_type: passport_motif.name_with_location_type,
-                                                       public_link_organisation_id: organisation.id, duration: 50))
+
+      expect(page).to have_link("modifier", href: prendre_rdv_path(
+        departement: organisation.territory.departement_number,
+        public_link_organisation_id: organisation.id,
+        duration: 50
+      ))
+
+      expect(page).to have_link("modifier", href: prendre_rdv_path(
+        departement: organisation.territory.departement_number,
+        motif_name_with_location_type: passport_motif.name_with_location_type,
+        public_link_organisation_id: organisation.id,
+        duration: 50
+      ))
+
+      expect(page).to have_link("modifier", href: prendre_rdv_path(
+        departement: organisation.territory.departement_number,
+        lieu_id: lieu.id,
+        motif_name_with_location_type: passport_motif.name_with_location_type,
+        public_link_organisation_id: organisation.id, duration: 50
+      ))


Oui s'il vous plaît merci beaucoup 🤩

francois-ferrandis · 2024-12-05T14:46:23Z

app/controllers/super_admins/comptes_controller.rb

-      if compte.save
+      if compte.save!


Un peu étrange de voir un if et une méthode bang, c'est contraire à la convention Rails de ne pas retourner de valeur pour les méthodes bang, mais plutôt de raise des exceptions.

Le choix de ce ! est-il motivé par la capacité de la méthode à lever des exceptions ?

Bref, c'est du chipotage, mais j'ai dû aller regarder l'implem de la méthode pour confirmer que c'était pas une erreur d'usage.

oui, cette méthode peut lever des exceptions, j'ai ajouté ça pour le rendre plus clair, mais j'ai eu la flemme de changer l'implém du controller pour mieux gérer ça

victormours · 2024-12-09T14:03:50Z

vu avec Matis et Léa : il faut vérifier comment les catégories de motifs s'afficheront pour les organisations qui n'ont pas vocation à se brancher à l'ants, et ça pose peut-être la question plus large de comment on clarifie ce branchement pour les agents.

victormours · 2024-12-11T14:49:46Z

Okay, ça marche dans le cas où on commence par créer une mairie, et ensuite un ouvre un CCAS.
Ça ne marche pas encore sans intervention des devs si on a un CCAS et ensuite on ouvre une mairie.
C'est validé avec Léa

adipasquale · 2024-12-09T08:32:14Z

spec/features/super_admin/creating_a_new_account_spec.rb

-    fill_in(:compte_agent_last_name, with: "Factice") # Plusieurs champs ont le label "Nom", donc on utilise le name de l'input
-    fill_in("Adresse mail", with: "francis@factice.org")
-    select("Urbanisme", from: "Service")
+    fill_in_form


chipotage : je pense qu’il aurait mieux valu dupliquer les appels plutôt que génériser, et éventuellement changer les valeurs pour la cohérence par ex fill_in("Nom de la première organisation", with: "Mairie de Romainville")

adipasquale · 2024-12-09T08:36:32Z

spec/features/users/online_booking/on_rdv_mairie_spec.rb

+        departement: organisation.territory.departement_number,
+        lieu_id: lieu.id,
+        motif_name_with_location_type: passport_motif.name_with_location_type,
+        public_link_organisation_id: organisation.id, duration: 50


chipotage ultime : le duration ne revient pas à la ligne

gros 👍 pour le réalignement en tout cas

adipasquale · 2024-12-09T08:44:19Z

app/controllers/api/ants/editor_controller.rb

@@ -52,7 +52,7 @@ def search_application_ids

  def self.lieux
    Lieu.joins(:organisation)
-      .where(organisations: { territory_id: Territory.mairies&.id })
+      .where(organisations: { ants_connectable: true })


très cool en effet 🙇

En revanche, ça n’est pas 100% sûr avec la migration non ?
Si l’ANTS nous requête sur cet endpoint entre le moment où on déploie et le moment où la migration est achevée, on ne va pas renvoyer les bons lieux il me semble.

si, vu qu'on ajoute la colonne avant que le nouveau code soit actif

victormours marked this pull request as draft December 5, 2024 10:44

victormours marked this pull request as ready for review December 5, 2024 13:47

francois-ferrandis approved these changes Dec 5, 2024

View reviewed changes

victormours marked this pull request as draft December 9, 2024 14:02

victormours force-pushed the mairies-with-full-territories branch from 4e5a578 to 05e9c92 Compare December 11, 2024 14:22

victormours marked this pull request as ready for review December 11, 2024 14:49

adipasquale reviewed Dec 11, 2024

View reviewed changes

victormours force-pushed the mairies-with-full-territories branch from 9d5166a to 42402ae Compare December 11, 2024 17:04

victormours added 20 commits December 11, 2024 18:04

Add column

e692cc1

backfill

493f448

Add checkbox to mairie compte form

df00052

wip

df77ff6

add column to anon rules

05ff18e

working account creation

319ce9b

fix ants connection rule

415e4bf

update ants specs

f4ad5be

don't create example motif for mairies

db82daf

goodbye mairie compte

019a2d8

update integration spec

82e107f

fix

ab05a9c

remove mairie compte routes

30c58bb

update more specs

965e727

direct link in menu

8eddb94

add spec for motif categories

89e31ac

fix

82a9768

fix

1980622

fix

a40d7ae

fix

75543e4

spec improvements after review

8468759

victormours force-pushed the mairies-with-full-territories branch from 42402ae to 8468759 Compare December 11, 2024 17:05

victormours enabled auto-merge (squash) December 11, 2024 17:06

victormours merged commit 30bcc13 into production Dec 11, 2024
15 checks passed

victormours deleted the mairies-with-full-territories branch December 11, 2024 17:18

francois-ferrandis mentioned this pull request Jan 3, 2025

Afficher le numéro ANTS lors de l'impression de la liste de RDV #4942

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unifier les créations de comptes pour les mairies et les comptes Saas #4871

Unifier les créations de comptes pour les mairies et les comptes Saas #4871

victormours commented Dec 5, 2024 •

edited

Loading

francois-ferrandis left a comment

francois-ferrandis Dec 5, 2024

francois-ferrandis Dec 5, 2024

francois-ferrandis Dec 5, 2024

francois-ferrandis Dec 5, 2024

victormours Dec 11, 2024

victormours commented Dec 9, 2024

victormours commented Dec 11, 2024 •

edited

Loading

adipasquale Dec 9, 2024

adipasquale Dec 9, 2024

adipasquale Dec 9, 2024

victormours Dec 11, 2024

		.where(organisations: { territory_id: Territory.mairies&.id })
		.where(organisations: { ants_connectable: true })

Unifier les créations de comptes pour les mairies et les comptes Saas #4871

Unifier les créations de comptes pour les mairies et les comptes Saas #4871

Conversation

victormours commented Dec 5, 2024 • edited Loading

Avant

Après

Contexte

Solution

francois-ferrandis left a 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

victormours commented Dec 9, 2024

victormours commented Dec 11, 2024 • 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

victormours commented Dec 5, 2024 •

edited

Loading

victormours commented Dec 11, 2024 •

edited

Loading