Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(champs): do not depend on attributes hash key in old code #10365

Conversation

tchak
Copy link
Member

@tchak tchak commented Apr 22, 2024

Pour comprendre cette PR, il faut comprendre comment fonctionnent les attributs imbriqués de rails.
Par défaut, les attributs sont dans un tableau avec pour indices 0, 1, 2, etc. Cela nous posait problème, car cela signifie qu'il est impossible de régénérer un fragment du formulaire sans savoir combien de fragments sont déjà dans le formulaire. Comme rails n'utilise pas les indices du tableau, nous avons décidé d'utiliser champ.id comme indice dans le tableau. Plus tard, nous avons écrit du code qui avait besoin de savoir quels champs étaient inclus dans les attributs imbriqués. Par flemme, j'ai utilisé params.keys - ce qui donne la liste des champ.id. J'aurais dû chercher l'attribut [id] dans chaque champ imbriqué dès le départ. Nous en arrivons à notre refactoring qui remplace les indices par les champ.public_id et en profitons pour ne plus répéter l'information. Dans le nouveau code, nous n'avons plus besoin de l'attribut [id]. En revanche, nous avons besoin d'un attribut pour indiquer que nous envoyons dans un nouveau format, d'où [with_public_id].

  1. { [123]: { id: 123, value: 'toto' } }
  2. { ['123-rowid']: { id: 123, value: 'toto' } } || { ['123-rowid']: { with_public_id: true, value: 'toto' } }
  3. { ['123-rowid']: { value: 'toto' } }

Passer de 1 à 2 est safe car Rails n'utilise pas les index du tableau des attributs imbriqués en interne. Ce sont des valeurs obscures de son point de vue.

Ce que je cherche à faire, c'est d'éviter de mettre un if dans le code qui génère l'attribut name d'un champ.

Copy link

codecov bot commented Apr 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.71%. Comparing base (32a2191) to head (95bb09a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10365      +/-   ##
==========================================
+ Coverage   80.68%   80.71%   +0.03%     
==========================================
  Files        1192     1192              
  Lines       25021    25019       -2     
  Branches     4497     4499       +2     
==========================================
+ Hits        20187    20195       +8     
+ Misses       4834     4824      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tchak tchak force-pushed the refactor-dossier-champ-by-stable-id-attribute-keys branch 2 times, most recently from a2f1058 to 1f8115b Compare April 22, 2024 16:04
@tchak tchak force-pushed the refactor-dossier-champ-by-stable-id-attribute-keys branch from 1f8115b to 95bb09a Compare April 22, 2024 16:39
@LeSim LeSim added this pull request to the merge queue Apr 23, 2024
Merged via the queue into demarches-simplifiees:main with commit 9ae734c Apr 23, 2024
18 checks passed
@LeSim LeSim deleted the refactor-dossier-champ-by-stable-id-attribute-keys branch April 23, 2024 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants