Skip to content

Conversation

@AaronDDM
Copy link
Contributor

What did you do?

  • Updated _build_form_request to use content_id if exists, otherwise fallback to indexed file#

License

I confirm that this contribution is made under the terms of the MIT license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

- Modified _build_form_request to use content_id as field name when provided
- Falls back to file{index} naming when content_id is not specified
- Maintains full backward compatibility for existing attachments
- Added comprehensive tests for content_id handling
- Added example demonstrating inline attachment usage with content_id
- Updated CHANGELOG.md

Fixes issue where large inline attachments (>3MB) ignored content_id
and used generic file{index} names, breaking inline image references
in HTML emails.
Removed conflicting duplicate assertions in test_build_form_request_backwards_compatibility
that were causing test failures.
@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.73%. Comparing base (f30867b) to head (3421870).
⚠️ Report is 57 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #440      +/-   ##
==========================================
- Coverage   98.85%   98.73%   -0.13%     
==========================================
  Files          45       52       +7     
  Lines        1753     2289     +536     
==========================================
+ Hits         1733     2260     +527     
- Misses         20       29       +9     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@playerzero-ai
Copy link

playerzero-ai bot commented Sep 24, 2025

Pull Request Summary

Summary of what's changing and why (CUST-4448)

  • Core fix: When building multipart/form-data for attachments, the multipart field name now uses an attachment's content_id (if provided) instead of always using "file{index}". This ensures inline attachments can be sent with explicit content IDs so email clients/APIs can reference them via cid:...
  • Backward compatible: If content_id is absent, behavior falls back to the previous "file0", "file1", ... naming.

Product / functionality impact

  • Fixes a bug where large inline attachments (and attachments in general) were not being sent with their content_id, preventing cid: references from working reliably.
  • Enables correct sending of inline images that must be referenced by content_id in HTML email bodies (embedded images will render inline instead of appearing as generic attachments).
  • No other behavior of the API/library is changed beyond the multipart field-name handling.

Developer / UX improvements

  • New runnable example (examples/inline_attachment_demo/inline_attachment_example.py) showing:
    • How to send a message and a draft with inline attachments referenced in HTML via using content_id and is_inline flags.
    • The difference between inline (content_id present) and regular (no content_id) attachments.
    • Prints message/draft/thread IDs and basic error handling for easy verification.
    • Demonstrates a simulated large-image case to show multipart/form-data behavior for bigger files.
  • Clearer guidance for integrators on how to ensure images render inline.

Tests

  • Added tests validating:
    • content_id is used as the multipart field name when present.
    • backward compatibility for attachments without content_id (file0, file1...).
  • Minor cleanup of an existing test to avoid redundant assertions.

Operational notes / caveats

  • Example requires NYLAS_API_KEY and replacing placeholder grant IDs.
  • Large files (> ~3MB) may be handled differently by the multipart logic — the example simulates this but real large files will trigger that code path.

Net effect

  • Fixes CUST-4448 by honoring content_id in multipart requests for attachments, enabling reliable inline image embedding while preserving prior behavior for attachments lacking content_id.

Files Changed

File Name Summary
nylas/utils/file_utils.py Changed multipart form field naming for attachments: if an attachment dict contains "content_id" it is used as the multipart field name; otherwise falls back to "file{index}". Backward-compatible; no other behavior changes.
examples/inline_attachment_demo/inline_attachment_example.py New example showing how to send messages and drafts with inline attachments referenced in HTML via content_id (cid:). Demonstrates an inline PNG (BytesIO) referenced as , a regular attachment for comparison, printing of IDs, and basic error handling. Notes: requires NYLAS_API_KEY and grant placeholders; simulates large-file behavior.
tests/utils/test_file_utils.py Added two tests: test_build_form_request_with_content_id (verifies content_id used as field name and correct filename/bytes/MIME) and test_build_form_request_backwards_compatibility (verifies file0/file1 fallback). Removed two filename/bytes assertions from an existing test. Increases coverage; no production code changes.

View more in PlayerZero
updated: Sep 24 @ 02:59 PM UTC

- Replace placeholder binary data with actual PNG image base64 data
- Use proper base64 decoding for both small and large image examples
- Add base64 import to handle image decoding
- Maintain example functionality while using realistic image data
Copy link
Contributor

@samLRodrigues samLRodrigues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AaronDDM AaronDDM merged commit df2f0c9 into main Sep 24, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants