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

use SimpleDelegator #20

Closed

Conversation

gschammah
Copy link

Hi there,

I am making 2 small changes in order to fix some problems I had with the library.

  1. I am changing the MultiMail::Message::Base class to inherit from SimpleDelegator instead of inherit from Mail::Message. The reason behind this is that when an instance of MultiMail::Message::Base is created, the original Mail::Message is serialized into a string and then parsed back into another Mail::Message object. Apart of being more performant, this fixes a problem I was having with the encodings.

If you have some UTF-8 characters into the html part, when serialized is then converted into an US-ASCII string, and when parsed again and converted into a Mail::Message object, the html body is converted into a 7bit string.
If you then call JSON.dump and you pass that string, it fails saying it can't convert that unknown 7bit character into a UTF-8 one. Conserving the original Mail::Message solves this

  1. In MultiMail::Sender::Base I am changing the attr_reader for @settings into an attr_accessor, to maintain compatibility with the original delivery_methods of the Mail gem.

Thanks

@coveralls
Copy link

Coverage Status

Coverage decreased (-24.44%) when pulling f4a340e on gschammah:refactor/simple-delegator into 3990e3d on opennorth:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.92%) when pulling 1896628 on gschammah:refactor/simple-delegator into 3990e3d on opennorth:master.

@jpmckinney
Copy link
Owner

@gschammah For whatever reason, Travis is failing, but when I run the tests locally, they pass. I've committed your two changes as separate commits. I'll close this issue if they pass.

@jpmckinney
Copy link
Owner

Aha, apparently, pull requests don't pick up the secure variables on Travis. Tests are passing. Thanks!

@jpmckinney jpmckinney closed this Jan 8, 2014
@gschammah
Copy link
Author

thanks for including the changes! cheers

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

Successfully merging this pull request may close these issues.

None yet

3 participants