43717 backend [ mail ] Create a switchable SMTP email client.

[EBirkenfeld]

Add switchable SMTP email client and route notifications through src.notifications.services.email.EmailService._dispatch_email with provider selection via settings.EMAIL_PROVIDER

Introduce a pluggable email client layer with CustomerIOEmailClient and SMTPEmailClient, centralize send routing in EmailService._dispatch_email, add account-scoped EmailTemplateModel, and add HTML templates for task, digest, and auth notifications. Replace direct Customer.io usage with get_email_client() and update imports, tests, and settings to select provider at runtime.

📍Where to Start

Start with provider selection in get_email_client in backend/src/notifications/clients/utils.py, then review dispatch logic in EmailService._dispatch_email in backend/src/notifications/services/email.py.

Changes since #82 opened

• Implemented switchable email client architecture with account-scoped SMTP client [c2de1e3]
• Added standard variable injection and UTM-tagged deep links to all email payloads [c2de1e3]
• Replaced email templates with new Django template structure [c2de1e3]
• Renamed EmailTemplate enum to EmailType and template_types model field to email_types [c2de1e3]
• Removed automatic injection of standard variables in email service and added manual logo_lg inclusion [0333a4d]
• Updated email template documentation to reflect new variable model [0333a4d]
• Changed footer year rendering to use Django template tag [0333a4d]
• Updated email service tests to use new enums, patch timezone, verify dispatch behavior, and validate explicit data structures [0333a4d]
• Fixed EmailService._send_email_to_console method to properly construct message_vars string [81d6de7]
• Added test coverage for email service methods [81d6de7]
• Added summary statistics and hierarchical count breakdowns to digest email template [8868b89]
• Implemented conditional rendering for dynamic content blocks across notification email templates [8868b89]
• Refactored base email template layout structure [8868b89]
• Adjusted typography styling for help and URL sections in notification templates [8868b89]
• Added HTML-formatted template variable documentation to the email template admin interface [c564826]
• Replaced synchronous email service calls with asynchronous Celery task dispatches across all notification flows [0e939f5]
• Refactored EmailService class from class-method-based architecture to instance-method-based architecture with unified dispatch and validation [0e939f5]
• Modified email template rendering to escape HTML content and restructured template variables [0e939f5]
• Refactored SMTPEmailClient template selection from dynamic method-based lookup to static mapping [0e939f5]
• Removed COMPLETE_TASK from EmailType enum and added five new constants to NotificationMethod enum with centralized title mapping [0e939f5]
• Created five new Celery task functions and helper functions for digest and account lifecycle notifications [0e939f5]
• Updated all test suites to mock Celery task .delay calls instead of EmailService methods and assert with explicit user identifiers [0e939f5]
• Expanded EmailTemplateAdmin help text to document additional template variables for auth, digests, and guest notifications [0e939f5]
• Deleted backend/src/templates/emails/user_deactivated.txt text email template file and regenerated migration to remove COMPLETE_TASK from email template choices [0e939f5]

---

📊 Macroscope summarized 0e939f5. 19 files reviewed, 56 issues evaluated, 49 issues filtered, 1 comment posted

🗂️ Filtered Issues

backend/src/accounts/services/user.py — 0 comments posted, 2 evaluated, 2 filtered

• line 205: Calling send_user_deactivated_notification.delay(...) without error handling can raise at runtime (e.g., broker unavailable), causing the caller to fail after deactivation side effects have committed. This creates a partial-failure scenario where the user is deactivated but the request may return an error, with no retry/compensation or graceful degradation. [ Low confidence ]
• line 209: user.account is dereferenced without a guard when passing logo_lg=user.account.logo_lg. If user.account can be None on any reachable call path to _deactivate_actions (e.g., if the method is invoked directly with a user lacking an account), this will raise AttributeError. There is no local guard or constructor guarantee in this method establishing non-nullability. [ Low confidence ]

backend/src/accounts/services/user_invite.py — 0 comments posted, 3 evaluated, 3 filtered

