Create an appointment when someone requests a booking. Delete it when…

… denied (#329)

* Create an appointment when someone requests a booking. Delete it when denied.

* Remove deprecated tests, mark some more things as deprecated, fix up tests to include appointment check.

* 🌐 complete German translation

* 💚 reduce row height in list view

---------

Co-authored-by: Andreas Müller <[email protected]>

backend/src/appointment/database/models.py

@@ -189,11 +189,11 @@ class Slot(Base):
     booking_expires_at = Column(DateTime)
     booking_status = Column(Enum(BookingStatus), default=BookingStatus.none)
 
-    appointment = relationship("Appointment", back_populates="slots")
-    schedule = relationship("Schedule", back_populates="slots")
+    appointment: Appointment = relationship("Appointment", back_populates="slots")
+    schedule: 'Schedule' = relationship("Schedule", back_populates="slots")
 
-    attendee = relationship("Attendee", cascade="all,delete", back_populates="slots")
-    subscriber = relationship("Subscriber", back_populates="slots")
+    attendee: Attendee = relationship("Attendee", cascade="all,delete", back_populates="slots")
+    subscriber: Subscriber = relationship("Subscriber", back_populates="slots")
 
 
 class Schedule(Base):

backend/src/appointment/database/repo.py

@@ -296,13 +296,14 @@ def delete_subscriber_calendar_by_subscriber_id(db: Session, subscriber_id: int)
 """
 
 
-def create_calendar_appointment(db: Session, appointment: schemas.AppointmentFull, slots: list[schemas.SlotBase]):
+def create_calendar_appointment(db: Session, appointment: schemas.AppointmentFull, slots: list[schemas.SlotBase] = []):
     """create new appointment with slots for calendar"""
     db_appointment = models.Appointment(**appointment.dict())
     db.add(db_appointment)
     db.commit()
     db.refresh(db_appointment)
-    add_appointment_slots(db, slots, db_appointment.id)
+    if len(slots) > 0:
+        add_appointment_slots(db, slots, db_appointment.id)
     return db_appointment
 
 
@@ -374,7 +375,7 @@ def delete_calendar_appointments_by_subscriber_id(db: Session, subscriber_id: in
 """
 
 
-def get_slot(db: Session, slot_id: int):
+def get_slot(db: Session, slot_id: int) -> models.Slot | None:
     """retrieve slot by id"""
     if slot_id:
         return db.get(models.Slot, slot_id)
@@ -429,7 +430,7 @@ def schedule_slot_exists(db: Session, slot: schemas.SlotBase, schedule_id: int):
     return db_slot is not None
 
 
-def book_slot(db: Session, slot_id: int):
+def book_slot(db: Session, slot_id: int) -> models.Slot | None:
     """update booking status for slot of given id"""
     db_slot = get_slot(db, slot_id)
     db_slot.booking_status = models.BookingStatus.booked
@@ -480,9 +481,6 @@ def delete_slot(db: Session, slot_id: int):
 def slot_is_available(db: Session, slot_id: int):
     """check if slot is still available for booking"""
     slot = get_slot(db, slot_id)
-    isAttended = slot.attendee_id or slot.subscriber_id
-    if slot.appointment:
-        return slot and not isAttended
     if slot.schedule:
         return slot and slot.booking_status == models.BookingStatus.requested
     return False

backend/src/appointment/defines.py

@@ -5,3 +5,6 @@
 
 # list of redis keys
 REDIS_REMOTE_EVENTS_KEY = 'rmt_events'
+
+APP_ENV_DEV = 'dev'
+APP_ENV_TEST = 'test'

backend/src/appointment/main.py

@@ -8,7 +8,7 @@
 from starlette_context.middleware import RawContextMiddleware
 from fastapi import Request
 
-from .l10n import l10n
+from .defines import APP_ENV_DEV, APP_ENV_TEST
 from .middleware.l10n import L10n
 # Ignore "Module level import not at top of file"
 # ruff: noqa: E402
@@ -141,10 +141,15 @@ def server():
     async def warn_about_deprecated_routes(request: Request, call_next):
         """Warn about clients using deprecated routes"""
         response = await call_next(request)
-        if (os.getenv('APP_ENV') == 'dev'
-            and request.scope.get('route')
-            and request.scope['route'].deprecated):
-            logging.warning(f"Use of deprecated route: `{request.scope['route'].path}`!")
+        if request.scope.get('route') and request.scope['route'].deprecated:
+            app_env = os.getenv('APP_ENV')
+            if app_env == APP_ENV_DEV:
+                logging.warning(f"Use of deprecated route: `{request.scope['route'].path}`!")
+            elif app_env == APP_ENV_TEST:
+                # Stale test runtime error
+                #raise RuntimeError(f"Test uses deprecated route: `{request.scope['route'].path}`!")
+                # Just log for this PR, we'll fix it another PR.
+                logging.error(f"Test uses deprecated route: `{request.scope['route'].path}`!")
         return response
 
     @app.exception_handler(DefaultCredentialsError)

backend/src/appointment/routes/api.py

@@ -412,7 +412,7 @@ def delete_my_appointment(id: int, db: Session = Depends(get_db), subscriber: Su
     return repo.delete_calendar_appointment(db=db, appointment_id=id)
 
 
-@router.get("/apmt/public/{slug}", response_model=schemas.AppointmentOut)
+@router.get("/apmt/public/{slug}", response_model=schemas.AppointmentOut, deprecated=True)
 def read_public_appointment(slug: str, db: Session = Depends(get_db)):
     """endpoint to retrieve an appointment from db via public link and only expose necessary data"""
     a = repo.get_public_appointment(db, slug=slug)
@@ -429,7 +429,7 @@ def read_public_appointment(slug: str, db: Session = Depends(get_db)):
     )
 
 
-@router.put("/apmt/public/{slug}", response_model=schemas.SlotAttendee)
+@router.put("/apmt/public/{slug}", response_model=schemas.SlotAttendee, deprecated=True)
 def update_public_appointment_slot(
     slug: str,
     s_a: schemas.SlotAttendee,

backend/src/appointment/routes/schedule.py

@@ -12,7 +12,7 @@
 from ..controller.calendar import CalDavConnector, Tools, GoogleConnector
 from ..controller.apis.google_client import GoogleClient
 from ..controller.auth import signed_url_by_subscriber
-from ..database import repo, schemas
+from ..database import repo, schemas, models
 from ..database.models import Subscriber, CalendarProvider, random_slug, BookingStatus, MeetingLinkProviderType, ExternalConnectionType
 from ..database.schemas import ExternalConnection
 from ..dependencies.auth import get_subscriber, get_subscriber_from_signed_url
@@ -209,22 +209,51 @@ def request_schedule_availability_slot(
     slot.booking_expires_at = datetime.now() + timedelta(days=1)
     slot.booking_status = BookingStatus.requested
     slot = repo.add_schedule_slot(db, slot, schedule.id)
+
     # create attendee for this slot
     attendee = repo.update_slot(db, slot.id, s_a.attendee)
+
     # generate confirm and deny links with encoded booking token and signed owner url
     url = f"{signed_url_by_subscriber(subscriber)}/confirm/{slot.id}/{token}"
+
     # human readable date in subscribers timezone
     # TODO: handle locale date representation
     date = slot.start.replace(tzinfo=timezone.utc).astimezone(ZoneInfo(subscriber.timezone)).strftime("%c")
     date = f"{date}, {slot.duration} minutes"
 
+    # Create a pending appointment
+    attendee_name = slot.attendee.name if slot.attendee.name is not None else slot.attendee.email
+    subscriber_name = subscriber.name if subscriber.name is not None else subscriber.email
+    title = f"Appointment - {subscriber_name} and {attendee_name}"
+
+    appointment = repo.create_calendar_appointment(db, schemas.AppointmentFull(
+        title=title,
+        details=schedule.details,
+        calendar_id=db_calendar.id,
+        duration=slot.duration,
+        status=models.AppointmentStatus.opened,
+        location_type=schedule.location_type,
+        location_url=schedule.location_url,
+    ))
+
+    # Update the slot
+    slot.appointment_id = appointment.id
+    db.add(slot)
+    db.commit()
+
     # Sending confirmation email to owner
     background_tasks.add_task(send_confirmation_email, url=url, attendee=attendee, date=date, to=subscriber.email)
 
     # Sending pending email to attendee
     background_tasks.add_task(send_pending_email, owner=subscriber, date=date, to=slot.attendee.email)
 
-    return True
+    # Mini version of slot, so we can grab the newly created slot id for tests
+    return schemas.SlotOut(
+        id=slot.id,
+        start=slot.start,
+        duration=slot.duration,
+        attendee_id=slot.attendee_id,
+    )
 
 
 @router.put("/public/availability/booking", response_model=schemas.AvailabilitySlotAttendee)
@@ -281,8 +310,14 @@ def decide_on_schedule_availability_slot(
         date = f"{date}, {slot.duration} minutes"
         # send rejection information to bookee
         background_tasks.add_task(send_rejection_email, owner=subscriber, date=date, to=slot.attendee.email)
-        # delete the scheduled slot to make the time available again
-        repo.delete_slot(db, slot.id)
+
+        if slot.appointment_id:
+            # delete the appointment, this will also delete the slot.
+            repo.delete_calendar_appointment(db, slot.appointment_id)
+        else:
+            # delete the scheduled slot to make the time available again
+            repo.delete_slot(db, slot.id)
+
     # otherwise, confirm slot and create event
     else:
         slot = repo.book_slot(db, slot.id)
@@ -316,10 +351,13 @@ def decide_on_schedule_availability_slot(
                 if os.getenv('SENTRY_DSN') != '':
                     capture_exception(err)
 
-        attendee_name = slot.attendee.name if slot.attendee.name is not None else slot.attendee.email
-        subscriber_name = subscriber.name if subscriber.name is not None else subscriber.email
+        if not slot.appointment:
+            attendee_name = slot.attendee.name if slot.attendee.name is not None else slot.attendee.email
+            subscriber_name = subscriber.name if subscriber.name is not None else subscriber.email
 
-        title = f"Appointment - {subscriber_name} and {attendee_name}"
+            title = f"Appointment - {subscriber_name} and {attendee_name}"
+        else:
+            title = slot.appointment.title
 
         event = schemas.Event(
             title=title,

backend/test/integration/test_appointment.py

@@ -306,25 +306,6 @@ def test_read_public_missing_appointment(self, with_client, make_appointment):
         response = with_client.get(f"/apmt/public/{generated_appointment.slug}-that-isnt-real")
         assert response.status_code == 404, response.text
 
-    def test_attendee_selects_appointment_slot(self, with_client, make_appointment):
-        generated_appointment = make_appointment()
-
-        response = with_client.put(
-            f"/apmt/public/{generated_appointment.slug}",
-            json={
-                "slot_id": generated_appointment.slots[0].id,
-                "attendee": {
-                    "email": "[email protected]",
-                    "name": "John Doe",
-                },
-            },
-        )
-        assert response.status_code == 200, response.text
-        data = response.json()
-        assert data["slot_id"] == generated_appointment.slots[0].id
-        assert data["attendee"]["email"] == "[email protected]"
-        assert data["attendee"]["name"] == "John Doe"
-
     def test_read_public_appointment_after_attendee_selection(self, with_db, with_client, make_appointment, make_attendee, make_appointment_slot):
         generated_appointment = make_appointment()
         generated_attendee = make_attendee()
@@ -379,15 +360,6 @@ def test_attendee_selects_missing_slot_of_existing_appointment(self, with_client
         )
         assert response.status_code == 404, response.text
 
-    def test_attendee_provides_invalid_email_address(self, with_client, make_appointment):
-        generated_appointment = make_appointment()
-
-        response = with_client.put(
-            f"/apmt/public/{generated_appointment.slug}",
-            json={"slot_id": generated_appointment.slots[0].id, "attendee": {"email": "a", "name": "b"}},
-        )
-        assert response.status_code == 400, response.text
-
     def test_get_remote_caldav_events(self, with_client, make_appointment, monkeypatch):
         """Test against a fake remote caldav, we're testing the route controller, not the actual caldav connector here!"""
         from appointment.controller.calendar import CalDavConnector

backend/test/integration/test_schedule.py

@@ -4,7 +4,7 @@
 
 from appointment.controller.auth import signed_url_by_subscriber
 from appointment.controller.calendar import CalDavConnector
-from appointment.database import schemas
+from appointment.database import schemas, repo
 from appointment.exceptions import validation
 from defines import DAY1, DAY5, DAY14, auth_headers, DAY2
 
@@ -298,8 +298,8 @@ def list_events(self, start, end):
             assert slots[0]['start'] == '2025-06-30T09:00:00-07:00'
             assert slots[-1]['start'] == '2025-07-11T16:30:00-07:00'
 
-
-    def test_request_schedule_availability_slot(self, monkeypatch, with_client, make_pro_subscriber, make_caldav_calendar, make_schedule):
+    def test_request_schedule_availability_slot(self, monkeypatch, with_db, with_client, make_pro_subscriber, make_caldav_calendar, make_schedule):
+        """Test that a user can request a booking from a schedule"""
         start_date = date(2024, 4, 1)
         start_time = time(9)
         start_datetime = datetime.combine(start_date, start_time)
@@ -331,7 +331,7 @@ def bust_cached_events(self, all_calendars = False):
 
         subscriber = make_pro_subscriber()
         generated_calendar = make_caldav_calendar(subscriber.id, connected=True)
-        make_schedule(
+        schedule = make_schedule(
             calendar_id=generated_calendar.id,
             active=True,
             start_date=start_date,
@@ -365,7 +365,7 @@ def bust_cached_events(self, all_calendars = False):
             },
             headers=auth_headers,
         )
-        print(response.status_code, response.json())
+
         assert response.status_code == 403, response.text
         data = response.json()
 
@@ -390,4 +390,11 @@ def bust_cached_events(self, all_calendars = False):
         )
         assert response.status_code == 200, response.text
         data = response.json()
-        assert data is True
+        assert data.get('id')
+
+        slot_id = data.get('id')
+
+        # Look up the slot
+        with with_db() as db:
+            slot = repo.get_slot(db, slot_id)
+            assert slot.appointment_id