From ca5533d96b1a7d667216b26b72cba8082feae3fd Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 4 Nov 2025 12:45:06 -0800 Subject: [PATCH 1/5] Analyze the problem and form a plan --- ISSUE-202-ANALYSIS-AND-PLAN.md | 244 +++++++++++++++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 ISSUE-202-ANALYSIS-AND-PLAN.md diff --git a/ISSUE-202-ANALYSIS-AND-PLAN.md b/ISSUE-202-ANALYSIS-AND-PLAN.md new file mode 100644 index 0000000..4079784 --- /dev/null +++ b/ISSUE-202-ANALYSIS-AND-PLAN.md @@ -0,0 +1,244 @@ +# Issue #202: MIME Structure Bug - Analysis and Fix Plan + +## The Problem in Plain English + +When you send an email with gmailr that has: +- Both text and HTML body content, AND +- File attachments (like .txt, .pdf, .xlsx, .png) + +Some parts of the email disappear: +- With `.txt` attachments → the message body vanishes +- With `.pdf` attachments → the attachment itself vanishes +- Similar problems occur with `.xlsx`, `.png`, and other file types + +## Why This Happens + +MIME messages need different structures depending on their content: + +1. **Simple email (text only)**: Just the text +2. **Multipart/alternative** (text + HTML, no attachments): Both versions wrapped together so email clients can choose which to display +3. **Multipart/mixed** (attachments involved): A container that holds both the message content AND the attachments as separate parts + +**The Bug**: gmailr is using `multipart/alternative` when it should be using `multipart/mixed` for emails with attachments. + +Think of it like packing boxes: +- ❌ gmailr puts everything in an "alternative" box (designed for holding just text vs HTML) +- ✅ gmailr should put everything in a "mixed" box (designed for holding message + attachments) + +## The Correct MIME Structure + +For an email with text+HTML body AND attachments, the structure should be: + +``` +multipart/mixed (the outer container) +├─ multipart/alternative (the message part) +│ ├─ text/plain (plain text version) +│ └─ text/html (HTML version) +├─ application/pdf (attachment) +└─ text/plain (another attachment, if any) +``` + +## Root Cause in Code + +In `as.character.mime()` function in `R/gm_mime.R` (lines 242-299): + +**Lines 246-254**: If both text and HTML parts exist, set `content_type = "multipart/alternative"` +```r +if ( + x$attr$content_type %!=% + "multipart/alternative" && + exists_list(x$parts, TEXT_PART) && + exists_list(x$parts, HTML_PART) +) { + x$attr$content_type <- "multipart/alternative" +} +``` + +**Lines 257-258**: If parts exist, set `content_type = "multipart/mixed"` ONLY if not already set +```r +if (length(x$parts) > 0L) { + x$attr$content_type <- x$attr$content_type %||% "multipart/mixed" + # ... +} +``` + +**The problem**: Step 1 sets the content_type to "multipart/alternative", so step 2's `%||%` operator keeps that value instead of changing it to "multipart/mixed". + +## Current gmailr Structure (Wrong) + +Currently, gmailr creates a flat structure: + +``` +multipart/alternative (WRONG - should be multipart/mixed) +├─ text/plain (part 1) +├─ text/html (part 2) +└─ application/pdf (part 3 - attachment) +``` + +Gmail's API then drops parts because they don't belong in a multipart/alternative container. + +--- + +# Fix Plan + +## Proposed Solution + +**Approach**: Restructure parts to create nested MIME structure when attachments are present. + +Modify `as.character.mime()` to detect when both conditions exist: +1. Text and HTML body parts (stored in indices 1 and 2) +2. Attachments (index 3 and beyond) + +When both conditions are met: +1. Create a new nested MIME part of type `multipart/alternative` containing parts 1 and 2 +2. Replace the parts list with: `[alternative_wrapper, attachment_1, attachment_2, ...]` +3. Set the outer content_type to `multipart/mixed` + +## Implementation Steps + +### 1. Modify `as.character.mime()` in R/gm_mime.R + +Add logic before line 246 to restructure parts when needed: + +```r +as.character.mime <- function(x, newline = "\r\n", ...) { + # encode headers + x$header <- lapply(x$header, header_encode) + + # NEW: Check if we need nested structure (text + HTML + attachments) + has_attachments <- length(x$parts) > 2L + has_both_bodies <- exists_list(x$parts, TEXT_PART) && + exists_list(x$parts, HTML_PART) + + if (has_both_bodies && has_attachments) { + # Create nested multipart/alternative for text + HTML + alternative_part <- gm_mime( + attr = list(content_type = "multipart/alternative"), + parts = list(x$parts[[TEXT_PART]], x$parts[[HTML_PART]]) + ) + + # Restructure: [alternative_wrapper, attachment1, attachment2, ...] + attachment_parts <- x$parts[3:length(x$parts)] + x$parts <- c(list(alternative_part), attachment_parts) + + # Set outer to multipart/mixed + x$attr$content_type <- "multipart/mixed" + } else if (has_both_bodies) { + # No attachments, just text + HTML + x$attr$content_type <- "multipart/alternative" + } + + # ... rest of existing logic continues unchanged +} +``` + +### 2. The existing code after this change + +After the restructuring, the existing code (lines 257-298) will handle the nested structure correctly because: +- It already knows how to serialize parts that are themselves mime objects +- The boundary logic works recursively +- The encoding logic applies to each part appropriately + +### 3. Remove or update the old text+HTML check + +The check at lines 246-254 can be simplified or removed since we're now handling it earlier. + +## Testing Strategy + +### New Test Cases to Add to tests/testthat/test-gm_mime.R + +Create a new test block: `test_that("MIME - Messages with attachments and alternative bodies", { ... })` + +**Test 1: Text + HTML + PDF attachment** +```r +msg <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Test with attachment") |> + gm_text_body("Plain text version") |> + gm_html_body("HTML version") |> + gm_attach_file(test_path("fixtures", "test.pdf")) + +msg_chr <- as.character(msg) + +# Verify outer is multipart/mixed +expect_match(msg_chr, "Content-Type: multipart/mixed", label = "Outer is multipart/mixed") + +# Verify nested multipart/alternative exists +expect_match(msg_chr, "Content-Type: multipart/alternative", label = "Nested alternative exists") + +# Verify all parts present +expect_match(msg_chr, "Plain text version", label = "Text body present") +expect_match(msg_chr, "HTML version", label = "HTML body present") +expect_match(msg_chr, "test\\.pdf", label = "Attachment present") +``` + +**Test 2: Text + HTML + TXT attachment (the original bug report)** +- Same structure as Test 1, use .txt file + +**Test 3: Text only + attachment (no HTML)** +```r +msg <- gm_mime() |> + gm_text_body("Plain text only") |> + gm_attach_file(test_path("fixtures", "test.pdf")) + +msg_chr <- as.character(msg) + +# Should be multipart/mixed, NOT alternative +expect_match(msg_chr, "Content-Type: multipart/mixed") +expect_no_match(msg_chr, "multipart/alternative") +``` + +**Test 4: Regression test - Text + HTML, no attachments** +- Verify this still works as before (existing test at line 138 should cover this) +- Should be `multipart/alternative` (not mixed, not nested) + +**Test 5: Multiple attachments** +```r +msg <- gm_mime() |> + gm_text_body("Text") |> + gm_html_body("HTML") |> + gm_attach_file(test_path("fixtures", "file1.pdf")) |> + gm_attach_file(test_path("fixtures", "file2.png")) + +# Verify both attachments present +# Verify correct nesting +``` + +### Manual Verification Steps + +After tests pass: +1. Create and send a test email with the problem combination +2. Check received email in Gmail to confirm all parts visible +3. Test with `gm_send_message()` and `gm_send_draft()` + +## Files to Modify + +1. **R/gm_mime.R** - Primary change in `as.character.mime()` function +2. **tests/testthat/test-gm_mime.R** - Add comprehensive test coverage + +## Edge Cases to Consider + +1. **Only HTML + attachment (no text)**: Should be flat `multipart/mixed`, no nesting +2. **Empty parts**: Existing `exists_list()` checks handle this +3. **Manual content_type**: Line 248 check suggests users might set this manually - our new logic should take precedence +4. **Single attachment**: Should work the same as multiple + +## Risks and Mitigations + +| Risk | Mitigation | +|------|------------| +| Breaking existing messages without attachments | Only restructure when `length(x$parts) > 2L`; add regression tests | +| Incorrect attachment detection | Clear criterion: parts 3+ are attachments (by design of `gm_attach_file()`) | +| Nested MIME serialization issues | Existing code already handles parts as mime objects; test thoroughly | + +## Success Criteria + +- [ ] Messages with text+HTML+attachments use `multipart/mixed` outer structure +- [ ] Nested `multipart/alternative` wraps text and HTML when attachments present +- [ ] All existing tests pass (no regressions) +- [ ] New tests cover all reported file types (.txt, .pdf, .xlsx, .png) +- [ ] Manual testing confirms correct display in Gmail +- [ ] Code follows tidyverse style guide +- [ ] Run `devtools::document()` if roxygen comments changed +- [ ] Run `air format .` after code changes From 230dd9d32962ea68a12b6fbe5f3a14f254e16290 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 4 Nov 2025 14:16:12 -0800 Subject: [PATCH 2/5] Refactoring the existing tests --- tests/testthat/test-gm_mime.R | 153 ++++++++++++---------------------- 1 file changed, 54 insertions(+), 99 deletions(-) diff --git a/tests/testthat/test-gm_mime.R b/tests/testthat/test-gm_mime.R index e6c62e0..f4cd75c 100644 --- a/tests/testthat/test-gm_mime.R +++ b/tests/testthat/test-gm_mime.R @@ -1,46 +1,30 @@ test_that("MIME - Basic functions", { - # Create a new Email::Stuffer object msg <- gm_mime() - expect_equal(class(msg), "mime", label = "msg object has correct class") - - expect_true( - length(msg$header) > 0, - label = "Even the default object has headers" - ) + expect_s3_class(msg, "mime") + expect_true(length(msg$header) > 0) rv <- gm_to(msg, "adam@ali.as") - expect_equal( - header_encode(rv$header$To), - "adam@ali.as", - label = "to sets To Header" - ) + expect_equal(header_encode(rv$header$To), "adam@ali.as") rv <- gm_from(msg, "bob@ali.as") - expect_equal( - header_encode(rv$header$From), - "bob@ali.as", - label = "from sets From Header" - ) + expect_equal(header_encode(rv$header$From), "bob@ali.as") rv <- gm_to(msg, c("adam@ali.as", "another@ali.as", "bob@ali.as")) expect_equal( header_encode(rv$header$To), - "adam@ali.as, another@ali.as, bob@ali.as", - label = "to (multiple) sets To header" + "adam@ali.as, another@ali.as, bob@ali.as" ) rv <- gm_cc(msg, c("adam@ali.as", "another@ali.as", "bob@ali.as")) expect_equal( header_encode(rv$header$Cc), - "adam@ali.as, another@ali.as, bob@ali.as", - label = "cc (multiple) sets To header" + "adam@ali.as, another@ali.as, bob@ali.as" ) rv <- gm_bcc(msg, c("adam@ali.as", "another@ali.as", "bob@ali.as")) expect_equal( header_encode(rv$header$Bcc), - "adam@ali.as, another@ali.as, bob@ali.as", - label = "bcc (multiple) sets To header" + "adam@ali.as, another@ali.as, bob@ali.as" ) }) @@ -68,62 +52,51 @@ test_that("header_encode encodes non-ascii values as base64", { }) test_that("MIME - More Complex", { - msg <- gm_mime() - msg <- gm_from(msg, "Jim Hester") - msg <- gm_to(msg, "james.f.hester@gmail.com") - msg <- gm_subject(msg, "Hello To:!") - msg <- gm_text_body(msg, "I am an email") - - msg1 <- gm_attach_file(msg, test_path("fixtures", "volcano.png")) - + msg <- gm_mime() |> + gm_from("Gargle Testuser ") |> + gm_to("jenny+gmailr-tests@posit.co") |> + gm_subject("Hello To:!") |> + gm_text_body("I am an email") + + # Text body with PNG attachment + msg1 <- msg |> gm_attach_file(test_path("fixtures", "volcano.png")) msg1_chr <- as.character(msg1) - - expect_match(msg1_chr, "Jim Hester", label = "Email contains from name") - expect_match(msg1_chr, "gmail", label = "Email contains to string") - expect_match(msg1_chr, "Hello", label = "Email contains subject string") - expect_match(msg1_chr, "I am an email", label = "Email contains text_body") - expect_match(msg1_chr, "volcano", label = "Email contains file name") - - msg2 <- gm_attach_file( - msg, - test_path("fixtures", "test.ini"), - content_type = "text/plain" - ) - + expect_match(msg1_chr, "Gargle Testuser") + expect_match(msg1_chr, "posit") + expect_match(msg1_chr, "Hello") + expect_match(msg1_chr, "I am an email") + expect_match(msg1_chr, "volcano") + + # Text body with text attachment + msg2 <- msg |> + gm_attach_file( + test_path("fixtures", "test.ini"), + content_type = "text/plain" + ) msg2_chr <- as.character(msg2) - - expect_match(msg2_chr, "Jim Hester", label = "Email contains from name") - expect_match(msg2_chr, "gmail", label = "Email contains to string") - expect_match(msg2_chr, "Hello", label = "Email contains subject string") - expect_match(msg2_chr, "I am an email", label = "Email contains text_body") + expect_match(msg2_chr, "I am an email") expect_match( msg2_chr, - "Content-Type: application/octet-stream; name=test\\.ini", - label = "Email contains attachment Content-Type" - ) - - msg3 <- gm_html_body(msg, "I am an html email
") - msg3 <- gm_attach_file( - msg3, - test_path("fixtures", "test.ini"), - content_type = "application/octet-stream" + "Content-Type: application/octet-stream; name=test\\.ini" ) + # Text + HTML body with attachment + msg3 <- msg |> + gm_html_body("

I am an html email

") |> + gm_attach_file( + test_path("fixtures", "test.ini"), + content_type = "application/octet-stream" + ) msg3_chr <- as.character(msg3) - - expect_match(msg3_chr, "Jim Hester", label = "Email contains from name") - expect_match(msg3_chr, "gmail", label = "Email contains to string") - expect_match(msg3_chr, "Hello", label = "Email contains subject string") - expect_match(msg3_chr, "I am an email", label = "Email contains text_body") + expect_match(msg3_chr, "I am an email") expect_match( msg3_chr, - base64url_encode("I am an html email
"), - label = "Email contains html_body" + base64encode(charToRaw("

I am an html email

")), + fixed = TRUE ) expect_match( msg3_chr, - "Content-Type: application/octet-stream; name=test\\.ini", - label = "Email contains attachment Content-Type" + "Content-Type: application/octet-stream; name=test\\.ini" ) skip_if_no_token() @@ -136,44 +109,26 @@ test_that("MIME - More Complex", { }) test_that("MIME - Alternative emails contain correct parts", { - msg <- gm_mime() - msg <- gm_from(msg, "Jim Hester") - msg <- gm_to(msg, "james.f.hester@gmail.com") - msg <- gm_subject(msg, "Hello To:!") - msg <- gm_text_body(msg, "I am an email") - msg <- gm_html_body(msg, "I am a html email") + msg <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Hello!") |> + gm_text_body("I am an email") |> + gm_html_body("I am a html email") email_chr <- as.character(msg) - expect_match(email_chr, "Jim Hester", label = "Email contains from name") - expect_match(email_chr, "james.f.hester", label = "Email contains to string") - expect_match(email_chr, "Hello", label = "Email contains subject string") - expect_match( - email_chr, - "Content-Type: multipart/alternative", - label = "Email content type" - ) - expect_match( - email_chr, - "Content-Type: text/plain", - label = "Email content type" - ) - expect_match( - email_chr, - "Content-Type: text/html", - label = "Email content type" - ) - - expect_match( - email_chr, - quoted_printable_encode("I am an email"), - label = "Email contains text body" - ) + expect_match(email_chr, "test@example\\.com") + expect_match(email_chr, "user@example\\.com") + expect_match(email_chr, "Hello") + expect_match(email_chr, "Content-Type: multipart/alternative") + expect_match(email_chr, "Content-Type: text/plain") + expect_match(email_chr, "Content-Type: text/html") + expect_match(email_chr, quoted_printable_encode("I am an email")) expect_match( email_chr, base64encode(charToRaw("I am a html email")), - fixed = TRUE, - label = "Email contains html body" + fixed = TRUE ) }) From ef3aa7e599925f2be6b6e9a278a4883cb9d5f253 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 4 Nov 2025 14:31:19 -0800 Subject: [PATCH 3/5] Fix + tests when there's plain text & html body AND attachment(s) --- R/gm_mime.R | 30 ++++++++++---- tests/testthat/test-gm_mime.R | 78 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/R/gm_mime.R b/R/gm_mime.R index 36fff70..ca31931 100644 --- a/R/gm_mime.R +++ b/R/gm_mime.R @@ -243,14 +243,28 @@ as.character.mime <- function(x, newline = "\r\n", ...) { # encode headers x$header <- lapply(x$header, header_encode) - # if we have both the text part and html part, we have to embed them in a multipart/alternative message - if ( - x$attr$content_type %!=% - "multipart/alternative" && - exists_list(x$parts, TEXT_PART) && - exists_list(x$parts, HTML_PART) - ) { - x$attr$content_type <- "multipart/alternative" + # Check if we need nested structure ((text + HTML) + attachments) + has_both_bodies <- exists_list(x$parts, TEXT_PART) && + exists_list(x$parts, HTML_PART) + # Attachments, if present, always start at index 3 + has_attachments <- length(x$parts) > 2 + + if (has_both_bodies) { + if (has_attachments) { + # Need a nested structure: + # multipart/mixed containing [multipart/alternative [text, html], attachment1, ...] + alternative_part <- gm_mime( + attr = list(content_type = "multipart/alternative"), + parts = list(x$parts[[TEXT_PART]], x$parts[[HTML_PART]]) + ) + + attachment_parts <- x$parts[3:length(x$parts)] + x$parts <- c(list(alternative_part), attachment_parts) + + x$attr$content_type <- "multipart/mixed" + } else { + x$attr$content_type <- "multipart/alternative" + } } # if a multipart message diff --git a/tests/testthat/test-gm_mime.R b/tests/testthat/test-gm_mime.R index f4cd75c..1db4523 100644 --- a/tests/testthat/test-gm_mime.R +++ b/tests/testthat/test-gm_mime.R @@ -132,6 +132,84 @@ test_that("MIME - Alternative emails contain correct parts", { ) }) +test_that("MIME - Messages with attachments and alternative bodies", { + # Test 1: text + HTML + attachment should have nested structure + msg1 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Test with attachment") |> + gm_text_body("Plain text version") |> + gm_html_body("HTML version") |> + gm_attach_file(test_path("fixtures", "volcano.png")) + + msg1_chr <- as.character(msg1) + + # Verify outer is multipart/mixed + expect_match(msg1_chr, "Content-Type: multipart/mixed") + # Verify nested multipart/alternative exists + expect_match(msg1_chr, "Content-Type: multipart/alternative") + # Verify all parts present + expect_match(msg1_chr, "Plain text version") + expect_match( + msg1_chr, + base64encode(charToRaw("HTML version")), + fixed = TRUE + ) + expect_match(msg1_chr, "volcano\\.png") + + # Test 2: text + HTML + text attachment + # https://github.com/r-lib/gmailr/issues/202 + msg2 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Test with text attachment") |> + gm_text_body("Email body text") |> + gm_html_body("

