markets / maily

📫 Rails Engine to preview emails in the browser
https://rubygems.org/gems/maily
MIT License
707 stars 31 forks source link

The edit is not working with heroku deployment #5

Closed melsatar closed 10 years ago

melsatar commented 10 years ago

Everything is working fine locally, but in heroku environment does not, do you know why?

markets commented 10 years ago

Ei @melsatar,

Thanks for trying. I didn't test the gem in heroku yet.

Only edition fails? Can you provide some error trace?

melsatar commented 10 years ago

The weird thing that, there is no errors and it saves the changes, but when i view them again, i cannot see the changes in the view screen, but they still can be viewed in edit view.

And, i tested to send the mail and it sent the old content not the new.

On Mon, May 12, 2014 at 7:57 PM, Marc Anguera Insa <notifications@github.com

wrote:

Ei @melsatar https://github.com/melsatar,

Thanks for trying. I didn't test the gem in heroku yet.

Only edition fails? Can you provide some error trace?

— Reply to this email directly or view it on GitHubhttps://github.com/markets/maily/issues/5#issuecomment-42865546 .

Mohamed Sami

markets commented 10 years ago

ah! I think this is because: Heroku boots app with production (at least, config.cache_classes = true) environment. In production, code is not reloaded between requests and code is eager-loaded. Since edition feature it's just a file (file system) edition, Rails doesn't take in account this change (without restarting the server). Into the Edit section it's visible because it's a directly File.read(email_template_path) (implementation).

Probably it's a good idea to add a note in the Readme. And, since allowing arbitrary ruby code evalution (<% User.destroy_all %>) is potentially dangerous, it would be safer to disable edition by default when booting production.

I implemented this feature with the idea to edit the emails easy in the browser, but just like a feature for development environment. It can be useful in some cases.

Any kind of suggestion or idea is welcome :+1: (stars are also welcome :smiley:).

melsatar commented 10 years ago

So, what the solution, actually I liked the gem and how easy it is, and I implemented it in production environment.

So, how can I fix that?

Mohamed Sami

