API Particulier x DataPass : handle Formulaire QF modality #1579

skelz0r · 2024-06-21T15:37:02Z

skelz0r · 2024-06-25T06:02:54Z

Ça se lit commit par commit.

Il manque le processCode pour le formulaire QF, à récupérer auprès d'HubEE.

Pour un peu de contexte: j'ai jugé qu'il était plus logique de s'occuper de la création/gestion des accès à la suite de la validation d'une demande côté APIM (ici donc) plutôt que côté DataPass.

Pourquoi ? Parce que ce n'est pas le job de DataPass. DataPass s'occupe juste de notifier des divers évenements qui surviennent sur les habilitations. En pratique ici on crée déjà chez nous l'accès via un jeton, c'est donc naturel, qu'en fonction des modalités, que l'on crée l'accès sur HubEE de notre côté.
Typiquement si on veut activer de manière rétroactive sur des habilitations existantes on doit le faire de notre côté, pas sur DataPass. En d'autres termes: les habilitations et les accès sont décorelés.

Samuelfaure

C'est ptêtre juste une impression mais j'ai le feeling que tu en as eu un peu marre à la fin x) le code perd un peu en clarté au fur et à mesure

Sinon quelques remarques mais rien de critique.

Samuelfaure · 2024-06-27T02:25:09Z

spec/organizers/datapass_webhook/v2/api_particulier_spec.rb

+  context 'when modalities does no include params' do
+    before do
+      datapass_webhook_params['data']['data']['modalities'] = ['formulaire_qf']
+    end
+
+    it 'does not create a token' do
+      expect {
+        subject
+      }.not_to change(Token, :count)
+    end
+  end
+


J'ai pas la tête dans datapass ni le formulaire QF donc je vois pas du tout ce que ça implique que modalities inclue params ou pas

Un describe supplémentaire pour documenter peut-être serait pas de trop

Je pense que ce qui me confuse c'est le fait que :

sur un modalities = 'formulaire qf' on skip

sur un modalities = 'params' on avance

Et le titre c'est "no params modalities". Ca fait beaucoup de params.

Je changerai bien le return if context.modalities.exclude?('params') en return if !from_datapass

Niveau modalité sur API Particulier il y a:

params, qui correspond en fait à "faire des appels à l'API avec des paramètres d'entrée"

formulaire qf, qui correspond à utiliser le formulaire QF

franceconnect qui utilise uniquement FC pour accéder à l'API

A noter que je n'ai pas trop réfléchi aux ids techniques, ça peut changer encore.

Pour 2. et 3. il n'y a pas besoin de jeton, seul 1. a besoin d'un jeton.
Si il n'y a pas la modalité params il n'y a aucune raison de créer un jeton. En pratique à court terme params sera toujours présent je pense.

Je changerai bien le return if context.modalities.exclude?('params') en return if !from_datapass

Tout vient de DataPass, je ne comprends pas bien ta remarque du coup.

spec/support/fixtures_helpers.rb

app/clients/hubee_api_authentication.rb

Samuelfaure · 2024-06-27T02:32:48Z

spec/clients/hubee_api_client_spec.rb

+        stub_request(:get, "#{host}/referential/v1/organizations/SI-#{siret}-#{code_commune}").to_return(
+          status: 200,
+          headers: { 'Content-Type' => 'application/json' },
+          body: valid_payload.to_json
+        )
+      end
+
+      it 'renders a valid json from payload' do
+        expect(find_organization_payload).to eq(valid_payload)
+      end
+    end
+
+    context 'when API returns 404' do
+      before do
+        stub_request(:get, "#{host}/referential/v1/organizations/SI-#{siret}-#{code_commune}").to_return(
+          status: 404,
+          headers: { 'Content-Type' => 'application/json' },
+          body: {
+            'header' => {
+              'statut' => 404,
+              'message' => "Aucun élément trouvé pour le siret #{siret}"
+            }
+          }.to_json
+        )


Je mettrais les stub requests dans un module comme tu as fait pour l'INSEE

Pour l'INSEE j'ai cherry-pick, j'ai hésité à jarter le module car en pratique c'est seulement utilisé dans le test du client, donc aucun intérêt de l'isoler dans un module. (contrairement à DataPass).

Du coup je vais potentiellement faire l'inverse et simplifier en jartant le module INSEE, ok pour toi ?

spec/clients/hubee_api_client_spec.rb