• line 201: The Celery task is enqueued inside a surrounding database transaction (e.g., in invite_user via _transfer_existent_user -> _send_transfer_email). If the transaction later rolls back, the email task still runs, causing side effects that do not reflect committed state. Use transaction.on_commit(lambda: send_user_transfer_notification.delay(...)) to ensure dispatch only after a successful commit. [ Low confidence ]
• line 204: self.request_user is optional per __init__ (can be None), but _send_transfer_email unconditionally dereferences it (self.request_user.account_id, self.request_user.get_full_name(), and self.request_user.account.name). If request_user was not provided, calling _send_transfer_email will raise AttributeError, crashing the flow during invite/transfer or resend paths. [ Low confidence ]
• line 208: logo_lg=current_account_user.account.logo_lg is passed as a Celery task kwarg. If logo_lg is a Django ImageField/FileField (i.e., a FieldFile), it is not JSON-serializable and Celery (with JSON serializer) will raise TypeError: Object of type FieldFile is not JSON serializable. Convert to a primitive (e.g., str(current_account_user.account.logo_lg.url or '') or name) before enqueueing. [ Low confidence ]

backend/src/authentication/views/signin.py — 0 comments posted, 3 evaluated, 3 filtered

• line 50: Possible unhandled exceptions when fetching the account owner with user.account.users.get(is_account_owner=True). If no owner exists or multiple owners exist, Django will raise DoesNotExist or MultipleObjectsReturned, causing a 500 instead of a controlled error during sign-in for timed-out accounts. [ Low confidence ]
• line 51: No error handling around send_verification_notification.delay(...). If the broker is unavailable or kwargs are not serializable, the call can raise and abort the request, preventing the intended AuthenticationFailed response and leaking a 500. [ Low confidence ]
• line 58: Passing logo_lg=user.account.logo_lg into a Celery task (send_verification_notification.delay(...)) may not be JSON-serializable (e.g., ImageFieldFile). Celery (JSON serializer by default) will raise a serialization error at enqueue time, causing a 500. [ Already posted ]

backend/src/authentication/views/signup.py — 0 comments posted, 2 evaluated, 2 filtered

• line 56: No error handling around send_verification_notification.delay(...) in after_signup. Broker/serialization errors will raise and abort the request, potentially preventing subsequent logic (inc_anonymous_user_account_counter) and returning a 500. [ Low confidence ]
• line 62: Passing logo_lg=account.logo_lg to the Celery task may not be JSON-serializable (e.g., ImageFieldFile), leading to serialization errors at enqueue time and a 500 during signup completion. [ Already posted ]

backend/src/authentication/views/verification.py — 0 comments posted, 3 evaluated, 3 filtered

• line 74: Possible unhandled exceptions when fetching the account owner with request.user.account.users.get(is_account_owner=True). If the owner is missing or duplicated, .get() raises and the resend endpoint returns a 500. [ Low confidence ]
• line 77: No error handling around send_verification_notification.delay(...) in the resend endpoint. Publish/serialization failures will raise and produce a 500 instead of returning the expected response. [ Low confidence ]
• line 83: Passing logo_lg=user.account.logo_lg to the Celery task can fail JSON serialization (e.g., ImageFieldFile), causing a 500 when resending verification. [ Low confidence ]

backend/src/notifications/admin.py — 0 comments posted, 2 evaluated, 2 filtered

• line 59: Using email_types (a PostgreSQL ArrayField) in list_filter is not supported by Django’s built-in admin filters and will raise ImproperlyConfigured when loading the changelist. list_filter expects fields like BooleanField, DateField, ForeignKey, or CharField with choices; but an ArrayField of CharField(choices=...) does not register a default FieldListFilter. Provide a custom SimpleListFilter or a custom FieldListFilter for ArrayField, or remove this entry. [ Already posted ]
• line 67: Including email_types (a PostgreSQL ArrayField) in search_fields will cause database errors when the admin performs a search, because Django applies icontains by default, producing SQL like email_types ILIKE %...%, which is invalid for text[] (operator does not exist: text[] ILIKE ...). Remove email_types from search_fields or implement a custom search via get_search_results using array-aware lookups (e.g., __contains). [ Already posted ]

backend/src/notifications/clients/customerio.py — 0 comments posted, 3 evaluated, 2 filtered