-----Original Message----- From: "Marc Anguera Insa" notifications@github.com Sent: ‎5/‎12/‎2014 9:04 PM To: "markets/maily" maily@noreply.github.com Cc: "Mohamed Sami" melsatar@gmail.com Subject: Re: [maily] The edit is not working with heroku deployment (#5)

ah! I think this is because: Heroku boots app with production (at least, config.cache_classes = true) environment. In production, code is not reloaded between requests and code is eager-loaded. Since edition feature it's just a file (file system) edition, Rails doesn't take in account this change (without restarting the server). Into the Edit section it's visible because it's a directly File.read(email_template_path) (implementation). Probably it's a good idea to add a note in the Readme. And, since allowing arbitrary ruby code evalution (<% User.destroy_all %>) is potentially dangerous, it would be safer to disable edition by default when booting production. I implemented this feature with the idea to edit the emails easy in the browser, but just like a feature for development environment. It can be useful in some cases. Any kind of suggestion or idea is welcome (stars are also welcome ). — Reply to this email directly or view it on GitHub.

jrdi commented 10 years ago

@melsatar @markets before to try to fix it we should think on what are you doing if you edit these email templates on production environment. If you edit these template in production you are making changes to it outside of your repository, isn't it?

IMHO, edit in production environment is not a really good feature.

markets commented 10 years ago

I recommend you to disable edition in production (or staging; for environments where eager-load doesn't work). You can achieve this via the initializer (config/initializers/maily.rb):

Maily.setup do |config|
  ...
  config.allow_edition = Rails.env.production? ? false : true
  ...
end
melsatar commented 10 years ago

This feature for administrators only, what do you suggest as alternative?

Mohamed Sami

-----Original Message----- From: "Jordi Villar" notifications@github.com Sent: ‎5/‎12/‎2014 9:58 PM To: "markets/maily" maily@noreply.github.com Cc: "Mohamed Sami" melsatar@gmail.com Subject: Re: [maily] The edit is not working with heroku deployment (#5)

@melsatar @markets before to try to fix it we should think on what are you doing if you edit these email templates on production environment. If you edit these template in production you are making changes to it outside of your repository, isn't it? IMHO, edit in production environment is not a really good feature. — Reply to this email directly or view it on GitHub.

melsatar commented 10 years ago

So, this will make me lose an important feature

Mohamed Sami

-----Original Message----- From: "Marc Anguera Insa" notifications@github.com Sent: ‎5/‎12/‎2014 9:59 PM To: "markets/maily" maily@noreply.github.com Cc: "Mohamed Sami" melsatar@gmail.com Subject: Re: [maily] The edit is not working with heroku deployment (#5)

I recommend you to disable edition in production (or staging; environments with eager-load don't work for now). You can achieve this via the initializer (config/initializers/maily.rb): Maily.setup do |config| ... config.allow_edition = Rails.env.production? ? false : true ... end — Reply to this email directly or view it on GitHub.

melsatar commented 10 years ago

Also, the gem benefits in editing actually

— Sent from Mailbox

On Mon, May 12, 2014 at 10:02 PM, Mohamed Sami melsatar@gmail.com wrote:

So, this will make me lose an important feature Mohamed Sami -----Original Message----- From: "Marc Anguera Insa" notifications@github.com Sent: ‎5/‎12/‎2014 9:59 PM To: "markets/maily" maily@noreply.github.com Cc: "Mohamed Sami" melsatar@gmail.com Subject: Re: [maily] The edit is not working with heroku deployment (#5) I recommend you to disable edition in production (or staging; environments with eager-load don't work for now). You can achieve this via the initializer (config/initializers/maily.rb): Maily.setup do |config| ... config.allow_edition = Rails.env.production? ? false : true ... end — Reply to this email directly or view it on GitHub.

markets commented 10 years ago

@jrdi yes, all ruby code inside <% %> or <%= %> will be evaluated when rendering the email template, dangerous in production env.

To have a full safe edition mode, we would need, for example, a persisted (db) model, and some overrides of Rails (action_mailer) related behavior. I think this is out of Maily scope. The main goal is to preview emails easy into a web browser.

Sorry for any inconvenience, but at least for now, I recommend to disable it in production.

I'll update docs with some clarifications.

I'm totally opened to suggestions or discuss changes (code/docs).

melsatar commented 10 years ago

So, how I can administrator edit the mail and save the template, do you have a solution, other gem even if you want the rails model to be shown On May 12, 2014 10:20 PM, "Marc Anguera Insa" notifications@github.com wrote:

@jrdi https://github.com/jrdi yes, all ruby code inside <% %> or <%= %> will be evaluated when rendering the email template, dangerous in production env.

To have a full safe edition mode, we would need, for example, a persisted (db) model, and some overrides of Rails (action_mailer) related behavior. I think this is out of Maily scope. The main goal is to preview emails easy into a web browser.

Sorry for any inconvenience, but at least for now, I recommend to disable it in production.

I'll update docs with some clarifications.

I'm totally opened to suggestions or discuss changes (code/docs).

— Reply to this email directly or view it on GitHubhttps://github.com/markets/maily/issues/5#issuecomment-42882850 .

markets commented 10 years ago

We can implement a new feature like: 'Send new template to admin' => it sends an email to a configured email address (or addresses) with the new view file, or a git patch, ... as an attachment.

What do you think @melsatar @jrdi ? Could be really useful? I can't see risk here.

melsatar commented 10 years ago

where is the attachment ?

markets commented 10 years ago

I mean, send an email with the template changes as an attached file to a pre-configured email addresses (admins or developers). Then this people should download the changes an apply it to app. In that way, developer are able to review the changes (no risk) and deploy it to production.

It's just a proposal and I think it could be useful.

Button label proposals:

melsatar commented 10 years ago

So, do you know how to modify your code to make the code saved in database ?

Mohamed Sami

-----Original Message----- From: "Marc Anguera Insa" notifications@github.com Sent: ‎5/‎14/‎2014 10:54 AM To: "markets/maily" maily@noreply.github.com Cc: "Mohamed Sami" melsatar@gmail.com Subject: Re: [maily] The edit is not working with heroku deployment (#5)

I mean, send an email with the template changes as an attached file to a pre-configured email addresses (admins or developers). Then this people should download the changes an apply it to app. In that way, developer are able to review the changes (no risk) and deploy it to production. It's just a proposal and I think it could be useful. Button label proposals: Notify changes to admin/developer Send new template to admin/developer — Reply to this email directly or view it on GitHub.

markets commented 10 years ago

It implies a very big big change (totally rewrite the gem) and some Rails behaviour overrides. It is out of Maily scope man (edition feature was designed to easy change templates in development). That's why I proposed another solution.

Of course, you can fork the project and try to implement these changes :+1: (and even send a PR to discuss it).

Maily is mostly a tool to help you previewing emails in the browser (+ some extra friendly features), a tool to help you to work with Rails emails, not an emails CMS.