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

Calling From() in Build() to override globalFrom in Mailing #321

Closed
lwestfall opened this issue Dec 15, 2022 · 3 comments
Closed

Calling From() in Build() to override globalFrom in Mailing #321

lwestfall opened this issue Dec 15, 2022 · 3 comments

Comments

@lwestfall
Copy link

Describe the solution you'd like
Calling .From("[email protected]") in Build() doesn't seem to override the globalFrom address. As discussed in #320 it's better if calling .From() would override the globalFrom mailbox. Essentially this would mean that globalFrom acts more as a default from address that can easily be overridden conditionally or on a Mailable implementation basis.

I can work on this one and #320 together. Might be a couple of days before I get to it.

@lwestfall
Copy link
Author

lwestfall commented Dec 15, 2022

Found some free time today to take a stab - I think it was pretty easy (basically just swapping the null coalescing precedence on what was this._globalFrom ?? @from).

Issue I'm having is writing tests:

  • Can't easily test on SmtpMailer with Mailable._from being private. SmtpMailer.RenderAsync returns a string with just the razor page rendered as html, so can't find the From address in there
  • AssertMailer doesn't have a globalFrom
  • There isn't a test class for FileLogMailer, but if there was we could inspect the header that's written at the top of the file

I'm assuming we want tests for at least SmtpMailer or FileLogMailer (if not both). Here are a few options as I see them, not mutually exclusive:

  1. Provide a public getter for Mailable._from (or could mark field or getter internal and make internal members visible to the test assembly if we want to keep _from invisible to client assemblies)
  2. Write a test class for FileLogMailer and use approach detailed in 3rd bullet above
  3. Give a globalFrom to AssertMailer (but is sort of moot since it doesn't test my changes to SmtpMailer and FileMailer)
  4. Am I missing anything?

Hope this all makes sense, let me know if you have any suggestions.

@jamesmh
Copy link
Owner

jamesmh commented Dec 31, 2022

Thanks for looking into this.

I'm okay with adding some public getters, as long as we keep the setters private. Notably, for the SMTP Mailer this is needed for the tests to work.

For the FileLogMailer, if you want to check the headers that would work 👍

I'm indifferent about the AssertMailer since it's more-or-less a testing tool vs. production thing.

Let me know if there's anything that I missed, etc. Thanks!

@jamesmh
Copy link
Owner

jamesmh commented Sep 20, 2024

This is fixed in Mailer 6.0.0. https://docs.coravel.net/Mailing/#sender

@jamesmh jamesmh closed this as completed Sep 20, 2024
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

No branches or pull requests

2 participants