Send email notifications for replies (#62)

cbothner · web-flow · commit 02f9f1598e3c · 2017-07-06T12:00:54.000-04:00
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,4 +1,6 @@
 AllCops:
   TargetRubyVersion: 2.3
+Style/AsciiComments:
+  Enabled: false
 Security/YAMLLoad:
   Enabled: false
diff --git a/app/assets/stylesheets/forms.sass b/app/assets/stylesheets/forms.sass
@@ -16,7 +16,7 @@ select, input, textarea, button
   font-family: $sansFont
 
 .edit_reader, .new_reader
-  label
+  label:not(.notification)
     font: 600 80% $sansFont
     text-transform: uppercase
     letter-spacing: 0.05em
@@ -34,7 +34,7 @@ select, input, textarea, button
   .pt-checkbox
     margin: 5px 0 0 0
 
-  .pt-control
+  .pt-control:not(.notification)
     text-transform: uppercase
 
   .actions
diff --git a/app/controllers/readers_controller.rb b/app/controllers/readers_controller.rb
@@ -85,7 +85,7 @@ def reader_params
       @reader_can_set_password = @reader && !@reader.created_password
     end
 
-    permitted = %i[name initials email locale]
+    permitted = %i[name initials email locale send_reply_notifications]
     permitted << :password if @reader_can_set_password
 
     params.require(:reader).permit(*permitted)
diff --git a/app/helpers/comment_threads_helper.rb b/app/helpers/comment_threads_helper.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module CommentThreadsHelper
+  def comment_thread_url(locale, comment_thread)
+    "#{case_url(locale, comment_thread.card.case.slug)}" \
+      "/#{comment_thread.card.element.case_element.position}" \
+      "/cards/#{comment_thread.card_id}/comments"
+  end
+end
diff --git a/app/jobs/reply_notification_broadcast_job.rb b/app/jobs/reply_notification_broadcast_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class NotificationBroadcastJob < ActiveJob::Base
+class ReplyNotificationBroadcastJob < ActiveJob::Base
   queue_as :default
 
   def perform(notification)
@@ -12,7 +12,6 @@ def perform(notification)
   private
 
   def render_notification(notification)
-    ApplicationController.renderer.render partial: 'notifications/notification',
-                                          locals: { notification: notification }
+    ApplicationController.renderer.render notification
   end
 end
diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb
@@ -2,7 +2,9 @@
 
 class ApplicationMailer < ActionMailer::Base
   helper :application
-  default from: 'Michigan Sustainability Cases <hello@learnmsc.org>'
+
+  FROM_ADDRESS = 'hello@learnmsc.org'
+  default from: "Michigan Sustainability Cases <#{FROM_ADDRESS}>"
 
   layout 'mailer'
 end
diff --git a/app/mailers/reply_notification_mailer.rb b/app/mailers/reply_notification_mailer.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+class ReplyNotificationMailer < ApplicationMailer
+  helper :cases
+  helper :comment_threads
+
+  def notify(notification)
+    @notification = notification
+    reader = @notification.reader
+    return unless reader.send_reply_notifications
+
+    mail(to: reader.name_and_email,
+         from: from_header,
+         subject: subject_header) do |format|
+      format.text
+      format.html
+    end
+  end
+
+  private
+
+  # Build the from header. We’re setting the from name to the name of the
+  # reader whose comment triggered the notification, but the from address
+  # remains our notification address so as not to trip spam filters
+  def from_header
+    "#{@notification.notifier.name} <#{ApplicationMailer::FROM_ADDRESS}>"
+  end
+
+  # Build the email subject as follows
+  # RE: [National Adaptation] “I understand that Ethiopia would be a...” (# 46)
+  #
+  # Every notification of a reply to the same original comment will have the
+  # same subject so that they can be threaded
+  def subject_header
+    comment_thread = @notification.comment_thread
+    "RE: [#{@notification.case.kicker}] " \
+      "“#{comment_thread.comments.first.content.truncate(40)}” " \
+      "(##{comment_thread.id})"
+  end
+end
diff --git a/app/models/comment.rb b/app/models/comment.rb
@@ -6,7 +6,7 @@ class Comment < ApplicationRecord
 
   translates :content
 
-  acts_as_list scope: :comment_thread
+  default_scope { order :created_at }
 
   validates :content, presence: :true
 
@@ -20,18 +20,17 @@ def timestamp
 
   private
 
-  def notification_data
-    { notifier_id: reader.id,
-      comment_thread_id: comment_thread.id,
-      card_id: comment_thread.card.id,
-      case_id: comment_thread.card.case.id,
-      page_id: comment_thread.card.element.id }
-  end
-
   def send_notifications_of_reply
-    (comment_thread.collocutors - [reader]).each do |r|
-      Notification.create reader: r, category: :reply_to_thread,
-                          data: notification_data
+    card = comment_thread.card
+    comment_thread.collocutors.each do |other_reader|
+      next if other_reader == reader
+      ReplyNotification.create reader: other_reader,
+                               notifier: reader,
+                               comment: self,
+                               comment_thread: comment_thread,
+                               card: card,
+                               case: card.case,
+                               page: card.element
     end
   end
 end
diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb
@@ -6,7 +6,6 @@ class CommentThread < ApplicationRecord
   belongs_to :reader
   belongs_to :group
   belongs_to :card
-  belongs_to :case
   has_many :comments, dependent: :restrict_with_error
 
   def collocutors
diff --git a/app/models/notification.rb b/app/models/notification.rb
diff --git a/app/models/reply_notification.rb b/app/models/reply_notification.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class ReplyNotification < ApplicationRecord
+  belongs_to :reader
+  belongs_to :notifier, class_name: 'Reader'
+  belongs_to :comment
+  belongs_to :comment_thread
+  belongs_to :case
+  belongs_to :page
+  belongs_to :card
+
+  after_create_commit { ReplyNotificationBroadcastJob.perform_now self }
+  after_create_commit { ReplyNotificationMailer.notify(self).deliver }
+
+  def message
+    I18n.t 'notifications.replied_to_your_comment',
+           notifier: notifier.name,
+           case_kicker: self.case.kicker
+  end
+end
diff --git a/app/views/layouts/mailer.text.erb b/app/views/layouts/mailer.text.erb
@@ -1,4 +1,5 @@
-<%= sanitize yield, tags: [] %>
+<% if content_for? :headline %># <%= content_for :headline %>
+<% end %><%= sanitize yield, tags: [] %>
 
 --
 <% if content_for? :email_footer %>
diff --git a/app/views/notifications/_notification.json.jbuilder b/app/views/notifications/_notification.json.jbuilder
diff --git a/app/views/readers/_form.html.haml b/app/views/readers/_form.html.haml
@@ -18,6 +18,14 @@
       = t 'activerecord.attributes.user.email'
       = f.email_field :email, class: "pt-input pt-fill", tabindex: 3
 
+    %h2= t '.notification_settings'
+    %div{ style: 'padding-bottom: 1em'}
+
+      = f.label :send_reply_notifications, class: 'notification pt-control pt-checkbox' do
+        = t 'activerecord.attributes.reader.send_reply_notifications'
+        = f.check_box :send_reply_notifications
+        %span.pt-control-indicator
+
     .actions
       = f.submit t('.save'), class: "o-button pt-button pt-intent-primary"
 
diff --git a/app/views/reply_notification_mailer/notify.markerb b/app/views/reply_notification_mailer/notify.markerb
@@ -0,0 +1,16 @@
+<% headline one_liner "#{@notification.notifier.name} replied to your comment" %>
+<% background_image_url ix_cover_image @notification.case, :email %>
+
+<%= @notification.comment.content %>
+
+<%= md_button_to "Reply online",
+                 comment_thread_url(I18n.locale, @notification.comment_thread) %>
+
+<% email_footer <<-FOOTER
+You are receiving this email because someone replied to a comment you made on a
+Michigan Sustainability Case you’re studying. If you do not want to receive
+emails like this, you can [change your notification settings](#{
+  edit_reader_url @notification.reader.locale, @notification.reader
+}).
+FOOTER
+%>
diff --git a/app/views/reply_notifications/_reply_notification.json.jbuilder b/app/views/reply_notifications/_reply_notification.json.jbuilder
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+json.key_format! camelize: :lower
+
+json.extract! reply_notification, :id, :message, :card_id, :comment_thread_id
+json.notifier do
+  json.extract! reply_notification.notifier, :id, :name, :initials
+end
+json.case do
+  json.extract! reply_notification.case, :slug, :kicker
+end
+json.element do
+  json.extract! reply_notification.page.case_element, :position
+end
diff --git a/circle.yml b/circle.yml
@@ -5,9 +5,6 @@ dependencies:
   override:
     - yarn
     - bundle install
