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

Quelques améliorations pour rendre le code du backend plus abordable #6246

Closed
14 of 16 tasks
Arnaud-D opened this issue Feb 19, 2022 · 2 comments
Closed
14 of 16 tasks
Labels
S-Évolution Ajoute de nouvelles fonctionnalités

Comments

@Arnaud-D
Copy link
Contributor

Arnaud-D commented Feb 19, 2022

Notre code est un peu vieillissant par endroit et par moment alambiqué, ce qui ne facilite pas l'intégration de nouveaux développeurs, ou pour ceux actuels d'être à l'aise.

Je ne pense pas qu'il y ait de grosse action facile pour faciliter ça. Par contre, il y a une somme de petites modifications qui, je pense, mise bout à bout peuvent rendre le code plus homogène, plus facile à explorer et à lire et donc plus abordable. Bref, enlever la poussière dans les coins pour être plus accueillants. ^^

C'est ouvert à discussion ou expansion évidemment. Ce sont juste des choses qui me sont passées par la tête.

Organisation des fichiers

Les fichiers au mauvais endroit ou trop long nuisent à la découvrabilité du code. Je sais que sans la recherche de l'IDE, j'aurais du mal à trouver beaucoup de choses planquées derrière des noms trop génériques ou dans des fichiers géants. Sans parler que ça nuit aussi à la performance des outils de l'IDE qui vérifient le code en temps réel.

  • Déplacer les factories.py dans les dossier tests , pour les séparer du véritable code et les rapprocher de leur rôle, à savoir faciliter les tests.
  • Scinder les fichiers de tests trop long. Tous les fichiers ne sont pas faciles à scinder de cette manière, parce que les tests ne sont pas forcément très unitaires partout.
  • Scinder d'autres fichiers trop long. Il y a un tutorialv2/forms.py de plus de 1300 lignes. Je ne sais pas trop comment on a fini par en arriver là. Une commande qui aide pas mal : git ls-files zds | xargs wc -l | awk '$1 > 600' | sort -gr
  • Supprimer zds/search ? Je crois que ça sert plus à rien. On a un fichier vide et un autre avec des constantes qui ne semblent pas utilisées, mais à vérifier plus en détail.
  • Déplacer utils/mps.py, qui aurait plus sa place avec... les mps
  • Déplacer ce qu'on peut de utils/forums.py, qui concerne les forums.
  • on a aussi tout ce qui tourne autour de HelpWriting qui devrait être dans tutorialv2 probablement.
  • de manière générale, utils ne devrait jamais importer de trucs des autres modules, c'est un coup à créer des dépendances croisées.

Gestion des urls

Les fichiers de définition des routes sont un peu en chaos par moment. Pourtant, ils sont centraux pour naviguer dans le code, puisque c'est l'endroit par excellence qui mène l'URL qu'on connaît tous sur le site à la vue qui la gère, dont on ne peut pas deviner le nom a priori.

  • Simplifier les modules urls alambiqués. Certains sont excessivement compliqués pour rien :
    re_path(r"^contenus/", include(("zds.tutorialv2.urls.urls_contents", "zds.tutorialv2.urls"), namespace="content")),
    .
  • ça dégrade trop la lisibilité à mon goût, probablement à éviter dans un premier temps Factoriser les déclarations de routes qui peuvent l'être, comme par exemple :
    re_path(r"^parametres/profil/$", UpdateMember.as_view(), name="update-member"),
    re_path(r"^parametres/github/$", UpdateGitHubToken.as_view(), name="update-github"),
    re_path(r"^parametres/github/supprimer/$", remove_github_token, name="remove-github"),
    re_path(r"^parametres/profil/maj_avatar/$", UpdateAvatarMember.as_view(), name="update-avatar-member"),
    re_path(r"^parametres/compte/$", UpdatePasswordMember.as_view(), name="update-password-member"),
    re_path(r"^parametres/user/$", UpdateUsernameEmailMember.as_view(), name="update-username-email-member"),
    Il y a des solutions pour ça dans la doc de Django.
  • harmoniser le vocabulaire dans les routes. On a des endroits avec editer et d'autres avec modifier (le meilleur terme est probablement modifier en français). Pareil avec nouveau et creer. Il y en a peut-être d'autres. Ça change les URL, donc faire attention si on a besoin de redirections.

Utiliser les fonctionnalités de Django et simplifier notre code

Django a évolué depuis le début du site, et notre code, pas forcément. Autant profiter de ce que la bibliothèque a à offrir et documente mieux que nous.

  • remplacer notre implémentation perso de LoginRequiredMixin par celle de Django
  • remplacer notre implémentation perso de PermissionRequiredMixin par celle de Django
  • envisager de remplacer notre login_view par un dérivé de LoginView. Beaucoup de notre logique basique serait déportée dans Django.
  • envisager de remplacer notre logout_view par un dérivé de LogoutView
  • à voir aussi s'il faut faire pareil pour le changement de mot de passe et le reset (Django propose différentes vues pour les deux cas : PasswordChangeView et PasswordResetView).
@Arnaud-D Arnaud-D added S-BUG Corrige un problème S-Évolution Ajoute de nouvelles fonctionnalités and removed S-BUG Corrige un problème labels Feb 19, 2022
@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Feb 23, 2022

J'ai ajouté quelques points au message initial. Ça concerne l'usage des fonctionnalités de Django quand elles existent plutôt que de maintenir nos propres implémentations des fonctionnalités basiques.

@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented May 7, 2022

Ce ticket a rempli son objectif. Presque tous les items ont été abordés à différents niveaux de profondeur ; je ferme.

Il sera temps d'en ouvrir d'autres lors de prochaines découvertes de potentielles refactorisations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Évolution Ajoute de nouvelles fonctionnalités
Projects
None yet
Development

No branches or pull requests

1 participant