notification_data: Create common source for trigger strings.

This reduces loose strings in the codebase, and allows us to not worry
about the exact naming (`stream_email_enabled` or `stream_emails_enabled`?)
and tense (`mentioned` or `mention`?).

Ideally this new class should have been in `lib/notification_data.py`,
which is our file for things like this. But, the next commit requires
using this data in `models.py`, and importing from `notification_data.py`
to `models.py` causes recursive imports.
This commit is contained in:
Abhijeet Prasad Bodas
2021-07-09 17:08:12 +05:30
committed by Tim Abbott
parent a748a2f03c
commit c3319a5231
4 changed files with 43 additions and 25 deletions

View File

@@ -2,6 +2,7 @@ from dataclasses import dataclass
from typing import Collection, Dict, List, Optional, Set from typing import Collection, Dict, List, Optional, Set
from zerver.lib.mention import MentionData from zerver.lib.mention import MentionData
from zerver.models import NotificationTriggers
@dataclass @dataclass
@@ -74,13 +75,13 @@ class UserMessageNotificationsData:
return None return None
if private_message: if private_message:
return "private_message" return NotificationTriggers.PRIVATE_MESSAGE
elif self.mentioned: elif self.mentioned:
return "mentioned" return NotificationTriggers.MENTION
elif self.wildcard_mention_notify: elif self.wildcard_mention_notify:
return "wildcard_mentioned" return NotificationTriggers.WILDCARD_MENTION
elif self.stream_push_notify: elif self.stream_push_notify:
return "stream_push_notify" return NotificationTriggers.STREAM_PUSH
else: else:
return None return None
@@ -102,13 +103,13 @@ class UserMessageNotificationsData:
return None return None
if private_message: if private_message:
return "private_message" return NotificationTriggers.PRIVATE_MESSAGE
elif self.mentioned: elif self.mentioned:
return "mentioned" return NotificationTriggers.MENTION
elif self.wildcard_mention_notify: elif self.wildcard_mention_notify:
return "wildcard_mentioned" return NotificationTriggers.WILDCARD_MENTION
elif self.stream_email_notify: elif self.stream_email_notify:
return "stream_email_notify" return NotificationTriggers.STREAM_EMAIL
else: else:
return None return None

View File

