From 0419a5e731982baba3aca8e007253c4a63cd377a Mon Sep 17 00:00:00 2001 From: Fred Pauchet Date: Tue, 20 Oct 2020 11:47:36 +0200 Subject: [PATCH 1/3] [WIP] Reviews planning application --- src/planning/admin.py | 11 +-- src/planning/forms.py | 8 +- src/planning/models.py | 99 ++++++++++++++---------- src/planning/tests/__init__.py | 0 src/planning/{ => tests}/tests_models.py | 5 +- src/planning/urls.py | 1 - src/planning/views.py | 1 - 7 files changed, 71 insertions(+), 54 deletions(-) create mode 100644 src/planning/tests/__init__.py rename src/planning/{ => tests}/tests_models.py (89%) diff --git a/src/planning/admin.py b/src/planning/admin.py index a5ef875..071e96f 100644 --- a/src/planning/admin.py +++ b/src/planning/admin.py @@ -1,6 +1,9 @@ -# coding=UTF-8 +"""Administration des plannings, évènements et saisons.""" + from django.contrib import admin +from django_extensions.admin import ForeignKeyAutocompleteAdmin + from .models import ( EventType, Event, @@ -14,19 +17,17 @@ from .models import ( Round, PlanningLine, ) -from django_extensions.admin import ForeignKeyAutocompleteAdmin def duplicate_record(modeladmin, request, queryset): - """ - Duplication de record sélectionner. + """*Custom action* permettant de dupliquer plusieurs enregistrements. """ for object in queryset: object.id = None object.save() -duplicate_record.short_description = "Duplicate selected record" +duplicate_record.short_description = "Duplicate selected records" class SeasonAdmin(admin.ModelAdmin): diff --git a/src/planning/forms.py b/src/planning/forms.py index 0e76be7..c291708 100644 --- a/src/planning/forms.py +++ b/src/planning/forms.py @@ -1,11 +1,11 @@ -# coding=UTF-8 + +from datetime import date from django import forms -from datetime import date -from .models import Unavailability, Event, PlanningLine -from people.models import Gymnast from django.contrib.admin.widgets import FilteredSelectMultiple +from people.models import Gymnast +from .models import Unavailability, Event, PlanningLine class UnavailabilityForm(forms.ModelForm): class Meta: diff --git a/src/planning/models.py b/src/planning/models.py index e94419b..8f29768 100644 --- a/src/planning/models.py +++ b/src/planning/models.py @@ -1,18 +1,23 @@ -# coding=UTF-8 -from django.db import models +from datetime import datetime, date, time, timedelta + from django.contrib.auth.models import User -from datetime import datetime, date, time -from base.models import Markdownizable +from django.db import models from django.utils import timezone -from people.models import Gymnast -from location.models import Club -from datetime import datetime, timedelta + import pendulum +from base.models import Markdownizable +from location.models import Club +from people.models import Gymnast + def get_week(a_date): """ + + Remarks: + Je ne comprends pas trop cette fonction... + Tu pars d'une date, et tu récupères le lundi et le samedi de la semaine correspondant ? """ the_date = pendulum.parse(a_date) day = the_date.weekday() @@ -33,6 +38,18 @@ def get_number_of_weeks_between(start, stop): :param stop: date de fin de la période :type stop: datetime.date :return: Le nombre de semaines entre les deux dates. + + Remarks: + Proposition d'utiliser isocalendar() sur une date. + L'indice 1 de la valeur de retour donne la semaine correspondant. + + Eg. + >>> from datetime import date + >>> d = date(2020, 9, 27) + >>> d.isocalendar() + (2020, 39, 7) + + -> Est-ce qu'il ne suffirait pas de faire la différence ? """ tmp = stop - start @@ -87,9 +104,8 @@ class TemporizableQuerySet(models.QuerySet): class Temporizable(models.Model): - """ - Classe abstraite définissant deux dates (une da te de début, une date de - fin) et des méthodes de calculs sur base de ces dates. + """Classe abstraite définissant une période comprise entre deux dates. + """ class Meta: @@ -198,12 +214,13 @@ class EventType(models.Model): class Event(Markdownizable, Temporizable): - """ - Classe représentant les évènements. Un évènement est caractèrisé par : - - un nom, - - un lieu (place), - - un type (compétition, démonstration, …), - - des gymnastes (participation prévue). + """Classe représentant les évènements. + + Un évènement est caractérisé par : + * un nom, + * un lieu (place), + * un type (compétition, démonstration, …), + * des gymnastes (participation prévue). Je ne me rapelle plus à quoi sert le club. """ @@ -257,14 +274,15 @@ class Event_Participation(models.Model): class Course(Markdownizable, Temporizable): - """ - Classe représentant les cours. Un cours est défini par : - - une heure de début et une heure de fin, - - une date de début et une date de fin (un cours est considéré comme donné hebdromadairement entre + """Classe représentant les cours. + + Un cours est défini par : + * une heure de début et une heure de fin, + * une date de début et une date de fin (un cours est considéré comme donné hebdromadairement entre ces deux dates) (hérite de la classe `Temporizable`) - - est associé à un ou plusieurs entraineurs, - - est associé à un club - - est associé à un jour de la semaine (numéro du jour dans la semaine : 0 = lundi, 6 = dimanche). + * est associé à un ou plusieurs entraineurs, + * est associé à un club + * est associé à un jour de la semaine (numéro du jour dans la semaine : 0 = lundi, 6 = dimanche). """ class Meta: @@ -319,8 +337,7 @@ class Course(Markdownizable, Temporizable): class Group(models.Model): - """ - Classe représentant les groupes (Loisir, D1, D2, A, B, …). + """Classe représentant les groupes (Loisir, D1, D2, A, B, …). Un groupe appartient à un club. """ @@ -343,10 +360,12 @@ class Group(models.Model): class Subgroup(models.Model): - """ - Classe représentant les sous-groupes. + """Classe représentant les sous-groupes. - Un sous-groupe appartient à un groupe (lui-même lié à un club). De la sorte, quand un gymnaste est mis dans un sous-groupe, en remontant via le groupe, nous pouvons connaître le(s) club(s) du gymnaste pour chaque saison. + Un sous-groupe appartient à un groupe (lui-même lié à un club). + + De cette manière, quand un gymnaste est mis dans un sous-groupe, en remontant via le groupe, + nous pouvons connaître le(s) club(s) du gymnaste pour chaque saison. """ class Meta: @@ -367,8 +386,7 @@ class Subgroup(models.Model): class UnavailabilityManager(models.Manager): - """ - Classe représentant le manager de la classe `Unavailability`. + """Classe représentant le manager de la classe `Unavailability`. """ def next(self, count): @@ -379,8 +397,7 @@ class UnavailabilityManager(models.Manager): class Unavailability(Markdownizable, Temporizable): - """ - Classe représentant les indisponibilités. + """Classe représentant les indisponibilités. """ class Meta: @@ -405,10 +422,11 @@ class Unavailability(Markdownizable, Temporizable): class Training(models.Model): - """ - Classe représentant les entraînements. Un entraînement est une occurence - d'un cours auquel sont présent des gymnastes pour une date donnée. Un objet - de cette classe lie donc un cours et un gymnaste à une date donnée. + """Classe représentant les entraînements. + + Un entraînement est une occurence d'un cours pendant lequel des gmnastes sont présents. + + Un objet de cette classe lie donc un cours et un gymnaste à une date donnée. """ class Meta: @@ -441,9 +459,9 @@ class Training(models.Model): class Round(Markdownizable): - """ - Classe représentant les passages des élèves lors d'un entrainement. Chaque record - représente un passage. Il est donc lié à un record de la classe `Training`. + """Classe représentant les passages des élèves lors d'un entrainement. + + Chaque record représente un passage. Il est donc lié à un record de la classe `Training`. """ class Meta: @@ -483,8 +501,7 @@ class Round(Markdownizable): class PlanningLine(Markdownizable): - """ - Classe représentant les passages prévisionnels (incubating idea). + """Classe représentant les passages prévisionnels (incubating idea). """ class Meta: diff --git a/src/planning/tests/__init__.py b/src/planning/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/planning/tests_models.py b/src/planning/tests/tests_models.py similarity index 89% rename from src/planning/tests_models.py rename to src/planning/tests/tests_models.py index 47dfb1e..3416ed2 100644 --- a/src/planning/tests_models.py +++ b/src/planning/tests/tests_models.py @@ -1,12 +1,13 @@ -# coding: utf-8 from datetime import datetime from django.test import TestCase -from .models import get_number_of_weeks_between, Season +from ..models import get_number_of_weeks_between, Season class TestUtils(TestCase): def test_get_number_of_weeks(self): + """Evalue le nombre de semaines qu'il y a entre deux dates. + """ x = datetime(2016, 1, 1) y = datetime(2016, 2, 5) z = datetime(2016, 2, 4) diff --git a/src/planning/urls.py b/src/planning/urls.py index 3ea1da6..73400a7 100644 --- a/src/planning/urls.py +++ b/src/planning/urls.py @@ -1,4 +1,3 @@ -# coding=UTF-8 from django.urls import path, re_path diff --git a/src/planning/views.py b/src/planning/views.py index a05b3b9..84b0180 100644 --- a/src/planning/views.py +++ b/src/planning/views.py @@ -1,4 +1,3 @@ -# coding=UTF-8 from django.shortcuts import render, get_object_or_404 from django.http import ( From 2d8cc1b7645783cd13eea35ef6ed8c523a89e97a Mon Sep 17 00:00:00 2001 From: Fred Pauchet Date: Tue, 20 Oct 2020 12:15:22 +0200 Subject: [PATCH 2/3] =?UTF-8?q?Premi=C3=A8re=20passe=20de=20relecture=20de?= =?UTF-8?q?=20l'application=20`planning`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/planning/views.py | 85 ++++++++++++++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 21 deletions(-) diff --git a/src/planning/views.py b/src/planning/views.py index 84b0180..26dcf6e 100644 --- a/src/planning/views.py +++ b/src/planning/views.py @@ -64,16 +64,25 @@ def event_lookup(request): @login_required @require_http_methods(["GET", "POST"]) def event_create_or_update(request, eventid=None): - """ - Création ou mise à jour d'un event. + """Création ou mise à jour d'un évènement. + + Remarks: + * Il faut éviter de cibler une URL précise. + + Pour cela, il faut passer par la fonction `reverse`: + https://docs.djangoproject.com/fr/3.1/ref/urlresolvers/ + + Si jamais l'URL `/event` devait être modifiée, cela ne fonctionnerait plus. + + * J'ai modifié l'ordre des conditions pour un peu plus de clarté. + Notamment, tu faisais une première requête pour récupérer le nom du lieu + Mais tu n'utilisais cette valeur que dans le cas d'un GET. + + Dans ce cas-ci, une CBV serait intéressante, parce qu'elle découplerait + complètement le GET du POST. """ - if eventid: - event = get_object_or_404(Event, pk=eventid) - data = {"place_related": event.place.name} - else: - event = None - data = {} + if request.method == "POST": form = EventForm(request.POST, instance=event) @@ -85,6 +94,14 @@ def event_create_or_update(request, eventid=None): else: return HttpResponseRedirect("/event/") else: + data = {} + + if eventid: + event = get_object_or_404(Event, pk=eventid) + data["place_related"] = event.place.name + else: + event = None + form = EventForm(instance=event, initial=data) context = {"form": form, "eventid": eventid} @@ -93,8 +110,16 @@ def event_create_or_update(request, eventid=None): @require_http_methods(["GET"]) def link_gymnast_to_event(request, eventid, gymnastid): - """ - Crée un lien entre un gymnaste et un évènement. + """Crée un lien entre un gymnaste et un évènement. + + Returns: + Si tout se passe bien, un code 200 (Success) est retourné. + + Excepts: + Si une erreur se produit lors de l'association, un code HTTP 409 (Conflict) est retourné. + + Remarks: + Tu ne veux pas retourner le lien qui vient d'être créé ? """ try: gymnast = get_object_or_404(Gymnast, pk=gymnastid) @@ -109,8 +134,14 @@ def link_gymnast_to_event(request, eventid, gymnastid): @require_http_methods(["GET"]) def remove_link_between_gymnast_and_event(request, eventid, gymnastid): - """ - Supprime le lien entre un gymnaste et un évènement. + """Supprime le lien entre un gymnaste et un évènement. + + Remarks: + En fait, tes fonctions `link_gymnast_to_event` + et `remove_link_between_gymnast_and_event` sont _très_ similaires. + + Il faudrait sans doute mieux passer par une CBV, voire mieux: DRF ;-) + Surtout qu'ici, on gère directement des `link_between_g_and_e`, à ajouter ou supprimer. """ try: gymnast = get_object_or_404(Gymnast, pk=gymnastid) @@ -124,20 +155,30 @@ def remove_link_between_gymnast_and_event(request, eventid, gymnastid): def __get_event_list(request): + """Récupère une liste d'évènement. + + Par défaut, la liste est triée chronologiquement - le plus ancien étant le premier élément. + + Cette fonction est utilisée pour l'affichage des évènements et au niveau du calendrier. + + Args: + request (HttpRequest): La requête en entrée + pattern (str?): Optionnel. Permet de spécifier un pattern à appliquer à la recherche. + """ pattern = request.GET.get("pattern", None) + if pattern: - event_list = Event.objects.filter(name__icontains=pattern).order_by("datebegin") + event_list = Event.objects.filter(name__icontains=pattern) else: - event_list = Event.objects.all().order_by("datebegin") - return event_list + event_list = Event.objects.all() + + return event_list.order_by("datebegin") @login_required @require_http_methods(["GET"]) def calendar(request): - """ - Récupère la liste de tous évènements suivant un pattern si celui-ci est - définit. + """Récupère la liste de tous évènements suivant un pattern si celui-ci est définit. """ event_list = __get_event_list(request) context = {"event_list": event_list} @@ -216,7 +257,7 @@ def event_detail(request, eventid): @require_http_methods(["GET"]) def course_detail(request, courseid, date=None): """ - Récupère toutes les informations d'un course. + Récupère toutes les informations d'un cours. :return: une instance de la classe `Course`, une liste de gymnaste associés à ce cours, … @@ -1238,8 +1279,10 @@ def program_date_listing(request, gymnastid=None): @login_required @require_http_methods(["GET", "POST"]) def planningline_update(request, planninglineid=None): - """ - Mise à jour d'une ligne de programme. + """Mise à jour d'une ligne de programme. + + Remarks: + Pourquoi ne pas juste faire un `form.save()` plutôt que de réattribuer chaque valeur ? """ planningline = get_object_or_404(PlanningLine, pk=planninglineid) From 92f240a91508218a521443ece1b3c10f47b78bf9 Mon Sep 17 00:00:00 2001 From: Fred Pauchet Date: Tue, 20 Oct 2020 15:54:17 +0200 Subject: [PATCH 3/3] =?UTF-8?q?Premi=C3=A8re=20vague=20de=20questions=20su?= =?UTF-8?q?r=20la=20gestion=20des=20personnes.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/people/admin.py | 14 ++++++--- src/people/forms.py | 2 +- src/people/models.py | 69 ++++++++++++++++++++++++++++++++++++-------- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/people/admin.py b/src/people/admin.py index 361567a..4e82b5e 100644 --- a/src/people/admin.py +++ b/src/people/admin.py @@ -1,4 +1,14 @@ +"""Administration des gymnastes et des personnes. + +Remarks: + * Je ne pense pas qu'il faille encore passer par `ForeignKeyAutocompleteAdmin`. + https://docs.djangoproject.com/fr/3.1/ref/contrib/admin/#django.contrib.admin.ModelAdmin.autocomplete_fields +""" + from django.contrib import admin + +from django_extensions.admin import ForeignKeyAutocompleteAdmin + from .models import ( Gymnast, Accident, @@ -7,10 +17,6 @@ from .models import ( GymnastHasRoutine, ) -# from objective.models import Educative -from django_extensions.admin import ForeignKeyAutocompleteAdmin - - class CanDoRelationAdmin(ForeignKeyAutocompleteAdmin): model = CanDoRelation diff --git a/src/people/forms.py b/src/people/forms.py index d6a10a5..19e520f 100644 --- a/src/people/forms.py +++ b/src/people/forms.py @@ -1,4 +1,4 @@ -# coding=UTF-8 +"""Formulaires de gestion des données entrantes pour les gymnastes et accidents.""" from django import forms diff --git a/src/people/models.py b/src/people/models.py index 9bb60dd..af6e9d9 100644 --- a/src/people/models.py +++ b/src/people/models.py @@ -1,18 +1,64 @@ -# coding=UTF-8 +"""Modélisation des gymnastes, accidents et relations à faire/faites. + +Notes: + Est-ce qu'il ne faudrait pas refactoriser les GSM père/mère ? + Avec une table d'association, et un champ qui indique la qualité du contact ? + Du coup, cela permettrait se débarasser des champs phone, gsm, gsm_father et gsm_mother. + + Comment sont gérées les évolutions ? Changement de clubs, de fédération, + + A quoi correspond le champ `year_of_practice` ? + Comment se comportera-t-il dans un an ? Dans deux ans ? + Est-ce qu'il ne faudrait pas une date de début, plutôt ? + + Idem pour la méthode `actual_year_of_pratice`. + Comme elle se base sur le champ `created_at`, il suffit que l'enregistrement ne soit pas + réalisé correctement pour que la méthode retourne une mauvaise information. + + Que signifie qu'un gymnaste soit actif ou inactif ? Est-ce que cela ne devrait pas plutôt + être géré au niveau des utilisateurs ? + + Au niveau des utilisateurs, comme un User Django a déjà les champs lastname/firstname + pourquoi ne pas les réutiliser ? On garde la clé OneToOne, mais on déplace dans l'autre + classe les champs qui peuvent l'être. `Email` s'y retrouve également. + + Les relations `cando`, `haveToDo` et `have_routine` ne sont pas correctement nommées. + Si tu as une instance de `Gymnast`, tu devrais faire ceci : + + >>> gregg = Gymnast.objects.first() + >>> gregg.have_routine <-- pas bien + >>> gregg.routines <-- ok + + Idéalement, cela pourrait même être une indirection. + + >>> gregg.routines <-- retourne la relation de type `have_routine` + >>> gregg.educatives.can_do <-- retourne les éducatifs qu'il **peut** faire + >>> gregg.educatives.must_do <-- retourne les éducatifs qu'il **doit** faire + (j'avoue ne pas tout à fait comprendre la nuance entre les deux) + + Tu as des fonctions qui ne sont pas du tout utilisées et qui pourrissent un peu le fichier. + >>> next_age() ? N'est appelé nulle part ailleurs. + >>> known_skills() <-- peut être récupéré directement via l'attribut `cando` + (du coup, tu as sans doute une piste de renommage ;-)) +""" -from django.db import models -from django.contrib.auth.models import User from datetime import date -from base.models import Markdownizable + +from django.contrib.auth.models import User from django.db import models + import pendulum -# from location.models import Club +from base.models import Markdownizable class Gymnast(Markdownizable): - """ - Représente un gymnaste. En plus des données personnels (nom, prénom, date de naissance, …) un gymnaste aura une photo et une orientation (de vrille). Un gymnaste peut être actif ou inactif. + """Représente un gymnaste. + + En plus de sa signalétique (nom, prénom, date de naissance, ...), + un gymnaste aura une photo et une orientation (de vrille). + + Un gymnaste peut être actif ou inactif. """ class Meta: @@ -90,16 +136,16 @@ class Gymnast(Markdownizable): @property def next_birthday(self): """Définit la prochaine date (de fête) d'anniversaire pour cette personne. - + Returns: Soit le jour/mois pour cette année Soit le jour/mois pour l'année prochaine. - + Examples: en supposant qu'on soit le 23/05/2019 >>> from datetime import date >>> gregg = People(name='Tru', firstname='Gregg', birthdate=date(1982, 2, 5) >>> gregg.next_birthday() - Date(2020, 2, 5) + Date(2020, 2, 5) """ now = pendulum.now() @@ -149,8 +195,7 @@ class Gymnast(Markdownizable): class Accident(Markdownizable): - """ - Classe représentant un accident. Un accident lie un saut à un gymnaste pour une date donnée. + """La classe `Accident` permet d'indiquer qu'un gymnaste est tombé durant un saut. """ class Meta: