Skip to content

Commit a6fd839

Browse files
leeandherclaude
authored andcommitted
fix(alerts): respect 24-hour clock preference in email notifications (#106884)
emails now check if you're a 24-hour time person and format accordingly. issue alerts, digests, and feedback emails all got the change. Fixes #106581 the html is kinda miserable to review but it works <img width="586" height="113" alt="24tz" src="https://github.com/user-attachments/assets/a5d44869-aabb-42c6-a13f-bf9639b38d9b" /> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent be171f2 commit a6fd839

File tree

8 files changed

+108
-46
lines changed

8 files changed

+108
-46
lines changed

src/sentry/notifications/notifications/digest.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,12 @@ def get_subject(self, context: Mapping[str, Any] | None = None) -> str:
8181
# This shouldn't be possible but adding a message just in case.
8282
return "Digest Report"
8383

84-
# Use timezone from context if available (added by get_recipient_context)
84+
# Use timezone and clock format from context (added by get_recipient_context)
8585
timezone = context.get("timezone")
86-
return get_digest_subject(context["group"], context["counts"], context["start"], timezone)
86+
clock_24_hours = context.get("clock_24_hours", False)
87+
return get_digest_subject(
88+
context["group"], context["counts"], context["start"], timezone, clock_24_hours
89+
)
8790

8891
def get_notification_title(
8992
self, provider: ExternalProviders, context: Mapping[str, Any] | None = None
@@ -118,18 +121,23 @@ def get_recipient_context(
118121
self, recipient: Actor, extra_context: Mapping[str, Any]
119122
) -> MutableMapping[str, Any]:
120123
tz: tzinfo = UTC
124+
clock_24_hours = False
121125
if recipient.is_user:
122126
user_options = user_option_service.get_many(
123-
filter={"user_ids": [recipient.id], "keys": ["timezone"]}
127+
filter={"user_ids": [recipient.id], "keys": ["timezone", "clock_24_hours"]}
124128
)
125129
user_tz = get_option_from_list(user_options, key="timezone", default="UTC")
130+
clock_24_hours = bool(
131+
get_option_from_list(user_options, key="clock_24_hours", default=False)
132+
)
126133
try:
127134
tz = zoneinfo.ZoneInfo(user_tz)
128135
except (ValueError, zoneinfo.ZoneInfoNotFoundError):
129136
pass
130137
return {
131138
**super().get_recipient_context(recipient, extra_context),
132139
"timezone": tz,
140+
"clock_24_hours": clock_24_hours,
133141
}
134142

135143
def get_context(self) -> MutableMapping[str, Any]:

src/sentry/notifications/notifications/rules.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,23 @@ def get_recipient_context(
138138
self, recipient: Actor, extra_context: Mapping[str, Any]
139139
) -> MutableMapping[str, Any]:
140140
tz: tzinfo = UTC
141+
clock_24_hours = False
141142
if recipient.is_user:
142143
user_options = user_option_service.get_many(
143-
filter={"user_ids": [recipient.id], "keys": ["timezone"]}
144+
filter={"user_ids": [recipient.id], "keys": ["timezone", "clock_24_hours"]}
144145
)
145146
user_tz = get_option_from_list(user_options, key="timezone", default="UTC")
147+
clock_24_hours = bool(
148+
get_option_from_list(user_options, key="clock_24_hours", default=False)
149+
)
146150
try:
147151
tz = zoneinfo.ZoneInfo(user_tz)
148152
except (ValueError, zoneinfo.ZoneInfoNotFoundError):
149153
pass
150154
return {
151155
**super().get_recipient_context(recipient, extra_context),
152156
"timezone": tz,
157+
"clock_24_hours": clock_24_hours,
153158
}
154159

155160
def get_image_url(self) -> str | None:

src/sentry/notifications/utils/digest.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,24 @@
1616

1717

1818
def get_digest_subject(
19-
group: Group, counts: Counter[Group], date: datetime, timezone: tzinfo | None = None
19+
group: Group,
20+
counts: Counter[Group],
21+
date: datetime,
22+
timezone: tzinfo | None = None,
23+
clock_24_hours: bool = False,
2024
) -> str:
2125
# Convert date to user's timezone if provided
2226
if timezone:
2327
date = date.astimezone(timezone)
2428

29+
# Use 24-hour format (H:i) or 12-hour format (P) based on user preference
30+
date_format = "N j, Y, H:i e" if clock_24_hours else "N j, Y, P e"
31+
2532
return "{short_id} - {count} new {noun} since {date}".format(
2633
short_id=group.qualified_short_id,
2734
count=len(counts),
2835
noun="alert" if len(counts) == 1 else "alerts",
29-
date=dateformat.format(date, "N j, Y, P e"),
36+
date=dateformat.format(date, date_format),
3037
)
3138

3239

src/sentry/templates/sentry/emails/digests/body.html

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,26 @@
2424

2525
<div class="container" style="padding-top: 30px;">
2626
<h2>{{ counts|length }} new alert{{ counts|pluralize }} from <a href="{{ project.get_absolute_url }}">{{ project.slug }}</a></h2>
27-
{% if timezone %}
28-
{% with start=start|timezone:timezone|date:"N j, Y, P e" end=end|timezone:timezone|date:"N j, Y, P e" %}
29-
<div class="dateline">{{ start }}{% if start != end %} to {{ end }}{% endif %}</div>
30-
{% endwith %}
27+
{% if clock_24_hours %}
28+
{% if timezone %}
29+
{% with start=start|timezone:timezone|date:"N j, Y, H:i e" end=end|timezone:timezone|date:"N j, Y, H:i e" %}
30+
<div class="dateline">{{ start }}{% if start != end %} to {{ end }}{% endif %}</div>
31+
{% endwith %}
32+
{% else %}
33+
{% with start=start|date:"N j, Y, H:i e" end=end|date:"N j, Y, H:i e" %}
34+
<div class="dateline">{{ start }}{% if start != end %} to {{ end }}{% endif %}</div>
35+
{% endwith %}
36+
{% endif %}
3137
{% else %}
32-
{% with start=start|date:"N j, Y, P e" end=end|date:"N j, Y, P e" %}
33-
<div class="dateline">{{ start }}{% if start != end %} to {{ end }}{% endif %}</div>
34-
{% endwith %}
38+
{% if timezone %}
39+
{% with start=start|timezone:timezone|date:"N j, Y, P e" end=end|timezone:timezone|date:"N j, Y, P e" %}
40+
<div class="dateline">{{ start }}{% if start != end %} to {{ end }}{% endif %}</div>
41+
{% endwith %}
42+
{% else %}
43+
{% with start=start|date:"N j, Y, P e" end=end|date:"N j, Y, P e" %}
44+
<div class="dateline">{{ start }}{% if start != end %} to {{ end }}{% endif %}</div>
45+
{% endwith %}
46+
{% endif %}
3547
{% endif %}
3648
</div>
3749

@@ -62,11 +74,13 @@ <h2>{{ counts|length }} new alert{{ counts|pluralize }} from <a href="{{ project
6274
<td class="event-detail">
6375
{% include "sentry/emails/_group.html" %}
6476
<div>
65-
{% if timezone %}
66-
<small>{{ records.0.datetime|timezone:timezone|date:"N j, Y, g:i:s a e" }}</small>
67-
{% else %}
68-
<small>{{ records.0.datetime|date:"N j, Y, g:i:s a e" }}</small>
69-
{% endif %}
77+
<small>
78+
{% if clock_24_hours %}
79+
{% if timezone %}{{ records.0.datetime|timezone:timezone|date:"N j, Y, H:i:s e" }}{% else %}{{ records.0.datetime|date:"N j, Y, H:i:s e" }}{% endif %}
80+
{% else %}
81+
{% if timezone %}{{ records.0.datetime|timezone:timezone|date:"N j, Y, g:i:s a e" }}{% else %}{{ records.0.datetime|date:"N j, Y, g:i:s a e" }}{% endif %}
82+
{% endif %}
83+
</small>
7084
{% if show_replay_links and group.has_replays %}
7185
<span class="replay-link">
7286
<a href="{{ group.get_absolute_url }}replays?referrer=alert_email">

src/sentry/templates/sentry/emails/digests/body.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{% load timezone from tz %}
22
{% load sentry_helpers %}
33
{% load sentry_features %}Notifications for {{ project.slug }}
4-
{% if timezone %}{{ start|timezone:timezone|date:"N j, Y, P e" }} to {{ end|timezone:timezone|date:"N j, Y, P e" }}{% else %}{{ start|date:"N j, Y, P e" }} to {{ end|date:"N j, Y, P e" }}{% endif %}
4+
{% if clock_24_hours %}{% if timezone %}{{ start|timezone:timezone|date:"N j, Y, H:i e" }} to {{ end|timezone:timezone|date:"N j, Y, H:i e" }}{% else %}{{ start|date:"N j, Y, H:i e" }} to {{ end|date:"N j, Y, H:i e" }}{% endif %}{% else %}{% if timezone %}{{ start|timezone:timezone|date:"N j, Y, P e" }} to {{ end|timezone:timezone|date:"N j, Y, P e" }}{% else %}{{ start|date:"N j, Y, P e" }} to {{ end|date:"N j, Y, P e" }}{% endif %}{% endif %}
55

66
{% for rule, groups in digest.items %}{{ rule.label }}
77
{% for group, records in groups.items %}{% with event_count=event_counts|get_item:group.id user_count=user_counts|get_item:group.id %}

src/sentry/templates/sentry/emails/feedback.html

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,13 @@ <h2 {% if notification_reason %}style="margin-bottom: 4px"{% else %}style="margi
6767
{% if enhanced_privacy %}
6868
<div class="event">
6969
<div class="event-id">ID: {{ event.event_id }}</div>
70-
{% if timezone %}
71-
<div class="event-date">{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}</div>
72-
{% else %}
73-
<div class="event-date">{{ event.datetime|date:"N j, Y, g:i:s a e"}}</div>
74-
{% endif %}
70+
<div class="event-date">
71+
{% if clock_24_hours %}
72+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, H:i:s e"}}{% else %}{{ event.datetime|date:"N j, Y, H:i:s e"}}{% endif %}
73+
{% else %}
74+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}{% else %}{{ event.datetime|date:"N j, Y, g:i:s a e"}}{% endif %}
75+
{% endif %}
76+
</div>
7577
</div>
7678

7779
<div class="notice">Details about this feedback are not shown in this notification since enhanced privacy
@@ -101,11 +103,13 @@ <h3>Message</h3>
101103
{% endif %}
102104
<div class="event">
103105
<div class="event-id">ID: {{ event.event_id }}</div>
104-
{% if timezone %}
105-
<div class="event-date">{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}</div>
106-
{% else %}
107-
<div class="event-date">{{ event.datetime|date:"N j, Y, g:i:s a e"}}</div>
108-
{% endif %}
106+
<div class="event-date">
107+
{% if clock_24_hours %}
108+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, H:i:s e"}}{% else %}{{ event.datetime|date:"N j, Y, H:i:s e"}}{% endif %}
109+
{% else %}
110+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}{% else %}{{ event.datetime|date:"N j, Y, g:i:s a e"}}{% endif %}
111+
{% endif %}
112+
</div>
109113
</div>
110114
</div>
111115
{% endwith %}

