Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to send an email with a LinkedResource #17863

Open
wants to merge 2 commits into
base: contrib
Choose a base branch
from

Conversation

Stanislaw-Rudnicki
Copy link

@Stanislaw-Rudnicki Stanislaw-Rudnicki commented Dec 24, 2024

This pull request adds support for linked resources in the EmailMessage model. It also updates the ToMimeMessage method to handle these linked resources, ensuring they are properly included in email messages. A new constructor is introduced to maintain backward compatibility.

Changes:

  1. Updated the EmailMessage class:

    • Added a new property LinkedResources of type IList<EmailMessageLinkedResource>.
    • Introduced a new constructor to avoid breaking changes.
    • Implemented HasLinkedResources for checking linked resource presence.
  2. Added a new EmailMessageLinkedResource class:

    • Supports streams for linked resources in emails.
  3. Updated the ToMimeMessage extension method:

    • Enhanced it to include LinkedResources when converting to MimeMessage.

Testing Steps:

  1. Unit Test for Backward Compatibility:

    • Create an EmailMessage object using the old constructor (without LinkedResources).
    • Verify that the LinkedResources property is null.
    • Ensure the ToMimeMessage method works as expected, without errors.
  2. Unit Test for New Functionality:

    • Create an EmailMessage object using the new constructor, including LinkedResources:

    using var stream = new FileStream(
            Path.Combine(_env.WebRootPath, "images", "logo.png"),
            FileMode.Open,
            FileAccess.Read
        );
    
    var contentId = MimeUtils.GenerateMessageId();
    var linkedResource = new EmailMessageLinkedResource(stream, "logo.png", contentId);
        
    var email = new EmailMessage(
            from: "[email protected]",
            to: new[] { "[email protected]" },
            cc: null,
            bcc: null,
            replyTo: null,
            subject: "Test Email",
            body: $"<img src=\"cid:{contentId}\" />",
            isBodyHtml: true,
            attachments: null,
            linkedResources: new[] { linkedResource }
        );
    • Verify that the ToMimeMessage method includes the linked resource with the correct ContentId.
  3. Edge Cases:

    • Test with empty or null values for LinkedResources to ensure no exceptions occur.
    • Pass a mix of valid and null entries in the LinkedResources collection and ensure only valid resources are included.
  4. Integration Testing:

    • Use the updated EmailMessage model in an actual email-sending flow.
    • Verify that emails with linked resources render correctly in email clients (e.g., embedded images display as expected).
  5. Code Review:

    • Verify that changes maintain compatibility with existing functionality and do not introduce breaking changes.

Expected Results:

  • Emails sent without linked resources should behave as before.
  • Emails with linked resources should correctly embed those resources with specified ContentId.
  • No exceptions or regressions should occur during testing.

Copy link

github-actions bot commented Dec 24, 2024

Hi there @Stanislaw-Rudnicki, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

Copy link
Contributor

@lauraneto lauraneto left a comment

Choose a reason for hiding this comment

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

Hi @Stanislaw-Rudnicki ,

Thank you for the pull request! 🙌
Your code changes were introducing a breaking change, so I left a comment on how to make it non breaking.

Would you also be able to update the description of the pull request to include testing steps? That would be very helpful!
Thanks! 🙏

src/Umbraco.Core/Models/Email/EmailMessage.cs Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants