[openstreetmap/openstreetmap-website] Dressed up notification mails (#1401)

[Tom Hughes]

tomhughes requested changes on this pull request.

A few more review comments. There's also a live version running at http://tomh.apis.dev.openstreetmap.org/ if people want to test things.

> +    link_to(
+      display_name,
+      user_url(display_name, :host => SERVER_URL),
+      :target => "_blank",
+      :style => "text-decoration: none; color: #222; font-weight: bold"
+    )
+  end
+
+  def message_body(&block)
+    render(
+      :partial => "message_body",
+      :locals => { :body => capture(&block) }
+    )
+  end
+
+  def apply_inline_css(html)

I think `apply_inline_css` is a bit too vague for the name - something like `style_message` would make it clearer what it does.

> @@ -185,5 +185,17 @@ def stub_hostip_requests
       stub_request(:get, "http://api.hostip.info/country.php?ip=0.0.0.0")
       stub_request(:get, "http://api.hostip.info/country.php?ip=127.0.0.1")
     end
+
+    def email_text_parts(message)
+      text_parts = []
+      message.parts.each do |part|

You could avoid initialising the array and then returning it by using `each_with_object` here. So something like:

```
message.parts.each_with_object([]) do |part,text_parts|
```

and then get rid of the first and last lines of the method.

>  
-<p><%= raw t 'notifier.changeset_comment_notification.details', :url => link_to(@changeset_url, @changeset_url) %></p>
+<% content_for :footer do %>
+  <p>
+    <%= raw t 'notifier.changeset_comment_notification.unsubscribe', :url => link_to(@changeset_url, @changeset_url, :style => "color: #222; white-space: nowrap") %>

If you're going to add completely new information to the message then it should really be added to the text version as well.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/1401#pullrequestreview-18976344
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20170128/dffb406f/attachment.html>