-
-
Notifications
You must be signed in to change notification settings - Fork 199
[#33] Parsed headers support #34
[#33] Parsed headers support #34
Conversation
@calebthompson thoughts? |
@@ -39,6 +41,18 @@ def self.extract_reply_body(body) | |||
end | |||
end | |||
|
|||
def self.extract_headers(raw_headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'd get a simpler, more robust result by just using the RobustEmailParser
and not defaulting to the regexp matching. This would also remove the need to add the option to override the parser_class.
I assume your reason for not doing this by default was to not require a dependency on Mail
, is that correct? If that's the case, Rails (via ActionMailer) already has that dependency, and we depend upon Rails as Griddler is an Engine, so there's no actual added dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, totally agreed 💎
header_fields.inject({}) do |header_hash, header_field| | ||
header_hash[header_field.name.to_s] = header_field.value.to_s | ||
header_hash | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is very unfortunate that Mail::Header
doesn't provide a to_hash
for this.
Emails received from services other than SendGrid will not likely have the headers provided. I just checked and it won't affect functionality, but we may want to note that in the documentation.
|
This is looking good. Would you rebase, squash fixup commits, and force push into this branch? |
end | ||
|
||
it 'handles no matched headers' do | ||
headers = header_from_email("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single quotes please
Uses Mail gem to parse optional headers in the params hash.
Thanks, @svanderbleek |
Adds default headers Regexps and support for configuration to add more header
extracting Regexps. Updated documentation and test coverage. Allows
EmailParser to be injected through configuration similar to
EmailProcessor. Includes an example of this using the Mail gem.
Let me know what you think! Thanks.