[codex] Add onboarding email command#330
Conversation
📝 WalkthroughWalkthroughAdds a deterministic onboarding email builder, onboarding SMTP configuration, a Discord cog exposing ChangesOnboarding Email Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Pull request overview
Adds a shared, deterministic onboarding email generator (HTML + plain text) and exposes it via a new Admin-only Discord slash command to draft downloadable email files and optionally send them over SMTP, along with configuration/docs and unit tests.
Changes:
- Introduces
five08.onboarding_emailwith deterministic, state-driven email rendering for consistent onboarding messaging. - Adds an Admin-only
/onboarding-emailDiscord command that generates drafts and can send via configured SMTP (send=true). - Documents and wires new onboarding SMTP configuration, plus unit tests for the builder and cog behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/five08/onboarding_email.py |
New deterministic onboarding email builder producing text + HTML drafts. |
apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py |
New Discord cog implementing /onboarding-email draft + SMTP send flow. |
apps/discord_bot/src/five08/discord_bot/config.py |
Adds onboarding email SMTP/sender settings (with fallbacks/aliases). |
tests/unit/test_onboarding_email.py |
Unit tests for deterministic draft content and validation. |
tests/unit/test_onboarding_email_cog.py |
Unit tests for draft generation, Reply-To resolution behavior, and send gating. |
ENVIRONMENT.md |
Documents onboarding email SMTP environment variables and behavior. |
.env.example |
Adds example onboarding SMTP configuration entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| port = settings.onboarding_email_smtp_port | ||
| timeout = settings.onboarding_email_smtp_timeout_seconds | ||
| if settings.onboarding_email_smtp_use_ssl: | ||
| with smtplib.SMTP_SSL(smtp_server, port, timeout=timeout) as smtp: | ||
| smtp.login(smtp_username, smtp_password) | ||
| smtp.send_message(message) | ||
| return | ||
|
|
||
| with smtplib.SMTP(smtp_server, port, timeout=timeout) as smtp: | ||
| if settings.onboarding_email_smtp_starttls: | ||
| smtp.starttls() | ||
| smtp.login(smtp_username, smtp_password) | ||
| smtp.send_message(message) |
| def onboarding_cog(mock_bot: Mock) -> OnboardingEmailCog: | ||
| with patch("five08.discord_bot.cogs.onboarding_email.settings") as mock_settings: | ||
| mock_settings.espo_api_key = "token" | ||
| mock_settings.espo_base_url = "https://crm.example.com" | ||
| mock_settings.audit_api_base_url = "https://audit.example.com" | ||
| mock_settings.api_shared_secret = "secret" | ||
| mock_settings.audit_api_timeout_seconds = 5.0 | ||
| mock_settings.discord_logs_webhook_url = None | ||
| mock_settings.discord_logs_webhook_wait = False | ||
| mock_settings.onboarding_email_sender_email = "onboarding@508.dev" | ||
| mock_settings.onboarding_email_smtp_server = "smtp.migadu.com" | ||
| mock_settings.onboarding_email_smtp_port = 465 | ||
| mock_settings.onboarding_email_smtp_use_ssl = True | ||
| mock_settings.onboarding_email_smtp_starttls = False | ||
| mock_settings.onboarding_email_smtp_username = "onboarding@508.dev" | ||
| mock_settings.onboarding_email_smtp_password = "secret" | ||
| mock_settings.onboarding_email_smtp_timeout_seconds = 20.0 | ||
| cog = OnboardingEmailCog(mock_bot) | ||
| cog.crm = Mock() | ||
| return cog |
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
66a4ee4 to
c41076c
Compare
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
c41076c to
850070a
Compare
| resolved_sender_name = sender_name.strip() if sender_name else "" | ||
| if not resolved_sender_name: | ||
| resolved_sender_name = self._display_name(interaction) |
| except (PermissionError, ValueError, EspoAPIError) as exc: | ||
| self._audit_command_safe( | ||
| interaction=interaction, | ||
| action="onboarding.email", | ||
| result="denied", | ||
| metadata={ | ||
| "candidate_name": candidate_name, | ||
| "recipient_email": recipient_email, | ||
| "has_contributed": has_contributed, | ||
| "discord_joined": discord_joined, | ||
| "agreement_signed": agreement_signed, | ||
| "send": send, | ||
| "error": str(exc), | ||
| }, | ||
| resource_type="onboarding_email", | ||
| resource_id=recipient_email, | ||
| ) | ||
| await interaction.followup.send( | ||
| f"⚠️ {exc}", | ||
| allowed_mentions=NO_MENTIONS, | ||
| ephemeral=True, | ||
| ) |
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
850070a to
568deac
Compare
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
568deac to
1d6dd98
Compare
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
1d6dd98 to
2204102
Compare
| if len(candidate_contacts) > 1: | ||
| view = OnboardingEmailContactSelectView( | ||
| cog=self, | ||
| requester_id=interaction.user.id, | ||
| state=state, | ||
| contacts=candidate_contacts, | ||
| ) | ||
| await interaction.followup.send( | ||
| "⚠️ Multiple CRM contacts match this onboarding email. " | ||
| "Select the candidate to continue. Already-onboarded contacts " | ||
| "are labelled with `status: onboarded`.", | ||
| allowed_mentions=NO_MENTIONS, | ||
| view=view, | ||
| ephemeral=True, | ||
| ) | ||
| return |
| chunks = [ | ||
| draft_message[index : index + 1900] | ||
| for index in range(0, len(draft_message), 1900) | ||
| ] | ||
| for index, chunk in enumerate(chunks, 1): | ||
| await interaction.followup.send( | ||
| f"**Copy/paste draft ({index}/{len(chunks)}):**\n{chunk}", | ||
| allowed_mentions=NO_MENTIONS, | ||
| ephemeral=True, | ||
| ) |
![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: 2204102a64
ℹ️ 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".
| await self.view.cog._complete_onboarding_email( | ||
| interaction, | ||
| state=self.view.state, | ||
| selected_contact=contact, | ||
| ) |
There was a problem hiding this comment.
When the command finds multiple CRM matches, this component callback defers the interaction and calls _complete_onboarding_email without the error handling used by the slash-command path. If the selected contact is not assigned to the requester, send=true is missing a Reply-To, or CRM/SMTP raises, the exception escapes after the defer, so the user gets a failed interaction instead of the warning/audit response and the selector remains usable. Wrap this call in the same PermissionError/ValueError/EspoAPIError/SMTP handling used by onboarding_email.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py`:
- Around line 118-123: The contact selector callback currently calls
_complete_onboarding_email directly and therefore bypasses the centralized
try/except + auditing used by onboarding_email; modify the code so both
onboarding_email and OnboardingEmailContactSelect.callback call a
new/centralized helper _run_onboarding_email_flow that wraps the call to
_complete_onboarding_email in the existing try/except and uses
_audit_command_safe for PermissionError/ValueError/EspoAPIError/SMTP errors, and
ensure interaction responses are resolved in the same error paths as the slash
command so selector failures are audited and the interaction is not left
unresolved.
- Around line 521-530: The SMTP calls in onboarding_email.py currently
instantiate smtplib.SMTP_SSL(...) and call smtp.starttls() without an explicit
SSLContext; create a secure context via ssl.create_default_context() and pass it
to SMTP_SSL as the context argument when settings.onboarding_email_smtp_use_ssl
is true, and pass the same context to smtp.starttls(context=...) when
settings.onboarding_email_smtp_starttls is true; ensure the new context is used
before any login/send_message calls (refer to the SMTP_SSL(...) call, the
starttls() call, and the blocks guarded by
settings.onboarding_email_smtp_use_ssl and
settings.onboarding_email_smtp_starttls).
In `@tests/unit/test_onboarding_email_cog.py`:
- Around line 23-42: The patch of settings in the onboarding_cog fixture
currently uses a with block that stops before tests run; change it to start the
patcher for the lifetime of the fixture and stop it in teardown so module-level
reads see the mocked values. Replace the with
patch("five08.discord_bot.cogs.onboarding_email.settings") as mock_settings
block by creating a patcher =
patch("five08.discord_bot.cogs.onboarding_email.settings"), calling
mock_settings = patcher.start(), constructing the OnboardingEmailCog(mock_bot),
yield or return the cog from the fixture, and ensure patcher.stop() runs after
the test (e.g., in a finally block or fixture teardown) so onboarding_cog,
mock_settings and the module-level settings remain mocked for the whole test.
🪄 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: 9388397f-79db-4acb-ba0b-67cfa3ea0d46
📒 Files selected for processing (8)
.env.exampleENVIRONMENT.mdapps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.pyapps/discord_bot/src/five08/discord_bot/config.pypackages/shared/src/five08/onboarding_email.pytests/unit/test_bot.pytests/unit/test_onboarding_email.pytests/unit/test_onboarding_email_cog.py
✅ Files skipped from review due to trivial changes (1)
- ENVIRONMENT.md
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/discord_bot/src/five08/discord_bot/config.py
- tests/unit/test_onboarding_email.py
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
2204102 to
5f656ec
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f656ec3e2
ℹ️ 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".
| username_candidates = { | ||
| str(getattr(interaction.user, "name", "") or "").strip(), | ||
| str(getattr(interaction.user, "display_name", "") or "").strip(), | ||
| } |
There was a problem hiding this comment.
When a Discord ID lookup misses, this fallback trusts interaction.user.name/display_name to resolve the CRM contact, and _requester_508_username() then uses that contact's c508Email for the designated-onboarder authorization check. In servers where members can change their nickname/display name, an unlinked user can set it to a real onboarder's stored cDiscordUsername, run /onboarding-email for that onboarder's assigned contact, and be authorized as them (including send flows if they provide a Reply-To). Authorization should only use the immutable Discord user ID, or otherwise require a verified link before granting onboarder privileges.
Useful? React with 👍 / 👎.
| await interaction.followup.send( | ||
| summary, | ||
| allowed_mentions=NO_MENTIONS, | ||
| ephemeral=True, | ||
| ) |
| def _contact_for_recipient_email( | ||
| self, recipient_email: str | ||
| ) -> dict[str, object] | None: | ||
| response = self.crm.list_contacts( | ||
| { |
| draft = build_onboarding_email( | ||
| OnboardingEmailRequest( | ||
| candidate_name=candidate_name, | ||
| sender_name=state.signature_name, | ||
| has_contributed=state.has_contributed, | ||
| discord_joined=state.discord_joined, # type: ignore[arg-type] | ||
| membership_agreement_signed=state.agreement_signed, # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| try: | ||
| resolved_reply_to = await asyncio.to_thread( | ||
| self._reply_to_email_for_user, | ||
| interaction=interaction, | ||
| override=state.reply_to_email, | ||
| ) | ||
| except EspoAPIError: | ||
| if state.send and not state.reply_to_email: | ||
| raise | ||
| logger.warning( | ||
| "Unable to resolve onboarding email Reply-To from CRM", | ||
| exc_info=True, | ||
| ) | ||
| resolved_reply_to = None | ||
|
|
||
| authorization_source = await asyncio.to_thread( | ||
| self._authorize_onboarding_email, | ||
| interaction=interaction, | ||
| selected_contact=selected_contact, | ||
| ) |
michaelmwu
force-pushed
the
michaelmwu/onboarding-email-automation
branch
from
5f656ec to
9e8697c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e8697c7eb
ℹ️ 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".
| await interaction.response.defer(ephemeral=True) | ||
| await self.view.cog._run_onboarding_email_flow( | ||
| interaction, | ||
| state=self.view.state, | ||
| selected_contact=contact, | ||
| ) |
There was a problem hiding this comment.
In the selector path, this passes the contact snapshot captured when the menu was rendered directly into _run_onboarding_email_flow. If the candidate is marked onboarded, reassigned, or has their email changed during the 5-minute selection window, _complete_onboarding_email trusts the stale cOnboardingState, cOnboarder, and email fields and can generate/send an onboarding email that the current CRM state would block or route differently. Re-fetch the selected contact by ID in the callback before continuing so authorization and already-onboarded checks use fresh CRM data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py`:
- Around line 849-874: The try/except in the onboarding wrapper only catches a
fixed set of exceptions and misses other runtime errors; update the error
handling around the call to _complete_onboarding_email so any unexpected
Exception is also caught and forwarded to _handle_onboarding_email_error (use
"except Exception as exc:" after the existing specific except block or add a
final broad except) and pass the same parameters (interaction, exc, state,
candidate_name, recipient_email, has_contributed, discord_joined,
agreement_signed, send, selected_contact) to ensure deferred interactions are
resolved and audit/error classification runs; apply the same change to the
second wrapper that calls _complete_onboarding_email (the block mirrored around
lines ~1017) so both paths use the generic Exception handler.
🪄 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: c70c6525-e3ac-432c-9194-7c7fc44e1a64
📒 Files selected for processing (8)
.env.exampleENVIRONMENT.mdapps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.pyapps/discord_bot/src/five08/discord_bot/config.pypackages/shared/src/five08/onboarding_email.pytests/unit/test_bot.pytests/unit/test_onboarding_email.pytests/unit/test_onboarding_email_cog.py
✅ Files skipped from review due to trivial changes (1)
- ENVIRONMENT.md
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/unit/test_bot.py
- tests/unit/test_onboarding_email.py
- apps/discord_bot/src/five08/discord_bot/config.py
- packages/shared/src/five08/onboarding_email.py
| try: | ||
| await self._complete_onboarding_email( | ||
| interaction, | ||
| state=state, | ||
| selected_contact=selected_contact, | ||
| ) | ||
| except ( | ||
| PermissionError, | ||
| ValueError, | ||
| EspoAPIError, | ||
| OSError, | ||
| smtplib.SMTPException, | ||
| ) as exc: | ||
| await self._handle_onboarding_email_error( | ||
| interaction, | ||
| exc, | ||
| state=state, | ||
| candidate_name=state.candidate_name, | ||
| recipient_email=state.recipient_email, | ||
| has_contributed=state.has_contributed, | ||
| discord_joined=state.discord_joined, | ||
| agreement_signed=state.agreement_signed, | ||
| send=state.send, | ||
| selected_contact=selected_contact, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Catch unexpected exceptions in the onboarding flow wrapper.
Line 849 and Line 1017 only catch a fixed exception set. Any other runtime exception bypasses _handle_onboarding_email_error, which can leave deferred interactions without a user-visible failure message and skip the standardized audit/error classification path.
Proposed minimal fix
@@
- except (
- PermissionError,
- ValueError,
- EspoAPIError,
- OSError,
- smtplib.SMTPException,
- ) as exc:
+ except Exception as exc:
await self._handle_onboarding_email_error(
interaction,
exc,
@@
- except (
- PermissionError,
- ValueError,
- EspoAPIError,
- OSError,
- smtplib.SMTPException,
- ) as exc:
+ except Exception as exc:
await self._handle_onboarding_email_error(
interaction,
exc,Also applies to: 1017-1034
🤖 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 `@apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py` around
lines 849 - 874, The try/except in the onboarding wrapper only catches a fixed
set of exceptions and misses other runtime errors; update the error handling
around the call to _complete_onboarding_email so any unexpected Exception is
also caught and forwarded to _handle_onboarding_email_error (use "except
Exception as exc:" after the existing specific except block or add a final broad
except) and pass the same parameters (interaction, exc, state, candidate_name,
recipient_email, has_contributed, discord_joined, agreement_signed, send,
selected_contact) to ensure deferred interactions are resolved and audit/error
classification runs; apply the same change to the second wrapper that calls
_complete_onboarding_email (the block mirrored around lines ~1017) so both paths
use the generic Exception handler.
Summary
/onboarding-emailcommand that looks up candidate data from CRM, prompts with a selector for multiple matches, and labels/blocks already-onboarded contacts.Great Talking,plus the candidate first name on the next line; default sender signature comes from the command user's CRM first name.#prospective-membersto the Discord channel URL in Markdown/HTML drafts.onboarding@508.devwith command-user Reply-To, plus docs and tests.Validation
uv run pytest tests/unit/test_bot.py tests/unit/test_onboarding_email.py tests/unit/test_onboarding_email_cog.py tests/unit/test_discord_command_metadata.pyuv run ruff check packages/shared/src/five08/onboarding_email.py apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py tests/unit/test_onboarding_email.py tests/unit/test_onboarding_email_cog.py apps/discord_bot/src/five08/discord_bot/config.py tests/unit/test_bot.pyuv run ruff format --check packages/shared/src/five08/onboarding_email.py apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py tests/unit/test_onboarding_email.py tests/unit/test_onboarding_email_cog.py apps/discord_bot/src/five08/discord_bot/config.py tests/unit/test_bot.pyuv run mypy apps/discord_bot/src/five08/discord_bot/config.py apps/discord_bot/src/five08/discord_bot/cogs/onboarding_email.py packages/shared/src/five08/onboarding_email.pySummary by CodeRabbit
New Features
Documentation
Tests