Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

[#33] Parsed headers support #34

Merged
merged 1 commit into from
Mar 18, 2013

Conversation

svanderbleek
Copy link
Contributor

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.

@svanderbleek
Copy link
Contributor Author

@calebthompson thoughts?

@@ -39,6 +41,18 @@ def self.extract_reply_body(body)
end
end

def self.extract_headers(raw_headers)
Copy link
Contributor

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 :trollface: 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.

Copy link
Contributor Author

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
Copy link
Contributor

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.

@calebhearth
Copy link
Contributor

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.

2.0.0-p0 :001 > require 'mail'
 => true 
2.0.0-p0 :002 > Mail::Header.new('')
 => #<Mail::Header:0x007fe037c1ba50 @errors=[], @charset=nil, @raw_source="", @unfolded_header="", @fields=[]> 
2.0.0-p0 :003 > Mail::Header.new(nil)
 => #<Mail::Header:0x007fe037c238b8 @errors=[], @charset=nil, @raw_source=""> 
2.0.0-p0 :004 > Mail::Header.new(nil).fields
 => [] 

@calebhearth
Copy link
Contributor

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("")
Copy link
Contributor

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.
@calebhearth
Copy link
Contributor

Thanks, @svanderbleek

@calebhearth calebhearth merged commit 3552660 into thoughtbot:master Mar 18, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants