Skip to content

Handle also singlepart messsages - #5

Open
benningm wants to merge 1 commit into
geetotes:masterfrom
benningm:fix-singlepart-message
Open

Handle also singlepart messsages#5
benningm wants to merge 1 commit into
geetotes:masterfrom
benningm:fix-singlepart-message

Conversation

@benningm

Copy link
Copy Markdown

When a singlepart message is passed to the delivery method
an undefined method error is raised:

undefined method `body' for nil:NilClass>

This is because the message class expects the content to be a mime-part.
In case of a singlepart message the content is in the message body.

This will use the body on singlepart messages and the text_part for
multipart messages.
It also extends the tests to cover the changed code.

When a singlepart message is passed to the delivery method
an undefined method error is raised:

  undefined method `body' for nil:NilClass>

This is because the message class expects the content to be a mime-part.
In case of a singlepart message the content is in the message body.

This will use the body on singlepart messages and the text_part for
multipart messages.
It also extends the tests to cover the changed code.
@geetotes

Copy link
Copy Markdown
Owner

@benningm This looks good to me -- are you running this gem off your own branch? Looks like I've got some test stuff on my end to clean up, so I'd like to do this and merge your change in with that and release a new version

@benningm

Copy link
Copy Markdown
Author

The tests are broken. I fixed the existing tests. In my test runs the remaining error where 'pending' tests.

In delivery_method test i moved the parameters channel and webhook from the initializer to the right place in the mail.

In the message tests i added the .result() call to cover the templating and trigger the singlepart error in the tests.

I'm currently implementing slack messaging in one of my rails apps and wanted to use the action_mailer with your gem against a mattermost server (Slack API compatible).
I'll try use the gem from my modified branch for now.

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.

2 participants