Un3x

Is ok globalement pour moi.

J'trouve ca intéressant le fait qu'on passe pas par nos propres services pour aller fetch l'insee, mais je comprends parfaitement pourquoi.

J'trouve les clients un poil lourd, mais en même temps c'est pas grave, c'est assez isolé et si on veut les reprendre plus tard on pourra tjrs le faire.

Un3x · 2024-06-27T07:02:47Z

spec/organizers/datapass_webhook/v2/api_particulier_spec.rb

+  context 'when modalities does no include params' do
+    before do
+      datapass_webhook_params['data']['data']['modalities'] = ['formulaire_qf']
+    end
+
+    it 'does not create a token' do
+      expect {
+        subject
+      }.not_to change(Token, :count)
+    end
+  end
+


Je pense que ce qui me confuse c'est le fait que :

sur un modalities = 'formulaire qf' on skip

sur un modalities = 'params' on avance

Et le titre c'est "no params modalities". Ca fait beaucoup de params.

Je changerai bien le return if context.modalities.exclude?('params') en return if !from_datapass

skelz0r · 2024-06-27T08:18:34Z

C'est ptêtre juste une impression mais j'ai le feeling que tu en as eu un peu marre à la fin x) le code perd un peu en clarté au fur et à mesure

C'est un souci ici. Qu'est-ce qui n'est pas clair ?

skelz0r · 2024-06-27T08:27:30Z

J'trouve les clients un poil lourd, mais en même temps c'est pas grave, c'est assez isolé et si on veut les reprendre plus tard on pourra tjrs le faire.

Une suggestion pour rendre ça moins lourd ?

Un3x · 2024-06-27T08:47:17Z

Je pense que c'est juste le feeling des payloads des méthodes de subscription à hubee / insee.

On peut toujours pousser, je reprend l'exemple hubee on pourrait avoir une méthode create_element(url, payload) qui s'occupe uniquement de la requête + du raise et appeler cette méthode côté create_subscription et create_organization mais est ce qu'on s'en fiche pas un peu ? Ici on est dans une mise en place que j'imagine "temporaire" tant qu'on sait pas exactement jusqu'où ira FQF et HubEE

skelz0r · 2024-06-27T08:51:07Z

