Set all mail attributes including template #70
Conversation
WalkthroughUpdates Mailtrap::Mail.from_message to extract template_uuid and template_variables from headers and adjust address extraction to accept header objects. Adds specs for template fields, bumps version to 2.4.1, and updates the changelog accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant MailMessage as Mail::Message
participant Mail as Mailtrap::Mail
participant Base as Mailtrap::Mail::Base
App->>Mail: from_message(MailMessage)
activate Mail
Mail->>MailMessage: read headers: from, to, cc, bcc
Mail->>Mail: prepare_addresses(from_header)
Mail->>Mail: prepare_addresses(to_header)
Mail->>Mail: prepare_addresses(cc_header)
Mail->>Mail: prepare_addresses(bcc_header)
Mail->>MailMessage: read headers: template_uuid, template_variables
Mail->>Base: new(...addresses..., template_uuid, template_variables)
deactivate Mail
Base-->>App: Mail object
note over Mail,Base: prepare_addresses now accepts header objects
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CHANGELOG.md (1)
1-3: Changelog entry looks good; consider linking to the PR and clarifying Action Mailer context.Optional polish:
- Add a link to PR #70 next to the bullet so readers can trace discussion.
- Wording tweak for consistency with prior entries that start with a verb (“Add/Set/Improve” is fine) and maybe call out “via Action Mailer headers” explicitly.
Example:
- Set
template_uuidandtemplate_variableswhen building mail fromMail::Message(Action Mailer), see #70.lib/mailtrap/mail.rb (2)
192-207: Handle hyphenated header names and avoid header echo for template fields.Great addition exposing
template_uuid/template_variablesfromMail::Message. Two improvements to make this more robust:
- Action Mailer/custom headers are typically hyphenated (e.g.,
Template-UUID,Template-Variables,Custom-Variables). The current lookup uses underscored names, which may miss standard hyphenated headers. Support both to be resilient.Apply this minimal diff inside the changed block:
- custom_variables: message['custom_variables']&.unparsed_value, - template_uuid: message['template_uuid']&.unparsed_value, - template_variables: message['template_variables']&.unparsed_value + custom_variables: (message['Custom-Variables'] || message['custom_variables'])&.unparsed_value, + template_uuid: (message['Template-UUID'] || message['template_uuid'])&.unparsed_value, + template_variables: (message['Template-Variables'] || message['template_variables'])&.unparsed_value
- Since you now extract these into dedicated attributes, consider excluding
Template-UUIDandTemplate-Variablesfrommail.headersto prevent them from being forwarded as arbitrary headers. This mirrors howcategory/custom variablesare handled. You can extend the removal list (outside this hunk), for example:# Add alongside other SPECIAL_HEADERS: # templateuuid and templatevariables use normalized, hyphen-less names SPECIAL_HEADERS = %w[from to cc bcc subject category customvariables contenttype templateuuid templatevariables].freezeOptionally, if callers may pass JSON strings for variables, you might parse JSON when the header value is a String; otherwise leave as-is (happy to draft a helper if you want).
Would you like me to open a follow-up PR/commit that (a) adds those two tokens to SPECIAL_HEADERS and (b) adds specs asserting that template headers are not present in
mail.headers?
224-227: Switch to header-driven address parsing LGTM.Accepting a
Mail::Fieldand delegating toaddress_list(header)&.addressesis cleaner and centralizes error handling. This also preserves the prior behavior for empty headers ([]→nilforfrom.first) and keeps invalid headers raising viaaddress_list.Two tiny nits (optional):
- Update the YARD doc for the return type to reflect an Array of Hashes, e.g.,
@return [Array<Hash>].- Consider renaming param to
address_headerfor clarity.spec/mailtrap/mail_spec.rb (1)
146-156: Good coverage for template fields; add checks for hyphenated headers and header removal.To lock in the intended behavior and the suggested robustness from the implementation comment:
- Add a spec where
Template-UUID/Template-Variablesare set via hyphenated names to ensure they’re recognized.- Assert that
mail.headersdoes not includeTemplate-UUID/Template-Variablesonce extracted.Example additions (outside this hunk):
context 'when template headers are hyphenated' do let(:message_params) do super().merge( 'Template-UUID' => '6b9c8a83-8d75-4b7c-9e9d-9b1db88c0e2d', 'Template-Variables' => { first_name: 'Jane' } ) end its(:template_uuid) { is_expected.to eq('6b9c8a83-8d75-4b7c-9e9d-9b1db88c0e2d') } its(:template_variables) { is_expected.to eq('first_name' => 'Jane') } specify 'template headers are not forwarded' do expect(mail.headers.keys).not_to include('Template-UUID', 'Template-Variables') end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)lib/mailtrap/mail.rb(2 hunks)lib/mailtrap/version.rb(1 hunks)spec/mailtrap/mail_spec.rb(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/mailtrap/mail.rb (1)
lib/mailtrap/mail/base.rb (1)
attachments(71-73)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~1-~1: There might be a mistake here.
Context: ## [2.4.1] - 2025-08-21 - Set template_uuid and `template_variab...
(QB_NEW_EN)
🔇 Additional comments (1)
lib/mailtrap/version.rb (1)
4-4: Version bump consistent with PR scope.Matches CHANGELOG entry for 2.4.1 dated 2025-08-21. Please ensure the gemspec (if present) and any docs/snippets referencing the version are updated in the release commit.
If you want, I can generate a quick script to grep for stale version strings across the repo.
Motivation
Provide a possibility to use templates from action mailer. This approach is questionable, but let's give our users a "sharp knife" and let them decide how to use it.
Changes
template_uuidandtemplate_variablesfrom messageHow to test
Images and GIFs
N/A
Summary by CodeRabbit
New Features
Tests
Documentation
Chores