src/sentry/templates/sentry/emails/issue_alert_base.html

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,13 @@ <h2 {% if notification_reason %}style="margin-bottom: 4px"{% else %}style="margi
8080
{% if enhanced_privacy %}
8181
<div class="event">
8282
<div class="event-id">ID: {{ event.event_id }}</div>
83-
{% if timezone %}
84-
<div class="event-date">{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}</div>
85-
{% else %}
86-
<div class="event-date">{{ event.datetime|date:"N j, Y, g:i:s a e"}}</div>
87-
{% endif %}
83+
<div class="event-date">
84+
{% if clock_24_hours %}
85+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, H:i:s e"}}{% else %}{{ event.datetime|date:"N j, Y, H:i:s e"}}{% endif %}
86+
{% else %}
87+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}{% else %}{{ event.datetime|date:"N j, Y, g:i:s a e"}}{% endif %}
88+
{% endif %}
89+
</div>
8890
</div>
8991

9092
<div class="notice">Details about this issue are not shown in this notification since enhanced privacy
@@ -163,11 +165,13 @@ <h3>
163165
{% if not is_new_design %}
164166
<div class="event">
165167
<div class="event-id">ID: {{ event.event_id }}</div>
166-
{% if timezone %}
167-
<div class="event-date">{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}</div>
168-
{% else %}
169-
<div class="event-date">{{ event.datetime|date:"N j, Y, g:i:s a e"}}</div>
170-
{% endif %}
168+
<div class="event-date">
169+
{% if clock_24_hours %}
170+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, H:i:s e"}}{% else %}{{ event.datetime|date:"N j, Y, H:i:s e"}}{% endif %}
171+
{% else %}
172+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}{% else %}{{ event.datetime|date:"N j, Y, g:i:s a e"}}{% endif %}
173+
{% endif %}
174+
</div>
171175
</div>
172176
{% endif %}
173177

