From 01e43a99e73e4610762722ff7fb1a38ab9d634c4 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Mon, 27 Aug 2018 14:29:53 -0400 Subject: [PATCH] Fix /connect bug and add email-if-inactive functionality (#482) * Add basic functionality for connect message emails * Fix lint * Add connect messages email permission --- .../email_subscriptions_controller.rb | 1 + app/controllers/users_controller.rb | 1 + app/mailers/notify_mailer.rb | 7 ++++ app/models/chat_channel.rb | 14 +++++-- app/models/chat_channel_membership.rb | 2 +- app/models/message.rb | 26 +++++++++++- app/policies/user_policy.rb | 1 + .../notify_mailer/new_mention_email.html.erb | 2 +- .../notify_mailer/new_message_email.html.erb | 10 +++++ .../notify_mailer/new_message_email.text.erb | 3 ++ .../notify_mailer/new_reply_email.html.erb | 2 +- app/views/users/_notifications.html.erb | 4 ++ ...4411_add_email_unread_messages_to_users.rb | 5 +++ db/schema.rb | 7 +++- .../mailers/previews/notify_mailer_preview.rb | 4 ++ spec/models/message_spec.rb | 42 +++++++++++++++++++ 16 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 app/views/mailers/notify_mailer/new_message_email.html.erb create mode 100644 app/views/mailers/notify_mailer/new_message_email.text.erb create mode 100644 db/migrate/20180826174411_add_email_unread_messages_to_users.rb diff --git a/app/controllers/email_subscriptions_controller.rb b/app/controllers/email_subscriptions_controller.rb index 66e6f7917..0b0036e86 100644 --- a/app/controllers/email_subscriptions_controller.rb +++ b/app/controllers/email_subscriptions_controller.rb @@ -20,6 +20,7 @@ class EmailSubscriptionsController < ApplicationController email_comment_notifications: "comment notifications", email_follower_notifications: "follower notifications", email_mention_notifications: "mention notifications", + email_connect_messages: "connect messages", email_unread_notifications: "unread notifications", } emails_type[given_email_type] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index ce277b360..5dfb3a144 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -21,6 +21,7 @@ class UsersController < ApplicationController @tab_list = @user.settings_tab_list @tab = params["user"]["tab"] || "profile" authorize @user + # raise permitted_attributes(@user).to_s if @user.update(permitted_attributes(@user)) RssReader.new.delay.fetch_user(@user) if @user.feed_url.present? notice = "Your profile was successfully updated." diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index 3fc31a9f7..e0310a785 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -55,6 +55,13 @@ class NotifyMailer < ApplicationMailer mail(to: @user.email, subject: "Thank you for your report") end + def new_message_email(message) + @message = message + @user = message.direct_receiver + subject = "#{message.user.name} just messaged you" + mail(to: @user.email, subject: subject) + end + def reporter_resolution_email(report) @feedback_message = report @user = report.reporter diff --git a/app/models/chat_channel.rb b/app/models/chat_channel.rb index 582e99488..8fe6bbf9c 100644 --- a/app/models/chat_channel.rb +++ b/app/models/chat_channel.rb @@ -33,6 +33,14 @@ class ChatChannel < ApplicationRecord channel_type == "open" end + def direct? + channel_type == "direct" + end + + def invite_only? + channel_type == "invite_only" + end + def clear_channel messages.each(&:destroy!) Pusher.trigger(pusher_channels, "channel-cleared", { chat_channel_id: id }.to_json) @@ -84,9 +92,9 @@ class ChatChannel < ApplicationRecord end def pusher_channels - if channel_type == "invite_only" + if invite_only? "presence-channel-#{id}" - elsif channel_type == "open" + elsif open? "open-channel-#{id}" else chat_channel_memberships.pluck(:user_id).map { |id| "private-message-notifications-#{id}" } @@ -95,7 +103,7 @@ class ChatChannel < ApplicationRecord def adjusted_slug(user = nil, caller_type = "reciever") user ||= current_user - if channel_type == "direct" && caller_type == "reciever" + if direct? && caller_type == "reciever" "@" + slug.gsub("/#{user.username}", "").gsub("#{user.username}/", "") elsif caller_type == "sender" "@" + user.username diff --git a/app/models/chat_channel_membership.rb b/app/models/chat_channel_membership.rb index d5505855c..54b28aa8f 100644 --- a/app/models/chat_channel_membership.rb +++ b/app/models/chat_channel_membership.rb @@ -11,7 +11,7 @@ class ChatChannelMembership < ApplicationRecord private def permission - if chat_channel.channel_type == "direct" && chat_channel.slug.split("/").exclude?(user.username) + if chat_channel.direct? && chat_channel.slug.split("/").exclude?(user.username) errors.add(:user_id, "is not allowed in chat") end # To be possibly implemented in future diff --git a/app/models/message.rb b/app/models/message.rb index bc6666649..f142e7024 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -8,6 +8,7 @@ class Message < ApplicationRecord before_save :determine_user_validity before_validation :evaluate_markdown + after_create :send_email_if_appropriate after_create :update_chat_channel_last_message_at after_create :update_all_has_unopened_messages_statuses @@ -40,6 +41,11 @@ class Message < ApplicationRecord end end + def direct_receiver + return if chat_channel.channel_type != "direct" + chat_channel.users.where.not(id: user.id).first + end + private def update_chat_channel_last_message_at @@ -67,7 +73,7 @@ class Message < ApplicationRecord doc = Nokogiri::HTML(html) rich_style = "border: 1px solid #0a0a0a; border-radius: 3px; padding: 8px;" doc.css("a").each do |a| - if a["href"].include?("//#{ApplicationConfig['APP_DOMAIN']}/") && article = Article.find_by_slug(a["href"].split("/")[4].split("?")[0]) + if article = rich_link_article(a) html = html + "

@@ -94,4 +100,22 @@ class Message < ApplicationRecord membership = sub.user.chat_channel_memberships.order("last_opened_at DESC").first membership.last_opened_at > 40.seconds.ago end + + def rich_link_article(a) + if a["href"].include?("//#{ApplicationConfig['APP_DOMAIN']}/") && a["href"].split("/")[4] + Article.find_by_slug(a["href"].split("/")[4].split("?")[0]) + end + end + + def send_email_if_appropriate + recipient = direct_receiver + return if !chat_channel.direct? || + recipient.updated_at > 2.hours.ago || + recipient.chat_channel_memberships.order("last_opened_at DESC"). + first.last_opened_at > 18.hours.ago || + chat_channel.last_message_at > 45.minutes.ago || + recipient.email_connect_messages == false + + NotifyMailer.new_message_email(self).deliver + end end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index fda6cd21f..741f6c913 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -56,6 +56,7 @@ class UserPolicy < ApplicationPolicy email_follower_notifications email_membership_newsletter email_mention_notifications + email_connect_messages email_newsletter email_public email_unread_notifications diff --git a/app/views/mailers/notify_mailer/new_mention_email.html.erb b/app/views/mailers/notify_mailer/new_mention_email.html.erb index ce73da01b..30eef5ee1 100644 --- a/app/views/mailers/notify_mailer/new_mention_email.html.erb +++ b/app/views/mailers/notify_mailer/new_mention_email.html.erb @@ -3,5 +3,5 @@ <%= @mentionable.processed_html.html_safe %> - read & reply + view on dev.to diff --git a/app/views/mailers/notify_mailer/new_message_email.html.erb b/app/views/mailers/notify_mailer/new_message_email.html.erb new file mode 100644 index 000000000..16bc0209c --- /dev/null +++ b/app/views/mailers/notify_mailer/new_message_email.html.erb @@ -0,0 +1,10 @@ +

<%= link_to(@message.user.name, "https://dev.to/#{@message.user.username}") %> sent you a message on DEV Connect

+
+
+ <%= @message.message_html %> +

+ view on dev.to +
+
+ +You only get emails like this if you have not been active on dev.to/connect in the past day. You may disable this type of email in your settings. \ No newline at end of file diff --git a/app/views/mailers/notify_mailer/new_message_email.text.erb b/app/views/mailers/notify_mailer/new_message_email.text.erb new file mode 100644 index 000000000..7b13a395a --- /dev/null +++ b/app/views/mailers/notify_mailer/new_message_email.text.erb @@ -0,0 +1,3 @@ +<%= ActionController::Base.helpers.strip_tags(@message.message_html.html_safe) %> + +View now: https://dev.to/connect/@<%= @message.user.username %> diff --git a/app/views/mailers/notify_mailer/new_reply_email.html.erb b/app/views/mailers/notify_mailer/new_reply_email.html.erb index d48c137c1..34a874b41 100644 --- a/app/views/mailers/notify_mailer/new_reply_email.html.erb +++ b/app/views/mailers/notify_mailer/new_reply_email.html.erb @@ -6,5 +6,5 @@

re: <%= @comment.commentable.title %>

<%= @comment.processed_html.html_safe %> - read & reply read original post + view on dev.to read original post diff --git a/app/views/users/_notifications.html.erb b/app/views/users/_notifications.html.erb index 4d5291cad..b563c3c38 100644 --- a/app/views/users/_notifications.html.erb +++ b/app/views/users/_notifications.html.erb @@ -31,6 +31,10 @@ <%= f.check_box :email_badge_notifications %> <%= f.label :email_badge_notifications, "Send me an email when I receive a badge" %> +
+ <%= f.check_box :email_connect_messages %> + <%= f.label :email_connect_messages, "Send me an email when I receive a direct message (while inactive)" %> +
<%= f.check_box :email_unread_notifications %> <%= f.label :email_unread_notifications, "Send me occasional reminders that I have unread notifications on dev.to" %> diff --git a/db/migrate/20180826174411_add_email_unread_messages_to_users.rb b/db/migrate/20180826174411_add_email_unread_messages_to_users.rb new file mode 100644 index 000000000..607f2c186 --- /dev/null +++ b/db/migrate/20180826174411_add_email_unread_messages_to_users.rb @@ -0,0 +1,5 @@ +class AddEmailUnreadMessagesToUsers < ActiveRecord::Migration[5.1] + def change + add_column :users, :email_connect_messages, :boolean, default:true + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index 40b9fbf0d..b3e1b8d4b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180806142338) do +ActiveRecord::Schema.define(version: 20180826174411) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -378,6 +378,10 @@ ActiveRecord::Schema.define(version: 20180806142338) do create_table "messages", force: :cascade do |t| t.bigint "chat_channel_id", null: false t.datetime "created_at", null: false + t.text "encrypted_message_html" + t.text "encrypted_message_html_iv" + t.text "encrypted_message_markdown" + t.text "encrypted_message_markdown_iv" t.string "message_html", null: false t.string "message_markdown", null: false t.datetime "updated_at", null: false @@ -632,6 +636,7 @@ ActiveRecord::Schema.define(version: 20180806142338) do t.string "email", default: "", null: false t.boolean "email_badge_notifications", default: true t.boolean "email_comment_notifications", default: true + t.boolean "email_connect_messages", default: true t.boolean "email_digest_periodic", default: true, null: false t.boolean "email_follower_notifications", default: true t.boolean "email_membership_newsletter", default: false diff --git a/spec/mailers/previews/notify_mailer_preview.rb b/spec/mailers/previews/notify_mailer_preview.rb index a2a74b030..f37d8ca9c 100644 --- a/spec/mailers/previews/notify_mailer_preview.rb +++ b/spec/mailers/previews/notify_mailer_preview.rb @@ -27,4 +27,8 @@ class NotifyMailerPreview < ActionMailer::Preview def new_report_email NotifyMailer.new_report_email(FeedbackMessage.first) end + + def new_message_email + NotifyMailer.new_message_email(Message.last) + end end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 80dac8164..fe14f29cf 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -38,4 +38,46 @@ RSpec.describe Message, type: :model do message_markdown: "hello") expect(message).to be_valid end + + it "creates rich link in connect with proper link" do + article = create(:article) + message = create(:message, chat_channel_id: chat_channel.id, user_id: user.id, + message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}#{article.path}") + expect(message.message_html).to include(article.title) + expect(message.message_html).to include("data-content") + end + + it "creates rich link in connect with non-rich link" do + message = create(:message, chat_channel_id: chat_channel.id, user_id: user.id, + message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") + expect(message.message_html).not_to include("data-content") + end + + it "sends email if user not recently active on /connect" do + chat_channel.add_users([user, user2]) + chat_channel.update_column(:channel_type, "direct") + user2.update_column(:updated_at, 1.day.ago) + user2.chat_channel_memberships.last.update_column(:last_opened_at, 2.days.ago) + create(:message, chat_channel_id: chat_channel.id, user_id: user.id, + message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") + expect(EmailMessage.last.subject).to start_with("#{user.name} just messaged you") + end + + it "does not send email if user has been recently active" do + chat_channel.add_users([user, user2]) + create(:message, chat_channel_id: chat_channel.id, user_id: user.id, + message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") + expect(EmailMessage.all.size).to eq(0) + end + + it "does not send email if user has email_messages turned off" do + chat_channel.add_users([user, user2]) + chat_channel.update_column(:channel_type, "direct") + user2.update_column(:updated_at, 1.day.ago) + user2.update_column(:email_connect_messages, false) + user2.chat_channel_memberships.last.update_column(:last_opened_at, 2.days.ago) + create(:message, chat_channel_id: chat_channel.id, user_id: user.id, + message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") + expect(EmailMessage.all.size).to eq(0) + end end