diff --git a/.gitignore b/.gitignore index d7d26693..b1e0e840 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ __pycache__/ # virtualenv venv/ +.venv/ ENV/ # pipenv: https://github.com/kennethreitz/pipenv /Pipfile diff --git a/DECISIONS.md b/DECISIONS.md new file mode 100644 index 00000000..b41d11fd --- /dev/null +++ b/DECISIONS.md @@ -0,0 +1,138 @@ +# Mes Choix Techniques + +Ce document reflete l'état de mes reflexions au cours du test. + + +## Objectifs du test à remplir + +- Modéliser BusShift et BusStop +- Pas de chevauchements bus/chauffeur +- Interface admin pour créer/modifier les trajets +- Minimum 2 arrêts par trajet + +## Modélisation + +### 1. Structure des modèles + +BusShift +├── Bus (1) +├── Driver (1) +└── BusStop (2 min - indéfini). + └── Place (1) + +**BusShift** +- id (PrimaryKey) +- `bus` (ForeignKey => Bus, PROTECT) +- `driver` (ForeignKey => Driver, PROTECT) +- Properties calculées : `departure_time`, `arrival_time`, `total_duration` + +**BusStop** +- id (PrimaryKey) +- `shift` (ForeignKey → BusShift, CASCADE) +- `place` (ForeignKey → Place, CASCADE) +- `time` (DateTimeField) +- `order` (PositiveIntegerField) + +**Justification du champ `order`** : +- Garantit l'ordre des arrêts indépendamment du temps +- Rends l'ordre plus explicite que la validation + + +## Stratégie on_delete + +### Bus/Driver / PROTECT + +- Un bus avec des trajets historiques ne doit pas pouvoir être supprimé +- Protection des data +- soft delete préférable? + +### BusStop / CASCADE + +- Un arrêt sans trajet n'a pas de sens +- Simplification de la gestion + +## Interface Admin + +### Choix : TabularInline + +**Alternatives considérées :** +1. Modèles séparés => UX horrible, pas de vue d'ensemble +2. TabularInline => Vue tableau intégrée +3. StackedInline =>Trop verbeux pour plusieurs arrêts et obligation de scroll +4. Drag & drop => Trop coûteux en temps + +**Avantages du TabularInline :** +- Vue d'ensemble du trajet sur une page +- Impossible de créer des arrêts orphelins +- Validation du minimum 2 arrêts via FormSet +- UX claire et intuitive + +## Contraintes à valider + +### 1. Pas de chevauchements bus/driver + + Deux trajets se chevauchent si l'un commence avant que l'autre ne se termine. +`start1 < end2 AND start2 < end1` + +Idéalement il faudrait valider au niveau du modele ET de l'admin +Par manque de temps on privilégie LA ROBUSTESSE + +Une validation seule au niveau de l'admin est plus risquée: + +- Ne valide pas les données par script malveillant +- Ne protége pas l'API +- Ne valide pas les data crée dans le shell +- Ou data crées par des tests. + +Dans l'idéal il devrait y avoir double validation front + back. + +### 2. Minimum 2 arrêts par trajet + +**Validation via FormSet** : La validation se fait au niveau du FormSet car le BusShift.save() n'est pas rappelé après l'ajout des stops inline dans l'admin. + +## Améliorations Futures + +### Admin / UX + +1. **Améliorer la vue** + - Visualisation graphique des plannings par bus/chauffeur + - Drag & drop pour réorganiser les shifts + - Vue timeline pour détecter visuellement les conflits + +2. **Fonction Duplication de shifts** + - Action admin "Dupliquer ce trajet" + +3. **Filtres plus avancés** (dates, trajet, lieu) + - Filtrer par plage de dates + - Filtrer par durée de trajet + - Recherche par lieu + + +### Backend / Archi + +1. **Validation de l'ordre chronologique** + - Vérifier que `time[n] < time[n+1]` + - Cohérence entre `order` et `time` + +2. **Tests unitaires supplémentaires (les 4 plus critiques)** + + - **Shifts back-to-back** (10h-12h puis 12h-14h) : valider le comportement souhaité + - **Modification d'un stop créant un conflit** : s'assurer que l'édition est bien validée + - **empêcher les doublons** : + - **Suppression d'un stop** : bloquer si le shift n'a que 2 stops + +3. **Soft delete** + - Garder l'historique des trajets supprimés pour analytics + +4. **API** + + - Endpoints (GET et POST) pour BusShift et BusStop + +5. **Contraintes** + - Check constraint : `order >= 0` + - Check constraint : durée trajet < 24h + +## Ce que j'aurais aimé faire différement: +Renforcer la validation Backend des 2 arrêts pour plus de robustesse. Et ajouter une validation frontend pour les chevauchements. + +Mieux committer le code. diff --git a/Makefile b/Makefile index 4062f4c4..6d49a568 100644 --- a/Makefile +++ b/Makefile @@ -3,3 +3,19 @@ run: ## Run the test server. install: ## Install the python requirements. pip install -r requirements.txt + +migrate: + python manage.py migrate + +makemigrations: + python manage.py makemigrations + +import_fake_data: + python manage.py create_data + +create_admin: + python manage.py createsuperuser + +test: + python manage.py test padam_django.apps.fleet.tests + diff --git a/padam_django/apps/fleet/admin.py b/padam_django/apps/fleet/admin.py index 3fba5023..73a5ebec 100644 --- a/padam_django/apps/fleet/admin.py +++ b/padam_django/apps/fleet/admin.py @@ -1,4 +1,7 @@ from django.contrib import admin +from django.core.exceptions import ValidationError +from django.forms import BaseInlineFormSet + from . import models @@ -11,3 +14,40 @@ class BusAdmin(admin.ModelAdmin): @admin.register(models.Driver) class DriverAdmin(admin.ModelAdmin): pass + + +class BusStopFormSet(BaseInlineFormSet): + def clean(self): + super().clean() + + valid_stops = 0 + for form in self.forms: + if form.cleaned_data and not form.cleaned_data.get('DELETE', False): + if form.cleaned_data.get('order') is not None and form.cleaned_data.get('place') and form.cleaned_data.get('time'): + valid_stops += 1 + + if valid_stops < 2: + raise ValidationError("Un shift doit avoir au moins deux arrêts (départ et arrivée)") + + +class BusStopInline(admin.TabularInline): + model = models.BusStop + formset = BusStopFormSet + extra = 2 + fields = ['order', 'place', 'time'] + ordering = ['order'] + + def get_extra(self, request, obj=None, **kwargs): + if obj: + return 0 + return 2 + + +@admin.register(models.BusShift) +class BusShiftAdmin(admin.ModelAdmin): + list_display = ['id', 'bus', 'driver', 'departure_time', 'arrival_time', 'total_duration'] + list_filter = ['bus', 'driver'] + inlines = [BusStopInline] + + def get_queryset(self, request): + return super().get_queryset(request).select_related('bus', 'driver__user') diff --git a/padam_django/apps/fleet/migrations/0003_busshift_busstop.py b/padam_django/apps/fleet/migrations/0003_busshift_busstop.py new file mode 100644 index 00000000..8ee6e0e3 --- /dev/null +++ b/padam_django/apps/fleet/migrations/0003_busshift_busstop.py @@ -0,0 +1,41 @@ +# Generated by Django 4.2.16 on 2026-01-08 09:52 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('geography', '0001_initial'), + ('fleet', '0002_auto_20211109_1456'), + ] + + operations = [ + migrations.CreateModel( + name='BusShift', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('bus', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='bus_shifts', to='fleet.bus')), + ('driver', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='driver_shifts', to='fleet.driver')), + ], + options={ + 'verbose_name': 'Bus Shift', + 'verbose_name_plural': 'Bus Shifts', + }, + ), + migrations.CreateModel( + name='BusStop', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('time', models.DateTimeField(verbose_name='Stop time')), + ('order', models.PositiveIntegerField(verbose_name='Stop order')), + ('place', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='bus_stops', to='geography.place')), + ('shift', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='stops', to='fleet.busshift')), + ], + options={ + 'ordering': ['order'], + 'unique_together': {('shift', 'place', 'time')}, + }, + ), + ] diff --git a/padam_django/apps/fleet/models.py b/padam_django/apps/fleet/models.py index 4cd3f19d..8f8fa774 100644 --- a/padam_django/apps/fleet/models.py +++ b/padam_django/apps/fleet/models.py @@ -1,5 +1,6 @@ from django.db import models - +from django.db.models import Min, Max +from django.core.exceptions import ValidationError class Driver(models.Model): user = models.OneToOneField('users.User', on_delete=models.CASCADE, related_name='driver') @@ -16,3 +17,83 @@ class Meta: def __str__(self): return f"Bus: {self.licence_plate} (id: {self.pk})" + + +class BusShift(models.Model): + bus = models.ForeignKey(Bus, on_delete=models.PROTECT, related_name='bus_shifts') + driver = models.ForeignKey(Driver, on_delete=models.PROTECT, related_name='driver_shifts') + + class Meta: + verbose_name = "Bus Shift" + verbose_name_plural = "Bus Shifts" + + def __str__(self): + return f"Shift: Bus {self.bus.licence_plate}, Driver {self.driver.user.username} (id: {self.pk})" + + def clean(self): + super().clean() + if self.pk: + stop_count = self.stops.count() + if stop_count < 2: + raise ValidationError("Un shift doit avoir au moins deux arrêts (départ et arrivée)") + + def save(self, *args, **kwargs): + self.full_clean() + super().save(*args, **kwargs) + + @property + def departure_time(self): + first_stop = self.stops.order_by('order').first() + return first_stop.time if first_stop else None + + @property + def arrival_time(self): + last_stop = self.stops.order_by('order').last() + return last_stop.time if last_stop else None + + @property + def total_duration(self): + if self.departure_time and self.arrival_time: + return self.arrival_time - self.departure_time + return None + + +class BusStop(models.Model): + shift = models.ForeignKey(BusShift, on_delete=models.CASCADE, related_name='stops') + place = models.ForeignKey('geography.Place', on_delete=models.CASCADE, related_name='bus_stops') + time = models.DateTimeField("Stop time") + order = models.PositiveIntegerField("Stop order") + + class Meta: + unique_together = ['shift', 'place', 'time'] + ordering = ['order'] + + def clean(self): + super().clean() + if not self.time or not self.shift_id: + return + + current_stops = self.shift.stops.exclude(pk=self.pk) if self.pk else self.shift.stops.all() + times = list(current_stops.values_list('time', flat=True)) + [self.time] + + departure = min(times) + arrival = max(times) + + + def check_overlap(filter_field, resource, error_label): + overlapping = ( + BusShift.objects.filter(**{filter_field: resource}) + .exclude(pk=self.shift.pk) + .annotate(departure=Min('stops__time'), arrival=Max('stops__time')) + .filter(departure__lt=arrival, arrival__gt=departure) + ) + if overlapping.exists(): + raise ValidationError(f"{error_label} est déjà assigné pendant cette période") + + + check_overlap('bus', self.shift.bus, f"Le bus {self.shift.bus.licence_plate}") + check_overlap('driver', self.shift.driver, f"Le chauffeur {self.shift.driver.user.username}") + + def save(self, *args, **kwargs): + self.full_clean() + super().save(*args, **kwargs) diff --git a/padam_django/apps/fleet/tests/__init__.py b/padam_django/apps/fleet/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/padam_django/apps/fleet/tests/test_busshift.py b/padam_django/apps/fleet/tests/test_busshift.py new file mode 100644 index 00000000..5bdc7cdb --- /dev/null +++ b/padam_django/apps/fleet/tests/test_busshift.py @@ -0,0 +1,129 @@ +from datetime import datetime, timedelta +from django.test import TestCase +from django.core.exceptions import ValidationError +from django.utils import timezone + +from padam_django.apps.fleet.models import Bus, Driver, BusShift, BusStop +from padam_django.apps.geography.models import Place +from padam_django.apps.users.models import User + + +class BusShiftValidationTest(TestCase): + + def setUp(self): + self.base_time = timezone.make_aware(datetime(1995, 9, 1, 9, 0, 0)) + + self.hogwarts = Place.objects.create( + name='Hogwarts Castle', + latitude=57.1103, + longitude=-3.9403 + ) + self.hogsmeade = Place.objects.create( + name='Hogsmeade Village', + latitude=57.1200, + longitude=-3.9500 + ) + self.diagon_alley = Place.objects.create( + name='Diagon Alley', + latitude=51.5074, + longitude=-0.1278 + ) + self.kings_cross = Place.objects.create( + name="King's Cross Station", + latitude=51.5308, + longitude=-0.1238 + ) + + def create_driver(self, username, password='hogwarts123'): + user = User.objects.create_user(username=username, password=password) + return Driver.objects.create(user=user) + + def create_bus(self, plate): + return Bus.objects.create(licence_plate=plate) + + def create_shift_with_stops(self, bus, driver, stop_data): + shift = BusShift.objects.create(bus=bus, driver=driver) + for place, time_offset, order in stop_data: + BusStop.objects.create( + shift=shift, + place=place, + time=self.base_time + time_offset, + order=order + ) + return shift + + def test_minimum_two_stops_required(self): + """Test : Un shift doit avoir au moins 2 arrêts""" + hagrid = self.create_driver('rubeus.hagrid') + knight_bus = self.create_bus('KNIGHT-BUS') + journey = BusShift.objects.create(bus=knight_bus, driver=hagrid) + + BusStop.objects.create( + shift=journey, + place=self.hogwarts, + time=self.base_time, + order=1 + ) + + with self.assertRaises(ValidationError) as context: + journey.clean() + + self.assertIn("au moins deux arrêts", str(context.exception)) + + def test_bus_overlap_detection(self): + """Test : Un bus ne peut pas avoir deux shifts qui se chevauchent""" + harry = self.create_driver('harry.potter') + ron = self.create_driver('ron.weasley') + hogwarts_express = self.create_bus('HOGWARTS-EXP') + + self.create_shift_with_stops( + hogwarts_express, + harry, + [ + (self.kings_cross, timedelta(hours=0), 1), + (self.hogwarts, timedelta(hours=6), 2) + ] + ) + + conflicting_trip = BusShift.objects.create(bus=hogwarts_express, driver=ron) + overlapping_stop = BusStop( + shift=conflicting_trip, + place=self.diagon_alley, + time=self.base_time + timedelta(hours=3), + order=1 + ) + + with self.assertRaises(ValidationError) as context: + overlapping_stop.clean() + + self.assertIn("bus", str(context.exception).lower()) + self.assertIn("assigné", str(context.exception).lower()) + + def test_driver_overlap_detection(self): + """Test : Un chauffeur ne peut pas avoir deux shifts qui se chevauchent""" + hermione = self.create_driver('hermione.granger') + thestral_carriage = self.create_bus('THESTRAL-01') + hippogriff_carriage = self.create_bus('HIPPOGRIFF-01') + + self.create_shift_with_stops( + thestral_carriage, + hermione, + [ + (self.hogsmeade, timedelta(hours=0), 1), + (self.hogwarts, timedelta(hours=4), 2) + ] + ) + + impossible_route = BusShift.objects.create(bus=hippogriff_carriage, driver=hermione) + double_booking = BusStop( + shift=impossible_route, + place=self.diagon_alley, + time=self.base_time + timedelta(hours=2), + order=1 + ) + + with self.assertRaises(ValidationError) as context: + double_booking.clean() + + self.assertIn("chauffeur", str(context.exception).lower()) + self.assertIn("assigné", str(context.exception).lower())