Topic notification transition (#412)

### Description Sending messages to a topic doesn't work on iOS when the app is in background. This branch will fix this by sending the messages to the tokens of users subscribed to the topic, instead of sending them to the topic directly. We can send a message to 500 devices per request, so in most cases we won't need to make more than one request to firebase. The PR add tests for notifications. Due to the usage of Firebase messaging to send notifications, tests can not check notifications can be sent. Instead, they make sure our endpoints to register/unregister a device or subscribe to topics work. To prevent anyone from subscribing to `bookingadmin` topic and then be notified of all booking requests, the notification is now sent to all members of the room manager group. We way want to send the notification to just a few persons instead of the whole group. Fix #291 --------- Co-authored-by: Armand Didierjean <[email protected]>
aeecleclair · May 11, 2024 · ae39dbf · ae39dbf
1 parent 5256b03
commit ae39dbf
Show file tree

Hide file tree

Showing 11 changed files with 489 additions and 290 deletions.
diff --git a/app/core/notification/cruds_notification.py b/app/core/notification/cruds_notification.py
@@ -45,6 +45,20 @@ async def get_messages_by_firebase_token(
     return result.scalars().all()
 
 
+async def get_messages_by_context_and_firebase_tokens(
+    context: str,
+    firebase_tokens: list[str],
+    db: AsyncSession,
+) -> Sequence[models_notification.Message]:
+    result = await db.execute(
+        select(models_notification.Message).where(
+            models_notification.Message.context == context,
+            models_notification.Message.firebase_device_token.in_(firebase_tokens),
+        ),
+    )
+    return result.scalars().all()
+
+
 async def remove_message_by_context_and_firebase_device_token(
     context: str,
     firebase_device_token: str,
@@ -265,3 +279,29 @@ async def get_topic_membership_by_user_id_and_custom_topic(
         ),
     )
     return result.scalars().first()
+
+
+async def get_user_ids_by_topic(
+    custom_topic: CustomTopic,
+    db: AsyncSession,
+) -> list[str]:
+    result = await db.execute(
+        select(models_notification.TopicMembership.user_id).where(
+            models_notification.TopicMembership.topic == custom_topic.topic,
+            models_notification.TopicMembership.topic_identifier
+            == custom_topic.topic_identifier,
+        ),
+    )
+    return list(result.scalars().all())
+
+
+async def get_firebase_tokens_by_user_ids(
+    user_ids: list[str],
+    db: AsyncSession,
+) -> list[str]:
+    result = await db.execute(
+        select(models_notification.FirebaseDevice.firebase_device_token).where(
+            models_notification.FirebaseDevice.user_id.in_(user_ids),
+        ),
+    )
+    return list(result.scalars().all())
diff --git a/app/core/notification/endpoints_notification.py b/app/core/notification/endpoints_notification.py
@@ -1,4 +1,4 @@
-from datetime import UTC, datetime
+from datetime import UTC, datetime, timedelta
 
 from fastapi import APIRouter, Body, Depends, HTTPException, Path
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -61,21 +61,6 @@ async def register_firebase_device(
             db=db,
         )
 
-    # We also need to subscribe the new token to the topics the user is subscribed to
-    topic_memberships = await cruds_notification.get_topic_memberships_by_user_id(
-        user_id=user.id,
-        db=db,
-    )
-
-    for topic_membership in topic_memberships:
-        await notification_manager.subscribe_tokens_to_topic(
-            tokens=[firebase_token],
-            custom_topic=CustomTopic(
-                topic=topic_membership.topic,
-                topic_identifier=topic_membership.topic_identifier,
-            ),
-        )
-
     firebase_device = models_notification.FirebaseDevice(
         user_id=user.id,
         firebase_device_token=firebase_token,
@@ -105,21 +90,6 @@ async def unregister_firebase_device(
     """
     # Anybody may unregister a device if they know its token, which should be secret
 
-    # We also need to unsubscribe the token to the topics the user is subscribed to
-    topic_memberships = await cruds_notification.get_topic_memberships_by_user_id(
-        user_id=user.id,
-        db=db,
-    )
-
-    for topic_membership in topic_memberships:
-        await notification_manager.unsubscribe_tokens_to_topic(
-            tokens=[firebase_token],
-            custom_topic=CustomTopic(
-                topic=topic_membership.topic,
-                topic_identifier=topic_membership.topic_identifier,
-            ),
-        )
-
     await cruds_notification.delete_firebase_devices(
         firebase_device_token=firebase_token,
         db=db,
@@ -254,12 +224,12 @@ async def get_topic(
 
 
 @router.get(
-    "/notification/topics/{topic_str}",
+    "/notification/topics/{topic}",
     status_code=200,
     response_model=list[str],
 )
 async def get_topic_identifier(
-    topic_str: str,
+    topic: Topic,
     db: AsyncSession = Depends(get_db),
     user: models_core.CoreUser = Depends(is_user),
 ):
@@ -272,7 +242,7 @@ async def get_topic_identifier(
     memberships = await cruds_notification.get_topic_memberships_with_identifiers_by_user_id_and_topic(
         user_id=user.id,
         db=db,
-        topic=Topic(topic_str),
+        topic=topic,
     )
 
     return [
@@ -289,7 +259,6 @@ async def get_topic_identifier(
     status_code=201,
 )
 async def send_notification(
-    message: schemas_notification.Message,
     user: models_core.CoreUser = Depends(is_user_a_member_of(GroupType.admin)),
     notification_tool: NotificationTool = Depends(get_notification_tool),
 ):
@@ -298,6 +267,42 @@ async def send_notification(
 
     **Only admins can use this endpoint**
     """
+    message = schemas_notification.Message(
+        context="notification-test",
+        is_visible=True,
+        title="Test notification",
+        content="Ceci est un test de notification",
+        # The notification will expire in 3 days
+        expire_on=datetime.now(UTC) + timedelta(days=3),
+    )
+    await notification_tool.send_notification_to_user(
+        user_id=user.id,
+        message=message,
+    )
+
+
+@router.post(
+    "/notification/send/future",
+    status_code=201,
+)
+async def send_future_notification(
+    user: models_core.CoreUser = Depends(is_user_a_member_of(GroupType.admin)),
+    notification_tool: NotificationTool = Depends(get_notification_tool),
+):
+    """
+    Send ourself a test notification.
+
+    **Only admins can use this endpoint**
+    """
+    message = schemas_notification.Message(
+        context="future-notification-test",
+        is_visible=True,
+        title="Test notification",
+        content="Ceci est un test de notification",
+        # The notification will expire in 3 days
+        expire_on=datetime.now(UTC) + timedelta(days=3),
+        delivery_datetime=datetime.now(UTC) + timedelta(minutes=3),
+    )
     await notification_tool.send_notification_to_user(
         user_id=user.id,
         message=message,

diff --git a/app/core/notification/notification_types.py b/app/core/notification/notification_types.py
@@ -8,7 +8,6 @@ class Topic(str, Enum):
 
     cinema = "cinema"
     advert = "advert"
-    bookingadmin = "bookingadmin"
     amap = "amap"
     booking = "booking"
     event = "event"

diff --git a/app/modules/advert/endpoints_advert.py b/app/modules/advert/endpoints_advert.py
@@ -1,6 +1,6 @@
 import logging
 import uuid
-from datetime import UTC, datetime
+from datetime import UTC, datetime, timedelta
 
 from fastapi import Depends, File, HTTPException, Query, UploadFile
 from fastapi.responses import FileResponse
@@ -265,23 +265,19 @@ async def create_advert(
         result = await cruds_advert.create_advert(db_advert=db_advert, db=db)
     except ValueError as error:
         raise HTTPException(status_code=400, detail=str(error))
+    message = Message(
+        context=f"advert-new-{id}",
+        is_visible=True,
+        title=f"📣 Annonce - {result.title}",
+        content=result.content,
+        # The notification will expire in 3 days
+        expire_on=datetime.now(UTC) + timedelta(days=3),
+    )
 
-    try:
-        now = datetime.now(UTC)
-        message = Message(
-            context=f"advert-{result.id}",
-            is_visible=True,
-            title=f"📣 Annonce - {result.title}",
-            content=result.content,
-            # The notification will expire in 3 days
-            expire_on=now.replace(day=now.day + 3),
-        )
-        await notification_tool.send_notification_to_topic(
-            custom_topic=CustomTopic(topic=Topic.advert),
-            message=message,
-        )
-    except Exception as error:
-        hyperion_error_logger.error(f"Error while sending advert notification, {error}")
+    await notification_tool.send_notification_to_topic(
+        custom_topic=CustomTopic(Topic.advert),
+        message=message,
+    )
 
     return result
 

diff --git a/app/modules/amap/endpoints_amap.py b/app/modules/amap/endpoints_amap.py
@@ -1,6 +1,6 @@
 import logging
 import uuid
-from datetime import UTC, datetime
+from datetime import UTC, datetime, timedelta
 
 from fastapi import Depends, HTTPException, Response
 from redis import Redis
@@ -9,6 +9,7 @@
 from app.core import models_core
 from app.core.groups.groups_type import GroupType
 from app.core.module import Module
+from app.core.notification.notification_types import CustomTopic, Topic
 from app.core.notification.schemas_notification import Message
 from app.core.users import cruds_users
 from app.core.users.endpoints_users import read_user
@@ -706,6 +707,7 @@ async def open_ordering_of_delivery(
     delivery_id: str,
     db: AsyncSession = Depends(get_db),
     user: models_core.CoreUser = Depends(is_user_a_member_of(GroupType.amap)),
+    notification_tool: NotificationTool = Depends(get_notification_tool),
 ):
     delivery = await cruds_amap.get_delivery_by_id(db=db, delivery_id=delivery_id)
     if delivery is None:
@@ -719,6 +721,19 @@ async def open_ordering_of_delivery(
 
     await cruds_amap.open_ordering_of_delivery(delivery_id=delivery_id, db=db)
 
+    message = Message(
+        context=f"amap-open-ordering-{delivery_id}",
+        is_visible=True,
+        title="🛒 AMAP - Nouvelle livraison disponible",
+        content="Viens commander !",
+        # The notification will expire in 3 days
+        expire_on=datetime.now(UTC) + timedelta(days=3),
+    )
+    await notification_tool.send_notification_to_topic(
+        custom_topic=CustomTopic(Topic.amap),
+        message=message,
+    )
+
 
 @module.router.post(
     "/amap/deliveries/{delivery_id}/lock",
@@ -891,23 +906,18 @@ async def create_cash_of_user(
         db=db,
     )
 
-    try:
-        if result:
-            now = datetime.now(UTC)
-            message = Message(
-                context=f"amap-cash-{user_id}",
-                is_visible=True,
-                title="AMAP - Solde mis à jour",
-                content=f"Votre nouveau solde est de {result.balance} €.",
-                # The notification will expire in 3 days
-                expire_on=now.replace(day=now.day + 3),
-            )
-            await notification_tool.send_notification_to_user(
-                user_id=user_id,
-                message=message,
-            )
-    except Exception as error:
-        hyperion_error_logger.error(f"Error while sending AMAP notification, {error}")
+    message = Message(
+        context=f"amap-cash-{user_id}",
+        is_visible=True,
+        title="AMAP - Solde mis à jour",
+        content=f"Votre nouveau solde est de {cash} €.",
+        # The notification will expire in 3 days
+        expire_on=datetime.now(UTC) + timedelta(days=3),
+    )
+    await notification_tool.send_notification_to_user(
+        user_id=user_id,
+        message=message,
+    )
 
     return result
 

diff --git a/app/modules/booking/endpoints_booking.py b/app/modules/booking/endpoints_booking.py
@@ -1,14 +1,15 @@
 import logging
 import uuid
-from datetime import UTC, datetime
+from datetime import UTC, datetime, timedelta
+from zoneinfo import ZoneInfo
 
 from fastapi import Depends, HTTPException
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.core import models_core
+from app.core.groups import cruds_groups
 from app.core.groups.groups_type import GroupType
 from app.core.module import Module
-from app.core.notification.notification_types import CustomTopic, Topic
 from app.core.notification.schemas_notification import Message
 from app.dependencies import (
     get_db,
@@ -265,26 +266,29 @@ async def create_booking(
     )
     await cruds_booking.create_booking(booking=db_booking, db=db)
     result = await cruds_booking.get_booking_by_id(db=db, booking_id=db_booking.id)
+    manager_group_id = result.room.manager_id
+    manager_group = await cruds_groups.get_group_by_id(
+        db=db,
+        group_id=manager_group_id,
+    )
+    local_start = result.start.astimezone(ZoneInfo("Europe/Paris"))
+    applicant_nickname = user.nickname if user.nickname else user.firstname
+    content = f"{applicant_nickname} - {result.room.name} {local_start.strftime('%m/%d/%Y, %H:%M')} - {result.reason}"
+    # Setting time to Paris timezone in order to have the correct time in the notification
+
+    if manager_group:
+        message = Message(
+            context=f"booking-new-{id}",
+            is_visible=True,
+            title="📅 Réservations - Nouvelle réservation",
+            content=content,
+            # The notification will expire in 3 days
+            expire_on=datetime.now(UTC) + timedelta(days=3),
+        )
 
-    try:
-        if result:
-            now = datetime.now(UTC)
-            message = Message(
-                # We use sunday date as context to avoid sending the recap twice
-                context=f"booking-create-{result.id}",
-                is_visible=True,
-                title="Réservations - Nouvelle réservation 📅",
-                content=f"{result.applicant.nickname} - {result.room.name} {result.start.strftime('%m/%d/%Y, %H:%M')} - {result.reason}",
-                # The notification will expire the next sunday
-                expire_on=now.replace(day=now.day + 3),
-            )
-            await notification_tool.send_notification_to_topic(
-                custom_topic=CustomTopic(topic=Topic.bookingadmin),
-                message=message,
-            )
-    except Exception as error:
-        hyperion_error_logger.error(
-            f"Error while sending cinema recap notification, {error}",
+        await notification_tool.send_notification_to_users(
+            user_ids=[user.id for user in manager_group.members],
+            message=message,
         )
 
     return result
@@ -326,7 +330,7 @@ async def edit_booking(
             db=db,
         )
     except ValueError as error:
-        raise HTTPException(status_code=422, detail=str(error))
+        raise HTTPException(status_code=400, detail=str(error))
 
 
 @module.router.patch(
@@ -434,7 +438,7 @@ async def create_room(
         )
         return await cruds_booking.create_room(db=db, room=room_db)
     except ValueError as error:
-        raise HTTPException(status_code=422, detail=str(error))
+        raise HTTPException(status_code=400, detail=str(error))
 
 
 @module.router.patch(