search

betagouv / analyse-flux-insertion

Outil d'analyse des flux et échanges de données dans le domaine de l'insertion
2 stars 1 forks source link

Identifier les nouveaux demandeurs #19 #35

Closed aminedhobb closed 3 years ago

aminedhobb commented 3 years ago

Fonction principale

Dans cette PR j'introduis une nouvelle page qui sert à l'identification des nouveaux demandeurs RSA. Pour l'instant, on ne sélectionne que les top entrants (voir #19) pour lesquels on affiche les informations de contacts (Nom, Prénom, Rôle, Numéro RSA et NIR).

Implémentation secondaire

En plus de cette fonctionnalité, j'introduis une refactorisation de la manière avec laquelle on interagit avec les fichiers XML. J'ai créé des readers (un pour les flux instructions et un pour les flux bénéficiaires qui héritent d'un base reader) qui sont des simples classes JS qui prennent en entrée le contenu des fichiers et dans laquelle on va englober toute la logique liée à l'extraction de de données de ces fichiers. Ça servira à interagir plus facilement avec les fichiers depuis les différentes pages et de manière plus lisible (même si malheureusement le fait de devoir parcourir à chaque fois les flux bénéficiaires rend les choses un peu moins lisibles).

Remarque: Les fichiers sont tous traités à la volée et les informations non filtrées. On pourra penser à améliorer la visibilité en intégrant un système de pagination et en filtrant les doublons pour les fichiers volumineux.

aminedhobb commented 3 years ago

Je n'ai pas testé en local pour le front, mais ça me paraît tout bon. Pour le back, le principal souci vient des gros arrays de HTMLCollection qu'il faut éviter à mon avis (cf bug résolu hier)

Merci pour ta review @qblanc 🙌 ! J'ai répondu à certains de tes commentaires et appliqué quelques changements, dis-moi si tu as encore des suggestions !

qblanc commented 3 years ago

Pour BaseFluxReader, si la méthode n'est pas appellée dans FluxBeneficiaireReader, tu peux ignorer.

En revanche pour l'autre commentaire, je préfère vraiment ne pas prendre le risque de recréer le bug que l'on vient de supprimer, d'autant que le passage par la variable intermédiaire newApplicants ne me paraît pas apporter grand-chose (sinon pour des logiques d'organisation du code). Les fichiers mensuels sont vraiment énormes et je ne préfère pas parier sur le nombre de personnes en top entrants.

aminedhobb commented 3 years ago

Pour BaseFluxReader, si la méthode n'est pas appellée dans FluxBeneficiaireReader, tu peux ignorer.

Non elle n'est pas appelée par le reader du flux bénéficiaire, mais je vais quand même la mettre au niveaux du reader du flux d'instruction pour éviter ce genre de confusion 👍 .

En revanche pour l'autre commentaire, je préfère vraiment ne pas prendre le risque de recréer le bug que l'on vient de supprimer, d'autant que le passage par la variable intermédiaire newApplicants ne me paraît pas apporter grand-chose (sinon pour des logiques d'organisation du code). Les fichiers mensuels sont vraiment énormes et je ne préfère pas parier sur le nombre de personnes en top entrants.

Je t'ai répondu sur ce sujet ici. Par contre juste pour être précis, cette logique n'est utilisée que dans la nouvelle vue que j'ai créé (/identification-beneficiaire) et n'intervient pas sur les fonctionnalités précédentes.

qblanc commented 3 years ago

Je t'ai répondu ici. À mon sens les corrections que je propose ne coûtent rien et peuvent nous éviter des bugs, mais je suis peut-être trop frileux. Je te laisse décider si tu veux les implémenter ou pas.