Repository navigation
Finish transactional mailer: sender, reply_to, hardcoded URLs - #561
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Mailer item marked done (PR #561, code only — the real MAILER_FROM address on the sandbox Mailjet account is still the team's to pick). Litestream's env var mapping checked against the gem's source, no bug there — just the manual Scaleway provisioning left. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EpFUEkhFgTwEUezjTZBbex
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cb70a0a88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| default from: ENV["MAILER_FROM"] || "no-reply@lecircographe.fr" | ||
| # Sender address per environment: config.action_mailer.default_options (production/staging.rb), | ||
| # never from a local .env file (dev machines carry a personal Gmail there for SMTP testing). | ||
| default from: "no-reply@lecircographe.fr" |
There was a problem hiding this comment.
When staging is deployed with a sandbox-validated MAILER_FROM, this class-level default merges over the ActionMailer::Base defaults configured by config.action_mailer.default_options, so every mailer still sends from no-reply@lecircographe.fr, which the comments state is not validated on the staging Mailjet account. It also discards the production display name. Remove this from default so the new environment-specific settings actually control the sender.
Useful? React with 👍 / 👎.
| # utilisé en sandbox (voir config/environments/staging.rb). Pas un secret, juste une adresse | ||
| # email — mais pas de valeur par défaut fiable ici (dépend du compte Mailjet utilisé), à définir | ||
| # explicitement (ex. export local avant déploiement, ou variable GitHub Actions). | ||
| MAILER_FROM: <%= ENV.fetch("MAILER_FROM", "") %> |
There was a problem hiding this comment.
In the standard GitHub deployment, .github/workflows/deploy-staging.yml lines 44–50 export the other deployment values but never export MAILER_FROM, so this expression always renders a blank value in CI rather than the required sandbox-validated address. Kamal's env.clear configuration supplies these values to the container; after the subclass sender override is removed, staging therefore has no usable sender and Mailjet delivery cannot be validated. Add the corresponding Actions variable/secret to the deploy job or make this fetch fail when it is absent.
Useful? React with 👍 / 👎.
| end | ||
|
|
||
| it "répond vers contact@lecircographe.fr" do | ||
| expect(mail.reply_to).to eq([ "contact@lecircographe.fr" ]) |
There was a problem hiding this comment.
When this spec runs, mail is undefined: the only mail object in this describe block is a local variable created inside the preceding example, and RSpec does not share locals between examples or define a named subject here. This example therefore raises before checking reply_to; construct the message in this example or add a subject(:mail) for the block.
Useful? React with 👍 / 👎.
Closes the "Mailer transactionnel" todo item: - Sender address moves to config.action_mailer.default_options per environment (production.rb: fixed, validated domain; staging.rb: ENV MAILER_FROM, wired through Kamal's deploy.staging.yml clear env) instead of ApplicationMailer reading straight from ENV — a stray .env MAILER_FROM (personal Gmail, used for local SMTP testing) could otherwise leak into a deployed environment. - reply_to: contact@lecircographe.fr on mails where a reply makes sense (welcome x2, membership reminder, donation receipt). None on security mails (password reset/changed, account claim, email change verification) — specs lock that split both ways. - UserMailer's hardcoded "https://lecircographe.fr/" replaced with root_url (staging links pointed at prod before this). - contact@lecircographe.fr footer line in both mailer layouts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EpFUEkhFgTwEUezjTZBbex
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EpFUEkhFgTwEUezjTZBbex
4cb70a0 to
ef490a4
Compare
2 participants
Closes the "Mailer transactionnel" item in
docs/internal/todo.md.Changements
config.action_mailer.default_optionsdansproduction.rb(adresse fixe, domaine validé Mailjet) etstaging.rb(ENV.fetch("MAILER_FROM", ...), câblé viaconfig/deploy.staging.ymlenv clear).ApplicationMailerne lit plusENV["MAILER_FROM"]directement — évite qu'un.envlocal (Gmail perso de dev) ne fuite en environnement déployé.reply_to: contact@lecircographe.frsur les mails où une réponse a du sens (bienvenue ×2, rappel d'adhésion, reçu de don). Aucun sur les mails de sécurité (reset/changement de mot de passe, rattachement de compte, vérification de changement d'email) — verrouillé par specs dans les deux sens.UserMailerutilisait"https://lecircographe.fr/"en dur (donc les liens staging pointaient vers la prod) →root_url, résolu correctement par environnement (default_url_optionsdéjà bon partout, vérifié).contact@lecircographe.frdans les deux layouts mailer (html + text).Non testé en local
Le sandbox de cette session n'a pas Ruby 4.0.1 (celui du projet) installé — specs écrites et relues, syntaxe validée (
ruby -c), mais pas exécutées. La CI tranchera.Reste hors scope de cette PR
MAILER_FROMvalidée (valeur réelle à définir par vous, pas par moi)MAILER_DELIVERIES_ENABLEDreste àfalseen prod (déjà verrouillé par spec existante) — à activer après validation du parcours en stagingGenerated by Claude Code