From 72bb4788f29597fe1ad66490f22c77437b876943 Mon Sep 17 00:00:00 2001
From: imperosol <thgirod@hotmail.com>
Date: Mon, 30 Jun 2025 22:36:17 +0200
Subject: [PATCH] test: room and slots creation/edition

---
 club/templates/club/club_tools.jinja          |   4 +-
 com/templates/com/news_list.jinja             |  10 +-
 reservation/api.py                            |  10 +-
 reservation/models.py                         |  15 +--
 reservation/schemas.py                        |   9 +-
 .../components/room-scheduler-index.ts        |   6 +-
 reservation/tests/test_room.py                | 113 ++++++++++++++++++
 reservation/tests/test_room_api.py            |  42 -------
 reservation/tests/test_slot.py                | 104 +++++++++++++++-
 reservation/views.py                          |  10 +-
 10 files changed, 249 insertions(+), 74 deletions(-)
 create mode 100644 reservation/tests/test_room.py
 delete mode 100644 reservation/tests/test_room_api.py

diff --git a/club/templates/club/club_tools.jinja b/club/templates/club/club_tools.jinja
index 8361e75e..5d927df4 100644
--- a/club/templates/club/club_tools.jinja
+++ b/club/templates/club/club_tools.jinja
@@ -48,7 +48,9 @@
         {%- endfor -%}
       </ul>
     {%- else -%}
-      {% trans %}This club manages no reservable room{% endtrans %}
+      <p>
+        {% trans %}This club manages no reservable room{% endtrans %}
+      </p>
     {%- endif -%}
     <h4>{% trans %}Counters:{% endtrans %}</h4>
     <ul>
diff --git a/com/templates/com/news_list.jinja b/com/templates/com/news_list.jinja
index c606e8f7..a7c6ccce 100644
--- a/com/templates/com/news_list.jinja
+++ b/com/templates/com/news_list.jinja
@@ -211,10 +211,12 @@
               <i class="fa-solid fa-magnifying-glass fa-xl"></i>
               <a href="{{ url("matmat:search_clear") }}">{% trans %}Matmatronch{% endtrans %}</a>
             </li>
-            <li>
-              <i class="fa-solid fa-thumbtack fa-xl"></i>
-              <a href="{{ url("reservation:main") }}">{% trans %}Room reservation{% endtrans %}</a>
-            </li>
+            {% if user.has_perm("reservation.view_reservationslot") %}
+              <li>
+                <i class="fa-solid fa-thumbtack fa-xl"></i>
+                <a href="{{ url("reservation:main") }}">{% trans %}Room reservation{% endtrans %}</a>
+              </li>
+            {% endif %}
             <li>
               <i class="fa-solid fa-check-to-slot fa-xl"></i>
               <a href="{{ url("election:list") }}">{% trans %}Elections{% endtrans %}</a>
diff --git a/reservation/api.py b/reservation/api.py
index 06e5cae5..cd39be33 100644
--- a/reservation/api.py
+++ b/reservation/api.py
@@ -1,4 +1,3 @@
-from datetime import timedelta
 from typing import Any, Literal
 
 from django.core.exceptions import ValidationError
@@ -6,7 +5,6 @@ from ninja import Query
 from ninja_extra import ControllerBase, api_controller, paginate, route
 from ninja_extra.pagination import PageNumberPaginationExtra
 from ninja_extra.schemas import PaginatedResponseSchema
-from pydantic import FutureDatetime
 
 from api.permissions import HasPerm
 from reservation.models import ReservationSlot, Room
@@ -15,6 +13,7 @@ from reservation.schemas import (
     RoomSchema,
     SlotFilterSchema,
     SlotSchema,
+    UpdateReservationSlotSchema,
 )
 
 
@@ -52,11 +51,12 @@ class ReservationSlotController(ControllerBase):
             409: dict[Literal["detail"], dict[str, list[str]]],
             422: dict[Literal["detail"], list[dict[str, Any]]],
         },
+        url_name="change_reservation_slot",
     )
