Fix error when mail not configured#284
Conversation
- Send mail is only triggered when mail is configered
WalkthroughAdded EMAIL_CONFIGURED boolean in settings, used in views to conditionally send signup verification emails only when SMTP credentials exist. README wording updated in a setup step. No other logic or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #284 +/- ##
===============================================
- Coverage 92.35% 92.31% -0.05%
===============================================
Files 50 50
Lines 1845 1847 +2
===============================================
+ Hits 1704 1705 +1
- Misses 141 142 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
93-105: Clarify that verification emails are disabled without SMTP creds; fix boolean literal in snippet
- Readers should know that if EMAIL_HOST_USER/EMAIL_HOST_PASSWORD aren’t set, no verification emails are sent and admins won’t be notified automatically.
- The example shows
EMAIL_USE_TLS = TRUEwhich is invalid Python; it must beTrue.Apply this diff:
-2. Add +2. Add ``` EMAIL_HOST_USER=your-email EMAIL_HOST_PASSWORD=your-password ``` in this file. **Caution**: Be aware that the password is stored as plain text! + Note: If EMAIL_HOST_USER and EMAIL_HOST_PASSWORD are not set, EVOKS will not send verification emails. New accounts remain pending until a superuser approves them in the Django admin. 3. Open `evoks/evoks/settings.py` and configure the SMTP server. E.g. for gmail ``` EMAIL_HOST = 'smtp.gmail.com' EMAIL_PORT = 587 - EMAIL_USE_TLS = TRUE + EMAIL_USE_TLS = True ```
🧹 Nitpick comments (3)
evoks/evoks/settings.py (1)
95-98: Trim whitespace and provide DEFAULT_FROM_EMAILSmall robustness and ergonomics tweaks.
EMAIL_HOST_USER = get_env('EMAIL_HOST_USER', '') EMAIL_HOST_PASSWORD = get_env('EMAIL_HOST_PASSWORD', '') -EMAIL_CONFIGURED = bool(EMAIL_HOST_USER and EMAIL_HOST_PASSWORD) +EMAIL_CONFIGURED = bool(EMAIL_HOST_USER.strip() and EMAIL_HOST_PASSWORD.strip()) +# Used by email utilities; falls back to instance contact address +DEFAULT_FROM_EMAIL = EMAIL_HOST_USER or EVOKS_MAILevoks/evoks/views.py (2)
1-2: Avoid sending with empty recipient list; log when mail is skippedIf no superuser emails exist, send_mail may be a no-op or error depending on backend. Guard on recipients and log when email is intentionally skipped.
import os +import logging +logger = logging.getLogger(__name__) @@ - if EMAIL_CONFIGURED: - send_mail( + if EMAIL_CONFIGURED and emails: + send_mail( 'Verify a new Evoks account', 'Please visit the evoks admin panel {0} and verify the new user'.format( url), - EMAIL_HOST_USER, + EMAIL_HOST_USER, emails, fail_silently=False, ) + else: + logger.warning( + "Signup verification email not sent (EMAIL_CONFIGURED=%s, recipients=%d).", + EMAIL_CONFIGURED, len(emails) + )Also applies to: 106-115
15-15: Prefer django.conf.settings over importing from the settings moduleUsing
from django.conf import settingsis the Django convention and eases testing/overrides.-from evoks.settings import EMAIL_HOST_USER, EMAIL_CONFIGURED +from django.conf import settings @@ - EMAIL_HOST_USER, + getattr(settings, "DEFAULT_FROM_EMAIL", settings.EMAIL_HOST_USER),Also applies to: 111-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
README.md(1 hunks)evoks/evoks/settings.py(1 hunks)evoks/evoks/views.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-all
🔇 Additional comments (2)
evoks/evoks/settings.py (1)
95-98: Good guard flag for mail configurationComputing EMAIL_CONFIGURED from presence of user/password is a simple, effective gate for sending mail.
evoks/evoks/views.py (1)
106-114: All send_mail calls are properly gated
Only occurrence ofsend_mailinevoks/evoks/views.pyis wrapped byif EMAIL_CONFIGURED.
1 participant
Summary by CodeRabbit
Bug Fixes
Documentation