• line 25: cio_template_ids[template_code] can be None if the corresponding environment variable is unset, resulting in transactional_message_id=None. This likely causes the downstream customerio API call to fail at runtime. Add a check and return a clear error or skip sending when the template id is missing. [ Low confidence ]
• line 25: cio_template_ids[template_code] may raise KeyError if template_code is not one of the expected keys. The EmailClient.send_email contract accepts template_code: str without validation. Ensure template_code is validated or use .get(...) with explicit error handling to return a clear failure. [ Low confidence ]

backend/src/notifications/clients/smtp.py — 1 comment posted, 9 evaluated, 7 filtered

• line 36: Potential thread-safety issue: the class keeps a shared self.connection and uses it in send_email without synchronization. Django’s email backends are not documented as thread-safe; reusing a single connection across concurrent calls can cause interleaved writes or backend state corruption. If the same client instance is used across threads, this can manifest as intermittent send failures. Use per-send connections, a connection pool, or ensure single-threaded use with locking. [ Low confidence ]
• line 38: Using __del__ for connection cleanup is unsafe. At interpreter shutdown or in cyclic GC scenarios, module globals may be None and __del__ may not run or may run when dependencies are already torn down, potentially raising exceptions or skipping cleanup. Do not rely on __del__ for essential resource management; use explicit teardown or context management. [ Low confidence ]
• line 38: __del__ may raise during interpreter shutdown or not run at all, making cleanup unreliable and potentially emitting exceptions to stderr. Module globals used by the connection/close path may be None at GC time. Relying on __del__ for resource management is unsafe; prefer explicit close or context management. [ Low confidence ]
• line 45: Unvalidated recipient address to may be empty or invalid (e.g., ''), leading to runtime failures from the email backend or SMTP server during email.send(). Add basic validation (non-empty, valid email format) or return a clear error before attempting to send. [ Low confidence ]
• line 58: Rendering database-stored templates via _render_template_string lacks error handling. If template.subject or template.content contains invalid Django template syntax, Template(...) or .render(...) can raise (e.g., TemplateSyntaxError), causing send_email to raise and abort without fallback, unlike the default-file template path which has a try/except and a fallback. Consider catching template exceptions here and falling back to _get_fallback_template. [ Low confidence ]
• line 73: Possible concurrent use of a shared self.connection without synchronization. If a single SMTPEmailClient instance is used from multiple threads, concurrent calls to send_email will operate on the same Django email backend connection (django.core.mail.backends.smtp.EmailBackend), which is not documented as thread-safe. This can cause interleaved writes or backend errors during email.send() when using the shared connection. Consider per-call connections, a connection pool, or locking around send operations. [ Low confidence ]
• line 99: _get_default_template may receive template_name=None (when template_code is not present in DEFAULT_TEMPLATE_BY_TYPE), then calls get_template(template_name) which raises an exception that is immediately caught and ignored. This uses exceptions for normal control flow and hides genuine template-loading/rendering errors under the same catch-all, making diagnostics difficult and adding overhead. Guard template_name explicitly and skip loader call when it is falsy; narrow the except to expected exceptions. [ Code style ]

backend/src/notifications/services/email.py — 0 comments posted, 15 evaluated, 14 filtered