-    def update_slot(self, start: FutureDatetime, duration: timedelta, slot_id: int):
+    def update_slot(self, slot_id: int, params: UpdateReservationSlotSchema):
         slot = self.get_object_or_exception(ReservationSlot, id=slot_id)
-        slot.start_at = start
-        slot.end_at = start + duration
+        slot.start_at = params.start_at
+        slot.end_at = params.end_at
         try:
             slot.full_clean()
             slot.save()
diff --git a/reservation/models.py b/reservation/models.py
index 1e340821..534beefc 100644
--- a/reservation/models.py
+++ b/reservation/models.py
@@ -5,7 +5,6 @@ from typing import Self
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.db.models import F, Q
-from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
 from club.models import Club
@@ -39,9 +38,6 @@ class Room(models.Model):
     def __str__(self):
         return self.name
 
-    def get_absolute_url(self):
-        return reverse("reservation:room_detail", kwargs={"room_id": self.id})
-
     def can_be_edited_by(self, user: User) -> bool:
         # a user may edit a room if it has the global perm
         # or is in the owner club board
@@ -95,9 +91,10 @@ class ReservationSlot(models.Model):
             # so in this case, don't do the overlap check and let
             # Django manage the non-null constraint error.
             return
-        if (
-            ReservationSlot.objects.overlapping_with(self)
-            .filter(room_id=self.room_id)
-            .exists()
-        ):
+        overlapping = ReservationSlot.objects.overlapping_with(self).filter(
+            room_id=self.room_id
+        )
+        if self.id is not None:
+            overlapping = overlapping.exclude(id=self.id)
+        if overlapping.exists():
             raise ValidationError(_("There is already a reservation on this slot."))
diff --git a/reservation/schemas.py b/reservation/schemas.py
index 434da8cb..b97bc509 100644
--- a/reservation/schemas.py
+++ b/reservation/schemas.py
@@ -1,7 +1,7 @@
 from datetime import datetime
 
-from ninja import FilterSchema, ModelSchema
-from pydantic import Field
+from ninja import FilterSchema, ModelSchema, Schema
+from pydantic import Field, FutureDatetime
 
 from club.schemas import SimpleClubSchema
 from core.schemas import SimpleUserSchema
@@ -39,3 +39,8 @@ class SlotSchema(ModelSchema):
     start: datetime = Field(alias="start_at")
     end: datetime = Field(alias="end_at")
     author: SimpleUserSchema