Il est vrai que j'aurais pu wrapper dans un api_call avec un block, mais bon dtf en vrai ce qui fait que c'est un feeling un peu huge reste la tête de la payload (et encore, j'ai quasiment rien mis en l'état va falloir que j'ajoute des champs)

Samuelfaure · 2024-06-27T09:45:10Z

@skelz0r Pour clarifier mon commentaire, le code est toujours OK dans les dernier commits, sinon j'aurais relevé des points précis plutôt qu'un vague feeling

C'est OK en l'état pour moi

app/jobs/create_formulaire_qf_hubee_subscription_job.rb

spec/clients/hubee_api_client_spec.rb

app/clients/hubee_api_client.rb

app/organizers/datapass_webhook/api_particulier.rb

JeSuisUnCaillou · 2024-08-29T08:32:23Z

Remrque : Faut pas créer de jeton pour les communes sans éditeurs et sans dévellopeurs.
Je sais pas si c'est pertinent dans cette PR, ou si ça doit être géré dans datapass de ne pas envoyer de requête de création de jeton à api_particulier dans ces cas là.

CF https://linear.app/pole-api/issue/API-3170/sassurer-quaucun-jeton-api-particulier-ne-sera-cree-pour-les

JeSuisUnCaillou · 2024-09-02T10:06:15Z

Pour FQF, on voudrait pouvoir créer une subscription HubEE Active, en mode API, associée à un éditeur.
Pour ça il faut :

Créer une subscription "normale" : inactive, sans éditeur (delegationActor), sans accessMode

# POST /referential/v1/subscriptions
sub_body = {
  "datapassId"=>99999,
  "processCode"=>"CERTDC",
  "subscriber"=>{"type"=>"SI", "companyRegister"=>"21890254200016", "branchCode"=>"89254"},
  "notificationFrequency"=>"Unitaire",
  "validateDateTime"=>DateTime.now.iso8601,
  "updateDateTime"=>DateTime.now.iso8601,
  "status"=>"Inactif",
  "email"=>"[email protected]",
  "localAdministrator"=>{"email"=>"[email protected]"},
  # "accessMode"=>"API",
  # "delegationActor"=>{"type"=>"EDT", "companyRegister"=>"42424242424299", "branchCode"=>"92062"},
}

Update la subscription avec l'éditeur, et l'access mode, le status actif et la date d'activation. A noter que je ne sais pas ce qui est obligatoire dans le PUT, j'ai pas testé de retirer des champs un par un pour voir.

# PUT /referential/v1/subscriptions/#{subscription_id}
sub_update_body = {
  "datapassId"=>99999,
  "processCode"=>"CERTDC",
  "subscriber"=>{"type"=>"SI", "companyRegister"=>"21890254200016", "branchCode"=>"89254"},
  "notificationFrequency"=>"Unitaire",
  "validateDateTime"=>DateTime.now.iso8601, # à garder ?
  "updateDateTime"=>DateTime.now.iso8601, # à garder ?
  "email"=>"[email protected]",
  "status"=>"Inactif",
  "localAdministrator"=>{"email"=>"[email protected]"},
  "accessMode" => "API",
  "delegationActor"=>{"type"=>"EDT", "companyRegister"=>"42424242424299", "branchCode"=>"92062"},
  "activateDateTime"=>DateTime.now.iso8601, #NEEDED !
  "status"=>"Actif"
}

Prepare to exploit modalities field for access

… params modalities

Ignore specs file path name, does not handle well and not an issue.

Prepare to store HubEE ids

Huge cherry-pick from DataPass v2 Ref etalab/data_pass@7c36324

Will be used for HubEE

Co-authored-by: Jean-Baptiste Feldis <[email protected]>

Les abonnements HubEE doivent être créées avant d'être activés, ce commit met en place cette séparation. Pour associer un éditeur le webhook envoie maintenant le service_provider associé à la demande (stocké dans extra_infos). Une méthode pour récupérer un abonnement existant a aussi été ajoutée pour rendre le webhook plus robuste. La collectivité est aussi créée du côté du FormulaireQF.

jbfeldis · 2024-10-18T12:02:36Z

app/clients/formulaire_qf_api_client.rb

+      code_cog: organization.code_commune_etablissement,
+      departement: organization.code_postal_etablissement[0..1],
+      name: organization.denomination,
+      status: 'active',


@JeSuisUnCaillou on active directement ? comment on pourrait détecter si ça n'a pas fonctionné correctement côté HubEE ?

Je ne comprends pas ta remarque dans le cadre du client qui est complétement décorellé de HubEE.

Si tu parles du séquençage de l'organizer, c'est la responsabilité de l'organizer d'arrêter la séquence si il y a un souci.

jbfeldis · 2024-10-18T12:03:36Z

app/clients/hubee_api_client.rb

+
+    payload = subscription_payload.with_indifferent_access.merge({
+      status: 'Actif',
+      activateDateTime: DateTime.now.iso8601,


@skelz0r tu parlais d'une date d'activation dans le passé, y'avait une raison particulière ? ça semble fonctionner comme ça

ça me dit quelque chose, mais de toute manière ta ligne est une date dans le passé quasiment instantanément.

jbfeldis · 2024-10-18T12:09:16Z

@skelz0r je te repasse la patate, je serai en vacances la semaine prochaine

il faudra un token FQF de prod

ça fonctionne (j'ai bidouillé pas mal pour réussir à faire des tests E2E en local) et ça va avec cette PR (n'hésitez pas à la reprendre !) https://github.com/etalab/data_pass/pull/476/files

skelz0r

(je ne peux pas refuse vu que c'est ma PR)

2 gros points:

Faut encapsuler la complexité du create puis update pour créer la subscription comme il faut, ça simplifiera la logique du code et cantonnera les trucs wtf dans le client (et on comprendra tout de suite que c'est l'API qui est mal foutu et non le dev qui a fait n'imp :D)
Faut absolument séquencer avec gestion d'erreur la création de l'abo HubEE puis celle du FQF

skelz0r · 2024-10-20T18:20:01Z

app/clients/formulaire_qf_api_client.rb

+      code_cog: organization.code_commune_etablissement,
+      departement: organization.code_postal_etablissement[0..1],
+      name: organization.denomination,
+      status: 'active',


Je ne comprends pas ta remarque dans le cadre du client qui est complétement décorellé de HubEE.

skelz0r · 2024-10-20T18:20:03Z

app/clients/hubee_api_client.rb

+
+    payload = subscription_payload.with_indifferent_access.merge({
+      status: 'Actif',
+      activateDateTime: DateTime.now.iso8601,


skelz0r · 2024-10-20T18:21:21Z

app/clients/formulaire_qf_api_client.rb

+      code_cog: organization.code_commune_etablissement,
+      departement: organization.code_postal_etablissement[0..1],
+      name: organization.denomination,
+      status: 'active',


Je ne comprends pas ta remarque dans le cadre du client qui est complétement décorellé de HubEE.

skelz0r · 2024-10-20T18:22:38Z

app/clients/formulaire_qf_api_client.rb

+      code_cog: organization.code_commune_etablissement,
+      departement: organization.code_postal_etablissement[0..1],
+      name: organization.denomination,
+      status: 'active',


Si tu parles du séquençage de l'organizer, c'est la responsabilité de l'organizer d'arrêter la séquence si il y a un souci.

skelz0r · 2024-10-20T18:23:31Z

app/clients/hubee_api_client.rb

+
+    payload = subscription_payload.with_indifferent_access.merge({
+      status: 'Actif',
+      activateDateTime: DateTime.now.iso8601,


ça me dit quelque chose, mais de toute manière ta ligne est une date dans le passé quasiment instantanément.

skelz0r · 2024-10-20T18:27:08Z

app/clients/hubee_api_client.rb

+    request.body.first
+  end
+
+  def update_subscription(authorization_request, subscription_payload, editor_payload = {})


Je renommerai cette méthode en activate_subscription, ainsi que le create_subscription en create_inactive_subscription, et je wrapperai les 2 dans un create_subscription

Pourquoi tout faire ici ? Parce que c'est de la tambouille interne de l'API qui ne fonctionne pas comme il faudrait, et fondamentalement dans le séquençage on ne devrait que faire un create_subscription, et les implémentations wtf vivent dans le client qui s'occupe de la complexité de l'API.

Bonus points:

tu ne changes rien dans le code de l'organizer pour gérer ça, et demain si ils fixent, tu changes juste le client (ce qui est logique en fait) ;

ça se comprend 100x plus en 1 commit avec un message

skelz0r · 2024-10-20T18:29:14Z

app/interactors/datapass_webhook/find_or_create_authorization_request.rb

@@ -72,10 +72,17 @@ def authorization_request_attributes
    ).merge(authorization_request_attributes_for_current_event).merge(
      'last_update' => fired_at_as_datetime,
      'previous_external_id' => context.data['pass']['copied_from_enrollment_id'],
-      'api' => context.api
+      'api' => context.api,
+      'extra_infos' => extra_infos_with_service_provider


dans 100% des cas il s'agit d'un éditeur, autant mettre editor ici imo

skelz0r · 2024-10-20T18:30:43Z

app/interactors/datapass_webhook/schedule_create_formulaire_qf_hubee_subscription_job.rb

    CreateFormulaireQFHubEESubscriptionJob.perform_later(context.authorization_request.id)
+    CreateFormulaireQFCollectivityJob.perform_later(context.authorization_request.id)


Je comprends mieux ta question plus haut: en effet ici on ne peut pas scheduler 2 jobs dépendant de manière indépendante, il faudrait faire un CreateFormulaireQFAccessJob qui fait séquentiellement la création de l'accès HubEE, puis celui chez vous. Autrement c'est un sac de noeud à debug.

skelz0r · 2024-10-20T18:31:23Z

app/jobs/create_formulaire_qf_hubee_subscription_job.rb

+    service_provider = authorization_request.extra_infos['service_provider']
+    subscription_update_payload = {}
+
+    if service_provider.present? && service_provider['type'] == 'editor'


je ne comprend pas: ce n'est pas systématiquement le cas ?

skelz0r · 2024-10-20T18:41:35Z

@skelz0r je te repasse la patate, je serai en vacances la semaine prochaine

du coup si je comprends bien ça doit partir en prod semaine prochaine ?

JeSuisUnCaillou · 2024-10-21T08:28:53Z

du coup si je comprends bien ça doit partir en prod semaine prochaine ?

On n'a pas de hard deadline comme ça, mais on aimerait effectivement pouvoir embarquer plus facilement les nouveaux éditeurs et leurs communes en automatisant la création des communes/abonnements. Je sais qu'on en a quelques uns qui peuvent bientôt arriver, faut checker avec Miryad pour savoir si ça attend.

skelz0r self-assigned this Jun 21, 2024

skelz0r force-pushed the features/api-particulier-fqf-modality branch 3 times, most recently from 96a999d to 78e5316 Compare June 25, 2024 05:50

skelz0r marked this pull request as ready for review June 25, 2024 05:55

skelz0r requested review from Un3x and Samuelfaure June 25, 2024 05:55

Samuelfaure reviewed Jun 27, 2024

View reviewed changes

Un3x approved these changes Jun 27, 2024

View reviewed changes

skelz0r force-pushed the features/api-particulier-fqf-modality branch from 78e5316 to ec4f1b4 Compare July 2, 2024 14:39

skelz0r force-pushed the features/api-particulier-fqf-modality branch 2 times, most recently from a17cfb3 to 6747add Compare July 17, 2024 07:20

jbfeldis reviewed Aug 26, 2024

View reviewed changes

app/jobs/create_formulaire_qf_hubee_subscription_job.rb Outdated Show resolved Hide resolved

jbfeldis reviewed Aug 26, 2024

View reviewed changes

spec/clients/hubee_api_client_spec.rb Show resolved Hide resolved

app/clients/hubee_api_client.rb Outdated Show resolved Hide resolved

app/organizers/datapass_webhook/api_particulier.rb Show resolved Hide resolved

skelz0r added 8 commits October 15, 2024 10:06

DatapassWebhook::AdaptV2ToV1 affects all authorization request data

58b8e7b

Prepare to exploit modalities field for access

API Particulier DataPass webhook : do not create token if there is no…

4a12489

… params modalities

HubEE is an acronym

a52a934

Ignore specs file path name, does not handle well and not an issue.

INSEE is an acronym

56248bb

QF is an acronym

deb8673

Add extra_infos to authorization requests

9252ef3

Prepare to store HubEE ids

Introduce INSEESireneAPIClient

31df189

Huge cherry-pick from DataPass v2 Ref etalab/data_pass@7c36324

Introduce Organization model

03b42ac

Will be used for HubEE

skelz0r and others added 7 commits October 15, 2024 10:50

Introduce HubEEAPIClient

cf6e233

Introduce CreateFormulaireQFHubEESubscriptionJob

4238962

DatapassWebhook::APIParticulier handles formulaire QF HubEE subscription

d2f3d7e

Remove useless module

3191e73

Update HubEEAPIClient with latest specs

ec85fd5

Add valid process_code for Formulaire QF

a081984

Co-authored-by: Jean-Baptiste Feldis <[email protected]>

jbfeldis force-pushed the features/api-particulier-fqf-modality branch from 2545d90 to 4f1339e Compare October 18, 2024 11:59

jbfeldis reviewed Oct 18, 2024

View reviewed changes

Lint discutable

62e40b2

skelz0r commented Oct 20, 2024

View reviewed changes

		CreateFormulaireQFHubEESubscriptionJob.perform_later(context.authorization_request.id)
		CreateFormulaireQFCollectivityJob.perform_later(context.authorization_request.id)

API Particulier x DataPass : handle Formulaire QF modality #1579

Are you sure you want to change the base?

API Particulier x DataPass : handle Formulaire QF modality #1579

Conversation

skelz0r commented Jun 21, 2024 • edited Loading

skelz0r commented Jun 25, 2024 • edited Loading

Samuelfaure 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

Un3x left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skelz0r commented Jun 27, 2024

skelz0r commented Jun 27, 2024

Un3x commented Jun 27, 2024 • edited Loading

skelz0r commented Jun 27, 2024

Samuelfaure commented Jun 27, 2024 • edited Loading

JeSuisUnCaillou commented Aug 29, 2024 • edited Loading

JeSuisUnCaillou commented Sep 2, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbfeldis commented Oct 18, 2024

skelz0r 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

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

skelz0r commented Oct 20, 2024

JeSuisUnCaillou commented Oct 21, 2024

skelz0r commented Jun 21, 2024 •

edited

Loading

skelz0r commented Jun 25, 2024 •

edited

Loading

Un3x commented Jun 27, 2024 •

edited

Loading

Samuelfaure commented Jun 27, 2024 •

edited

Loading

JeSuisUnCaillou commented Aug 29, 2024 •

edited

Loading

JeSuisUnCaillou commented Sep 2, 2024 •

edited

Loading