Revue de l'application `communication`

* Ajout de commentaires sur certaines méthodes et fonctions
* Spécification des remarques ou des notes là où j'ai des questions
* Proposition de renommage de certains champs

src/communication/admin.py

@@ -1,17 +1,13 @@
+"""Module d'administration des messages."""
+
 from django.contrib import admin
 
-# Register your models here.
 from .models import Message
-from django_extensions.admin import ForeignKeyAutocompleteAdmin
 
 
+@admin.register(Message)
 class MessageAdmin(admin.ModelAdmin):
-    model = Message
-
     list_display = ("writer", "reader", "date_of_writing", "is_read", "date_of_reading")
     ordering = ("date_of_writing", "writer")
     search_fields = ("writer", "reader", "message_title")
     list_filter = ("writer", "reader", "is_read")
-
-
-admin.site.register(Message, MessageAdmin)

src/communication/forms.py

@@ -1,9 +1,11 @@
-# coding=UTF-8
+"""Configuration et représentation des forms liés aux messages."""
+
+from datetime import date
 
 from django import forms
-from datetime import date
-from .models import Message
+
 from people.models import Gymnast
+from .models import Message
 
 
 class MessageForm(forms.ModelForm):
@@ -19,10 +21,10 @@ class MessageForm(forms.ModelForm):
             "writer": forms.HiddenInput(),
             "reader": forms.HiddenInput(),
             "message_title": forms.TextInput(
-                attrs={"class": "form-control", "placeholder": "Title's message."}
+                attrs={"class": "form-control", "placeholder": "Message title"}
             ),
             "message_body": forms.Textarea(
-                attrs={"class": "form-control", "placeholder": "Body's'message.",}
+                attrs={"class": "form-control", "placeholder": "Message body",}
             ),
         }
 

src/communication/models.py

@@ -1,11 +1,40 @@
-# coding=UTF-8
 
 from django.db import models
 from django.contrib.auth.models import User
 
 class Message(models.Model):
-    """
-    Communication entre utilisateur de l'application
+    """Représente un message échangé entre deux utilisateurs.
+
+    Attributes:
+        writer (User): Association à l'utilisateur qui a écrit le message.
+        reader (User): Association au destinataire du message.
+        date_of_writing (datetime): Date à laquelle le message a été enregistré.
+        date_of_reading (datetime): Date à laquelle le message a été lu par son destinataire.
+        is_read (bool): Indique si le message a été lu ou non - par défaut: False
+        message_title (str): Titre ou sujet du message.
+        message_body (text): Contenu du message.
+
+    Remarks:
+        . Les champs `message_body` et `message_title` me posent problème,
+        parce qu'on est déjà dans la classe `Message`. Pourquoi pas juste `title` et `body` ?
+
+        . Le champ `is_read` est superflu. Si on veut savoir si le message est lu, il suffit de
+        faire une méthode avec un décorateur @property et retourner si `date_of_reading` est vide.
+
+        . Par convention, j'aurais appelé les champs `writer` et `reader` plutôt `sender` et `recipient`.
+
+        . Pareil pour les champs "date" auto-remplis. J'aime bien les conventions type `<nom_du_champ>_at`.()
+        (Ici, `written_at` et `read_at`).
+
+        . Est-ce qu'un message ne peut pas être envoyé à plusieurs personnes en même temps ?
+
+        . Tes related_names ne sont pas corrects: `have_write` et `have_read` n'ont pas de signification.
+        Quand on a une instance d'un utilisateur en main, ça reviendrait à faire `user_instance.have_read.all`.
+        Sémantiquement, c'est pas top. Il faudrait plutôt les appeler `sent_messages` et `received_messages`.
+        Dans le code, cela donnerait `user_instance.sent_messages.all()` ou `user_instance.sent_messages.filter(...)`.
+
+        . Est-ce qu'un `Message` ne devrait pas être `Markdownizable` ?
+        Mais alors le champ `information` devrait s'appeler `content` ou quelque chose de plus générique.
     """
 
     writer = models.ForeignKey(

src/communication/urls.py

@@ -1,10 +1,15 @@
-# coding=UTF-8
+"""Définition des routes d'actions permettant de contrôler les messages et la communication."""
 
 from django.urls import path, re_path
 
 from . import views
 
