Skip to content

Don't assume ActionMailer is available#608

Merged
justin808 merged 1 commit intoshakacode:masterfrom
tuzz:dont-assume-actionmailer
Nov 30, 2016
Merged

Don't assume ActionMailer is available#608
justin808 merged 1 commit intoshakacode:masterfrom
tuzz:dont-assume-actionmailer

Conversation

@tuzz
Copy link
Copy Markdown
Contributor

@tuzz tuzz commented Nov 17, 2016

If the Rails app that uses this gem doesn't include the
ActionMailer railtie, the gem raises this error:

uninitialized constant ReactOnRailsHelper::ActionMailer

This change introduces a check that the ActionMailer
constant is defined before setting the 'inMailer' option.

I'm not quite sure how to test this, so if anyone has
ideas, that would be helpful.


This change is Reviewable

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.08%) to 83.069% when pulling 4003930 on tuzz:dont-assume-actionmailer into 3a84802 on shakacode:master.

@justin808
Copy link
Copy Markdown
Member

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


app/helpers/react_on_rails_helper.rb, line 405 at r1 (raw file):

  def in_mailer?
    return unless controller.present?

this is not easy to read

please return a specific value

return false unless controller.present?

Comments from Reviewable

@justin808
Copy link
Copy Markdown
Member

please update per my suggestion


Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

If the Rails app that uses this gem doesn't include the
ActionMailer railtie, the gem raises this error:

`uninitialized constant ReactOnRailsHelper::ActionMailer`

This change introduces a check that the ActionMailer
constant is defined before setting the 'inMailer' option.

I'm not quite sure how to test this, so if anyone has
ideas, that would be helpful.
@tuzz
Copy link
Copy Markdown
Contributor Author

tuzz commented Nov 18, 2016

@justin808: updated as per your suggestion

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.08%) to 83.069% when pulling 396a1ed on tuzz:dont-assume-actionmailer into 3a84802 on shakacode:master.

@justin808
Copy link
Copy Markdown
Member

:lgtm:

Please provide a changelog entry: https://github.com/shakacode/react_on_rails/blob/master/CONTRIBUTING.md#summary


Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants