PrestoDoctor / griddler-sparkpost

SparkPost adapter for Griddler
MIT License
6 stars 4 forks source link

Unexpected #from[:email] when message sent from alias #1

Closed rapcal closed 8 years ago

rapcal commented 8 years ago

I've tested this with Google Apps, configured with SPF and DKIM. When you send an email using an alias, the primary email is passed to Griddler by the adapter, which seems unexpected to me.

I may not be seeing some other nasty side effect, but maybe this line could use msg['friendly_from'] instead of msg['msg_from'], which would fix the issue.

mfkp commented 8 years ago

Thanks @rapcal

I suppose I never tested this out with aliases, and the SparkPost documentation wasn't super clear on what friendly_from is ("Friendly sender or 'From' header in the original email").

https://support.sparkpost.com/customer/portal/articles/1976204-webhook-event-reference

When I was testing, they were always the same, so I believe I just picked msg['msg_from']

You think using the friendly_from makes more sense (e.g. using the alias)? If so, I'm willing to change that.

rapcal commented 8 years ago

@mfkp thanks for the super fast detailed reply! 👍 To me, it seems that using the alias is the result expected by most people. After all, the alias was the from address they explicitly set when sending the message. However, I guess some people will prefer the other way around - although I honestly believe it'll be a minority. Please feel free to handle it the way you see fit. Just my 2 cents. Thanks again! PS: this may be trickier than I thought initially. From here it seems that the friendly_from field can contain name + email address

mfkp commented 8 years ago

@rapcal just pushed out version 0.0.2 with this change, give it a shot and make sure it's working as intended. Thanks!

mfkp commented 8 years ago

Oh @rapcal - just saw your other comment with the link. Will check that out.

rapcal commented 8 years ago

Wow! That was fast! 😄 From the docs you linked to, seems it's just an email - and I never came across the name + address pattern. If you'd like, I'd be glad to get in touch with their support and try to figure it out.

mfkp commented 8 years ago

Yes, please, if you could. Their documentation could use some work :-)

Would be good to know the exact output of friendly_from so we don't break it for anyone - let me know once you find out!

rapcal commented 8 years ago

On it!

rapcal commented 8 years ago

Still waiting to hear from Sparkpost's support via their Slack channel, but even if the output can vary in format, maybe friendly_from could be handled like Griddler does in this line:

msg['friendly_from'].split('<').last.delete('>').strip

Or, alternatively, with something like:

msg['friendly_from'].match(/[\w+\-.]+@[a-z\d\-.]+\.[a-z]+/i)[0]
mfkp commented 8 years ago

I like that, adds some safety around it and will work in either case. I've updated it in this manner. Try out the new version that I just pushed, 0.0.3 and see if it looks right.

rapcal commented 8 years ago

Working like a charm, @mfkp!! Thanks a lot! Do you plan to push it to rubygems as 0.0.3? Version hasn't been bumped yet on master.

mfkp commented 8 years ago

@rapcal: it's already pushed (just forgot to push the version commit to github)

https://rubygems.org/gems/griddler-sparkpost

rapcal commented 8 years ago

Great! I'll let you know when I get the response from Sparkpost's support

Please let me know whenever I can be of assistance! Thanks for this fantastic gem!

mfkp commented 8 years ago

Will do, thanks for the help, appreciate it!

rapcal commented 8 years ago

@mfkp: According to Chris McFadden, VP of Engineering at SparkPost, Friendly from is the sending domain plus local part

Link to slack: https://sparkpost-community.slack.com/archives/community-support/p1461523413003097

I'd keep sanitizing the output of friendly_from just in case... But up to you, of course!

mfkp commented 8 years ago

Great, thanks for the clarification. I agree though, I think it's fine keeping it as-is with the sanitization. Can't hurt!