feat(2431): add email templates and send email test command#2479
Conversation
…izable by env vars
herzog0
left a comment
There was a problem hiding this comment.
Looks good to me!
Tested with my own Mailtrap profile and got the same results.
Also did some research on the amount of emails of each domain in our db and found that most of the unsupported ones are not truly relevant to our user base.
jlchilders11
left a comment
There was a problem hiding this comment.
Couple of small nits, but otherwise looks good to me!
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a complete transactional email system to the Boost website, including environment-driven SMTP configuration, reusable Django email templates with block-based customization, concrete templates for email confirmation and password reset, and a management command for testing email rendering and delivery. ChangesTransactional Email System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
users/management/commands/send_test_emails.py (1)
206-250: 💤 Low valueConsider wrapping connection usage in try/finally for clean resource release.
If an exception occurs during template rendering or sending (lines 217-248),
connection.close()at line 250 won't be reached. For a dev-only tool this is low-impact, but a try/finally ensures the connection is always released.♻️ Suggested improvement
connection = get_connection() + try: is_smtp = "smtp" in settings.EMAIL_BACKEND # ... rest of sending logic ... click.secho(f" sent: {key} — {subject!r}", fg="cyan") - connection.close() + finally: + connection.close() click.secho("Done.", fg="green")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@users/management/commands/send_test_emails.py` around lines 206 - 250, The code uses connection = get_connection() and calls connection.close() after the send loop, but if render_to_string, _send_inline, EmailMultiAlternatives, or msg.send() raise an exception the close() won't run; wrap the for-loop and send logic in a try/finally where the finally always calls connection.close() (or checks connection is not None before closing) so get_connection()/connection is reliably released even on errors when using TEMPLATES, render_to_string, _send_inline, EmailMultiAlternatives, or msg.send().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/emails/base_email.html`:
- Line 58: The template references email images via the custom tag large_static
(e.g., in templates/emails/base_email.html at the <img src="{% large_static
'img/emails/email-logo.png' %}"> usage), but those asset files are missing; fix
by adding the missing files to your project's static assets at img/emails
(create static/img/emails/email-logo.png, email-octopus.jpg, email-lock.png,
email-warning.png, email-social-x.png, email-social-bluesky.png,
email-social-mastodon.png, email-social-reddit.png, email-social-github.png,
email-social-linkedin.png) or alternatively change the large_static references
in base_email.html (and the other occurrences around lines 116–122) to point to
existing hosted URLs or to a fallback image path that is present in the repo so
emails won't break when assets are not provided externally.
In `@templates/emails/confirm_email.html`:
- Line 29: The templates templates/emails/confirm_email.html and
templates/emails/confirm_email.txt currently hardcode "This link expires in 24
hours."; change the text to use the allauth context variable (e.g., {{
EMAIL_CONFIRMATION_EXPIRE_DAYS }} or the exact allauth-supplied var) instead of
"24 hours", and ensure that the variable is supplied to the email template
context where the confirmation email is rendered (update the code path that
builds the email context—e.g., the allauth adapter/email-sending hook or the
function that calls render_to_string for confirm_email.html/confirm_email.txt—to
include EMAIL_CONFIRMATION_EXPIRE_DAYS derived from
ACCOUNT_EMAIL_CONFIRMATION_EXPIRE_DAYS or EMAIL_CONFIRMATION_EXPIRE_DAYS).
In `@templates/emails/password_reset.html`:
- Line 29: The templates currently hardcode "This link will expire in 1 hour." —
replace that hardcoded text in templates/emails/password_reset.html and .txt
with a dynamic placeholder (e.g. "This link will expire in {{ expiry }}.") and
update the code that renders/sends the password reset email (e.g. your
PasswordResetView/send_password_reset_email/send_mail helper or adapter that
calls render_to_string) to compute the actual expiry string from the configured
token lifetime (read from Django/allauth settings or fall back to the framework
default) and pass it into the template context as expiry; ensure formatting
(hours/days) matches the computed value so the user-facing message matches the
real token lifetime.
In `@users/management/commands/send_test_emails.py`:
- Around line 199-201: The code detects a missing URL scheme by splitting
base_url into scheme and host (variables scheme, host) but never updates
base_url, so later URL joins produce malformed URLs; fix by reconstructing
base_url when scheme was missing (e.g., set base_url = f"{scheme}://{host}"
right after assigning scheme="https" and host=base_url) so that subsequent uses
of base_url produce fully qualified URLs; update the logic in send_test_emails
(variables scheme, host, base_url) accordingly.
---
Nitpick comments:
In `@users/management/commands/send_test_emails.py`:
- Around line 206-250: The code uses connection = get_connection() and calls
connection.close() after the send loop, but if render_to_string, _send_inline,
EmailMultiAlternatives, or msg.send() raise an exception the close() won't run;
wrap the for-loop and send logic in a try/finally where the finally always calls
connection.close() (or checks connection is not None before closing) so
get_connection()/connection is reliably released even on errors when using
TEMPLATES, render_to_string, _send_inline, EmailMultiAlternatives, or
msg.send().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58498630-4e2d-4125-8e9b-ee197b3b4f6b
📒 Files selected for processing (10)
config/settings.pytemplates/emails/_social_icon.htmltemplates/emails/base_email.htmltemplates/emails/confirm_email.htmltemplates/emails/confirm_email.txttemplates/emails/confirm_email_subject.txttemplates/emails/password_reset.htmltemplates/emails/password_reset.txttemplates/emails/password_reset_subject.txtusers/management/commands/send_test_emails.py
| scheme, _, host = base_url.partition("://") | ||
| if not host: # base_url given without a scheme | ||
| scheme, host = "https", base_url |
There was a problem hiding this comment.
base_url not reconstructed when scheme is missing, causing malformed URLs.
If the user passes --base-url www.boost.org (without a scheme), lines 200-201 correctly set scheme="https" and host="www.boost.org", but base_url itself is never reassigned. Lines 227-228 then produce URLs like "www.boost.org/account/preferences" missing the scheme prefix.
🐛 Proposed fix
scheme, _, host = base_url.partition("://")
if not host: # base_url given without a scheme
scheme, host = "https", base_url
+ base_url = f"{scheme}://{host}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scheme, _, host = base_url.partition("://") | |
| if not host: # base_url given without a scheme | |
| scheme, host = "https", base_url | |
| scheme, _, host = base_url.partition("://") | |
| if not host: # base_url given without a scheme | |
| scheme, host = "https", base_url | |
| base_url = f"{scheme}://{host}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@users/management/commands/send_test_emails.py` around lines 199 - 201, The
code detects a missing URL scheme by splitting base_url into scheme and host
(variables scheme, host) but never updates base_url, so later URL joins produce
malformed URLs; fix by reconstructing base_url when scheme was missing (e.g.,
set base_url = f"{scheme}://{host}" right after assigning scheme="https" and
host=base_url) so that subsequent uses of base_url produce fully qualified URLs;
update the logic in send_test_emails (variables scheme, host, base_url)
accordingly.
Issue: #2431
Summary & Context
Adds the templates and images (sync down from S3) for Email Confirmation and Password Reset.
Changes
[ ] Get email credentials so it's easier to test emails in local development.Screenshots
Confirm email: HTML / Desktop
Confirm email: text
Confirm email: HTML mobile
Password reset: HTML Desktop
Password reset: HTML mobile
Password reset: text
Gmail
iOS
Web
Apple Mail
Self-review Checklist
Frontend
Summary by CodeRabbit
Release Notes
New Features
Chores