Email body HTML

") |> + gm_attach_file(test_path("fixtures", "test.ini")) + + msg2_chr <- as.character(msg2) + + expect_match(msg2_chr, "Content-Type: multipart/mixed") + expect_match(msg2_chr, "Content-Type: multipart/alternative") + expect_match(msg2_chr, "Email body text") + expect_match( + msg2_chr, + base64encode(charToRaw("

Email body HTML

")), + fixed = TRUE + ) + expect_match(msg2_chr, "test\\.ini") + + # Test 3: text only + attachment (no HTML) should be flat multipart/mixed + msg3 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Text only with attachment") |> + gm_text_body("Just plain text") |> + gm_attach_file(test_path("fixtures", "volcano.png")) + + msg3_chr <- as.character(msg3) + + expect_match(msg3_chr, "Content-Type: multipart/mixed") + # Should NOT have multipart/alternative since there's no HTML + expect_false(grepl("multipart/alternative", msg3_chr)) + + # Test 4: Multiple attachments + msg4 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Multiple attachments") |> + gm_text_body("Text body") |> + gm_html_body("HTML body") |> + gm_attach_file(test_path("fixtures", "test.ini")) |> + gm_attach_file(test_path("fixtures", "volcano.png")) + + msg4_chr <- as.character(msg4) + + expect_match(msg4_chr, "Content-Type: multipart/mixed") + expect_match(msg4_chr, "Content-Type: multipart/alternative") + expect_match(msg4_chr, "test\\.ini") + expect_match(msg4_chr, "volcano\\.png") +}) test_that("plain ascii should not be encoded", { expect_match( From 833d7cac81f7bde7f8ba039a7c52d253acd3da44 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 4 Nov 2025 15:30:20 -0800 Subject: [PATCH 4/5] Add NEWS bullet --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 487a8da..10d6e56 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,10 @@ * Legacy auth functions `clear_token()`, `gmail_auth()`, and `use_secret_file()` have been removed, following the same deprecation timeline as described above. +## Bug fixes + +* Fixed MIME structure for emails with text+HTML bodies and attachments. These messages now correctly use nested `multipart/mixed` (outer) containing `multipart/alternative` (text/HTML), preventing the loss of some of the message parts (#202). + # gmailr 2.0.0 ## Changes around the OAuth client From 0039852225b8294c9aaa88689eb5c5e4c9c1538b Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 4 Nov 2025 15:31:19 -0800 Subject: [PATCH 5/5] Remove claude notes and planning --- ISSUE-202-ANALYSIS-AND-PLAN.md | 244 --------------------------------- 1 file changed, 244 deletions(-) delete mode 100644 ISSUE-202-ANALYSIS-AND-PLAN.md diff --git a/ISSUE-202-ANALYSIS-AND-PLAN.md b/ISSUE-202-ANALYSIS-AND-PLAN.md deleted file mode 100644 index 4079784..0000000 --- a/ISSUE-202-ANALYSIS-AND-PLAN.md +++ /dev/null @@ -1,244 +0,0 @@ -# Issue #202: MIME Structure Bug - Analysis and Fix Plan - -## The Problem in Plain English - -When you send an email with gmailr that has: -- Both text and HTML body content, AND -- File attachments (like .txt, .pdf, .xlsx, .png) - -Some parts of the email disappear: -- With `.txt` attachments → the message body vanishes -- With `.pdf` attachments → the attachment itself vanishes -- Similar problems occur with `.xlsx`, `.png`, and other file types - -## Why This Happens - -MIME messages need different structures depending on their content: - -1. **Simple email (text only)**: Just the text -2. **Multipart/alternative** (text + HTML, no attachments): Both versions wrapped together so email clients can choose which to display -3. **Multipart/mixed** (attachments involved): A container that holds both the message content AND the attachments as separate parts - -**The Bug**: gmailr is using `multipart/alternative` when it should be using `multipart/mixed` for emails with attachments. - -Think of it like packing boxes: -- ❌ gmailr puts everything in an "alternative" box (designed for holding just text vs HTML) -- ✅ gmailr should put everything in a "mixed" box (designed for holding message + attachments) - -## The Correct MIME Structure - -For an email with text+HTML body AND attachments, the structure should be: - -``` -multipart/mixed (the outer container) -├─ multipart/alternative (the message part) -│ ├─ text/plain (plain text version) -│ └─ text/html (HTML version) -├─ application/pdf (attachment) -└─ text/plain (another attachment, if any) -``` - -## Root Cause in Code - -In `as.character.mime()` function in `R/gm_mime.R` (lines 242-299): - -**Lines 246-254**: If both text and HTML parts exist, set `content_type = "multipart/alternative"` -```r -if ( - x$attr$content_type %!=% - "multipart/alternative" && - exists_list(x$parts, TEXT_PART) && - exists_list(x$parts, HTML_PART) -) { - x$attr$content_type <- "multipart/alternative" -} -``` - -**Lines 257-258**: If parts exist, set `content_type = "multipart/mixed"` ONLY if not already set -```r -if (length(x$parts) > 0L) { - x$attr$content_type <- x$attr$content_type %||% "multipart/mixed" - # ... -} -``` - -**The problem**: Step 1 sets the content_type to "multipart/alternative", so step 2's `%||%` operator keeps that value instead of changing it to "multipart/mixed". - -## Current gmailr Structure (Wrong) - -Currently, gmailr creates a flat structure: - -``` -multipart/alternative (WRONG - should be multipart/mixed) -├─ text/plain (part 1) -├─ text/html (part 2) -└─ application/pdf (part 3 - attachment) -``` - -Gmail's API then drops parts because they don't belong in a multipart/alternative container. - ---- - -# Fix Plan - -## Proposed Solution - -**Approach**: Restructure parts to create nested MIME structure when attachments are present. - -Modify `as.character.mime()` to detect when both conditions exist: -1. Text and HTML body parts (stored in indices 1 and 2) -2. Attachments (index 3 and beyond) - -When both conditions are met: -1. Create a new nested MIME part of type `multipart/alternative` containing parts 1 and 2 -2. Replace the parts list with: `[alternative_wrapper, attachment_1, attachment_2, ...]` -3. Set the outer content_type to `multipart/mixed` - -## Implementation Steps - -### 1. Modify `as.character.mime()` in R/gm_mime.R - -Add logic before line 246 to restructure parts when needed: - -```r -as.character.mime <- function(x, newline = "\r\n", ...) { - # encode headers - x$header <- lapply(x$header, header_encode) - - # NEW: Check if we need nested structure (text + HTML + attachments) - has_attachments <- length(x$parts) > 2L - has_both_bodies <- exists_list(x$parts, TEXT_PART) && - exists_list(x$parts, HTML_PART) - - if (has_both_bodies && has_attachments) { - # Create nested multipart/alternative for text + HTML - alternative_part <- gm_mime( - attr = list(content_type = "multipart/alternative"), - parts = list(x$parts[[TEXT_PART]], x$parts[[HTML_PART]]) - ) - - # Restructure: [alternative_wrapper, attachment1, attachment2, ...] - attachment_parts <- x$parts[3:length(x$parts)] - x$parts <- c(list(alternative_part), attachment_parts) - - # Set outer to multipart/mixed - x$attr$content_type <- "multipart/mixed" - } else if (has_both_bodies) { - # No attachments, just text + HTML - x$attr$content_type <- "multipart/alternative" - } - - # ... rest of existing logic continues unchanged -} -``` - -### 2. The existing code after this change - -After the restructuring, the existing code (lines 257-298) will handle the nested structure correctly because: -- It already knows how to serialize parts that are themselves mime objects -- The boundary logic works recursively -- The encoding logic applies to each part appropriately - -### 3. Remove or update the old text+HTML check - -The check at lines 246-254 can be simplified or removed since we're now handling it earlier. - -## Testing Strategy - -### New Test Cases to Add to tests/testthat/test-gm_mime.R - -Create a new test block: `test_that("MIME - Messages with attachments and alternative bodies", { ... })` - -**Test 1: Text + HTML + PDF attachment** -```r -msg <- gm_mime() |> - gm_from("test@example.com") |> - gm_to("user@example.com") |> - gm_subject("Test with attachment") |> - gm_text_body("Plain text version") |> - gm_html_body("HTML version") |> - gm_attach_file(test_path("fixtures", "test.pdf")) - -msg_chr <- as.character(msg) - -# Verify outer is multipart/mixed -expect_match(msg_chr, "Content-Type: multipart/mixed", label = "Outer is multipart/mixed") - -# Verify nested multipart/alternative exists -expect_match(msg_chr, "Content-Type: multipart/alternative", label = "Nested alternative exists") - -# Verify all parts present -expect_match(msg_chr, "Plain text version", label = "Text body present") -expect_match(msg_chr, "HTML version", label = "HTML body present") -expect_match(msg_chr, "test\\.pdf", label = "Attachment present") -``` - -**Test 2: Text + HTML + TXT attachment (the original bug report)** -- Same structure as Test 1, use .txt file - -**Test 3: Text only + attachment (no HTML)** -```r -msg <- gm_mime() |> - gm_text_body("Plain text only") |> - gm_attach_file(test_path("fixtures", "test.pdf")) - -msg_chr <- as.character(msg) - -# Should be multipart/mixed, NOT alternative -expect_match(msg_chr, "Content-Type: multipart/mixed") -expect_no_match(msg_chr, "multipart/alternative") -``` - -**Test 4: Regression test - Text + HTML, no attachments** -- Verify this still works as before (existing test at line 138 should cover this) -- Should be `multipart/alternative` (not mixed, not nested) - -**Test 5: Multiple attachments** -```r -msg <- gm_mime() |> - gm_text_body("Text") |> - gm_html_body("HTML") |> - gm_attach_file(test_path("fixtures", "file1.pdf")) |> - gm_attach_file(test_path("fixtures", "file2.png")) - -# Verify both attachments present -# Verify correct nesting -``` - -### Manual Verification Steps - -After tests pass: -1. Create and send a test email with the problem combination -2. Check received email in Gmail to confirm all parts visible -3. Test with `gm_send_message()` and `gm_send_draft()` - -## Files to Modify - -1. **R/gm_mime.R** - Primary change in `as.character.mime()` function -2. **tests/testthat/test-gm_mime.R** - Add comprehensive test coverage - -## Edge Cases to Consider - -1. **Only HTML + attachment (no text)**: Should be flat `multipart/mixed`, no nesting -2. **Empty parts**: Existing `exists_list()` checks handle this -3. **Manual content_type**: Line 248 check suggests users might set this manually - our new logic should take precedence -4. **Single attachment**: Should work the same as multiple - -## Risks and Mitigations - -| Risk | Mitigation | -|------|------------| -| Breaking existing messages without attachments | Only restructure when `length(x$parts) > 2L`; add regression tests | -| Incorrect attachment detection | Clear criterion: parts 3+ are attachments (by design of `gm_attach_file()`) | -| Nested MIME serialization issues | Existing code already handles parts as mime objects; test thoroughly | - -## Success Criteria - -- [ ] Messages with text+HTML+attachments use `multipart/mixed` outer structure -- [ ] Nested `multipart/alternative` wraps text and HTML when attachments present -- [ ] All existing tests pass (no regressions) -- [ ] New tests cover all reported file types (.txt, .pdf, .xlsx, .png) -- [ ] Manual testing confirms correct display in Gmail -- [ ] Code follows tidyverse style guide -- [ ] Run `devtools::document()` if roxygen comments changed -- [ ] Run `air format .` after code changes