@@ -206,10 +210,10 @@ <h3>
206210
<tr>
207211
<th>Start Time</th>
208212
<td>
209-
{% if timezone %}
210-
{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}
213+
{% if clock_24_hours %}
214+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, H:i:s e"}}{% else %}{{ event.datetime|date:"N j, Y, H:i:s e"}}{% endif %}
211215
{% else %}
212-
{{ event.datetime|date:"N j, Y, g:i:s a e"}}
216+
{% if timezone %}{{ event.datetime|timezone:timezone|date:"N j, Y, g:i:s a e"}}{% else %}{{ event.datetime|date:"N j, Y, g:i:s a e"}}{% endif %}
213217
{% endif %}
214218
</td>
215219
</tr>

tests/sentry/mail/test_adapter.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -764,18 +764,20 @@ def test_notify_users_with_utf8_subject(self) -> None:
764764
msg = mail.outbox[0]
765765
assert msg.subject == "[Sentry] BAR-1 - רונית מגן"
766766

767-
def test_notify_users_with_their_timezones(self) -> None:
767+
def test_notify_users_with_their_time_preferences(self) -> None:
768768
"""
769-
Test that ensures that datetime in issue alert email is in the user's timezone
769+
Test that ensures that datetime in issue alert email is in the user's timezone, and their
770+
preferred clock format (24h or 12h)
770771
"""
771772
from django.template.defaultfilters import date
772773

