-
-
Notifications
You must be signed in to change notification settings - Fork 35
feat!: Email support using lettre #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat!: Email support using lettre #419
Conversation
Initial commit of email.rs using lettre Initial commit of Add email support using lettre crate Added the message printout if debug mode is enabled Added a test and deferred the extra headers and attachment features. Added tests for config and send_email(ignore).
…/cot into feat-add-email-support
…ded to improve test coverage.
| [resolver] | ||
| incompatible-rust-versions = "fallback" | ||
|
|
||
| [target.x86_64-pc-windows-msvc] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default linker on windows fails when compiling with chumsky crate which is used by lettre. A workaround is to use the llvm linker and set the symbol mangling version to v0. Also need to use [target.x86_64-pc-windows-msvc] specifically as [target.'cfg(target_os = "windows")'] fails in the CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is definitely a sort of error I totally wouldn't expect! Can we add a # TODO comment to remove this part of the config when the upstream bug is fixed? The links to the bug reports should be included in TODO.
This is sort of unfortunate, though, as it will most likely prevent Cot from being built on Windows with the email feature enabled, unless users add similar config to their project (I see you added the config to the default project template - good!). Hopefully this gets resolved soon!
| type Error = EmailMessageError; | ||
|
|
||
| fn try_from(message: EmailMessage) -> Result<Self, Self::Error> { | ||
| let from_mailbox = message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would have loved to add a server_email in the config to use here as a default if not specified for convenience, but there's no nice way to do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive email support to the Cot framework using the lettre library, providing both console (development) and SMTP (production) transport backends for sending emails.
Key Changes:
- Implements high-level
EmailAPI withEmailMessagebuilder pattern for constructing and sending emails - Adds console and SMTP transport backends with pluggable architecture via
Transporttrait - Integrates email service into the request context and project configuration system
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
examples/send-email/ |
Complete example application demonstrating email functionality with HTML form and status feedback |
cot/src/email/ |
Core email module with message building, transport abstraction, and backend implementations |
cot/src/config.rs |
Email configuration structures supporting console and SMTP transports with URL-based config |
cot/src/request.rs |
Request extension trait additions to access email service |
cot/src/project.rs |
Email service initialization and integration into project bootstrap lifecycle |
cot/Cargo.toml |
New email feature flag and lettre dependency |
deny.toml |
Apache-2.0 WITH LLVM-exception license allowance for new dependencies |
cot-cli/src/project_template/ |
Default email configuration in project templates |
.github/workflows/rust.yml |
CI toolchain version updates |
.cargo/config.toml |
Windows linker configuration for faster builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
m4tx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a really nice piece of work! I'll need to have another look at the actual email logic in the PR, but this overall looks decent. Please make sure to have a look at the Copilot review as well - it has found a few nitpicks.
| [resolver] | ||
| incompatible-rust-versions = "fallback" | ||
|
|
||
| [target.x86_64-pc-windows-msvc] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is definitely a sort of error I totally wouldn't expect! Can we add a # TODO comment to remove this part of the config when the upstream bug is fixed? The links to the bug reports should be included in TODO.
This is sort of unfortunate, though, as it will most likely prevent Cot from being built on Windows with the email feature enabled, unless users add similar config to their project (I see you added the config to the default project template - good!). Hopefully this gets resolved soon!
This makes the API more ergonomic, as `Arc` should really just be an implementation detail, rather than something exposed to the user. This is a followup to the discussion here: #419 (comment)
This is a continuation of #250