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.

Sign up for GitHub

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

Jump to bottom

Account for namespaced mailers #8

Closed
licatajustin opened this issue Mar 18, 2016 · 6 comments
Closed

Account for namespaced mailers #8

licatajustin opened this issue Mar 18, 2016 · 6 comments

Comments

@licatajustin
Copy link
Contributor

We ran into a small issue running this with a mounted engine in our Rails app. The template_path wasn't accounting for the namespace.

template = /path/to/app/views/namespace/user_mailer/mailer_method.html.erb
template_path = template.inspect.split("/").slice(-2, 2).join("/")
# => user_mailer/mailer_method.html.erb

My forked version patches this by splitting the template at the "views" string, then keeps the rest of the path.

template = /path/to/app/views/namespace/user_mailer/mailer_method.html.erb
template_path = template.inspect.split("views")[1][1..-1]
# => namespace/user_mailer/mailer_method.html.erb

I will submit a Pull Request with the patch, please let me know if anything else is needed. I also noticed that all the tests were failing locally for me (prior to the patch). Is there something else needed to get them to pass...I see they passed their latest build on Travis.

Thanks for the gem! This gem really brightens up the sad state of HTML emails!
@billhorsman
Copy link
Owner

A patch would be wonderful, thanks!

What errors/failures are you getting from your tests?

@licatajustin
Copy link
Contributor Author

I get the following error for all 6 tests.

InlineStylesMailer Switch parts order text/plain first (default) should inline the CSS
     Failure/Error: case _layout

     ArgumentError:
       wrong number of arguments (0 for 1)
     # ./lib/inline_styles_mailer.rb:96:in `layout_to_use'
     # ./lib/inline_styles_mailer.rb:84:in `block (3 levels) in mail'
     # ./lib/inline_styles_mailer.rb:81:in `block (2 levels) in mail'
     # ./lib/inline_styles_mailer.rb:72:in `each'
     # ./lib/inline_styles_mailer.rb:72:in `block in mail'
     # ./lib/inline_styles_mailer.rb:60:in `mail'
     # ./spec/foo_mailer.rb:7:in `foo'

If I comment out that method, then 5/6 tests pass. The failing test is

1) InlineStylesMailer Inlining CSS Default CSS file should inline the CSS
     Failure/Error: mail.body.parts[1].body.should =~ /<p style="color: red;">Testing foo text\/html\.<\/p>/

       expected: /<p style="color: red;">Testing foo text\/html\.<\/p>/
            got: #<Mail::Body:0x007fcfd6c73460 @boundary=nil, @preamble=nil, @epilogue=nil, @charset="US-ASCII", @part_sort_order=["text/plain", "text/enriched", "text/html"], @parts=[], @raw_source="<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\" \"http://www.w3.org/TR/REC-html40/loose.dtd\">\n<html><body style=\"background: yellow;\"><p style=\"color: blue;\">Testing foo text/html.</p></body></html>\n", @encoding="7bit"> (using =~)
       Diff:
       @@ -1,2 +1,11 @@
       -/<p style="color: red;">Testing foo text\/html\.<\/p>/
       +#<Mail::Body:0x007fcfd6c73460
       + @boundary=nil,
       + @charset="US-ASCII",
       + @encoding="7bit",
       + @epilogue=nil,
       + @part_sort_order=["text/plain", "text/enriched", "text/html"],
       + @parts=[],
       + @preamble=nil,
       + @raw_source=
       +  "<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\" \"http://www.w3.org/TR/REC-html40/loose.dtd\">\n<html><body style=\"background: yellow;\"><p style=\"color: blue;\">Testing foo text/html.</p></body></html>\n">

     # ./spec/inline_styles_mailer/inline_styles_mailer_spec.rb:20:in `block (4 levels) in <top (required)>'

@billhorsman
Copy link
Owner

Damn. Using the _layout method of ActionView isn't very smart of me. It's liable to break without notice, which I think it does in Rails 5. See Rails commit face604. We might need to do some tests on that method before knowing how to call it reliably.

I don't believe that problem is related to your pull request but thanks for finding it :)

I'll keep this issue open until we fix that failing test for you in Rails 5.

@billhorsman
Copy link
Owner

Justin. Would you like to take a look at the rails5 branch? I think the tests should pass for you there. If they do, I'll fix it on master and close this issue.

@licatajustin
Copy link
Contributor Author

@billhorsman, sorry for the delayed response.

That fix did the trick for Rails 5. 👍

billhorsman added a commit that referenced this issue Mar 25, 2016
@billhorsman
Bump to version 1
5e5312a 
This is because we’ve fixed issue #8.
@billhorsman
Copy link
Owner

I've just released version 1.0.0. Thanks for your help, Justin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@billhorsman @licatajustin