@@ -28,6 +28,7 @@ from zerver.lib.user_groups import access_user_group_by_id
from zerver.models import ( from zerver.models import (
ArchivedMessage, ArchivedMessage,
Message, Message,
NotificationTriggers,
PushDeviceToken, PushDeviceToken,
Recipient, Recipient,
UserMessage, UserMessage,
@@ -559,19 +560,25 @@ def get_gcm_alert(message: Message, mentioned_user_group_name: Optional[str] = N
""" """
sender_str = message.sender.full_name sender_str = message.sender.full_name
display_recipient = get_display_recipient(message.recipient) display_recipient = get_display_recipient(message.recipient)
if message.recipient.type == Recipient.HUDDLE and message.trigger == "private_message": if (
message.recipient.type == Recipient.HUDDLE
and message.trigger == NotificationTriggers.PRIVATE_MESSAGE
):
return f"New private group message from {sender_str}" return f"New private group message from {sender_str}"
elif message.recipient.type == Recipient.PERSONAL and message.trigger == "private_message": elif (
message.recipient.type == Recipient.PERSONAL
and message.trigger == NotificationTriggers.PRIVATE_MESSAGE
):
return f"New private message from {sender_str}" return f"New private message from {sender_str}"
elif message.is_stream_message() and message.trigger == "mentioned": elif message.is_stream_message() and message.trigger == NotificationTriggers.MENTION:
if mentioned_user_group_name is None: if mentioned_user_group_name is None:
return f"{sender_str} mentioned you in #{display_recipient}" return f"{sender_str} mentioned you in #{display_recipient}"
else: else:
return f"{sender_str} mentioned @{mentioned_user_group_name} in #{display_recipient}" return f"{sender_str} mentioned @{mentioned_user_group_name} in #{display_recipient}"
elif message.is_stream_message() and message.trigger == "wildcard_mentioned": elif message.is_stream_message() and message.trigger == NotificationTriggers.WILDCARD_MENTION:
return f"{sender_str} mentioned everyone in #{display_recipient}" return f"{sender_str} mentioned everyone in #{display_recipient}"
else: else:
assert message.is_stream_message() and message.trigger == "stream_push_notify" assert message.is_stream_message() and message.trigger == NotificationTriggers.STREAM_PUSH
return f"New stream message from {sender_str} in #{display_recipient}" return f"New stream message from {sender_str} in #{display_recipient}"
@@ -722,14 +729,14 @@ def get_apns_alert_subtitle(
""" """
On an iOS notification, this is the second bolded line. On an iOS notification, this is the second bolded line.
""" """
if message.trigger == "mentioned": if message.trigger == NotificationTriggers.MENTION:
if mentioned_user_group_name is not None: if mentioned_user_group_name is not None:
return _("{full_name} mentioned @{user_group_name}:").format( return _("{full_name} mentioned @{user_group_name}:").format(
full_name=message.sender.full_name, user_group_name=mentioned_user_group_name full_name=message.sender.full_name, user_group_name=mentioned_user_group_name
) )
else: else:
return _("{full_name} mentioned you:").format(full_name=message.sender.full_name) return _("{full_name} mentioned you:").format(full_name=message.sender.full_name)
elif message.trigger == "wildcard_mentioned": elif message.trigger == NotificationTriggers.WILDCARD_MENTION:
return _("{full_name} mentioned everyone:").format(full_name=message.sender.full_name) return _("{full_name} mentioned everyone:").format(full_name=message.sender.full_name)
elif message.recipient.type == Recipient.PERSONAL: elif message.recipient.type == Recipient.PERSONAL:
return "" return ""

View File

@@ -3331,6 +3331,15 @@ class MissedMessageEmailAddress(models.Model):
self.save(update_fields=["times_used"]) self.save(update_fields=["times_used"])
class NotificationTriggers:
# "private_message" is for 1:1 PMs as well as huddles
PRIVATE_MESSAGE = "private_message"
MENTION = "mentioned"
WILDCARD_MENTION = "wildcard_mentioned"
STREAM_PUSH = "stream_push_notify"
STREAM_EMAIL = "stream_email_notify"
class ScheduledMessage(models.Model): class ScheduledMessage(models.Model):
id: int = models.AutoField(auto_created=True, primary_key=True, verbose_name="ID") id: int = models.AutoField(auto_created=True, primary_key=True, verbose_name="ID")
sender: UserProfile = models.ForeignKey(UserProfile, on_delete=CASCADE) sender: UserProfile = models.ForeignKey(UserProfile, on_delete=CASCADE)

View File

@@ -67,6 +67,7 @@ from zerver.lib.test_helpers import mock_queue_publish
from zerver.lib.user_groups import create_user_group from zerver.lib.user_groups import create_user_group
from zerver.models import ( from zerver.models import (
Message, Message,
NotificationTriggers,
PushDeviceToken, PushDeviceToken,
RealmAuditLog, RealmAuditLog,
Recipient, Recipient,
@@ -1509,7 +1510,7 @@ class TestGetAPNsPayload(PushNotificationTest):
"Content of personal message", "Content of personal message",
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = "private_message" message.trigger = NotificationTriggers.PRIVATE_MESSAGE
payload = get_message_payload_apns(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
"alert": { "alert": {
@@ -1543,7 +1544,7 @@ class TestGetAPNsPayload(PushNotificationTest):
self.sender, [self.example_user("othello"), self.example_user("cordelia")] self.sender, [self.example_user("othello"), self.example_user("cordelia")]
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = "private_message" message.trigger = NotificationTriggers.PRIVATE_MESSAGE
payload = get_message_payload_apns(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
"alert": { "alert": {
@@ -1576,7 +1577,7 @@ class TestGetAPNsPayload(PushNotificationTest):
def test_get_message_payload_apns_stream_message(self) -> None: def test_get_message_payload_apns_stream_message(self) -> None:
stream = Stream.objects.filter(name="Verona").get() stream = Stream.objects.filter(name="Verona").get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = "push_stream_notify" message.trigger = NotificationTriggers.STREAM_PUSH
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(self.sender, message) payload = get_message_payload_apns(self.sender, message)
expected = { expected = {
@@ -1608,7 +1609,7 @@ class TestGetAPNsPayload(PushNotificationTest):
user_profile = self.example_user("othello") user_profile = self.example_user("othello")
stream = Stream.objects.filter(name="Verona").get() stream = Stream.objects.filter(name="Verona").get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = "mentioned" message.trigger = NotificationTriggers.MENTION
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
@@ -1641,7 +1642,7 @@ class TestGetAPNsPayload(PushNotificationTest):
user_group = create_user_group("test_user_group", [user_profile], get_realm("zulip")) user_group = create_user_group("test_user_group", [user_profile], get_realm("zulip"))
stream = Stream.objects.filter(name="Verona").get() stream = Stream.objects.filter(name="Verona").get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = "mentioned" message.trigger = NotificationTriggers.MENTION
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(user_profile, message, user_group.id, user_group.name) payload = get_message_payload_apns(user_profile, message, user_group.id, user_group.name)
expected = { expected = {
@@ -1675,7 +1676,7 @@ class TestGetAPNsPayload(PushNotificationTest):
user_profile = self.example_user("othello") user_profile = self.example_user("othello")
stream = Stream.objects.filter(name="Verona").get() stream = Stream.objects.filter(name="Verona").get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = "wildcard_mentioned" message.trigger = NotificationTriggers.WILDCARD_MENTION
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
@@ -1710,7 +1711,7 @@ class TestGetAPNsPayload(PushNotificationTest):
self.sender, [self.example_user("othello"), self.example_user("cordelia")] self.sender, [self.example_user("othello"), self.example_user("cordelia")]
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = "private_message" message.trigger = NotificationTriggers.PRIVATE_MESSAGE
payload = get_message_payload_apns(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
"alert": { "alert": {
@@ -1813,7 +1814,7 @@ class TestGetGCMPayload(PushNotificationTest):
def test_get_message_payload_gcm_private_message(self) -> None: def test_get_message_payload_gcm_private_message(self) -> None:
message = self.get_message(Recipient.PERSONAL, 1) message = self.get_message(Recipient.PERSONAL, 1)
message.trigger = "private_message" message.trigger = NotificationTriggers.PRIVATE_MESSAGE
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm(hamlet, message) payload, gcm_options = get_message_payload_gcm(hamlet, message)
self.assertDictEqual( self.assertDictEqual(
@@ -1846,7 +1847,7 @@ class TestGetGCMPayload(PushNotificationTest):
def test_get_message_payload_gcm_stream_notifications(self) -> None: def test_get_message_payload_gcm_stream_notifications(self) -> None:
stream = Stream.objects.get(name="Denmark") stream = Stream.objects.get(name="Denmark")
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = "stream_push_notify" message.trigger = NotificationTriggers.STREAM_PUSH
message.stream_name = "Denmark" message.stream_name = "Denmark"
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm(hamlet, message) payload, gcm_options = get_message_payload_gcm(hamlet, message)
@@ -1883,7 +1884,7 @@ class TestGetGCMPayload(PushNotificationTest):
def test_get_message_payload_gcm_redacted_content(self) -> None: def test_get_message_payload_gcm_redacted_content(self) -> None:
stream = Stream.objects.get(name="Denmark") stream = Stream.objects.get(name="Denmark")
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = "stream_push_notify" message.trigger = NotificationTriggers.STREAM_PUSH
message.stream_name = "Denmark" message.stream_name = "Denmark"
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm(hamlet, message) payload, gcm_options = get_message_payload_gcm(hamlet, message)