diff --git a/.gitignore b/.gitignore index d7d26693..5a61dde4 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ __pycache__/ # virtualenv venv/ ENV/ +mise.toml # pipenv: https://github.com/kennethreitz/pipenv /Pipfile diff --git a/.idea/.gitignore b/.idea/.gitignore deleted file mode 100644 index 73f69e09..00000000 --- a/.idea/.gitignore +++ /dev/null @@ -1,8 +0,0 @@ -# Default ignored files -/shelf/ -/workspace.xml -# Datasource local storage ignored files -/dataSources/ -/dataSources.local.xml -# Editor-based HTTP Client requests -/httpRequests/ diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml deleted file mode 100644 index 03d9549e..00000000 --- a/.idea/inspectionProfiles/Project_Default.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/inspectionProfiles/profiles_settings.xml b/.idea/inspectionProfiles/profiles_settings.xml deleted file mode 100644 index 105ce2da..00000000 --- a/.idea/inspectionProfiles/profiles_settings.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/misc.xml b/.idea/misc.xml deleted file mode 100644 index 574ec96e..00000000 --- a/.idea/misc.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/modules.xml b/.idea/modules.xml deleted file mode 100644 index 2253d389..00000000 --- a/.idea/modules.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - \ No newline at end of file diff --git a/.idea/padam-django-tech-test.iml b/.idea/padam-django-tech-test.iml deleted file mode 100644 index c7ffe09b..00000000 --- a/.idea/padam-django-tech-test.iml +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/Makefile b/Makefile index 4062f4c4..5ea64905 100644 --- a/Makefile +++ b/Makefile @@ -3,3 +3,20 @@ run: ## Run the test server. install: ## Install the python requirements. pip install -r requirements.txt + +migrate: ## Apply migrations. + python manage.py migrate + +makemigrations: ## Create new migrations based on the models. + python manage.py makemigrations + +create_admin_user: ## Create a superuser for the admin interface. + python manage.py createsuperuser --username admin --email admin@test.com + +create_data: ## Create initial data. + python manage.py create_data + +test: ## Run all tests. + python manage.py test padam_django.tests + +setup_and_run: install migrate create_data create_admin_user run diff --git a/README.md b/README.md index f99d629d..a67452ad 100644 --- a/README.md +++ b/README.md @@ -193,3 +193,18 @@ en temps que d'autres ... - Privilégier la qualité et les bonnes pratiques. - Vous pouvez réduire le périmètre du projet si vous manquez de temps. Une ébauche de réponse est déjà une bonne chose. - Soyez prêt à présenter le sujet, à justifier vos choix et à parler de comment vous auriez fait les parties que vous avez laisser de côté. + + +--- + +### Notes Adèle +Things I would have done with more time: +- Make the BusStop form more user-friendly, sorting the places list by distance from the last bus stop coordinates + - in the context of a complete app dealing with bus routes, we would probably already have Postgis installed in a Postgres DB, so it could be a quick win +- Another nice option would be to be able to combine a classic select with a list and an autocomplete field (but I have not found a way to make it work with only django admin) +- Make the unique constraint take into account time to the minute would make it more useful + - I could not make it work in sqlite but I think with Postgres I could use TruncMinute("time") in the constraint +- Make error messages more explicit, referencing the id of the overlapping shift(s) +- Move BusShift and BusStop to a new app, as they might not really belong in "fleet" +- Create a few more tests to be exhaustive on BusShift error cases + - here I prioritized checking for overlap cases because it seemed like the riskiest part of the code (the rest being mostly managed by django) diff --git a/padam_django/apps/fleet/admin.py b/padam_django/apps/fleet/admin.py index 3fba5023..4df35bda 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.db.models import Min, Max +from django.forms import BaseInlineFormSet +from django.core.exceptions import ValidationError from . import models @@ -11,3 +14,71 @@ class BusAdmin(admin.ModelAdmin): @admin.register(models.Driver) class DriverAdmin(admin.ModelAdmin): pass + + +class BusStopFormSet(BaseInlineFormSet): + def _shift_has_overlap(self, shifts, departure_time, arrival_time): + if self.instance.pk: + shifts = shifts.exclude(pk=self.instance.pk) + return ( + shifts.annotate( + departure=Min("stops__time"), + arrival=Max("stops__time"), + ) + .filter( + departure__lte=arrival_time, + arrival__gte=departure_time, + ) + .exists() + ) + + def clean(self): + super().clean() + + # BusStops are not saved yet, so we have to get times from the forms + bus_stop_times = [ + form.cleaned_data.get("time") + for form in self.forms + if form.cleaned_data.get("time") and not form.cleaned_data.get("DELETE") + ] + + if len(bus_stop_times) < 2: + raise ValidationError("A bus shift must have at least two valid stops.") + + departure_time = min(bus_stop_times) + arrival_time = max(bus_stop_times) + + if self._shift_has_overlap( + self.instance.bus.shifts, departure_time, arrival_time + ): + raise ValidationError("This bus already has a conflicting shift.") + if self._shift_has_overlap( + self.instance.driver.shifts, departure_time, arrival_time + ): + raise ValidationError("This driver already has a conflicting shift.") + + +class BusStopInline(admin.TabularInline): + model = models.BusStop + formset = BusStopFormSet + min_num = 2 + extra = 0 + + +@admin.register(models.BusShift) +class BusShiftAdmin(admin.ModelAdmin): + fields = ["driver", "bus"] + list_display = ["__str__", "departure", "arrival", "duration"] + inlines = [BusStopInline] + + @admin.display(description="Departure") + def departure(self, obj): + return obj.departure_time + + @admin.display(description="Arrival") + def arrival(self, obj): + return obj.arrival_time + + @admin.display(description="Duration") + def duration(self, obj): + return obj.duration diff --git a/padam_django/apps/fleet/migrations/0003_busshift_busstop_busstop_unique_bus_stop_combination.py b/padam_django/apps/fleet/migrations/0003_busshift_busstop_busstop_unique_bus_stop_combination.py new file mode 100644 index 00000000..693e3878 --- /dev/null +++ b/padam_django/apps/fleet/migrations/0003_busshift_busstop_busstop_unique_bus_stop_combination.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.16 on 2025-12-30 20:27 + +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.RESTRICT, related_name='shifts', to='fleet.bus')), + ('driver', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='shifts', to='fleet.driver')), + ], + ), + migrations.CreateModel( + name='BusStop', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('time', models.DateTimeField(verbose_name='Scheduled time at the designated place for the related bus shift')), + ('bus_shift', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='stops', to='fleet.busshift')), + ('place', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='stops', to='geography.place')), + ], + ), + migrations.AddConstraint( + model_name='busstop', + constraint=models.UniqueConstraint(fields=('place', 'time', 'bus_shift'), name='unique_bus_stop_combination'), + ), + ] diff --git a/padam_django/apps/fleet/migrations/0004_alter_busstop_options_alter_busstop_time.py b/padam_django/apps/fleet/migrations/0004_alter_busstop_options_alter_busstop_time.py new file mode 100644 index 00000000..375dbc3e --- /dev/null +++ b/padam_django/apps/fleet/migrations/0004_alter_busstop_options_alter_busstop_time.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.16 on 2026-01-02 10:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('fleet', '0003_busshift_busstop_busstop_unique_bus_stop_combination'), + ] + + operations = [ + migrations.AlterModelOptions( + name='busstop', + options={'ordering': ['time']}, + ), + migrations.AlterField( + model_name='busstop', + name='time', + field=models.DateTimeField(verbose_name='Scheduled time of arrival at the stop'), + ), + ] diff --git a/padam_django/apps/fleet/models.py b/padam_django/apps/fleet/models.py index 4cd3f19d..80b9d324 100644 --- a/padam_django/apps/fleet/models.py +++ b/padam_django/apps/fleet/models.py @@ -1,8 +1,11 @@ from django.db import models +from padam_django.apps.geography.models import Place class Driver(models.Model): - user = models.OneToOneField('users.User', on_delete=models.CASCADE, related_name='driver') + user = models.OneToOneField( + "users.User", on_delete=models.CASCADE, related_name="driver" + ) def __str__(self): return f"Driver: {self.user.username} (id: {self.pk})" @@ -16,3 +19,51 @@ class Meta: def __str__(self): return f"Bus: {self.licence_plate} (id: {self.pk})" + + +class BusShift(models.Model): + driver = models.ForeignKey(Driver, on_delete=models.RESTRICT, related_name="shifts") + bus = models.ForeignKey(Bus, on_delete=models.RESTRICT, related_name="shifts") + + def __str__(self): + return f"BusShift: Driver {self.driver.user.username} with bus {self.bus.licence_plate} (id: {self.pk})" + + @property + def departure_time(self): + first_stop = self.stops.first() + return first_stop.time if first_stop else None + + @property + def arrival_time(self): + last_stop = self.stops.last() + return last_stop.time if last_stop else None + + @property + def duration(self): + arrival = self.arrival_time + departure = self.departure_time + if arrival and departure: + return arrival - departure + return None + + +class BusStop(models.Model): + place = models.ForeignKey(Place, on_delete=models.RESTRICT, related_name="stops") + bus_shift = models.ForeignKey( + BusShift, on_delete=models.CASCADE, related_name="stops" + ) + time = models.DateTimeField("Scheduled time of arrival at the stop") + + class Meta: + constraints = [ + models.UniqueConstraint( + # If a shift has two stops at the same place and time + # one must be a duplicate + fields=["place", "time", "bus_shift"], + name="unique_bus_stop_combination", + ) + ] + ordering = ["time"] + + def __str__(self): + return f"BusStop: {self.place.name} at {self.time.strftime('%H:%M')} (id: {self.pk})" diff --git a/padam_django/tests/__init__.py b/padam_django/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/padam_django/tests/test_fleet_admin.py b/padam_django/tests/test_fleet_admin.py new file mode 100644 index 00000000..6998f03e --- /dev/null +++ b/padam_django/tests/test_fleet_admin.py @@ -0,0 +1,334 @@ +from datetime import timedelta +from django.utils import timezone +from django.test import TestCase, RequestFactory +from django.contrib.admin.sites import AdminSite +from padam_django.apps.fleet.admin import ( + BusShiftAdmin, +) +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 BusStopFormSetTests(TestCase): + def setUp(self): + self.factory = RequestFactory() + self.admin_user = User.objects.create_superuser( + username="admin", password="password" + ) + self.bus = Bus.objects.create(licence_plate="ABC123") + self.user = User.objects.create_user(username="driver1", password="password") + self.driver = Driver.objects.create(user=self.user) + self.user2 = User.objects.create_user(username="driver2", password="password") + self.driver2 = Driver.objects.create(user=self.user2) + + self.place1 = Place.objects.create( + name="Place 1", latitude=48.8566, longitude=2.3522 + ) + self.place2 = Place.objects.create( + name="Place 2", latitude=48.8606, longitude=2.3376 + ) + self.place3 = Place.objects.create( + name="Place 3", latitude=48.8738, longitude=2.2950 + ) + + def _get_formset(self, bus_shift): + site = AdminSite() + admin = BusShiftAdmin(BusShift, site) + request = self.factory.get("/") + request.user = self.admin_user + inline_instance = admin.inlines[0](admin.model, site) + return inline_instance.get_formset(request, obj=bus_shift) + + def _create_existing_shift(self, start_time, end_time): + bus_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + BusStop.objects.create( + bus_shift=bus_shift, + place=self.place1, + time=start_time, + ) + BusStop.objects.create( + bus_shift=bus_shift, + place=self.place2, + time=end_time, + ) + return bus_shift + + def _make_formset_data(self, bus_shift, stops): + """Helper to create formset data + + Args: + bus_shift: The BusShift instance + stops: List of (place, time) tuples + """ + data = { + "stops-TOTAL_FORMS": str(len(stops)), + "stops-INITIAL_FORMS": "0", + "stops-MIN_NUM_FORMS": "0", + "stops-MAX_NUM_FORMS": "1000", + } + + for i, (place, time) in enumerate(stops): + data.update( + { + f"stops-{i}-place": place.pk, + f"stops-{i}-time_0": time.strftime("%Y-%m-%d"), + f"stops-{i}-time_1": time.strftime("%H:%M:%S"), + f"stops-{i}-id": "", + f"stops-{i}-bus_shift": bus_shift.pk, + } + ) + + return data + + def test_bus_shift_creation_success(self): + """Test creating a valid BusShift with 2 BusStops""" + bus_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + FormSet = self._get_formset(bus_shift) + + now = timezone.now() + formset_data = self._make_formset_data( + bus_shift, [(self.place1, now), (self.place2, now + timedelta(hours=2))] + ) + + formset = FormSet(data=formset_data, instance=bus_shift) + self.assertTrue( + formset.is_valid(), + f"Errors: {formset.errors}, Non-form errors: {formset.non_form_errors()}", + ) + formset.save() + self.assertEqual(BusStop.objects.count(), 2) + + def test_bus_shift_creation_failure_because_only_one_stop(self): + """Test creating a BusShift with only 1 BusStop fails""" + bus_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + FormSet = self._get_formset(bus_shift) + + now = timezone.now() + formset_data = self._make_formset_data(bus_shift, [(self.place1, now)]) + + formset = FormSet(data=formset_data, instance=bus_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "A bus shift must have at least two valid stops.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_failure_because_no_stops(self): + """Test creating a BusShift with no BusStops fails""" + bus_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + FormSet = self._get_formset(bus_shift) + + formset_data = self._make_formset_data(bus_shift, []) + + formset = FormSet(data=formset_data, instance=bus_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "A bus shift must have at least two valid stops.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_failure_because_duplicate_stops(self): + """Test creating a BusShift with duplicate BusStops fails""" + bus_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + FormSet = self._get_formset(bus_shift) + + now = timezone.now() + formset_data = self._make_formset_data( + bus_shift, [(self.place1, now), (self.place1, now)] + ) + + formset = FormSet(data=formset_data, instance=bus_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "Please correct the duplicate data for place and time, which must be unique.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_success_two_shifts_for_driver_and_bus_no_overlap(self): + """Test creating two BusShifts for the same driver and bus with no overlapping times""" + # Define the non-overlapping shift times + existing_shift_start = timezone.now() + existing_shift_end = existing_shift_start + timedelta(hours=1) + + new_shift_start = existing_shift_start + timedelta(hours=2) + new_shift_end = new_shift_start + timedelta(hours=3) + + # Create the first bus shift + self._create_existing_shift(existing_shift_start, existing_shift_end) + + # Create the second bus shift with non-overlapping times + second_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + FormSet = self._get_formset(second_shift) + + formset_data = self._make_formset_data( + second_shift, [(self.place2, new_shift_start), (self.place3, new_shift_end)] + ) + + formset = FormSet(data=formset_data, instance=second_shift) + self.assertTrue( + formset.is_valid(), + f"Errors: {formset.errors}, Non-form errors: {formset.non_form_errors()}", + ) + formset.save() + self.assertEqual(BusStop.objects.filter(bus_shift=second_shift).count(), 2) + + def test_bus_shift_creation_failure_because_overlapping_shift_both_bus_and_driver( + self, + ): + """Test creating a BusShift that overlaps with an existing shift as follows: + + Existing shift: |==========| + New shift: |===============| + """ + # Define the overlapping shift times + existing_shift_start = timezone.now() + existing_shift_end = existing_shift_start + timedelta(hours=2) + + new_shift_start = existing_shift_start + timedelta(hours=1) + new_shift_end = new_shift_start + timedelta(days=2) + + # Create a pre-existing bus shift in DB + self._create_existing_shift(existing_shift_start, existing_shift_end) + + # Create a new bus shift that will overlap + new_shift = BusShift.objects.create(bus=self.bus, driver=self.driver) + FormSet = self._get_formset(new_shift) + + formset_data = self._make_formset_data( + new_shift, [(self.place2, new_shift_start), (self.place3, new_shift_end)] + ) + + formset = FormSet(data=formset_data, instance=new_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "This bus already has a conflicting shift.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_failure_because_overlapping_shift_driver_only(self): + """Test creating a BusShift that overlaps with an existing shift for driver only + + Existing shift: |==================| + New shift: |=======| + """ + # Define the overlapping shift times + existing_shift_start = timezone.now() + existing_shift_end = existing_shift_start + timedelta(days=4) + + new_shift_start = existing_shift_start + timedelta(hours=1) + new_shift_end = existing_shift_start + timedelta(hours=3) + + # Create a pre-existing bus shift in DB + self._create_existing_shift(existing_shift_start, existing_shift_end) + + # Create a new bus shift that will overlap + new_bus = Bus.objects.create(licence_plate="DEF456") + new_shift = BusShift.objects.create(bus=new_bus, driver=self.driver) + FormSet = self._get_formset(new_shift) + + formset_data = self._make_formset_data( + new_shift, [(self.place2, new_shift_start), (self.place3, new_shift_end)] + ) + + formset = FormSet(data=formset_data, instance=new_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "This driver already has a conflicting shift.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_failure_because_overlapping_shift_bus_only(self): + """Test creating a BusShift that overlaps with an existing shift for bus only + + Existing shift: |=======| + New shift: |==================| + """ + # Define the overlapping shift times + existing_shift_start = timezone.now() + timedelta(hours=1) + existing_shift_end = existing_shift_start + timedelta(hours=2) + + new_shift_start = timezone.now() + new_shift_end = new_shift_start + timedelta(hours=4) + + # Create a pre-existing bus shift in DB + self._create_existing_shift(existing_shift_start, existing_shift_end) + + # Create a new bus shift that will overlap + new_shift = BusShift.objects.create(bus=self.bus, driver=self.driver2) + FormSet = self._get_formset(new_shift) + + formset_data = self._make_formset_data( + new_shift, [(self.place2, new_shift_start), (self.place3, new_shift_end)] + ) + + formset = FormSet(data=formset_data, instance=new_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "This bus already has a conflicting shift.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_failure_because_back_to_back_shift(self): + """Test creating a BusShift that overlaps with an existing shift with + an end time identical to the new shift's start time + + Existing shift: |=======| + New shift: |=======| + """ + # Define the overlapping shift times + existing_shift_start = timezone.now() + existing_shift_end = existing_shift_start + timedelta(hours=2) + + new_shift_start = existing_shift_end + new_shift_end = existing_shift_end + timedelta(hours=2) + + # Create a pre-existing bus shift in DB + self._create_existing_shift(existing_shift_start, existing_shift_end) + + # Create a new bus shift that will overlap + new_shift = BusShift.objects.create(bus=self.bus, driver=self.driver2) + FormSet = self._get_formset(new_shift) + + formset_data = self._make_formset_data( + new_shift, [(self.place2, new_shift_start), (self.place3, new_shift_end)] + ) + + formset = FormSet(data=formset_data, instance=new_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "This bus already has a conflicting shift.", + formset.non_form_errors(), + ) + + def test_bus_shift_creation_failure_because_identical_start_and_end(self): + """Test creating a BusShift that overlaps with an existing shift with + exactly the same start and end times + + Existing shift: |=======| + New shift: |=======| + """ + # Define the overlapping shift times + existing_shift_start = timezone.now() + existing_shift_end = existing_shift_start + timedelta(hours=2) + + new_shift_start = existing_shift_start + new_shift_end = existing_shift_end + + # Create a pre-existing bus shift in DB + self._create_existing_shift(existing_shift_start, existing_shift_end) + + # Create a new bus shift that will overlap + new_shift = BusShift.objects.create(bus=self.bus, driver=self.driver2) + FormSet = self._get_formset(new_shift) + + formset_data = self._make_formset_data( + new_shift, [(self.place2, new_shift_start), (self.place3, new_shift_end)] + ) + + formset = FormSet(data=formset_data, instance=new_shift) + self.assertFalse(formset.is_valid()) + self.assertIn( + "This bus already has a conflicting shift.", + formset.non_form_errors(), + ) diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 00000000..bbafd739 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +DJANGO_SETTINGS_MODULE = padam_django.settings