773774
timestamp = timezone.now()
774775
local_timestamp_s = timezone.localtime(timestamp, zoneinfo.ZoneInfo("Europe/Vienna"))
775-
local_timestamp = date(local_timestamp_s, "N j, Y, g:i:s a e")
776+
local_timestamp_24h = date(local_timestamp_s, "N j, Y, H:i:s e")
776777

777778
with assume_test_silo_mode(SiloMode.CONTROL):
778779
UserOption.objects.create(user=self.user, key="timezone", value="Europe/Vienna")
780+
UserOption.objects.create(user=self.user, key="clock_24_hours", value=True)
779781

780782
event = self.store_event(
781783
data={"message": "foobar", "level": "error", "timestamp": timestamp.isoformat()},
@@ -795,7 +797,14 @@ def test_notify_users_with_their_timezones(self) -> None:
795797
assert len(mail.outbox) == 1
796798
msg = mail.outbox[0]
797799
assert isinstance(msg, EmailMultiAlternatives)
798-
assert local_timestamp in str(msg.alternatives)
800+
assert local_timestamp_24h in str(msg.alternatives)
801+
802+
alert_notification = AlertRuleNotification(notification, ActionTargetType.ISSUE_OWNERS)
803+
recipient_context = alert_notification.get_recipient_context(
804+
Actor.from_orm_user(self.user), {}
805+
)
806+
assert recipient_context["timezone"] == zoneinfo.ZoneInfo("Europe/Vienna")
807+
assert recipient_context["clock_24_hours"] is True
799808

800809
def _test_invalid_timezone(self, s: str) -> None:
801810
with assume_test_silo_mode(SiloMode.CONTROL):
@@ -817,6 +826,17 @@ def test_context_invalid_timezone_empty_string(self) -> None:
817826
def test_context_invalid_timezone_garbage_value(self) -> None:
818827
self._test_invalid_timezone("not/a/real/timezone")
819828

829+
def test_recipient_context_clock_24_hours_false_by_default(self) -> None:
830+
event = self.store_event(
831+
data={"message": "foobar", "level": "error"},
832+
project_id=self.project.id,
833+
)
834+
notification = AlertRuleNotification(
835+
Notification(event=event), ActionTargetType.ISSUE_OWNERS
836+
)
837+
recipient_context = notification.get_recipient_context(Actor.from_orm_user(self.user), {})
838+
assert recipient_context["clock_24_hours"] is False
839+
820840
def test_notify_with_suspect_commits(self) -> None:
821841
repo = Repository.objects.create(
822842
organization_id=self.organization.id, name=self.organization.id

0 commit comments

Comments
 (0)