Setup sandbox email settings for staging#308
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughEmail backend configuration is being refactored across Django settings environments. The console email backend is removed from base settings and migrated to development settings. Staging now explicitly configures SMTP with environment variables. CI/CD workflow adds secret-backed email configuration variables for staging deployment. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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 |
Abdulwahab-CS
requested review from
abubakr-itqan,
hassaanalansary and
m-abulnasr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
config/settings/staging.py (2)
158-162: Silent empty defaults can mask misconfiguration.Using
default=""forEMAIL_HOST,EMAIL_HOST_USER, andEMAIL_HOST_PASSWORDmeans a missing or misnamed env var (see theSTAGINgG_EMAIL_HOST_PASSWORDtypo in.github/workflows/ci-cd.ymlline 179) will not raise at startup — SMTP will just fail at send time with an auth error, which is harder to diagnose. Since staging is meant to exercise real email flows, consider dropping the defaults (lettingdecoupleraiseUndefinedValueError) for the required fields, or adding a startup check that logs a warning when any are empty. This mirrors the same concern that should apply toproduction.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/settings/staging.py` around lines 158 - 162, The staging email settings currently use silent empty defaults for EMAIL_HOST, EMAIL_HOST_USER, and EMAIL_HOST_PASSWORD which can hide misconfigured or misspelled env vars; update the config so these three variables do not provide default="" (remove the default argument) to let decouple raise UndefinedValueError at startup, or alternatively add a startup validation function that reads EMAIL_HOST, EMAIL_HOST_USER, EMAIL_HOST_PASSWORD and logs or raises if any are empty; adjust the EMAIL_PORT handling only if you want a cast/default, but keep EMAIL_BACKEND, EMAIL_HOST, EMAIL_HOST_USER, and EMAIL_HOST_PASSWORD changes in config/settings/staging.py to ensure failures surface early.
165-165: Minor:DEFAULT_FROM_EMAILformatting edge case.When
_EMAIL_SENDER_NAMEcontains characters that require RFC 5322 quoting (commas, parentheses, non-ASCII), the naive f-stringf"{name} <{email}>"may produce a malformedFrom:header. Consider usingemail.utils.formataddr((name, email))for a safer format. This pattern also exists inconfig/settings/production.py, so the fix would apply to both.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/settings/staging.py` at line 165, DEFAULT_FROM_EMAIL is built with a naive f-string using _EMAIL_SENDER_NAME and _EMAIL_SENDER_EMAIL which can produce malformed RFC5322 From headers when the name contains commas, parentheses, or non-ASCII; change the construction to use email.utils.formataddr(( _EMAIL_SENDER_NAME, _EMAIL_SENDER_EMAIL )) when _EMAIL_SENDER_NAME is set (fall back to _EMAIL_SENDER_EMAIL when name is empty) and apply the same change to the equivalent DEFAULT_FROM_EMAIL definition in production (reference symbols: DEFAULT_FROM_EMAIL, _EMAIL_SENDER_NAME, _EMAIL_SENDER_EMAIL)..github/workflows/ci-cd.yml (1)
176-181: Consider quoting values written to.envto guard against special characters.Email host passwords commonly contain characters like
$,#, spaces, or quotes. The currentecho "KEY=${{ secrets.X }}"form interpolates the secret unquoted into the.envfile, which will breakdecouple/docker-compose parsing if the password contains spaces,#, or line breaks. Consider wrapping values in single quotes (or using a heredoc) so the resulting line isKEY='value'. This concern applies to all secret-backed values written in this block, not only the email ones.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci-cd.yml around lines 176 - 181, The echo statements that write secret values to the .env in .github/workflows/ci-cd.yml (the lines that emit STAGING_EMAIL_HOST, STAGING_EMAIL_PORT, STAGING_EMAIL_HOST_USER, STAGING_EMAIL_HOST_PASSWORD, STAGING_EMAIL_SENDER_NAME, STAGING_EMAIL_SENDER_EMAIL) must quote values to protect special characters; change each echo from echo "KEY=${{ secrets.X }}" to emit KEY='value' (or use a heredoc) so the secret is wrapped in single quotes when written to the .env, ensuring passwords with $, #, spaces or newlines are preserved and parsed correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci-cd.yml:
- Line 179: Fix the typo in the CI workflow: replace the incorrect env var key
"STAGINgG_EMAIL_HOST_PASSWORD" with the correct "STAGING_EMAIL_HOST_PASSWORD" so
the value exported by the workflow matches what config/settings/staging.py reads
(STAGING_EMAIL_HOST_PASSWORD) and prevents the SMTP password from falling back
to the empty default.
---
Nitpick comments:
In @.github/workflows/ci-cd.yml:
- Around line 176-181: The echo statements that write secret values to the .env
in .github/workflows/ci-cd.yml (the lines that emit STAGING_EMAIL_HOST,
STAGING_EMAIL_PORT, STAGING_EMAIL_HOST_USER, STAGING_EMAIL_HOST_PASSWORD,
STAGING_EMAIL_SENDER_NAME, STAGING_EMAIL_SENDER_EMAIL) must quote values to
protect special characters; change each echo from echo "KEY=${{ secrets.X }}" to
emit KEY='value' (or use a heredoc) so the secret is wrapped in single quotes
when written to the .env, ensuring passwords with $, #, spaces or newlines are
preserved and parsed correctly.
In `@config/settings/staging.py`:
- Around line 158-162: The staging email settings currently use silent empty
defaults for EMAIL_HOST, EMAIL_HOST_USER, and EMAIL_HOST_PASSWORD which can hide
misconfigured or misspelled env vars; update the config so these three variables
do not provide default="" (remove the default argument) to let decouple raise
UndefinedValueError at startup, or alternatively add a startup validation
function that reads EMAIL_HOST, EMAIL_HOST_USER, EMAIL_HOST_PASSWORD and logs or
raises if any are empty; adjust the EMAIL_PORT handling only if you want a
cast/default, but keep EMAIL_BACKEND, EMAIL_HOST, EMAIL_HOST_USER, and
EMAIL_HOST_PASSWORD changes in config/settings/staging.py to ensure failures
surface early.
- Line 165: DEFAULT_FROM_EMAIL is built with a naive f-string using
_EMAIL_SENDER_NAME and _EMAIL_SENDER_EMAIL which can produce malformed RFC5322
From headers when the name contains commas, parentheses, or non-ASCII; change
the construction to use email.utils.formataddr(( _EMAIL_SENDER_NAME,
_EMAIL_SENDER_EMAIL )) when _EMAIL_SENDER_NAME is set (fall back to
_EMAIL_SENDER_EMAIL when name is empty) and apply the same change to the
equivalent DEFAULT_FROM_EMAIL definition in production (reference symbols:
DEFAULT_FROM_EMAIL, _EMAIL_SENDER_NAME, _EMAIL_SENDER_EMAIL).
🪄 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
Run ID: 33872332-651f-4165-85be-40412ea83f90
📒 Files selected for processing (4)
.github/workflows/ci-cd.ymlconfig/settings/base.pyconfig/settings/development.pyconfig/settings/staging.py
💤 Files with no reviewable changes (1)
- config/settings/base.py
1 participant
Summary by CodeRabbit