Skip to content

Commit 1b405f0

Browse files
authored
Various notifications fixes (#844)
- Fix: log distinct usernames - Ditchline break and separate usernames with commas - Better type hints - Reuse an unused CRUD to get usernames by topic - Use `send_each` to get a `BatchResponse` when sending to a topic (not so useful unfortunately)
1 parent 9a05167 commit 1b405f0

File tree

2 files changed

+39
-23
lines changed

2 files changed

+39
-23
lines changed

app/core/notification/cruds_notification.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -231,16 +231,19 @@ async def get_topic_membership_by_user_id_and_topic_id(
231231
return result.scalars().first()
232232

233233

234-
async def get_user_ids_by_topic_id(
234+
async def get_usernames_by_topic_id(
235235
topic_id: UUID,
236236
db: AsyncSession,
237237
) -> list[str]:
238238
result = await db.execute(
239-
select(models_notification.TopicMembership.user_id).where(
240-
models_notification.TopicMembership.topic_id == topic_id,
241-
),
239+
select(models_users.CoreUser)
240+
.join(
241+
models_notification.TopicMembership,
242+
models_users.CoreUser.id == models_notification.TopicMembership.user_id,
243+
)
244+
.where(models_notification.TopicMembership.topic_id == topic_id),
242245
)
243-
return list(result.scalars().all())
246+
return [f"{u.firstname} {u.name}" for u in list(result.scalars().all())]
244247

245248

246249
async def get_firebase_tokens_by_user_ids(
@@ -265,6 +268,7 @@ async def get_usernames_by_firebase_tokens(
265268
models_notification.FirebaseDevice,
266269
models_users.CoreUser.id == models_notification.FirebaseDevice.user_id,
267270
)
268-
.where(models_notification.FirebaseDevice.firebase_device_token.in_(tokens)),
271+
.where(models_notification.FirebaseDevice.firebase_device_token.in_(tokens))
272+
.distinct(),
269273
)
270274
return [f"{u.firstname} {u.name}" for u in list(result.scalars().all())]

app/utils/communication/notifications.py

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,13 @@ async def _manage_firebase_batch_response(
8181
db=db,
8282
)
8383
hyperion_error_logger.error(
84-
"""
85-
Firebase: SenderId mismatch for notification '%s' (%s module) for %s/%s tokens (%s users) :
86-
%s
87-
""",
84+
"Firebase: SenderId mismatch for notification '%s' (%s module) for %s/%s tokens (%s users) : %s",
8885
message_content.title,
8986
message_content.action_module,
9087
len(mismatching_tokens),
9188
response.success_count + response.failure_count,
9289
len(usernames),
93-
"\n".join(usernames),
90+
", ".join(usernames),
9491
)
9592
hyperion_error_logger.info(
9693
f"{response.failure_count} messages failed to be send, removing their tokens from the database.",
@@ -140,7 +137,7 @@ async def _send_firebase_push_notification_by_tokens(
140137
),
141138
)
142139

143-
result = messaging.send_each_for_multicast(message)
140+
result: messaging.BatchResponse = messaging.send_each_for_multicast(message)
144141
except Exception:
145142
hyperion_error_logger.exception(
146143
"Notification: Unable to send firebase notification to tokens",
@@ -166,22 +163,34 @@ def _send_firebase_push_notification_by_topic(
166163
if not self.use_firebase:
167164
return
168165

169-
topic = str(topic_id)
170-
message = messaging.Message(
171-
topic=topic,
172-
notification=messaging.Notification(
173-
title=message_content.title,
174-
body=message_content.content,
175-
),
176-
)
177166
try:
178-
messaging.send(message)
179-
except messaging.FirebaseError:
167+
topic = str(topic_id)
168+
message = messaging.Message(
169+
topic=topic,
170+
data={"action_module": message_content.action_module},
171+
notification=messaging.Notification(
172+
title=message_content.title,
173+
body=message_content.content,
174+
),
175+
)
176+
177+
result: messaging.BatchResponse = messaging.send_each([message])
178+
except Exception:
180179
hyperion_error_logger.exception(
181180
f"Notification: Unable to send firebase notification for topic {topic}",
182181
)
183182
raise
184183

184+
if result.failure_count > 0:
185+
hyperion_error_logger.error(
186+
"Firebase: Failed to send notification '%s' for topic %s (%s module) for %s/%s tokens",
187+
message_content.title,
188+
topic,
189+
message_content.action_module,
190+
result.failure_count,
191+
result.success_count + result.failure_count,
192+
)
193+
185194
async def subscribe_tokens_to_topic(
186195
self,
187196
topic_id: UUID,
@@ -197,7 +206,10 @@ async def subscribe_tokens_to_topic(
197206
return
198207

199208
topic = str(topic_id)
200-
response = messaging.subscribe_to_topic(tokens, topic)
209+
response: messaging.TopicManagementResponse = messaging.subscribe_to_topic(
210+
tokens,
211+
topic,
212+
)
201213
if response.failure_count > 0:
202214
hyperion_error_logger.info(
203215
f"Notification: Failed to subscribe to topic {topic} due to {[error.reason for error in response.errors]}",

0 commit comments

Comments
 (0)