Remarques

[Ishydo]

Salut à vous ! 😄

Quelques remarques préliminaires après un petit survol de votre code et test de votre projet en local. Sachant que c'est encore en cours de dev, je n'ai pas tout analysé en détail, il y a donc peut-être des éléments sur lesquels vous travaillez déjà. Toutes ces infos sont à titre indicatif.

Bravo pour avoir découplé frontend et backend en faisant une app react. C'est ce genre de paradigme qu'on rencontre le plus souvent dans le dev en entreprise (moderne) 👍 Vous exploitez djangorestframework correctement pour arriver à un bon exercice de création d'API, c'est top. Vous montrez une bonne compréhension de django + restframework et utilisation des vues génériques.

Mes remarques concernent donc principalement des bonnes pratiques.

• Pour pousser le vice de la création d'API au maximum, de manière générale on explicite le fait qu'une route est un endpoint API et on le versionne. Ainsi, on aura des routes commençant par api/v1. Cela a l'avantage de vous permettre de facilement proposer une api/v2 à l'avenir en conservant une api/v1 fonctionnelle, si votre frontend doit évoluer. Ca se fait beaucoup en software-as-a-service (saas). Ici mailjet qui en est à la v3 de son API comme exemple : Mailjet API V3. Dans le contexte actuel, ce n'est évidemment pas nécessaire, surtout que vous êtes les seuls utilisateurs de votre API, mais c'est bien de le garder en tête. Lecture à ce propos ici (point 7)

• Votre fichier views tire un peu en longueur, il serait peut-être envisageable de déplacer de la logique métier dans les modèles correspondants (?)

• En terme d'UX, je pense qu'il peut être pertinent de prévoir une pagination pour la page de recherche (?)

• Je n'ai pas (encore) pris le temps de consulter votre code frontend. Cependant j'ai remarqué que dans la template index.php, vous incluez certaines dépendances via cdn.

<script src="https://code.jquery.com/jquery-3.4.1.slim.min.js" integrity="sha384-J6qa4849blE2+poT4WnyKhv5vZF5SrPo0iEjwBvKU7imGFAV0wwj1yYfoRSJoZ+n" crossorigin="anonymous" defer></script>
<script src="https://cdn.jsdelivr.net/npm/popper.js@1.16.0/dist/umd/popper.min.js" integrity="sha384-Q6E9RHvbIyZFJoft+2mJbHaEWldlvI9IOYy5n3zV9zzTtmI3UksdQRVvoxMfooAo" crossorigin="anonymous" defer></script>
<script src="https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/js/bootstrap.min.js" integrity="sha384-wfSDF2E50Y2D1uUdj0O3uMBJnjuUD4Ih7YwaYd1iqfktj0Uod8GCExl3Og8ifwB6" crossorigin="anonymous" defer></script>
<script src="https://cdn.jsdelivr.net/npm/chart.js@2.9.3/dist/Chart.min.js" integrity="sha256-R4pqcOYV8lt7snxMQO/HSbVCFRPMdrhAFMH+vr9giYI=" crossorigin="anonymous"></script>
<script src="{% static "csruby_frontend_app/main.js" %}"></script>

Bien que cela puisse se discuter, je suis plutôt d'avis que si vous avez un environnement utilisant npm, il est plus propre de faire passer toutes vos dépendances par le même canal. Ainsi, on fera plutôt des npm install bootstrap && npm install popper plutôt que des inclusions cdn.

• Je n'ai pas (encore) vu de fixtures permettant de facilement seeder la DB avec des données de base en local. Mais au vu de l'app déploée en prod, je me doute qu'elles doivent bien exister quelque part.

J'ajouterai d'éventuelles remarques supplémentaires ci-dessous au fur et à mesure.

Très bonne continuation à vous ! 😄

[sadikuar]

Bonjour,

Merci pour vos commentaires !

[sadikuar]

Ce qui a été fait :

• Déplacement de la logique (majorité) dans les modèles
• Ajout d'un "infinite scroll" pour améliorer l'UX (remplace pagination)
• Ajout d'un seeder (fixtures)

Ce qui n'a pas été fait :

• Ajout de api avec version dans l'url
  • Raison : ceci a déjà été discuté avec M. Grunenwald et donc la décision a été de ne pas l'ajouté. Néanmoins, ceci reste une bonne pratique si plusieurs version de l'api.

Ce qui reste à faire :

• Utiliser les paquets NPM plutôt que les CDNs