-# Message
+
+""" NOTE: Attention à bien nommer les URLs. `message_sent` -> `sent_messages`...
+
+"""
+
+
 message_urlpatterns = [
     path(r"sent/", views.get_message_sent, name="message_sent"),
     path(r"received/", views.get_message_received, name="message_received"),

src/communication/views.py

@@ -1,28 +1,57 @@
-# coding=UTF-8
+"""Vues et fonctions pour tout ce qui touche à la communication entre plusieurs utilisateurs."""
 
-from django.views.decorators.http import require_http_methods
-from django.http import HttpResponse, HttpResponseRedirect
 from django.contrib.auth.decorators import login_required
-from django.shortcuts import render, get_object_or_404
 from django.contrib.auth.models import User
+from django.http import HttpResponse, HttpResponseRedirect
+from django.shortcuts import render, get_object_or_404
+from django.views.decorators.http import require_http_methods
 from django.urls import reverse
+
 from .forms import MessageForm
 from .models import Message
 
 
 # @login_required
 def get_number_unreaded_message(request):
+    """Récupère le nombre de messages non lus associés à l'utilisateur en session.
+
+    Remarks:
+        La fonction s'appelle `get_number_of_unread_message`,
+        mais retourne l'ensemble des messages de l'utilisateur.
+
+        Pourquoi avoir commenté le décorateur @login_required, alors qu'il est clair
+        qu'il faut avoir un utilisateur en session pour y accéder ?
+
+        Le participe passé de "non lus", en anglais, c'est "unread", pas "unreaded"
     """
-    """
-    # user = User.objects.get(pk=userid)
-    number_unreaded_message = Message.objects.filter(reader=request.user).count()
-    return number_unreaded_message
+
+    return Message.objects.filter(reader=request.user).count()
 
 
 @login_required
 @require_http_methods(["GET"])
 def get_messages(request, message_type="received"):
-    """
+    """Récupère des messages associés l'utilisateur actuellement connecté.
+
+    Args:
+        request (django.http.HttpRequest): (voir mes notes ci-dessous)
+        message_type (str): { received | sent }
+
+    Returns:
+        Retourne les messages reçus ou envoyés par l'utilisateur connecté.
+        Si le paramètre `message_type` est vide, la liste renvoyée est vide également.
+
+    Remarks:
+        Je vois bien l'idée, mais par simplification, j'aurais plutôt fait une fonction
+        qui récupère une liste de messages, et qui retourne cette liste à la fonction appelante.
+
+        Note aussi qu'avec la modification sur les related_name, tu peux changer la ligne
+            Message.objects.filter(reader=request.user)
+
+            -> request.user.received_messages.all()
+
+        En mixant le tout, tu ne dois plus passer la requête, ni les décorateurs.
+        Tu peux juste passer l'utilisateur et ton paramètre `received` ou `sent`.
     """
     if message_type == "received":
         message_list = Message.objects.filter(reader=request.user)
@@ -38,7 +67,11 @@ def get_messages(request, message_type="received"):
 @login_required
 @require_http_methods(["GET"])
 def get_message_received(request):
-    """
+    """Idem que dans la fonction `get_messages`
+
+    Remarks:
+        La fonction s'appelle `get_message_received`, mais devrait s'appeller `get_received_messages`
+        (au pluriel et accordé).
     """
     return get_messages(request, "received")
 
@@ -46,7 +79,7 @@ def get_message_received(request):
 @login_required
 @require_http_methods(["GET"])
 def get_message_sent(request):
-    """
+    """Idem que `get_message_received`. Nomme la plutôt `get_sent_messages`.
     """
     return get_messages(request, "sent")
 
@@ -54,7 +87,7 @@ def get_message_sent(request):
 @login_required
 @require_http_methods(["GET"])
 def get_message_details(request, messageid):
-    """
+    """Récupère les détails (l'affichage ?) d'un message.
     """
     message = get_object_or_404(Message, pk=messageid)
     if not message.is_read and message.reader == request.user.id:
@@ -68,7 +101,22 @@ def get_message_details(request, messageid):
 @login_required
 @require_http_methods(["GET"])
 def delete_message(request, messageid):
-    """
+    """Supprime le message dont la clé est passée en paramètre.
+
+    Remarks:
+        . Il existe un verbe Http spécifique pour ce type d'action: DELETE
+
+        . Tu ne vérifies pas non plus si le message appartient à l'utilisateur qui l'a écrit ;-)
+
+        > Alice écrit un message et le sauve
+        > Le message porte l'identifiant 301
+        > Bob se connecte et appelle l'URL /delete/301
+        > Le message 301 est supprimé et Alice n'en sait rien.
+
+        . Est-ce qu'il ne vaudrait pas mieux aussi *invalider* un message
+        plutôt que de le supprimer ? Et enregister qui a effectué cette modification ?
+
+        . C'est quoi le code http 409 ?
     """
     try:
         Message.objects.get(pk=messageid).delete()
@@ -81,7 +129,11 @@ def delete_message(request, messageid):
 @login_required
 @require_http_methods(["GET", "POST"])
 def compose_message(request):
-    """
+    """Permet à l'utilisateur connecté de rédiger un nouveau message.
+
+    Remarks:
+        Tes messages d'erreur sont en anglais ou en français ?
+        "Form invalide" ou "Invalid form" ?
     """
 
     if request.method == "POST":