Fix notification delivery (#1879)

Another missed change in #1697.

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.

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],

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

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

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