• line 49: Misconfiguration silently falls back to SMTPEmailClient: if settings.EMAIL_PROVIDER is any value other than EmailProvider.CUSTOMERIO, the code selects SMTPEmailClient without validation. This can route emails via the wrong provider with no error or log, violating configuration expectations. [ Code style ]
• line 78: _send_email_to_console assumes data is a dict and calls data.items() without validation. If data is None or any non-mapping type, this raises AttributeError, crashing the call. Add a guard (e.g., default to {} or coerce/validate) before data.items() to ensure safe handling of empty or invalid inputs. [ Low confidence ]
• line 127: Potential KeyError when accessing settings.PROJECT_CONF['EMAIL'] without guarding against a missing key. If PROJECT_CONF or the EMAIL key is absent or misconfigured at runtime, _send will raise, aborting all email sends instead of failing gracefully. [ Out of scope ]
• line 127: Possible KeyError when accessing settings.PROJECT_CONF['EMAIL'] without a default or existence check. If PROJECT_CONF is missing the EMAIL key, _send will raise at runtime and abort sending/logging. Use get with a default (e.g., settings.PROJECT_CONF.get('EMAIL', False)) or guard the key. [ Low confidence ]
• line 351: In send_guest_new_task, when task_due_date is a string, it is parsed via parse_datetime without validating the result. If parsing fails (returns None) or returns a naive datetime, the subsequent subtraction task_due_date - timezone.now() will raise TypeError (NoneType subtraction or naive/aware mismatch). Add validation and timezone normalization before arithmetic. [ Out of scope ]
• line 353: send_guest_new_task: Potential aware/naive datetime subtraction task_due_date - timezone.now() can raise TypeError if task_due_date is naive (e.g., parsed from a string without timezone or passed as a naive datetime). Normalize to aware timezone (or make both naive) before subtraction. [ Low confidence ]
• line 353: send_guest_new_task: Unsafe handling of task_due_date when it is a string. parse_datetime(task_due_date) can return None for unparseable inputs, causing TypeError on subtraction at due_in = task_due_date - timezone.now(). Add a None check and handle invalid formats gracefully. [ Already posted ]
• line 602: send_workflows_digest calls .strftime(...) on date_from/date_to without validation. If None or a non-date/datetime is passed, it raises AttributeError. Add type checks/coercion or stricter typing. [ Low confidence ]
• line 602: send_workflows_digest and send_tasks_digest call .strftime(...) on date_from and date_to without validating types. If either is None or not a date/datetime, this raises AttributeError. Add type checks or conversions before formatting. [ Low confidence ]
• line 624: send_workflows_digest spreads **digest after setting core fields in data. Any overlapping keys in digest (e.g., unsubscribe_link, link, is_tasks_digest, status_labels) will silently override the intended values, potentially producing malformed or insecure email payloads. Place **digest first or sanitize/namespace its keys. [ Low confidence ]
• line 651: Inconsistent unsubscribe URL format for tasks digest emails. Other methods use '/accounts/emails/unsubscribe?token={token}', but send_tasks_digest uses '/accounts/unsubscribe/{token}'. This inconsistency can lead to broken links or mismatched backend routes. Align the URL format with the others or ensure both routes exist. [ Low confidence ]
• line 652: send_tasks_digest builds an unsubscribe URL inconsistent with other flows: '/accounts/unsubscribe/{token}' vs the established '/accounts/emails/unsubscribe?token=...'. If the former route does not exist (likely given other methods), users get a broken link. [ Low confidence ]
• line 662: send_tasks_digest calls .strftime(...) on date_from/date_to without validating types. Non-date inputs (including None) will raise AttributeError. Add strict typing and validation/coercion. [ Low confidence ]
• line 684: send_tasks_digest spreads **digest after setting fixed fields in data. Overlapping keys in digest can overwrite critical values (e.g., links, flags), producing malformed payloads. Move **digest earlier or whitelist allowed keys. [ Low confidence ]

backend/src/reports/services/tasks.py — 0 comments posted, 3 evaluated, 3 filtered

• line 109: _send_emails assumes digests are finalized, but it does not call put_tmp() or sort templates before sending. When _process_data triggers a bulk send (users_count > bulk_size), some users' TasksDigest.tmp may still hold the last template, causing incomplete data to be sent. _send_emails should finalize each TasksDigest (e.g., put_tmp() and required ordering) prior to enqueuing. [ Out of scope ]
• line 117: Passing non-JSON-serializable objects to Celery: date_from (a date), date_to (a datetime), and especially logo_lg (likely a Django FieldFile) are included in send_tasks_digest_notification.delay(...). With Celery's default JSON serializer, this will raise a serialization error at enqueue time. Convert to primitives (e.g., ISO strings) or ensure a compatible serializer. [ Low confidence ]
• line 124: Potential crash when accessing user.account.logo_lg: if the related account is missing/nullable, user.account access will raise RelatedObjectDoesNotExist. The code does not guard against None/missing relation before dereferencing logo_lg. [ Low confidence ]

backend/src/reports/services/workflows.py — 0 comments posted, 3 evaluated, 3 filtered