+
+
+class UpdateReservationSlotSchema(Schema):
+    start_at: FutureDatetime
+    end_at: FutureDatetime
diff --git a/reservation/static/bundled/reservation/components/room-scheduler-index.ts b/reservation/static/bundled/reservation/components/room-scheduler-index.ts
index 0dcf3e80..3f2a5554 100644
--- a/reservation/static/bundled/reservation/components/room-scheduler-index.ts
+++ b/reservation/static/bundled/reservation/components/room-scheduler-index.ts
@@ -80,9 +80,9 @@ export class RoomScheduler extends inheritHtmlElement("div") {
     const response = await reservationslotUpdateSlot({
       // biome-ignore lint/style/useNamingConvention: api is snake_case
       path: { slot_id: Number.parseInt(args.event.id) },
-      query: {
-        start: args.event.startStr,
-        duration: `PT${duration.getUTCHours()}H${duration.getUTCMinutes()}M${duration.getUTCSeconds()}S`,
+      body: {
+        start_at: args.event.startStr,
+        end_at: args.event.endStr,
       },
     });
     if (response.response.ok) {
diff --git a/reservation/tests/test_room.py b/reservation/tests/test_room.py
new file mode 100644
index 00000000..36ccd207
--- /dev/null
+++ b/reservation/tests/test_room.py
@@ -0,0 +1,113 @@
+import pytest
+from django.contrib.auth.models import Permission
+from django.test import Client
+from django.urls import reverse
+from model_bakery import baker
+from pytest_django.asserts import assertNumQueries, assertRedirects
+
+from club.models import Club
+from core.models import User
+from reservation.forms import RoomUpdateForm
+from reservation.models import Room
+
+
+@pytest.mark.django_db
+class TestFetchRoom:
+    @pytest.fixture
+    def user(self):
+        return baker.make(
+            User,
+            user_permissions=[Permission.objects.get(codename="view_room")],
+        )
+
+    def test_fetch_simple(self, client: Client, user: User):
+        rooms = baker.make(Room, _quantity=3, _bulk_create=True)
+        client.force_login(user)
+        response = client.get(reverse("api:fetch_reservable_rooms"))
+        assert response.status_code == 200
+        assert response.json() == [
+            {
+                "id": room.id,
+                "name": room.name,
+                "description": room.description,
+                "location": room.location,
+                "club": {"id": room.club.id, "name": room.club.name},
+            }
+            for room in rooms
+        ]
+
+    def test_nb_queries(self, client: Client, user: User):
+        client.force_login(user)
+        with assertNumQueries(5):
+            # 4 for authentication
+            # 1 to fetch the actual data
+            client.get(reverse("api:fetch_reservable_rooms"))
+
+
+@pytest.mark.django_db
+class TestCreateRoom:
+    def test_ok(self, client: Client):
+        perm = Permission.objects.get(codename="add_room")
+        club = baker.make(Club)
+        client.force_login(
+            baker.make(User, user_permissions=[perm], groups=[club.board_group])
+        )
+        response = client.post(
+            reverse("reservation:room_create"),
+            data={"club": club.id, "name": "test", "location": "BELFORT"},
+        )
+        assertRedirects(response, reverse("club:tools", kwargs={"club_id": club.id}))
+        room = Room.objects.last()
+        assert room is not None
+        assert room.club == club
+        assert room.name == "test"
+        assert room.location == "BELFORT"
+
+    def test_permission_denied(self, client: Client):
+        club = baker.make(Club)
+        client.force_login(baker.make(User))
+        response = client.get(reverse("reservation:room_create"))
+        assert response.status_code == 403
+        response = client.post(
+            reverse("reservation:room_create"),
+            data={"club": club.id, "name": "test", "location": "BELFORT"},
+        )
+        assert response.status_code == 403
+
+
+@pytest.mark.django_db
+class TestUpdateRoom:
+    def test_ok(self, client: Client):
+        club = baker.make(Club)
+        room = baker.make(Room, club=club)
+        client.force_login(baker.make(User, groups=[club.board_group]))
+        url = reverse("reservation:room_edit", kwargs={"room_id": room.id})
+        response = client.post(url, data={"name": "test", "location": "BELFORT"})
+        assertRedirects(response, url)
+        room.refresh_from_db()
+        assert room.club == club
+        assert room.name == "test"
+        assert room.location == "BELFORT"
+
+    def test_permission_denied(self, client: Client):
+        club = baker.make(Club)
+        room = baker.make(Room, club=club)
+        client.force_login(baker.make(User))
+        url = reverse("reservation:room_edit", kwargs={"room_id": room.id})
+        response = client.get(url)
+        assert response.status_code == 403
+        response = client.post(url, data={"name": "test", "location": "BELFORT"})
+        assert response.status_code == 403
+
+
+@pytest.mark.django_db
+class TestUpdateRoomForm:
+    def test_form_club_edition_rights(self):
+        """The club field should appear only if the request user can edit it."""
+        room = baker.make(Room)
+        perm = Permission.objects.get(codename="change_room")
+        user_authorized = baker.make(User, user_permissions=[perm])
+        assert "club" in RoomUpdateForm(request_user=user_authorized).fields
+
+        user_forbidden = baker.make(User, groups=[room.club.board_group])
+        assert "club" not in RoomUpdateForm(request_user=user_forbidden).fields
diff --git a/reservation/tests/test_room_api.py b/reservation/tests/test_room_api.py
deleted file mode 100644
index e2a8fa32..00000000
--- a/reservation/tests/test_room_api.py
+++ /dev/null
@@ -1,42 +0,0 @@
-import pytest
-from django.contrib.auth.models import Permission
-from django.test import Client
-from django.urls import reverse
-from model_bakery import baker
-from pytest_django.asserts import assertNumQueries
-
-from core.models import User
-from reservation.models import Room
-
-
-@pytest.mark.django_db
-class TestFetchRoom:
-    @pytest.fixture
-    def user(self):
-        return baker.make(
-            User,
-            user_permissions=[Permission.objects.get(codename="view_room")],
-        )
-
-    def test_fetch_simple(self, client: Client, user: User):
-        rooms = baker.make(Room, _quantity=3, _bulk_create=True)
-        client.force_login(user)
-        response = client.get(reverse("api:fetch_reservable_rooms"))
-        assert response.status_code == 200
-        assert response.json() == [
-            {
-                "id": room.id,
-                "name": room.name,
-                "description": room.description,
-                "location": room.location,
-                "club": {"id": room.club.id, "name": room.club.name},
-            }
-            for room in rooms
-        ]
-
-    def test_nb_queries(self, client: Client, user: User):
-        client.force_login(user)
-        with assertNumQueries(5):
-            # 4 for authentication
-            # 1 to fetch the actual data
-            client.get(reverse("api:fetch_reservable_rooms"))
diff --git a/reservation/tests/test_slot.py b/reservation/tests/test_slot.py
index 9131e14a..70ffe5db 100644
--- a/reservation/tests/test_slot.py
+++ b/reservation/tests/test_slot.py
@@ -17,10 +17,8 @@ from reservation.models import ReservationSlot, Room
 class TestFetchReservationSlotsApi:
     @pytest.fixture
     def user(self):
-        return baker.make(
-            User,
-            user_permissions=[Permission.objects.get(codename="view_reservationslot")],
-        )
+        perm = Permission.objects.get(codename="view_reservationslot")
+        return baker.make(User, user_permissions=[perm])
 
     def test_fetch_simple(self, client: Client, user: User):
         slots = baker.make(ReservationSlot, _quantity=5, _bulk_create=True)
@@ -55,6 +53,67 @@ class TestFetchReservationSlotsApi:
             client.get(reverse("api:fetch_reservation_slots"))
 
 
+@pytest.mark.django_db
+class TestUpdateReservationSlotApi:
+    @pytest.fixture
+    def user(self):
+        perm = Permission.objects.get(codename="change_reservationslot")
+        return baker.make(User, user_permissions=[perm])
+
+    @pytest.fixture
+    def slot(self):
+        return baker.make(
+            ReservationSlot,
+            start_at=now() + timedelta(hours=2),
+            end_at=now() + timedelta(hours=4),
+        )
+
+    def test_ok(self, client: Client, user: User, slot: ReservationSlot):
+        client.force_login(user)
+        new_start = (slot.start_at + timedelta(hours=1)).replace(microsecond=0)
+        response = client.patch(
+            reverse("api:change_reservation_slot", kwargs={"slot_id": slot.id}),
+            {"start_at": new_start, "end_at": new_start + timedelta(hours=2)},
+            content_type="application/json",
+        )
+        assert response.status_code == 200
+        slot.refresh_from_db()
+        assert slot.start_at.replace(microsecond=0) == new_start
+        assert slot.end_at.replace(microsecond=0) == new_start + timedelta(hours=2)
+
+    def test_change_past_event(self, client, user: User, slot: ReservationSlot):
+        """Test that moving a slot that already began is impossible."""
+        client.force_login(user)
+        new_start = now() - timedelta(hours=1)
+        response = client.patch(
+            reverse("api:change_reservation_slot", kwargs={"slot_id": slot.id}),
+            {"start_at": new_start, "end_at": new_start + timedelta(hours=2)},
+            content_type="application/json",
+        )
+
+        assert response.status_code == 422
+
+    def test_move_event_to_occupied_slot(
+        self, client: Client, user: User, slot: ReservationSlot
+    ):
+        client.force_login(user)
+        other_slot = baker.make(
+            ReservationSlot,
+            room=slot.room,
+            start_at=slot.end_at + timedelta(hours=1),
+            end_at=slot.end_at + timedelta(hours=3),
+        )
+        response = client.patch(
+            reverse("api:change_reservation_slot", kwargs={"slot_id": slot.id}),
+            {
+                "start_at": other_slot.start_at - timedelta(hours=1),
+                "end_at": other_slot.start_at + timedelta(hours=1),
+            },
+            content_type="application/json",
+        )
+        assert response.status_code == 409
+
+
 @pytest.mark.django_db
 class TestReservationForm:
     def test_ok(self):
@@ -109,3 +168,40 @@ class TestReservationForm:
         assert form.errors == {
             "__all__": ["Il y a déjà une réservation sur ce créneau."]
         }
+
+
+@pytest.mark.django_db
+class TestCreateReservationSlot:
+    @pytest.fixture
+    def user(self):
+        perms = Permission.objects.filter(
+            codename__in=["add_reservationslot", "view_reservationslot"]
+        )
+        return baker.make(User, user_permissions=list(perms))
+
+    def test_ok(self, client: Client, user: User):
+        client.force_login(user)
+        start = now() + timedelta(hours=2)
+        end = start + timedelta(hours=1)
+        room = baker.make(Room)
+        response = client.post(
+            reverse("reservation:make_reservation"),
+            {"room": room.id, "start_at": start, "end_at": end},
+        )
+        assert response.status_code == 200
+        assert response.headers.get("HX-Redirect", "") == reverse("reservation:main")
+        slot = ReservationSlot.objects.filter(room=room).last()
+        assert slot is not None
+        assert slot.start_at == start
+        assert slot.end_at == end
+        assert slot.author == user
+
+    def test_permissions_denied(self, client: Client):
+        client.force_login(baker.make(User))
+        start = now() + timedelta(hours=2)
+        end = start + timedelta(hours=1)
+        response = client.post(
+            reverse("reservation:make_reservation"),
+            {"room": baker.make(Room), "start_at": start, "end_at": end},
+        )
+        assert response.status_code == 403
diff --git a/reservation/views.py b/reservation/views.py
index 8e4fe55a..563345ef 100644
--- a/reservation/views.py
+++ b/reservation/views.py
@@ -2,7 +2,7 @@
 
 from django.contrib.auth.mixins import PermissionRequiredMixin
 from django.contrib.messages.views import SuccessMessageMixin
-from django.urls import reverse_lazy
+from django.urls import reverse, reverse_lazy
 from django.utils.translation import gettext_lazy as _
 from django.views.generic import CreateView, DeleteView, TemplateView, UpdateView
 
@@ -29,14 +29,13 @@ class ReservationFragment(PermissionRequiredMixin, FragmentMixin, CreateView):
 
 class ReservationScheduleView(PermissionRequiredMixin, UseFragmentsMixin, TemplateView):
     template_name = "reservation/schedule.jinja"
-    permission_required = "reservation.view_room"
+    permission_required = "reservation.view_reservationslot"
     fragments = {"add_slot_fragment": ReservationFragment}
 
 
-class RoomCreateView(SuccessMessageMixin, PermissionRequiredMixin, CreateView):
+class RoomCreateView(PermissionRequiredMixin, CreateView):
     form_class = RoomCreateForm
     template_name = "core/create.jinja"
-    success_message = _("%(name)s was created successfully")
     permission_required = "reservation.add_room"
 
     def get_initial(self):
@@ -47,6 +46,9 @@ class RoomCreateView(SuccessMessageMixin, PermissionRequiredMixin, CreateView):
                 init["club"] = Club.objects.filter(id=int(club_id)).first()
         return init
 
+    def get_success_url(self):
+        return reverse("club:tools", kwargs={"club_id": self.object.club_id})
+
 
 class RoomUpdateView(SuccessMessageMixin, CanEditMixin, UpdateView):
     model = Room