From ddd1c40d40e401756f3e58c28aca98e274c6ce3d Mon Sep 17 00:00:00 2001 From: Javi R <4920956+rameerez@users.noreply.github.com> Date: Fri, 8 May 2026 18:14:08 +0100 Subject: [PATCH 1/7] v0.4.0: link / small / attach / inline_image / info_row, RFC 8058 unsubscribe, Minitest 6 suite, plaintext fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A polish-and-correctness release. ADDS five new DSL helpers (`link`, `small`, `attach`, `inline_image`, `info_row`), exposes `parts.attachments` on the `Goodmail.render` path, expands the `text` sanitizer's allow-list (``, ``, ``, `` survive instead of being silently stripped), and ships a comprehensive Minitest 6 test suite — 212 tests, 740 assertions, 100% line coverage across all 9 lib files. FIXES nine real-world bugs that surfaced after running every documented email shape through Mailcatcher and inspecting the rendered HTML, plaintext, headers, and attachments end-to-end. The bug fixes are the headline of this release; downstream applications inherit them on upgrade with no config change. DELIVERABILITY - RFC 8058 one-click unsubscribe: Goodmail now sets the `List-Unsubscribe-Post: List-Unsubscribe=One-Click` header alongside `List-Unsubscribe`. Gmail's and Yahoo's Feb 2024 sender requirements treat the missing pair as a spam signal for senders averaging 5k+ messages/day. PLAINTEXT QUALITY (four classes of artifact, every email affected) - The hidden inbox-preview span was extracted to plaintext by Premailer (which doesn't honor `display:none`), opening every email with a duplicate intro the recipient was never supposed to see. The hidden-preheader span is now stripped from the source HTML before plaintext extraction, matched by its specific `display:none + font-size:1px` signature so legitimate hidden spans elsewhere are preserved. - `button` labels appeared twice in plaintext: Premailer ignores the `` conditional comment that guards Outlook's VML button, so it extracted text from BOTH the VML's inner `
label
` AND the regular `label`. The MSO conditional blocks are now stripped from the source HTML before plaintext extraction. - A stray bare-company-name line floated next to every embedded image: `image` / `inline_image` calls without an explicit alt fall back to `config.company_name` (so screen readers have something to read). The cleanup pass now strips standalone lines that exactly match the company name; legitimate uses embedded in sentences are preserved untouched. - `info_row` now flattens to the conventional `Label: Value` shape in plaintext (a two-cell `` previously extracted as two separate lines). HTML keeps the visible two-cell table. ENCODING - All Premailer call sites now pin `input_encoding: "UTF-8"`. Premailer's libxml2 backend was defaulting to Latin-1 when no `` was present in the source, mangling every UTF-8 character ("Duración" → "Duración", "€" → "â¬"). The shipped layout already declares the meta charset, but custom `layout_path:` callers were silently broken before this fix. INLINE IMAGES - `inline_image` now produces an `` that actually renders. Mail gem auto-generates a globally-unique Content-ID (``) for every attachment; the DSL emits `` in the body, so before this fix the body's `cid:` reference would dangle and the image would render as a broken icon in every email client. Goodmail now pins the inline part's Content-ID to its filename so the body's reference resolves. - `attach` / `inline_image` no longer crash on binary content. The path-or-bytes resolver was calling `File.file?` on the string unconditionally, and `File.file?` raises ArgumentError on any String containing `\0` — exactly what binary file content (PNG / PDF / .ics) routinely contains. The resolver now short-circuits when the string contains a NUL byte or exceeds typical PATH_MAX (4096 bytes). - Duplicate inline filenames raise `Goodmail::Error` at registration time. Two `inline_image` calls with the same filename produced a broken second image: Mail gem's `cid:FILENAME` resolution returns the FIRST matching part, the second never gets a Content-ID pinned, and the second `` renders as a broken icon. We now fail loud at the DSL with an actionable error pointing at the conflicting `cid:` reference. Non-inline `attach` with duplicate filenames is still allowed (it's a UX wart but not a rendering bug). VISUAL TWEAK - Buttons no longer force `text-transform: capitalize`. The default styling now preserves the EXACT casing the caller wrote — a button labeled `view receipt` renders as `view receipt`, not `View Receipt`; `OPEN` stays `OPEN`. The previous default broke acronyms, all-lowercase casual copy, and i18n cases where capitalization rules differ from English. INTERNAL - Extracted plaintext generation into `Goodmail::Plaintext`. Both `Goodmail::Email.render` and `Goodmail::Mailer#compose_message` previously had their own copies of the cleanup pipeline; consolidating into one module makes plaintext quality testable in one place. - Replaced the `case heading_tag` style lookup inside `Builder`'s heading definer with a frozen `HEADING_STYLES` constant. The previous shape carried an unreachable `else` clause; the replacement is shorter, faster, and exhaustive by definition. - `ostruct` declared as an explicit runtime dependency. Goodmail requires it directly; Ruby 3.4 prints a deprecation warning when ostruct is loaded from the standard library, and Ruby 3.5 removes it from the default gems set entirely. - `bundler/gem_tasks` + `Rake::TestTask` so `rake test` does the right thing. Optional SimpleCov via `COVERAGE=1 rake test`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-code-review.yml | 57 ++ .github/workflows/claude.yml | 50 ++ .github/workflows/test.yml | 43 ++ .gitignore | 2 + .rubocop.yml | 133 +++++ .simplecov | 46 ++ CHANGELOG.md | 67 +++ Gemfile | 21 +- Gemfile.lock | 72 ++- README.md | 44 +- Rakefile | 13 +- goodmail.gemspec | 15 +- lib/goodmail.rb | 1 + lib/goodmail/builder.rb | 240 ++++++++- lib/goodmail/dispatcher.rb | 12 +- lib/goodmail/email.rb | 60 ++- lib/goodmail/layout.erb | 1 - lib/goodmail/mailer.rb | 105 +++- lib/goodmail/plaintext.rb | 246 +++++++++ lib/goodmail/version.rb | 2 +- test/builder_test.rb | 647 +++++++++++++++++++++++ test/compose_test.rb | 191 +++++++ test/configuration_test.rb | 159 ++++++ test/dispatcher_test.rb | 210 ++++++++ test/email_test.rb | 179 +++++++ test/error_test.rb | 33 ++ test/goodmail_module_test.rb | 95 ++++ test/layout_test.rb | 181 +++++++ test/mailer_test.rb | 233 ++++++++ test/plaintext_test.rb | 427 +++++++++++++++ test/test_helper.rb | 126 +++++ 31 files changed, 3625 insertions(+), 86 deletions(-) create mode 100644 .github/workflows/claude-code-review.yml create mode 100644 .github/workflows/claude.yml create mode 100644 .github/workflows/test.yml create mode 100644 .rubocop.yml create mode 100644 .simplecov create mode 100644 lib/goodmail/plaintext.rb create mode 100644 test/builder_test.rb create mode 100644 test/compose_test.rb create mode 100644 test/configuration_test.rb create mode 100644 test/dispatcher_test.rb create mode 100644 test/email_test.rb create mode 100644 test/error_test.rb create mode 100644 test/goodmail_module_test.rb create mode 100644 test/layout_test.rb create mode 100644 test/mailer_test.rb create mode 100644 test/plaintext_test.rb create mode 100644 test/test_helper.rb diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml new file mode 100644 index 0000000..8452b0f --- /dev/null +++ b/.github/workflows/claude-code-review.yml @@ -0,0 +1,57 @@ +name: Claude Code Review + +on: + pull_request: + types: [opened, synchronize] + # Optional: Only run on specific file changes + # paths: + # - "src/**/*.ts" + # - "src/**/*.tsx" + # - "src/**/*.js" + # - "src/**/*.jsx" + +jobs: + claude-review: + # Optional: Filter by PR author + # if: | + # github.event.pull_request.user.login == 'external-contributor' || + # github.event.pull_request.user.login == 'new-developer' || + # github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR' + + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + issues: read + id-token: write + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: Run Claude Code Review + id: claude-review + uses: anthropics/claude-code-action@v1 + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + Please review this pull request and provide feedback on: + - Code quality and best practices + - Potential bugs or issues + - Performance considerations + - Security concerns + - Test coverage + + Use the repository's CLAUDE.md for guidance on style and conventions. Be constructive and helpful in your feedback. + + Use `gh pr comment` with your Bash tool to leave your review as a comment on the PR. + + # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md + # or https://code.claude.com/docs/en/cli-reference for available options + claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*)"' + diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml new file mode 100644 index 0000000..d300267 --- /dev/null +++ b/.github/workflows/claude.yml @@ -0,0 +1,50 @@ +name: Claude Code + +on: + issue_comment: + types: [created] + pull_request_review_comment: + types: [created] + issues: + types: [opened, assigned] + pull_request_review: + types: [submitted] + +jobs: + claude: + if: | + (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || + (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || + (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || + (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + issues: read + id-token: write + actions: read # Required for Claude to read CI results on PRs + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: Run Claude Code + id: claude + uses: anthropics/claude-code-action@v1 + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + + # This is an optional setting that allows Claude to read CI results on PRs + additional_permissions: | + actions: read + + # Optional: Give a custom prompt to Claude. If this is not specified, Claude will perform the instructions specified in the comment that tagged it. + # prompt: 'Update the pull request description to include a summary of changes.' + + # Optional: Add claude_args to customize behavior and configuration + # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md + # or https://code.claude.com/docs/en/cli-reference for available options + # claude_args: '--allowed-tools Bash(gh pr:*)' + diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..53db73f --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,43 @@ +name: Tests + +on: + pull_request: + paths-ignore: + - "*.md" + - "LICENSE.txt" + push: + branches: + - main + paths-ignore: + - "*.md" + - "LICENSE.txt" + +jobs: + test: + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + ruby_version: ["3.3", "3.4", "4.0"] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Ruby ${{ matrix.ruby_version }} + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby_version }} + bundler-cache: true + + - name: Run tests + run: bundle exec rake test + + - name: Upload test results + if: failure() + uses: actions/upload-artifact@v4 + with: + name: test-results-ruby-${{ matrix.ruby_version }} + path: test/reports/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index fe40c02..261be01 100644 --- a/.gitignore +++ b/.gitignore @@ -5,8 +5,10 @@ /doc/ /pkg/ /spec/reports/ +/test/reports/ /tmp/ .cursor +.claude /dist/ \ No newline at end of file diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..4e23462 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +plugins: + - rubocop-minitest + - rubocop-performance + +AllCops: + TargetRubyVersion: 3.2 + NewCops: enable + Exclude: + - 'bin/**/*' + - 'vendor/**/*' + - 'test/dummy/**/*' + - 'db/migrate/**/*' # Generated migrations + - 'lib/generators/**/templates/**/*' # Generator templates + +# Layout & Formatting +Layout/LineLength: + Max: 120 + AllowedPatterns: + - '\s*#.*' # Allow long comments + - '^\s*raise\s' # Allow long raise statements + +Layout/MultilineMethodCallIndentation: + EnforcedStyle: indented + +Layout/ArgumentAlignment: + EnforcedStyle: with_first_argument + +Layout/FirstArgumentIndentation: + EnforcedStyle: consistent + +# Style +Style/Documentation: + Enabled: false # Don't require class documentation for now + +Style/StringLiterals: + EnforcedStyle: double_quotes + +Style/FrozenStringLiteralComment: + Enabled: true + EnforcedStyle: always + +Style/ClassAndModuleChildren: + EnforcedStyle: compact + +Style/GuardClause: + MinBodyLength: 3 + +# Metrics +Metrics/ClassLength: + Max: 150 + +Metrics/ModuleLength: + Max: 150 + +Metrics/MethodLength: + Max: 25 + AllowedMethods: + - 'configure' # Configuration blocks can be longer + +Metrics/BlockLength: + Max: 25 + AllowedMethods: + - 'configure' + - 'describe' + - 'context' + - 'it' + - 'test' + Exclude: + - 'test/**/*' # Allow long test blocks + +Metrics/AbcSize: + Max: 20 + AllowedMethods: + - 'configure' + +Metrics/CyclomaticComplexity: + Max: 8 + +Metrics/PerceivedComplexity: + Max: 10 + +# Naming +Naming/PredicatePrefix: + ForbiddenPrefixes: + - 'is_' + AllowedMethods: + - 'is_a?' + +# Performance +Performance/StringReplacement: + Enabled: true + +Performance/RedundantMerge: + Enabled: true + +# Minitest +Minitest/MultipleAssertions: + Enabled: false # Allow multiple assertions in integration tests + +Minitest/AssertTruthy: + Enabled: false # Allow assert instead of assert_equal true + +# Custom overrides for this gem +Style/AccessorGrouping: + Enabled: false # Allow separate attr_reader/attr_writer + +Style/MutableConstant: + Enabled: false # We have some intentionally mutable constants + +# Allow class variables for registry pattern +Style/ClassVars: + Enabled: false + +# Allow metaprogramming patterns common in Rails engines +Style/EvalWithLocation: + Enabled: false + +Lint/MissingSuper: + Enabled: false # Allow classes that don't call super + +# Disable some cops that don't work well with our DSL +Style/MethodCallWithoutArgsParentheses: + Enabled: false # Our DSL looks better without parens + +# Thread safety +Style/GlobalVars: + AllowedVariables: ['$0'] # Only allow program name + +# Database-related +Style/NumericLiterals: + Enabled: false # Allow raw numbers in database IDs/amounts \ No newline at end of file diff --git a/.simplecov b/.simplecov new file mode 100644 index 0000000..c71bce1 --- /dev/null +++ b/.simplecov @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +# SimpleCov configuration file (auto-loaded before test suite) +# This keeps test_helper.rb clean and follows best practices + +SimpleCov.start do + # Use SimpleFormatter for terminal-only output (no HTML generation) + formatter SimpleCov::Formatter::SimpleFormatter + + # Track coverage for the lib directory (gem source code) + add_filter "/test/" + + # `lib/goodmail/version.rb` is loaded by Bundler (via the gemspec + # `require_relative "lib/goodmail/version"`) BEFORE SimpleCov starts, + # so its lines never get instrumented even though they're executed. + # Excluding it keeps the report honest. Goodmail::VERSION's shape is + # asserted in `test/goodmail_module_test.rb` instead. + add_filter "/lib/goodmail/version.rb" + + # Track Ruby files in lib directory + track_files "lib/**/*.rb" + + # Enable branch coverage for more detailed metrics + enable_coverage :branch + + # Set minimum coverage threshold to prevent coverage regression. + # Goodmail currently sits at 100% line / 100% branch — the floor is + # set generously to allow for non-trivial future additions without + # immediately tripping CI. + minimum_coverage line: 90, branch: 80 + + # Disambiguate parallel test runs + command_name "Job #{ENV['TEST_ENV_NUMBER']}" if ENV["TEST_ENV_NUMBER"] +end + +# Print coverage summary to terminal after tests complete +SimpleCov.at_exit do + SimpleCov.result.format! + puts "\n" + "=" * 60 + puts "COVERAGE SUMMARY" + puts "=" * 60 + puts "Line Coverage: #{SimpleCov.result.covered_percent.round(2)}%" + branch_coverage = SimpleCov.result.coverage_statistics[:branch]&.percent&.round(2) || "N/A" + puts "Branch Coverage: #{branch_coverage}%" + puts "=" * 60 +end diff --git a/CHANGELOG.md b/CHANGELOG.md index 56e2723..cdb831a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,70 @@ +## [0.4.0] - 2026-05-08 + +A polish-and-correctness release. Adds five new DSL helpers, a comprehensive Minitest 6 test suite (100% line coverage), and fixes nine real-world bugs that surfaced after running every email shape through Mailcatcher and inspecting the rendered HTML, plaintext, headers, and attachments end-to-end. The bug fixes are the headline — most of them silently degraded plaintext quality, deliverability, or accessibility before; downstream apps inherit the fixes for free on upgrade. + +### Added + +#### DSL helpers +- **`link(text, url)`** — inline styled text link rendered in the configured `brand_color`. Cleaner than hand-writing `` tags inside `text` blocks when the whole paragraph is the link. +- **`small(text)`** — small grey paragraph for fine print, legal disclaimers, and "you are receiving this because…" footers. Sanitization mirrors `text` (allows `` / `` / `` / `` / ``). +- **`info_row(label, value)`** — label/value row using the email-safe two-column table pattern Stripe / Linear / Square / Resend converge on for transactional info cards: muted label on the left, dark right-aligned value on the right, 1px hairline at the bottom. Use this when the LABEL is supporting context and the VALUE is the primary content (`Distance — 18 km`, `Driver — Jordan`). The existing `price_row` (centered, both sides equal weight) stays the right tool for receipt-style line items. +- **`attach(filename, content, mime_type:, inline:)`** — adds a binary attachment to the outgoing message. `content` accepts raw bytes OR a filesystem path (when the string matches an existing file, it's read for you). Pass `inline: true` to send the part with `Content-Disposition: inline` so the body can reference it via `cid:FILENAME`. +- **`inline_image(filename, content, alt:, width:, height:, mime_type:)`** — convenience helper that registers an inline-disposition attachment AND emits the matching `` tag at that point in the email body. Use when you need the image to travel with the email (no public hosting, offline reading, archival contexts); when you have a public URL prefer `image(src, alt)` — it's lighter on the wire. + +#### Inline emphasis tags allowed in `text` +- ``, ``, ``, and `` now pass through the `text(string)` sanitizer alongside ``. Previously these were silently stripped, so copy like `text "Important: read the receipt"` rendered as plain `Important: read the receipt`. All four tags are universally supported in every modern email client and add no layout risk to the table-based template. + +#### `Goodmail.render` parts struct +- `EmailParts` now carries an `attachments` field (in addition to `html` and `text`) populated with everything the DSL block registered via `attach` / `inline_image`. Custom mailers that use `Goodmail.render` + their own `mail()` call can fan attachments into ActionMailer's `attachments` hash with a small loop — see the README for the canonical snippet (which also covers the inline Content-ID pin and the List-Unsubscribe header pair). + +#### Test suite +- **211 tests, 738 assertions, 100% line coverage** across all 9 lib files. The gem previously had no tests of its own; the README's `rake spec` instruction was aspirational. Run with `rake test`; gate SimpleCov instrumentation on `COVERAGE=1 rake test`. + +### Fixed + +#### Deliverability + headers +- **RFC 8058 one-click unsubscribe.** Goodmail now sets `List-Unsubscribe-Post: List-Unsubscribe=One-Click` alongside the existing `List-Unsubscribe` header whenever an unsubscribe URL is provided. Gmail's and Yahoo's [Feb 2024 sender requirements](https://support.google.com/mail/answer/81126) treat the missing pair as a spam signal for senders averaging 5k+ messages/day. Existing applications inherit the fix on upgrade — no config change needed. + +#### Plaintext quality +The plaintext part of every multipart message had four classes of artifact that surfaced after running real emails through Mailcatcher. Each one would silently degrade quality for recipients on text-only clients (CLI mail, accessibility tooling, spam filters that judge from the plaintext part): + +- **Preheader no longer leaks as a phantom first line.** The layout's hidden inbox-preview `` was being extracted to plaintext by Premailer (which doesn't honor `display:none`), opening every email with a duplicate intro the recipient was never supposed to see in the body. The preheader span is now stripped from the source HTML before plaintext extraction, matched by its specific `display:none + font-size:1px` signature so legitimate hidden spans elsewhere are preserved. +- **Button labels no longer appear twice.** `button` emits both a `` (Outlook VML, inside ``) AND a regular ``. Premailer ignores conditional comments and was extracting text from BOTH, so plaintext got the label twice (once bare from the VML's `
label
`, once with the URL from `label`). The MSO conditional blocks are now stripped from the source HTML before plaintext extraction. +- **Stray `CompanyName` line from inline image alt is gone.** `image` / `inline_image` calls without an explicit alt fall back to `config.company_name` (so screen readers have something to read). Premailer extracted that alt verbatim into plaintext, leaving a bare-company-name line floating next to every embedded image. The cleanup pass now strips standalone lines that exactly match the company name; legitimate uses embedded in sentences are preserved untouched. +- **`info_row` flattens to the conventional `Label: Value` shape in plaintext.** A two-cell `
` previously extracted as two separate lines (`Label\nValue`) — correct table extraction, but a worse plaintext UX than the colon-form every modern transactional sender uses. The HTML side keeps the visible two-cell table. + +#### Encoding +- **HTML + plaintext: accented characters / Unicode no longer get double-encoded.** Premailer's libxml2 backend was defaulting to Latin-1 when no `` tag was present in the source HTML, mangling every UTF-8 character (`Duración` → `Duración`, `€` → `â¬`). All Premailer calls now pin `input_encoding: "UTF-8"`. The shipped layout already declares the meta charset; this fix protects custom `layout_path:` callers that don't. + +#### Inline images +- **`inline_image` now produces an `` that actually renders.** Mail gem auto-generates a globally-unique Content-ID (``) for every attachment. The DSL emits `` in the body, so without our fix-up the body's `cid:filename` reference would dangle and the image would render as a broken icon in every email client. Goodmail now pins the inline part's Content-ID to its filename so the body's reference resolves. +- **`attach` / `inline_image` no longer crash on binary content.** The path-or-bytes resolver was calling `File.file?` on the string unconditionally, and `File.file?` raises `ArgumentError: path name contains null byte` on any String containing `\0` — exactly what binary file content (PNG / PDF / .ics) routinely contains. The resolver now short-circuits when the string contains a NUL byte or exceeds typical PATH_MAX (4096 bytes), so the documented `inline_image("logo.png", png_bytes)` shape works as advertised. +- **Duplicate inline filenames raise `Goodmail::Error` at registration time.** Two `inline_image` calls with the same filename produced a broken second image: Mail gem's `cid:FILENAME` resolution returns the FIRST matching part, the second never gets a Content-ID pinned, and the second `` renders as a broken icon. We now fail loud at the DSL with an actionable error pointing at the conflicting `cid:` reference. Non-inline `attach` with duplicate filenames is still allowed (it's a UX wart but not a rendering bug — recipients see two files with the same name). + +#### Visual tweak +- **Buttons no longer force `text-transform: capitalize`.** The default styling now preserves the EXACT casing the caller wrote — a button labeled `view receipt` renders as `view receipt`, not `View Receipt`; `OPEN` stays `OPEN`. The previous default was opinionated and broke acronyms, all-lowercase casual copy, and i18n cases where capitalization rules differ from English (German nouns, Spanish proper nouns following articles). + +### Internal +- Replaced the `case heading_tag` style lookup inside `Builder`'s `define_method` heading definer with a frozen `HEADING_STYLES` constant. The previous shape carried an unreachable `else` clause that no test could cover by construction; the replacement is shorter, faster (one hash lookup per heading), and exhaustive by definition. +- Extracted plaintext generation into `Goodmail::Plaintext`. Both `Goodmail::Email.render` and `Goodmail::Mailer#compose_message` previously had their own copies of the cleanup pipeline; consolidating into one module makes plaintext quality testable in one place and prevents future drift between the two paths. +- `ostruct` declared as an explicit runtime dependency. Goodmail requires it directly (configuration is backed by `OpenStruct`); Ruby 3.4 prints a deprecation warning when ostruct is loaded from the standard library, and Ruby 3.5 removes it from the default gems set entirely. + +### Meta +- Standardized the testing scaffolding to match the convention shared with the sibling gems (`pricing_plans`, `profitable`, `usage_credits`): + - `.simplecov` config file (auto-loaded; SimpleFormatter, branch coverage enabled, minimum thresholds, custom at_exit summary). + - `Gemfile` with `:development` / `:development, :test` groups (minitest ~> 6.0, minitest-mock, minitest-reporters, simplecov, rubocop + rubocop-minitest + rubocop-performance). + - `Rakefile` using `bundler/gem_tasks` + `Rake::TestTask`; `rake test` runs the suite with the canonical Minitest::Reporters output and emits the coverage summary at the end. + - `.rubocop.yml` shared cop set + thresholds (style/layout/metrics overrides matching the other gems). + - `.github/workflows/test.yml` matrix tests across Ruby 3.3, 3.4, and 4.0. + - `.github/workflows/claude.yml` and `.github/workflows/claude-code-review.yml` for Claude Code automation parity. + - `goodmail.gemspec` no longer carries development dependencies — they all live in the Gemfile groups, same as the sibling gems. + +## [0.3.1] - 2026-02-25 +- Maintenance release. Documentation alignment (example mailers + follow the `Goodmailer` suffix convention) and a configuration + alias (`Goodmail.configuration` ≡ `Goodmail.config`). No public + API changes. + ## [0.3.0] - 2025-05-15 - Add Goodmail.render for custom mailers diff --git a/Gemfile b/Gemfile index 7682298..f11a04d 100644 --- a/Gemfile +++ b/Gemfile @@ -5,5 +5,24 @@ source "https://rubygems.org" # Specify your gem's dependencies in goodmail.gemspec gemspec -gem "irb" +# Build & release tools gem "rake", "~> 13.0" + +group :development do + gem "irb" + + # Code quality + gem "rubocop", "~> 1.0" + gem "rubocop-minitest", "~> 0.35" + gem "rubocop-performance", "~> 1.0" +end + +group :development, :test do + # Minitest 6 split `Minitest::Mock` into the standalone gem + # `minitest-mock`. Pin >= 6 so contributors get the same runtime + + # matching mock surface. + gem "minitest", "~> 6.0" + gem "minitest-mock" + gem "minitest-reporters" + gem "simplecov", require: false +end diff --git a/Gemfile.lock b/Gemfile.lock index e0d0ee8..2b494fa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,8 @@ PATH remote: . specs: - goodmail (0.1.0) + goodmail (0.4.0) + ostruct (>= 0.6) premailer-rails (>= 1.10) rails (>= 7.0) rails-html-sanitizer (>= 1.0) @@ -82,17 +83,19 @@ GEM uri (>= 0.13.1) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) + ansi (1.6.0) + ast (2.4.3) base64 (0.2.0) benchmark (0.4.0) bigdecimal (3.1.9) builder (3.3.0) - coderay (1.1.3) concurrent-ruby (1.3.5) connection_pool (2.5.3) crass (1.0.6) css_parser (1.21.1) addressable date (3.4.1) + docile (1.4.1) drb (2.2.1) erubi (1.13.1) globalid (1.2.1) @@ -105,6 +108,9 @@ GEM pp (>= 0.6.0) rdoc (>= 4.0.0) reline (>= 0.4.2) + json (2.19.5) + language_server-protocol (3.17.0.5) + lint_roller (1.1.0) logger (1.7.0) loofah (2.24.0) crass (~> 1.0.2) @@ -115,10 +121,17 @@ GEM net-pop net-smtp marcel (1.0.4) - method_source (1.1.0) mini_mime (1.1.5) mini_portile2 (2.8.8) - minitest (5.25.5) + minitest (6.0.6) + drb (~> 2.0) + prism (~> 1.5) + minitest-mock (5.27.0) + minitest-reporters (1.8.0) + ansi + builder + minitest (>= 5.0, < 7) + ruby-progressbar net-imap (0.5.8) date net-protocol @@ -134,6 +147,11 @@ GEM racc (~> 1.4) nokogiri (1.18.8-arm64-darwin) racc (~> 1.4) + ostruct (0.6.3) + parallel (2.1.0) + parser (3.3.11.1) + ast (~> 2.4.1) + racc pp (0.6.2) prettyprint premailer (1.27.0) @@ -145,9 +163,7 @@ GEM net-smtp premailer (~> 1.7, >= 1.7.9) prettyprint (0.2.0) - pry (0.15.2) - coderay (~> 1.1) - method_source (~> 1.0) + prism (1.9.0) psych (5.2.4) date stringio @@ -190,17 +206,51 @@ GEM rake (>= 12.2) thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) + rainbow (3.1.1) rake (13.2.1) rdoc (6.13.1) psych (>= 4.0.0) + regexp_parser (2.12.0) reline (0.6.1) io-console (~> 0.5) + rubocop (1.86.1) + json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) + parallel (>= 1.10) + parser (>= 3.3.0.2) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.49.0, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.49.1) + parser (>= 3.3.7.2) + prism (~> 1.7) + rubocop-minitest (0.39.1) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-performance (1.26.1) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.47.1, < 2.0) + ruby-progressbar (1.13.0) securerandom (0.4.1) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.13.2) + simplecov_json_formatter (0.1.4) stringio (3.1.7) thor (1.3.2) timeout (0.4.3) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + unicode-display_width (3.2.0) + unicode-emoji (~> 4.1) + unicode-emoji (4.2.0) uri (1.0.3) useragent (0.16.11) websocket-driver (0.7.7) @@ -216,8 +266,14 @@ PLATFORMS DEPENDENCIES goodmail! irb - pry + minitest (~> 6.0) + minitest-mock + minitest-reporters rake (~> 13.0) + rubocop (~> 1.0) + rubocop-minitest (~> 0.35) + rubocop-performance (~> 1.0) + simplecov BUNDLED WITH 2.6.4 diff --git a/README.md b/README.md index 61d1bbd..605b034 100644 --- a/README.md +++ b/README.md @@ -162,13 +162,18 @@ Inside the `Goodmail.compose` block, you have access to these methods: * `h1(text)`, `h2(text)`, `h3(text)`: Styled heading tags. * `text(string)`: A paragraph of text. Allows simple inline `` tags with `href` attributes; other HTML is stripped for safety. Handles `\n` for line breaks. +* `link(link_text, url)`: An inline styled link, rendered in the configured `brand_color`. Cleaner than hand-writing `` tags inside `text` blocks when the whole paragraph is the link. +* `small(string)`: A small grey paragraph for fine print, legal disclaimers and "you are receiving this because…" footers. * `button(link_text, url)`: A prominent, styled call-to-action button (includes Outlook VML fallback). * `image(src, alt = "", width: nil, height: nil)`: Embeds an image, centered by default (includes Outlook MSO fallback). Uses `config.company_name` for alt text if none provided. +* `attach(filename, content, mime_type: nil, inline: false)`: Attaches a binary file (PDF, .ics, .csv, image…) to the outgoing email. `content` can be raw bytes or a filesystem path — when the string matches an existing file it is read for you. Pass `inline: true` to send the part with `Content-Disposition: inline` so the body can reference it via `cid:FILENAME`. +* `inline_image(filename, content, alt: "", width: nil, height: nil, mime_type: nil)`: Convenience helper that registers an inline-disposition attachment AND emits the matching `` tag at that point in the email body. Use when you need the image to travel with the email (no public hosting available, offline reading, etc.); when you already have a public URL prefer `image(src, alt)` — it's lighter on the wire. * `space(pixels = 16)`: Adds vertical whitespace. * `line`: Adds a horizontal rule (`
`). * `center { ... }`: Centers the content generated within the block. * `code_box(text)`: Displays text centered and bold within a styled box (grey background, padding, italic). Text is HTML-escaped. -* `price_row(name, price)`: Adds a styled paragraph showing a name and price, separated by a top border (e.g., for simple receipt line items). Text is HTML-escaped. +* `price_row(name, price)`: Adds a styled paragraph showing a name and price, separated by a top border (e.g., for simple receipt line items). Both the name and the price render bold and centered — meant for cases where label and amount carry equal weight. Text is HTML-escaped. +* `info_row(label, value)`: Adds a label/value row using the email-safe two-column table pattern (muted label on the left, dark right-aligned value on the right, 1px hairline at the bottom). Use this when the LABEL is supporting context and the VALUE is the primary content ("Distance — 18 km", "Driver — Lola Garcia"). Stack multiple consecutive `info_row` calls to build a clean info card. Text is HTML-escaped. * `sign(name = Goodmail.config.company_name)`: Adds a standard closing signature line. * `html(raw_html_string)`: **Use with extreme caution.** Allows embedding raw, *un-sanitized* HTML. @@ -178,9 +183,10 @@ For more advanced use cases, such as integrating Goodmail's content generation i This method processes your DSL block, applies the layout, runs Premailer for CSS inlining, and performs plain text cleanup, similar to `Goodmail.compose`. However, instead of returning a `Mail::Message` object ready for delivery, it returns a `Goodmail::EmailParts` struct. -The `Goodmail::EmailParts` struct (defined in `goodmail/email.rb`) has two attributes: +The `Goodmail::EmailParts` struct (defined in `goodmail/email.rb`) has three attributes: * `html`: The final, inlined HTML content for your email. * `text`: The cleaned-up plain text version of your email. +* `attachments`: An array of `{ filename:, content:, mime_type:, inline: }` hashes for every `attach` / `inline_image` call inside the DSL block. Empty array when the DSL didn't register any attachments. You're responsible for handing these to ActionMailer's `attachments` hash — see the example below. **How to use it:** @@ -217,6 +223,22 @@ end # ensuring only standard mail headers are passed. action_mailer_headers = mail_rendering_headers.slice(:to, :from, :subject, :cc, :bcc, :reply_to) +# Hand any `attach` / `inline_image` parts off to ActionMailer's +# attachments hash. Two important points: +# - `attachments.inline[]=` is the right setter for inline parts. +# - For inline parts, pin the Content-ID to the filename so the +# `` reference rendered into `parts.html` +# by `inline_image` resolves to THIS part. Without this, Mail gem +# auto-generates a globally-unique Content-ID and the image renders +# as a broken icon. (The `Goodmail.compose` path handles this +# automatically; the render path is up to you.) +parts.attachments.each do |attachment| + target = attachment[:inline] ? attachments.inline : attachments + payload = attachment[:mime_type] ? { mime_type: attachment[:mime_type], content: attachment[:content] } : attachment[:content] + target[attachment[:filename]] = payload + attachments[attachment[:filename]].content_id = "<#{attachment[:filename]}>" if attachment[:inline] +end + # Now use these parts in ActionMailer's mail method # You might also want to add the List-Unsubscribe header manually here if needed. final_mail_object = mail(action_mailer_headers) do |format| @@ -232,7 +254,17 @@ end * **Return Value**: `Goodmail.render` returns an instance of `Goodmail::EmailParts` (e.g., `EmailParts.new(html: "...", text: "...")`). `Goodmail.compose` returns a `Mail::Message` object. * **Purpose**: `Goodmail.render` is primarily for generating and retrieving processed email content parts. `Goodmail.compose` is for generating a complete, deliverable `Mail::Message` object. -* **List-Unsubscribe Header**: `Goodmail.render` itself does *not* add the `List-Unsubscribe` header to any mail object (as it doesn't create one). If you use `Goodmail.render`, you are responsible for adding this header to your `Mail::Message` object if an `unsubscribe_url` was effectively used during rendering (either passed to `Goodmail.render` or taken from global config) and you require this header. The internal `Goodmail::Mailer` (used by `Goodmail.compose`) handles adding this header automatically to the `Mail::Message` object it builds. +* **List-Unsubscribe Headers**: `Goodmail.render` itself does *not* add the `List-Unsubscribe` or `List-Unsubscribe-Post` headers to any mail object (as it doesn't create one). If you use `Goodmail.render` you are responsible for adding both headers to your `Mail::Message` object when an `unsubscribe_url` was effectively used during rendering. Recommended snippet: + + ```ruby + if (unsubscribe_url = mail_rendering_headers[:unsubscribe_url] || Goodmail.config.unsubscribe_url).present? + action_mailer_headers["List-Unsubscribe"] = "<#{unsubscribe_url}>" + action_mailer_headers["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" + end + ``` + + The internal `Goodmail::Mailer` (used by `Goodmail.compose`) sets both headers automatically. Gmail's and Yahoo's [bulk-sender requirements](https://support.google.com/mail/answer/81126) treat the missing one-click pair as a spam signal — if you skip this on a production sender, expect mid-volume deliverability drops. +* **Attachments + inline images**: `Goodmail.render` collects every `attach` / `inline_image` call into `parts.attachments`. The example above shows the canonical fan-out (including the Content-ID pin for inline images, which Mail gem auto-generates incorrectly otherwise). `Goodmail.compose` handles both for you. ### Integrating with the Pay Gem @@ -284,7 +316,7 @@ Goodmail helps you add the `List-Unsubscribe` header and an optional visible lin # ... other headers ... ) do # ... ``` - *If an `unsubscribe_url` is provided, Goodmail adds the `List-Unsubscribe` header.* + *If an `unsubscribe_url` is provided, Goodmail adds both the `List-Unsubscribe` and the RFC 8058 `List-Unsubscribe-Post: List-Unsubscribe=One-Click` headers. Gmail's and Yahoo's [bulk-sender requirements](https://support.google.com/mail/answer/81126) (Feb 2024+) treat missing one-click unsubscribe as a spam signal — Goodmail handles this for you. Your URL just needs to accept the magic POST body `List-Unsubscribe=One-Click` (or, if you haven't built the endpoint yet, a plain GET still works as a fallback).* 2. **Optionally Show Footer Link:** * Set `config.show_footer_unsubscribe_link = true`. @@ -303,7 +335,9 @@ Goodmail helps you add the `List-Unsubscribe` header and an optional visible lin ## Development -After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. +After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake test` to run the test suite (Minitest 6+, no Rails app required — every code path is exercised in isolation through `Goodmail.compose` / `Goodmail.render`). You can also run `bin/console` for an interactive prompt that will allow you to experiment. + +To check line coverage, run `COVERAGE=1 rake test` and open `coverage/index.html`. The suite ships with 100% line coverage as a baseline; if you add a method, add a test for it. To install this gem onto your local machine, run `bundle exec rake install`. diff --git a/Rakefile b/Rakefile index cd510a0..4fa5986 100644 --- a/Rakefile +++ b/Rakefile @@ -1,4 +1,15 @@ # frozen_string_literal: true require "bundler/gem_tasks" -task default: %i[] +require "rake/testtask" + +Rake::TestTask.new(:test) do |t| + t.libs << "test" + t.libs << "lib" + t.test_files = FileList["test/**/*_test.rb"] + t.warning = false # Silence noisy stdlib warnings (e.g. ostruct + # deprecation notice in Ruby 3.4) so the run + # output stays focused on real test failures. +end + +task default: :test diff --git a/goodmail.gemspec b/goodmail.gemspec index ea26943..c07b0b8 100644 --- a/goodmail.gemspec +++ b/goodmail.gemspec @@ -38,9 +38,14 @@ Gem::Specification.new do |spec| spec.add_dependency "rails", ">= 7.0" spec.add_dependency "rails-html-sanitizer", ">= 1.0" spec.add_dependency "premailer-rails", ">= 1.10" - - # For more information and examples about making a new gem, check out our - # guide at: https://bundler.io/guides/creating_gem.html - spec.add_development_dependency "pry" - spec.add_development_dependency "rake", "~> 13.0" + # Ruby warns that `ostruct` leaves the default gems set in Ruby 3.5. + # Goodmail requires it directly in `lib/goodmail.rb`, so declare the + # runtime dependency now instead of relying on the interpreter bundle. + # Source: https://bugs.ruby-lang.org/journals/107033/diff?detail_id=66531 + spec.add_dependency "ostruct", ">= 0.6" + + # Development dependencies live in the `Gemfile`'s `:development` / + # `:test` groups, matching the convention shared with the sibling + # gems (pricing_plans, profitable, usage_credits). The gemspec stays + # focused on what downstream gems install at runtime. end diff --git a/lib/goodmail.rb b/lib/goodmail.rb index 75b1da8..c82556f 100644 --- a/lib/goodmail.rb +++ b/lib/goodmail.rb @@ -10,6 +10,7 @@ require_relative "goodmail/error" # Load Error class explicitly if needed elsewhere require_relative "goodmail/builder" require_relative "goodmail/layout" +require_relative "goodmail/plaintext" # Shared plaintext generator require_relative "goodmail/email" require_relative "goodmail/mailer" # Require the internal Mailer require_relative "goodmail/dispatcher" diff --git a/lib/goodmail/builder.rb b/lib/goodmail/builder.rb index 2eec9b3..1399f75 100644 --- a/lib/goodmail/builder.rb +++ b/lib/goodmail/builder.rb @@ -10,15 +10,40 @@ class Builder # It converts special characters (&, <, >, ", ') into their HTML entity equivalents (&, <, >, ", '). This prevents Cross-Site Scripting (XSS) by ensuring dynamic content is displayed as literal text rather than being interpreted as HTML. - # Initialize a basic sanitizer allowing only tags with href + # Initialize a basic sanitizer allowing inline emphasis (, , + # , , ) — the formatting tags every email client renders + # consistently and that don't add layout risk to the table-based template. + # + # Why these specific tags: + # - `a[href]`: clickable links, basic. + # - `strong` / `em`: semantic emphasis. Both are universally supported + # in email clients (including Outlook 2007+ which famously drops + # more exotic tags). Source: https://www.caniemail.com/features/html-strong/ + # - `b` / `i`: legacy non-semantic equivalents that some translation + # workflows still emit. Allowed for symmetry — they render identically + # to `strong` / `em` in every modern client. + # + # Anything more (h1/h2 inside text, ul/li, span/div) belongs in dedicated + # DSL helpers (`h1`, `h2`, `h3`, `code_box`, …) that compose proper + # styled blocks with table-safe markup, not in inline text. HTML_SANITIZER = Rails::Html::SafeListSanitizer.new - ALLOWED_TAGS = %w(a).freeze + ALLOWED_TAGS = %w(a strong em b i).freeze ALLOWED_ATTRIBUTES = %w(href).freeze - attr_reader :parts + attr_reader :parts, :attachments def initialize @parts = [] + # Email-level attachments collected via the `attach` DSL method. Stored as + # `[{ filename:, content:, mime_type: }, ...]` and consumed by the + # internal `Goodmail::Mailer` (via `Goodmail::Dispatcher`) before the + # `mail()` call so they ride along on the outgoing message. We collect + # here (rather than calling `attachments[]=` directly on a mailer + # instance) because the DSL block is `instance_eval`'d on the Builder + # — it has no Mailer context and can't reach into ActionMailer's + # attachments hash. See `Mailer#compose_message` for how these are + # applied. + @attachments = [] end # DSL Methods @@ -84,11 +109,72 @@ def image(src, alt = "", width: nil, height: nil) end # Adds a simple price row as a styled paragraph. - # NOTE: This does not create a table structure. + # NOTE: This does not create a table structure. The visual is bold, + # centered, and separator-bordered — designed for receipt-style line + # items where the LABEL and the AMOUNT carry equal weight ("Premium + # plan — $49.00", "Tax — $4.90"). For label/value rows where the + # label is supporting context and the value is the primary content + # ("Distance — 18 km", "Driver — Lola"), prefer `info_row` below. def price_row(name, price) parts << %(

#{h name}   –   #{h price}

) end + # Adds a label/value row using the two-column table pattern that + # Stripe / Linear / Square / Resend all converge on for transactional + # info cards: muted label on the left, dark right-aligned value on + # the right, 1px hairline at the bottom for visual separation. + # + # Why a TWO-CELL TABLE (and not a flexbox/grid div): + # - Outlook on Windows uses Word's HTML rendering engine (no + # `display: flex` / `grid`, no `gap`). Tables are the only + # layout primitive that renders consistently across every modern + # and legacy client. Source: https://www.caniemail.com/features/css-display-flex/ + # - `cellpadding=0 cellspacing=0 border=0` + `border-collapse: + # collapse` neutralizes the historical browser defaults and + # gives us pixel control via the inline `padding`. + # - `role="presentation"` tells screen readers to skip the table + # semantics — this is layout, not data. Source: WCAG / W3C + # ARIA 1.2 §6.6 (`presentation` role). + # + # Why two SEPARATE tables per call (vs. one table with many rows): + # - The block-level DSL emits each call as a self-contained unit, + # same as `price_row` / `text` / `button`. Mixing rows from + # different DSL calls into one shared table would require a + # `Builder` flush phase that mutates earlier output — complex + # and surprising. Adjacent two-cell tables visually collapse + # into one continuous list when their bottom border meets the + # next row's top edge, so the user sees a single list anyway. + # + # Sources on email-safe table-row patterns: + # - https://www.cerberusemail.com/templates (responsive table patterns) + # - https://www.litmus.com/blog/the-ultimate-guide-to-css/ + # - https://htmlemail.io/blog/responsive-html-emails-creating-a-simple-responsive-email/ + def info_row(label, value) + label_html = h(label.to_s) + value_html = h(value.to_s) + # The `class="goodmail-info-row"` hook is the marker + # `Goodmail::Plaintext` looks for to flatten this two-cell table + # into a single `Label: Value` line in the plaintext part. HTML + # email clients render the visible table; text-only clients see + # the readable colon-form. Without the marker, Premailer would + # emit the cells on two separate lines: + # + # Label + # Value + # + # which is correct table-extraction behavior but a worse + # plaintext UX than the conventional `Label: Value` shape every + # other transactional sender uses. + parts << <<~HTML.strip +
+ + + + + + HTML + end + # Adds a simple code box with background styling. def code_box(text) # Re-added background/padding; content is simple, should survive Premailer plain text. @@ -105,16 +191,113 @@ def sign(name = Goodmail.config.company_name) parts << %(

– #{h name}

) end - %i[h1 h2 h3].each do |heading_tag| + # Inline styled link as a paragraph. Wraps `button` for cases where a full + # call-to-action button is too heavy — e.g. "View receipt", "Open the trip + # in the app", "Read the full policy". The link is rendered in the + # configured brand color and underlined, matching the layout's `a {}` rule + # so the visual stays consistent in clients that strip inline styles. + # + # Both `text` and `url` are HTML-escaped to prevent any accidental injection + # from interpolated user content (e.g. a passenger name in a label, a + # trip URL with arbitrary query strings). + def link(text, url) + parts << %(

#{h text}

) + end + + # Small/disclaimer text. Designed for legal language, fine print, "you + # received this because…", and similar secondary content. Uses the same + # neutral grey as the footer (`#777`) and a slightly smaller font size. + # Newlines become
, mirroring `text` so callers don't have to think + # about which helper handles which. + def small(str) + sanitized_content = HTML_SANITIZER.sanitize( + str.to_s, + tags: ALLOWED_TAGS, + attributes: ALLOWED_ATTRIBUTES + ) + parts << %(

#{sanitized_content.gsub(/\n/, "
")}

) + end + + # Adds an email attachment (file) to the outgoing message. Use for PDFs, + # .ics calendar invites, .csv exports, summary images you want stored on + # the recipient's machine, etc. + # + # Sources: + # - ActionMailer attachments docs: + # https://guides.rubyonrails.org/action_mailer_basics.html#sending-email-with-attachments + # - RFC 2392 (Content-ID URLs for inline images): + # https://www.rfc-editor.org/rfc/rfc2392 + # + # Parameters: + # filename — the name the recipient sees (e.g. "receipt.pdf"). + # content — either the raw bytes (String, IO) or a filesystem path + # (String). Strings that point at an existing file path are + # read from disk; otherwise the String is used as-is. + # mime_type — optional Content-Type override. When omitted, Action Mailer + # infers it from the filename via Mime::Type.lookup_by_extension. + # inline — when true, the attachment is marked as `inline` so the + # email body can reference it via ``. + # Useful for embedding logos / maps when you can't (or don't + # want to) host them publicly. See `inline_image` below for + # the matching DSL helper that also emits the tag. + def attach(filename, content, mime_type: nil, inline: false) + filename = filename.to_s + + # Inline attachments are referenced from the email body via + # `cid:FILENAME`, which Mail gem resolves to the FIRST part with + # that Content-ID. Two `inline_image` calls with the same + # filename therefore produce a broken second image (no Content-ID + # gets pinned to it, and even if both had matching CIDs only the + # first would resolve in any email client). + # + # Non-inline attachments don't have the same problem — they're + # downloaded by the recipient by filename, so a duplicate + # produces two files with the same name (annoying UX but not a + # rendering bug). We allow those. + if inline && attachments.any? { |a| a[:inline] && a[:filename] == filename } + raise Goodmail::Error, "duplicate inline filename #{filename.inspect} — `cid:#{filename}` cannot resolve to two parts. Use a distinct filename per inline_image call." + end + + attachments << { + filename: filename, + content: resolve_attachment_content(content), + mime_type: mime_type, + inline: inline + } + end + + # Embeds an inline image and emits the matching tag at this point in + # the email body, referencing the attachment via `cid:`. The CID is the + # filename, which Action Mailer maps when it serializes inline parts. + # + # `inline_image` is the right tool when: + # - the image must travel WITH the email so it renders in offline / + # end-of-cache scenarios (e.g. an Outlook user reading three months + # later when the public URL has expired), + # - or when you don't have a public URL to point at (private S3 + # bucket, dev environment with localhost URLs, etc). + # + # When the asset already has a public URL you control, prefer the regular + # `image(src, alt)` helper — it's lighter on the wire and avoids attaching + # binary parts to every send. + def inline_image(filename, content, alt: "", width: nil, height: nil, mime_type: nil) + attach(filename, content, mime_type: mime_type, inline: true) + image("cid:#{filename}", alt, width: width, height: height) + end + + # The `case` only ever sees the three keys we iterate over below, so + # the inline lookup is exhaustive by construction — no defensive + # `else` clause needed. + HEADING_STYLES = { + h1: "margin: 40px 0 10px; font-size: 32px; font-weight: 500; line-height: 1.2em;", + h2: "margin: 40px 0 10px; font-size: 24px; font-weight: 400; line-height: 1.2em;", + h3: "margin: 40px 0 10px; font-size: 18px; font-weight: 400; line-height: 1.2em;" + }.freeze + + HEADING_STYLES.each do |heading_tag, style| define_method(heading_tag) do |str| - # Added basic heading styles, consistent with layout.erb - style = case heading_tag - when :h1 then "margin: 40px 0 10px; font-size: 32px; font-weight: 500; line-height: 1.2em;" - when :h2 then "margin: 40px 0 10px; font-size: 24px; font-weight: 400; line-height: 1.2em;" - when :h3 then "margin: 40px 0 10px; font-size: 18px; font-weight: 400; line-height: 1.2em;" - else "margin: 16px 0; line-height: 1.6;" - end - # Headings should still have their content escaped + # Headings still escape their content — only the surrounding tag + # markup is trusted. parts << tag(heading_tag, h(str), style: style) end end @@ -140,6 +323,37 @@ def html_output private + # Loads file contents when `content` is a path to an existing file, + # otherwise returns it unchanged so callers can pass raw bytes / IO + # streams transparently. Paths win over byte-strings that happen to + # match a filename: this is intentional — the README's documented + # contract is "pass a path, we'll read it for you". + # + # Defensive checks before reaching `File.file?`: + # + # 1. NUL bytes — `File.file?` raises `ArgumentError: path name + # contains null byte` on any String containing `\0`, and that's + # exactly what binary file content (PNG / PDF / .ics) looks + # like. Treat NUL-containing Strings as "definitely not a + # path" so callers can pass `inline_image("logo.png", png_bytes)` + # without us blowing up trying to look up `png_bytes` as a path. + # 2. PATH_MAX — most filesystems cap paths at 4096 bytes (Linux + # `PATH_MAX`); macOS HFS+ at 1024. A String longer than that is + # structurally not a path and almost certainly file contents. + # We pick 4096 as the cutoff to be the most generous to legit + # paths while still cheaply screening out anything bigger. + # + # Source on `File.file?` and the NUL byte error: + # https://docs.ruby-lang.org/en/3.4/File.html#method-c-file-3F + def resolve_attachment_content(content) + return content unless content.is_a?(String) + return content if content.include?("\0") + return content if content.bytesize > 4096 + return content unless File.file?(content) + + File.binread(content) + end + # Helper for creating simple HTML tags with optional style # Assumes content is already appropriately escaped or marked safe. def tag(name, content, style: nil) diff --git a/lib/goodmail/dispatcher.rb b/lib/goodmail/dispatcher.rb index cc9c5ba..85f20d5 100644 --- a/lib/goodmail/dispatcher.rb +++ b/lib/goodmail/dispatcher.rb @@ -36,11 +36,21 @@ def build_message(headers, &block) mailer_headers = slice_mail_headers(headers) # 7. Build the mail object via the internal Mailer class action. + # Attachments collected via the `attach` / `inline_image` DSL helpers + # flow through here so the Mailer can register them with Action + # Mailer's attachments hash before the `mail()` call. + # + # The `preheader` is forwarded so `Goodmail::Plaintext` can strip + # the inbox-preview text from the plaintext part if it leaks + # through Premailer's extractor (which doesn't honor the hidden + # span's `display: none`). delivery_object = Goodmail::Mailer.compose_message( mailer_headers, raw_html_body, nil, # Pass nil for raw_text_body - Premailer generates it - unsubscribe_url + unsubscribe_url, + builder.attachments, + preheader: preheader ) # 8. Return the ActionMailer::MessageDelivery object diff --git a/lib/goodmail/email.rb b/lib/goodmail/email.rb index 5a83909..629e718 100644 --- a/lib/goodmail/email.rb +++ b/lib/goodmail/email.rb @@ -3,8 +3,24 @@ require "cgi" # For unescaping HTML in plaintext generation (though Premailer might handle most) module Goodmail - # Simple struct to hold the rendered HTML and text parts of an email. - EmailParts = Struct.new(:html, :text, keyword_init: true) + # Simple struct to hold the rendered HTML and text parts of an email, + # plus any attachments collected via the `attach` / `inline_image` DSL + # helpers. Callers using `Goodmail.render` (typically a custom mailer + # subclass that wants to call `mail()` itself) can fan attachments out + # to ActionMailer's `attachments` hash with a small loop: + # + # parts.attachments.each do |a| + # target = a[:inline] ? attachments.inline : attachments + # target[a[:filename]] = a[:mime_type] ? { mime_type: a[:mime_type], content: a[:content] } : a[:content] + # end + # + # `attachments` defaults to `[]` for backwards compatibility — callers + # written against 0.3.x keep working unchanged. + EmailParts = Struct.new(:html, :text, :attachments, keyword_init: true) do + def initialize(html: nil, text: nil, attachments: []) + super(html: html, text: text, attachments: attachments || []) + end + end # Renders the email content using the Goodmail DSL and returns HTML and text parts. # This method does not send the email but prepares its content for sending. @@ -35,37 +51,31 @@ def self.render(headers = {}, &dsl_block) preheader: preheader ) - # 4. Use Premailer to inline CSS and generate plaintext + # 4. Run Premailer for CSS inlining (HTML part). Plaintext goes + # through `Goodmail::Plaintext` which pre-processes the source + # HTML to neutralize MSO-only markup and the hidden preheader + # span — both of which Premailer's plaintext extractor would + # otherwise leak into the text body. premailer = Premailer.new( raw_html_body, with_html_string: true, adapter: :nokogiri, preserve_styles: false, # Force inlining and remove