• line 75: Non-serializable arguments are passed to Celery in send_workflows_digest_notification.delay(...). Specifically: date_from (line 79) and date_to (line 80) are datetime.date objects, and logo_lg (line 83) is likely a Django FieldFile/ImageFieldFile. With Celery's default JSON serializer these will raise a serialization error on the producer side, preventing the task from being enqueued. Even with pickle, passing FieldFile objects is brittle and unnecessary. Convert dates to ISO strings and pass a primitive (e.g., URL or path) for the logo instead of the file object. [ Low confidence ]
• line 83: Possible AttributeError if user.account is None when accessing user.account.logo_lg (line 83). select_related('account') does not guarantee the relation exists. Add a null check and provide a fallback (e.g., pass None or a default logo URL). [ Low confidence ]
• line 84: user.last_digest_send_time is updated (line 84) and _sent_digests_count incremented (line 86) immediately after enqueuing the task, without confirming the task actually executed or even reached the broker. If enqueueing fails or the worker fails to send the email, the user will still be marked as having received a digest, causing missed notifications and blocking retries. [ Low confidence ]

backend/src/settings.py — 0 comments posted, 6 evaluated, 5 filtered

• line 77: BACKEND_URL = env.get('BACKEND_URL') is assumed non-empty and well-formed, but BACKEND_HOST = BACKEND_URL.split('//')[1].split(':')[0] will raise if BACKEND_URL is missing (None -> AttributeError) or lacks '//' (IndexError). Add validation and a clear error or a safe default before splitting. [ Out of scope ]
• line 111: CORS_ORIGIN_WHITELIST = [FRONTEND_URL, FORMS_URL] can include None if those env vars are unset. Some consumers (django-cors-headers) expect strings; None can cause misconfiguration or errors during origin checks. Filter out falsy values before assigning or provide defaults. [ Out of scope ]
• line 272: Behavioral change: previously EMAIL_BACKEND defaulted to console; now when EMAIL_PROVIDER is unset or not 'customerio', it defaults to SMTP and requires additional envs (EMAIL_HOST, etc.). This can cause runtime failures in environments that previously worked without SMTP configuration (e.g., local dev) when sending mail. Consider preserving the prior default (console) or gating SMTP behind an explicit EMAIL_PROVIDER == 'smtp'. [ Low confidence ]
• line 275: When EMAIL_PROVIDER == EmailProvider.CUSTOMERIO, the settings set EMAIL_BACKEND to 'django.core.mail.backends.console.EmailBackend' (line 275). This backend only prints emails to the console and does not send them, which contradicts the implied contract of selecting the customerio provider and will result in no emails being delivered in that mode. Given that CUSTOMERIO_* API keys are configured immediately after, the code likely intended to use a Customer.io-capable backend or integration rather than the console backend. This is a runtime misconfiguration leading to silent non-delivery of emails whenever EMAIL_PROVIDER is set to 'customerio'. [ Low confidence ]
• line 285: EMAIL_USE_TLS and EMAIL_USE_SSL are set independently from env without enforcing mutual exclusivity. If both resolve to true, Django email configuration is invalid and may raise or fail to connect. Add a guard to ensure only one is enabled, with a clear error if both are set. [ Low confidence ]

[pneumojoseph]

Remove any html markup from the code. Markup is only in the template; create "sub_title" variable if need.

sub_title = 'We got a request to reset your Pneumatic account password.'
content = 'A strong password includes eight or more '
                    'characters and a combination of uppercase and '
                    'lowercase letters, numbers and symbols, and is not '
                    'based on words in the dictionary.

Add more variables as "top_content", "middle_content", "bottom content", "title", "sub_title" or any other for more flexible template customization

[macroscopeapp]

_render_template_string doesn’t handle template errors. A bad DB template (e.g., syntax error) will raise and abort send_email, potentially leaving the SMTP connection open. Consider catching exceptions here and falling back to the raw string (or another safe default) so delivery continues or fails in a controlled way.

Suggested change

	template = Template(template_string)
	try:
	template = Template(template_string)
	return template.render(Context(context))
	except Exception:
	return template_string

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.