diff --git a/src/communication/admin.py b/src/communication/admin.py index 4c2d17b..bbffdb3 100644 --- a/src/communication/admin.py +++ b/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) diff --git a/src/communication/forms.py b/src/communication/forms.py index 1d63c2e..e00d7c0 100644 --- a/src/communication/forms.py +++ b/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",} ), } diff --git a/src/communication/models.py b/src/communication/models.py index b8dd048..1b48701 100644 --- a/src/communication/models.py +++ b/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 `_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( diff --git a/src/communication/urls.py b/src/communication/urls.py index 469ee38..0b440e3 100644 --- a/src/communication/urls.py +++ b/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"), diff --git a/src/communication/views.py b/src/communication/views.py index 440ab6b..2c41f72 100644 --- a/src/communication/views.py +++ b/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":