From 145f88ef70eaaec450be0154e0e856c0f02b0e32 Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Wed, 20 Sep 2023 18:28:41 +0530 Subject: [PATCH] Fix notification delivery (#1879) Another missed change in #1697. --- README.rst | 8 +++----- funnel/models/helpers.py | 3 +++ funnel/models/notification.py | 22 +++++++++++----------- funnel/transports/email/send.py | 4 +++- funnel/views/notification.py | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/README.rst b/README.rst index 631d0b080..31dad39ad 100644 --- a/README.rst +++ b/README.rst @@ -3,17 +3,15 @@ Hasgeek Code for Hasgeek.com at https://hasgeek.com/ -Copyright © 2010-2022 by Hasgeek +Copyright © 2010-2023 by Hasgeek This code is open source under the AGPL v3 license (see LICENSE.txt). We welcome your examination of our code to: * Establish trust and transparency on how it works, and * Allow contributions -To establish our intent, we use the AGPL v3 license, which requires you to release all your modifications to the public under the same license. You may not make a proprietary fork. To have your contributions merged back into the master repository, you must agree to assign copyright to Hasgeek, and must assert that you have the right to make this assignment. (We realise this sucks, so if you have a better idea, we’d like to hear it.) +To establish our intent, we use the AGPL v3 license, which requires you to release your modifications to the public under the same license. You may not make a proprietary fork. To have your contributions merged into the main repository, you must agree to assign copyright to Hasgeek, and must assert that you have the right to make this assignment. You will be asked to sign a Contributor License Agreement when you make a Pull Request. -Our workflow assumes this code is for use on a single production website. Using this to operate your own website is not recommended. Brand names and visual characteristics are not covered under the source code license. +Our workflow assumes this code is for use on a single production website. Using this to operate your own website is not recommended. Brand names, logos and visual characteristics are not covered under the source code license. We aim to have our source code useful to the larger community. Several key components are delegated to the Coaster library, available under the BSD license. Requests for liberal licensing of other components are also welcome. Please file an issue ticket. - -This repository uses Travis CI for test automation and has dependencies scanned by PyUp.io. diff --git a/funnel/models/helpers.py b/funnel/models/helpers.py index 2f1c5fdb7..3fb9d038e 100644 --- a/funnel/models/helpers.py +++ b/funnel/models/helpers.py @@ -683,6 +683,9 @@ def coerce(cls: type[_MC], key: str, value: Any) -> _MC: """Allow a composite column to be assigned a string value.""" return cls(value) + # TODO: Add `nullable` as a keyword parameter and add overloads for returning + # Mapped[str] or Mapped[str | None] based on nullable + @classmethod def create( cls: type[_MC], diff --git a/funnel/models/notification.py b/funnel/models/notification.py index dc1d9704f..0abcb5a6c 100644 --- a/funnel/models/notification.py +++ b/funnel/models/notification.py @@ -987,15 +987,15 @@ def _is_revoked_expression(cls) -> sa.ColumnElement[bool]: # --- Dispatch helper methods ------------------------------------------------------ - def user_preferences(self) -> NotificationPreferences: - """Return the user's notification preferences for this notification type.""" - prefs = self.user.notification_preferences.get(self.notification_pref_type) + def recipient_preferences(self) -> NotificationPreferences: + """Return the account's notification preferences for this notification type.""" + prefs = self.recipient.notification_preferences.get(self.notification_pref_type) if prefs is None: prefs = NotificationPreferences( - notification_type=self.notification_pref_type, account=self.user + notification_type=self.notification_pref_type, account=self.recipient ) db.session.add(prefs) - self.user.notification_preferences[self.notification_pref_type] = prefs + self.recipient.notification_preferences[self.notification_pref_type] = prefs return prefs def has_transport(self, transport: str) -> bool: @@ -1012,13 +1012,13 @@ def has_transport(self, transport: str) -> bool: # This property inserts the row if not already present. An immediate database # commit is required to ensure a parallel worker processing another notification # doesn't make a conflicting row. - main_prefs = self.user.main_notification_preferences - user_prefs = self.user_preferences() + main_prefs = self.recipient.main_notification_preferences + user_prefs = self.recipient_preferences() return ( self.notification.allow_transport(transport) and main_prefs.by_transport(transport) and user_prefs.by_transport(transport) - and self.user.has_transport(transport) + and self.recipient.has_transport(transport) ) def transport_for(self, transport: str) -> AccountEmail | AccountPhone | None: @@ -1032,14 +1032,14 @@ def transport_for(self, transport: str) -> AccountEmail | AccountPhone | None: 3. The user's per-type preference allows it 4. The user has this transport (verified email or phone, etc) """ - main_prefs = self.user.main_notification_preferences - user_prefs = self.user_preferences() + main_prefs = self.recipient.main_notification_preferences + user_prefs = self.recipient_preferences() if ( self.notification.allow_transport(transport) and main_prefs.by_transport(transport) and user_prefs.by_transport(transport) ): - return self.user.transport_for( + return self.recipient.transport_for( transport, self.notification.preference_context ) return None diff --git a/funnel/transports/email/send.py b/funnel/transports/email/send.py index c7fcc7407..8ed054bb3 100644 --- a/funnel/transports/email/send.py +++ b/funnel/transports/email/send.py @@ -172,6 +172,7 @@ def send_email( # sender reputation. There is no automated way to flag an email address as # no longer bouncing, so it'll require customer support intervention if ea.delivery_state.HARD_FAIL: + statsd.incr('email_address.send_hard_fail') raise TransportRecipientError( _( "This email address is bouncing messages: {email}. If you" @@ -180,6 +181,7 @@ def send_email( ) email_addresses.append(ea) except EmailAddressBlockedError as exc: + statsd.incr('email_address.send_blocked') raise TransportRecipientError( _("This email address has been blocked: {email}").format(email=email) ) from exc @@ -194,7 +196,6 @@ def send_email( message = _("This email address is not valid: {email}").format( email=list(exc.recipients.keys())[0] ) - else: if len(to) == len(exc.recipients): # We don't know which recipients were rejected, so the error message @@ -204,6 +205,7 @@ def send_email( message = _("These email addresses are not valid: {emails}").format( emails=_(", ").join(exc.recipients.keys()) ) + statsd.incr('email_address.send_smtp_refused') raise TransportRecipientError(message) from exc # After sending, mark the address as having received an email and also update the diff --git a/funnel/views/notification.py b/funnel/views/notification.py index d61e6b604..48dc4a651 100644 --- a/funnel/views/notification.py +++ b/funnel/views/notification.py @@ -516,7 +516,7 @@ def inner(notification_recipient_ids: Sequence[tuple[int, UUID]]) -> None: pass else: # TODO: Implement transport error handling code here - raise + pass return inner