-compile:
-  override:
-    - RAILS_ENV=test rails webpacker:compile
 test:
   override:
     - bundle exec rspec -r rspec_junit_formatter --format progress --format RspecJunitFormatter -o $CIRCLE_TEST_REPORTS/rspec/junit.xml:
diff --git a/config/locales/translations/en.yml b/config/locales/translations/en.yml
@@ -95,6 +95,7 @@ en:
         reset_password_token: Reset password token  #g
         uid: UID  #g
         initials: Initials
+        send_reply_notifications: Notify me when someone replies to a comment I wrote
 
       question:
         content: Content
diff --git a/db/migrate/20170705153826_add_reply_notification_flag_to_reader.rb b/db/migrate/20170705153826_add_reply_notification_flag_to_reader.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddReplyNotificationFlagToReader < ActiveRecord::Migration[5.0]
+  def change
+    add_column :readers, :send_reply_notifications, :boolean, default: true
+  end
+end
diff --git a/db/migrate/20170705164751_create_reply_notifications.rb b/db/migrate/20170705164751_create_reply_notifications.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class CreateReplyNotifications < ActiveRecord::Migration[5.0]
+  def change
+    create_table :reply_notifications do |t|
+      t.references :reader
+
+      t.integer :notifier_id
+      t.integer :comment_id
+      t.integer :comment_thread_id
+      t.integer :case_id
+      t.integer :page_id
+      t.integer :card_id
+    end
+  end
+end
diff --git a/db/migrate/20170705165135_drop_notifications.rb b/db/migrate/20170705165135_drop_notifications.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class DropNotifications < ActiveRecord::Migration[5.0]
+  def change
+    drop_table :notifications do |t|
+      t.boolean :email_sent
+      t.boolean :read
+      t.references :reader, foreign_key: true
+      t.integer :category
+      t.jsonb :data
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/spec/features/leaving_a_comment_spec.rb b/spec/features/leaving_a_comment_spec.rb
