fix: prevent duplicate newsletter sends from dev/test containers (#440)#441
Conversation
Dev, test, and production containers all share the same Buttondown API key (seeded from prod backups). Each independently fires the newsletter preview cron, resulting in 3 identical preview emails every Thursday. Add NEWSLETTER_SEND_ENABLED env var (default: true in prod, false in dev/test). When false, buttondownClient.js skips all outbound sends (addSubscriber, sendEmail, sendDraftToRecipients) while still allowing digest generation and UI testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a NEWSLETTER_SEND_ENABLED environment variable to control whether Buttondown newsletter operations (adding subscribers, sending emails, and sending drafts) are executed, defaulting to false for development and test environments. The feedback highlights two key issues: first, adding the variable to ENV_ARGS in run.sh is dead code, and the test environment configuration needs to explicitly disable this variable to prevent tests from accidentally sending emails; second, the check in buttondownClient.js is case-sensitive and should be updated to a case-insensitive check to avoid accidental sends if configured with values like FALSE or False.
| [ -n "$PGUSER" ] && ENV_ARGS="$ENV_ARGS -e PGUSER=$PGUSER" | ||
| [ -n "$PGPASSWORD" ] && ENV_ARGS="$ENV_ARGS -e PGPASSWORD=$PGPASSWORD" | ||
| [ -n "$PGDATABASE" ] && ENV_ARGS="$ENV_ARGS -e PGDATABASE=$PGDATABASE" | ||
| [ -n "$NEWSLETTER_SEND_ENABLED" ] && ENV_ARGS="$ENV_ARGS -e NEWSLETTER_SEND_ENABLED=$NEWSLETTER_SEND_ENABLED" |
There was a problem hiding this comment.
The ENV_ARGS variable is built here but is never actually used or passed to any podman run command in this script (the container instead relies on the mounted /etc/rotv/environment file). Adding NEWSLETTER_SEND_ENABLED to ENV_ARGS here is dead code and has no effect. Additionally, to ensure that the test container also has newsletter sends disabled by default, NEWSLETTER_SEND_ENABLED=false (or NEWSLETTER_SEND_ENABLED=${NEWSLETTER_SEND_ENABLED:-false}) should be added to the ~/.rotv/environment-test file generation block around line 244-255. Otherwise, since isSendEnabled() in buttondownClient.js defaults to true when the environment variable is undefined, the test container will run with newsletter sends enabled.
| function isSendEnabled() { | ||
| return process.env.NEWSLETTER_SEND_ENABLED !== 'false'; | ||
| } |
There was a problem hiding this comment.
The environment variable check is currently case-sensitive. If a user or environment configuration sets NEWSLETTER_SEND_ENABLED=FALSE or False, the check !== 'false' will evaluate to true, and newsletter sends will remain enabled. To prevent accidental sends due to casing differences, we should perform a case-insensitive check.
function isSendEnabled() { const val = process.env.NEWSLETTER_SEND_ENABLED; return !val || val.toLowerCase() !== 'false'; }1 participant
Summary
NEWSLETTER_SEND_ENABLEDenv var guard tobuttondownClient.js— whenfalse, all outbound Buttondown sends are skipped (addSubscriber, sendEmail, sendDraftToRecipients)falsein dev environments (run.sh template + .env.example); production is unaffected (defaults to enabled)Closes #440
Test plan
./run.sh buildpassesNEWSLETTER_SEND_ENABLED=falseto test container env on lotor🤖 Generated with Claude Code