From 208bbb37575f4076615a570ebcbf0db39d77e7a6 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 27 Aug 2019 21:54:09 -0400 Subject: [PATCH] Improve mod roundrobin notification and internal tooling (#3855) * Improve mod roundrobin notification and internal tooling * Fix tests --- app/assets/stylesheets/notifications.scss | 1 + app/controllers/internal/mods_controller.rb | 20 +++++++ .../moderation_notification_job.rb | 8 +-- app/labor/assign_tag_moderator.rb | 4 ++ app/mailers/notify_mailer.rb | 6 +++ app/models/comment.rb | 2 +- .../moderator/manage_activity_and_roles.rb | 3 ++ app/services/notifications/moderation.rb | 4 +- .../tag_adjustment_notification/send.rb | 1 - app/views/internal/mods/index.html.erb | 51 ++++++++++++++++++ .../notify_mailer/trusted_role_email.html.erb | 20 +++++++ .../notify_mailer/trusted_role_email.text.erb | 13 +++++ app/views/notifications/_comment.html.erb | 14 +++-- .../shared/_comment_box.html.erb | 6 ++- app/views/users/_notifications.html.erb | 9 ++++ config/routes.rb | 1 + ...27163358_add_mod_notifications_to_users.rb | 5 ++ db/schema.rb | 3 +- spec/mailers/notify_mailer_spec.rb | 26 ++++++++++ .../mailers/previews/notify_mailer_preview.rb | 4 ++ spec/requests/internal/mods_spec.rb | 33 ++++++++++++ spec/requests/notifications_spec.rb | 52 ++++++++++++++++++- .../tag_adjustment_notification/send_spec.rb | 12 ----- 23 files changed, 270 insertions(+), 28 deletions(-) create mode 100644 app/controllers/internal/mods_controller.rb create mode 100644 app/views/internal/mods/index.html.erb create mode 100644 app/views/mailers/notify_mailer/trusted_role_email.html.erb create mode 100644 app/views/mailers/notify_mailer/trusted_role_email.text.erb create mode 100644 db/migrate/20190827163358_add_mod_notifications_to_users.rb create mode 100644 spec/requests/internal/mods_spec.rb diff --git a/app/assets/stylesheets/notifications.scss b/app/assets/stylesheets/notifications.scss index b5031f7f3..63075e8f1 100644 --- a/app/assets/stylesheets/notifications.scss +++ b/app/assets/stylesheets/notifications.scss @@ -44,6 +44,7 @@ } .content { padding-bottom: 18px; + line-height: 1.35em; &.notification-content { a { color: $sky-blue; diff --git a/app/controllers/internal/mods_controller.rb b/app/controllers/internal/mods_controller.rb new file mode 100644 index 000000000..d08baa064 --- /dev/null +++ b/app/controllers/internal/mods_controller.rb @@ -0,0 +1,20 @@ +class Internal::ModsController < Internal::ApplicationController + layout "internal" + + def index + @mods = if params[:state] == "tag" + User.with_role(:tag_moderator).page(params[:page]).per(50) + elsif params[:state] == "potential" + User.without_role(:trusted).order("comments_count DESC").page(params[:page]).per(50) + else + User.with_role(:trusted).page(params[:page]).per(50) + end + @mods = @mods.where("users.username ILIKE :search OR users.name ILIKE :search", search: "%#{params[:search]}%") if params[:search].present? + end + + def update + @user = User.find(params[:id]) + AssignTagModerator.add_trusted_role(@user) + redirect_to "/internal/mods" + end +end diff --git a/app/jobs/notifications/moderation_notification_job.rb b/app/jobs/notifications/moderation_notification_job.rb index c42f4ab73..7d6c50e9a 100644 --- a/app/jobs/notifications/moderation_notification_job.rb +++ b/app/jobs/notifications/moderation_notification_job.rb @@ -3,14 +3,16 @@ module Notifications queue_as :send_moderation_notification def perform(notifiable_id, service = Notifications::Moderation::Send) - random_moderator = Notifications::Moderation.available_moderators.order(Arel.sql("RANDOM()")).first - return unless random_moderator + random_moderators = Notifications::Moderation.available_moderators.order(Arel.sql("RANDOM()")).first(2) + return unless random_moderators.any? # notifiable is currently only comment notifiable = Comment.find_by(id: notifiable_id) return unless notifiable - service.call(random_moderator, notifiable) + random_moderators.each do |mod| + service.call(mod, notifiable) + end end end end diff --git a/app/labor/assign_tag_moderator.rb b/app/labor/assign_tag_moderator.rb index b583c32fb..b27efb3e0 100644 --- a/app/labor/assign_tag_moderator.rb +++ b/app/labor/assign_tag_moderator.rb @@ -1,8 +1,12 @@ module AssignTagModerator def self.add_trusted_role(user) + return if user.has_role?(:trusted) + user.add_role :trusted user.update(email_community_mod_newsletter: true) MailchimpBot.new(user).manage_community_moderator_list + Rails.cache.delete("user-#{user.id}/has_trusted_role") + NotifyMailer.trusted_role_email(user).deliver end def self.add_to_chat_channel(user) diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index 229e41df3..65be0d8b1 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -87,4 +87,10 @@ class NotifyMailer < ApplicationMailer subject = "Congrats! You're the moderator for ##{tag_name}" mail(to: @user.email, subject: subject) end + + def trusted_role_email(user) + @user = user + subject = "You've been upgraded to #{ApplicationConfig['COMMUNITY_NAME']} Community mod status!" + mail(to: @user.email, subject: subject) + end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 9d068cd97..02eec6504 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -207,7 +207,7 @@ class Comment < ApplicationRecord end def send_to_moderator - return if user && user.comments_count > 10 + return if user && user.comments_count > 2 Notification.send_moderation_notification(self) end diff --git a/app/services/moderator/manage_activity_and_roles.rb b/app/services/moderator/manage_activity_and_roles.rb index 71c78ca60..e94b59ba5 100644 --- a/app/services/moderator/manage_activity_and_roles.rb +++ b/app/services/moderator/manage_activity_and_roles.rb @@ -133,8 +133,11 @@ module Moderator end def add_trusted_role + return if user.has_role?(:trusted) + user.add_role :trusted user.update(email_community_mod_newsletter: true) + NotifyMailer.trusted_role_email(user).deliver MailchimpBot.new(user).manage_community_moderator_list end diff --git a/app/services/notifications/moderation.rb b/app/services/notifications/moderation.rb index 8ab03dc76..81b8cd498 100644 --- a/app/services/notifications/moderation.rb +++ b/app/services/notifications/moderation.rb @@ -1,10 +1,10 @@ module Notifications module Moderation - MODERATORS_AVAILABILITY_DELAY = 28.hours + MODERATORS_AVAILABILITY_DELAY = 22.hours SUPPORTED = [Comment].freeze def self.available_moderators - User.with_role(:trusted).where("last_moderation_notification < ?", MODERATORS_AVAILABILITY_DELAY.ago) + User.with_role(:trusted).where("last_moderation_notification < ?", MODERATORS_AVAILABILITY_DELAY.ago).where(mod_roundrobin_notifications: true) end end end diff --git a/app/services/notifications/tag_adjustment_notification/send.rb b/app/services/notifications/tag_adjustment_notification/send.rb index 2e55f3a04..76cc70966 100644 --- a/app/services/notifications/tag_adjustment_notification/send.rb +++ b/app/services/notifications/tag_adjustment_notification/send.rb @@ -25,7 +25,6 @@ module Notifications notifiable_type: tag_adjustment.class.name, json_data: json_data, ) - article.user.update_column(:last_moderation_notification, Time.current) notification end diff --git a/app/views/internal/mods/index.html.erb b/app/views/internal/mods/index.html.erb new file mode 100644 index 000000000..d24ab247d --- /dev/null +++ b/app/views/internal/mods/index.html.erb @@ -0,0 +1,51 @@ + +

<%= params[:state] || "community" %> mods

+<%= form_tag("/internal/mods", method: "get") do %> + <%= label_tag(:search, "Find by tag name:") %> + <%= text_field_tag(:search, params[:search]) %> + <% if params[:state].present? %> + <%= hidden_field_tag(:state, params[:state]) %> + <% end %> + <%= submit_tag("Search") %> +<% end %> + +

+ ">General Community | + ">Tag Mods | + ">Potential Mods +

+ +<%= paginate @mods %> +
+
ID
+
Username
+
Comments count
+
Reactions count
+ <% if params[:state] == "potential" %> +
Action
+ <% end %> +
+<% @mods.each do |user| %> +
+
<%= user.id %>
+
@<%= user.username %>
+
<%= user.comments_count %>
+
<%= user.reactions_count %>
+ <% if params[:state] == "potential" %> +
+ <%= form_for([:internal, user], url: "/internal/mods/#{user.id}", method: :patch) do |f| %> + <%#= f.hidden_field :id, value: user.id %> + <%= f.submit "Make Trusted Mod" %> + <% end %> +
+ <% end %> +
+<% end %> +<%= paginate @mods %> diff --git a/app/views/mailers/notify_mailer/trusted_role_email.html.erb b/app/views/mailers/notify_mailer/trusted_role_email.html.erb new file mode 100644 index 000000000..da5b3f15e --- /dev/null +++ b/app/views/mailers/notify_mailer/trusted_role_email.html.erb @@ -0,0 +1,20 @@ +

+ Hey <%= @user.name %>! +

+

+ This is an email to let you know that we've upgraded your account to have basic moderation privileges. This means you can apply "negative reactions" to posts as well as certain other actions which help us maintain a constructive community and uphold our code of conduct. +

+

+ There are no specific responsibilities with this new privilege except that you use it respectfully and empathetically. If for any reason you'd like to forego these new features, let us know. +

+

+ You'll occasionally get on-site notifications asking you to take an action. You are never required to take any actions and feel free to unsubscribe in your notification settings at any point without giving up the rest of the features. +

+

+ Click here for full details about basic mod privileges. +

+

+ Happy Coding! +
+ - DEV Team +

diff --git a/app/views/mailers/notify_mailer/trusted_role_email.text.erb b/app/views/mailers/notify_mailer/trusted_role_email.text.erb new file mode 100644 index 000000000..d340bfc0b --- /dev/null +++ b/app/views/mailers/notify_mailer/trusted_role_email.text.erb @@ -0,0 +1,13 @@ +Hey <%= @user.name %>! + +This is an email to let you know that we've upgraded your account to have basic mod privileges. This means you can apply "negative reactions" to posts as well as certain other actions which help us maintain a constructive community and uphold our code of conduct. + +There are no specific responsibilities with this new privilege except that you use it respectfully and empathetically. If for any reason you'd like to forego these new features, let us know. + +You'll occasionally get on-site notifications asking you to take an action. You are never required to take any actions and feel free to unsubscribe in your notification settings at any point without giving up the rest of the features. + +Visit https://dev.to/community-moderation for full details of what you can do with these new privileges. + +Happy Coding! + +- DEV Team diff --git a/app/views/notifications/_comment.html.erb b/app/views/notifications/_comment.html.erb index f72bb6494..e1bfe7016 100644 --- a/app/views/notifications/_comment.html.erb +++ b/app/views/notifications/_comment.html.erb @@ -13,7 +13,7 @@ <% if notification.action.blank? %> "><%= json_data["user"]["name"] %> <% if json_data["comment"]["depth"] && json_data["comment"]["depth"] > 0 %> - replied to a thread in + replied to a thread in <% else %> commented on <% end %> @@ -25,16 +25,20 @@ <%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "default" %> <% elsif notification.action == "Moderation" %> - Hey there! <%= image_tag("emoji/apple-hugging-face.png", class: "reaction-image", alt: "Hugging face emoji") %> As a trusted member, could you react to this comment - so we know it fits our community code of conduct? + Hey there! <%= image_tag("emoji/apple-hugging-face.png", class: "reaction-image", alt: "Hugging face emoji") %>

+ ">@<%= json_data["user"]["username"] %> just left a comment. Since they are new to the community, could you leave a nice reply to help them feel welcome? +

+ Thank you! +

+ Alternatively, if this comment violates the code of conduct, please downvote/report as appropriate.

- Here's the article for context: + re: "> <%= h(json_data["comment"]["commentable"]["title"]) %> <%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "moderation" %>
-
All negative reactions are 100% private. If it is a good comment, consider also replying with some positive reinforcement or adding to the conversation.
+
All negative reactions are 100% private. Thank you for being a trusted <%= ApplicationConfig["COMMUNITY_NAME"] %> member.
<% elsif notification.action == "First" %> "><%= json_data["user"]["name"] %> wrote their first comment on: diff --git a/app/views/notifications/shared/_comment_box.html.erb b/app/views/notifications/shared/_comment_box.html.erb index 5bf458d69..5f282fbc0 100644 --- a/app/views/notifications/shared/_comment_box.html.erb +++ b/app/views/notifications/shared/_comment_box.html.erb @@ -37,7 +37,9 @@ <% end %> <% @comment = Comment.new %> - <%= form_for(@comment, authenticity_token: false, html: { id: "comment-form-for-#{json_data['comment']['id']}", onsubmit: "handleCommentSubmit.bind(this)(event)" }) do |f| %> + <%= form_for(@comment, + authenticity_token: false, + html: { id: "comment-form-for-#{json_data['comment']['id']}", onsubmit: "handleCommentSubmit.bind(this)(event)", onkeydown: "handleKeyDown(event)" }) do |f| %> <%= f.hidden_field :commentable_id, value: json_data["comment"]["commentable"]["id"] %> <%= f.hidden_field :commentable_type, value: json_data["comment"]["commentable"]["class"]["name"] %> @@ -46,7 +48,7 @@ <%= f.text_area :body_markdown, id: "comment-textarea-for-#{json_data['comment']['id']}" %>
- <%= f.submit "SUBMIT", id: "submit-button", onclick: "validateField(event)" %> + <%= f.submit "SUBMIT", id: "submit-button", class: "comment-action-button", onclick: "validateField(event)" %>
<% end %> <% end %> diff --git a/app/views/users/_notifications.html.erb b/app/views/users/_notifications.html.erb index 5d8df1008..fabcaf42f 100644 --- a/app/views/users/_notifications.html.erb +++ b/app/views/users/_notifications.html.erb @@ -62,6 +62,15 @@ <%= f.label :mobile_comment_notifications, "Notify me when someone replies to me in a comment thread" %> + <% if current_user.trusted %> +

Mod Notification Settings

+
+
+ <%= f.check_box :mod_roundrobin_notifications %> + <%= f.label :mod_roundrobin_notifications, "Send me occasional community-success mod notifications" %> +
+
+ <% end %>
<%= f.hidden_field :tab, value: @tab %> diff --git a/config/routes.rb b/config/routes.rb index bf90963ec..479497590 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,6 +37,7 @@ Rails.application.routes.draw do resources :feedback_messages, only: %i[index show] resources :listings, only: %i[index edit update destroy], controller: "classified_listings" resources :pages, only: %i[index new create edit update destroy] + resources :mods, only: %i[index update] resources :podcasts, only: %i[index edit update destroy] do member do post :add_admin diff --git a/db/migrate/20190827163358_add_mod_notifications_to_users.rb b/db/migrate/20190827163358_add_mod_notifications_to_users.rb new file mode 100644 index 000000000..46d4de493 --- /dev/null +++ b/db/migrate/20190827163358_add_mod_notifications_to_users.rb @@ -0,0 +1,5 @@ +class AddModNotificationsToUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :mod_roundrobin_notifications, :boolean, default: true + end +end diff --git a/db/schema.rb b/db/schema.rb index b68adff37..e6ea88ba5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_08_22_162434) do +ActiveRecord::Schema.define(version: 2019_08_27_163358) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1067,6 +1067,7 @@ ActiveRecord::Schema.define(version: 2019_08_22_162434) do t.string "medium_url" t.datetime "membership_started_at" t.boolean "mobile_comment_notifications", default: true + t.boolean "mod_roundrobin_notifications", default: true t.integer "monthly_dues", default: 0 t.string "mostly_work_with" t.string "name" diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index 16174029d..52a8a9850 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -331,4 +331,30 @@ RSpec.describe NotifyMailer, type: :mailer do expect(email.html_part.body).to include(CGI.escape("utm_campaign=tag_moderator_confirmation_email")) end end + + describe "#trusted_role_email" do + let(:tag) { create(:tag) } + + it "renders proper subject" do + email = described_class.trusted_role_email(user) + expect(email.subject).to eq("You've been upgraded to #{ApplicationConfig['COMMUNITY_NAME']} Community mod status!") + end + + it "renders proper receiver" do + email = described_class.trusted_role_email(user) + expect(email.to).to eq([user.email]) + end + + it "includes the tracking pixel" do + email = described_class.trusted_role_email(user) + expect(email.html_part.body).to include("open.gif") + end + + it "includes UTM params" do + email = described_class.trusted_role_email(user) + expect(email.html_part.body).to include(CGI.escape("utm_medium=email")) + expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer")) + expect(email.html_part.body).to include(CGI.escape("utm_campaign=trusted_role_email")) + end + end end diff --git a/spec/mailers/previews/notify_mailer_preview.rb b/spec/mailers/previews/notify_mailer_preview.rb index e055014fb..afa0846ab 100644 --- a/spec/mailers/previews/notify_mailer_preview.rb +++ b/spec/mailers/previews/notify_mailer_preview.rb @@ -35,6 +35,10 @@ class NotifyMailerPreview < ActionMailer::Preview NotifyMailer.tag_moderator_confirmation_email(User.first, "discuss") end + def trusted_role_email + NotifyMailer.trusted_role_email(User.first) + end + def feedback_message_resolution_email # change email_body text when you need to see a different version @user = User.first diff --git a/spec/requests/internal/mods_spec.rb b/spec/requests/internal/mods_spec.rb new file mode 100644 index 000000000..afc6fa87c --- /dev/null +++ b/spec/requests/internal/mods_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe "/internal/mods", type: :request do + let(:super_admin) { create(:user, :super_admin) } + let(:regular_user) { create(:user) } + + describe "GET /internal/mods" do + before do + sign_in super_admin + end + + it "displays mod user" do + regular_user.add_role(:trusted) + get "/internal/mods" + expect(response.body).to include(regular_user.username) + end + it "does not display non-mod" do + get "/internal/mods" + expect(response.body).not_to include(regular_user.username) + end + end + + describe "PUT /internal/mods" do + before do + sign_in super_admin + end + + it "displays mod user" do + put "/internal/mods/#{regular_user.id}" + expect(regular_user.reload.has_role?(:trusted)).to eq true + end + end +end diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index c951a4291..ff7be318f 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -221,7 +221,7 @@ RSpec.describe "NotificationsIndex", type: :request do end it "renders the proper message" do - expect(response.body).to include "As a trusted member" + expect(response.body).to include "Since they are new to the community, could you leave a nice reply" end it "renders the article's path" do @@ -233,6 +233,56 @@ RSpec.describe "NotificationsIndex", type: :request do end end + context "when a user should not receive moderation notification" do + let(:user2) { create(:user) } + let(:article) { create(:article, user_id: user.id) } + let(:comment) { create(:comment, user_id: user2.id, commentable_id: article.id, commentable_type: "Article") } + + before do + sign_in user + Notification.send_moderation_notification_without_delay(comment) + get "/notifications" + end + + it "renders the proper message" do + expect(response.body).not_to include "Since they are new to the community, could you leave a nice reply" + end + + it "renders the article's path" do + expect(response.body).not_to include article.path + end + + it "renders the comment's processed HTML" do + expect(response.body).not_to include comment.processed_html + end + end + + context "when a user has unsubscribed from mod roundrobin notifications" do + let(:user2) { create(:user) } + let(:article) { create(:article, user_id: user.id) } + let(:comment) { create(:comment, user_id: user2.id, commentable_id: article.id, commentable_type: "Article") } + + before do + user.add_role :trusted + user.update(mod_roundrobin_notifications: false) + sign_in user + Notification.send_moderation_notification_without_delay(comment) + get "/notifications" + end + + it "renders the proper message" do + expect(response.body).not_to include "Since they are new to the community, could you leave a nice reply" + end + + it "renders the article's path" do + expect(response.body).not_to include article.path + end + + it "renders the comment's processed HTML" do + expect(response.body).not_to include comment.processed_html + end + end + context "when a user has a new welcome notification" do before do sign_in user diff --git a/spec/services/notifications/tag_adjustment_notification/send_spec.rb b/spec/services/notifications/tag_adjustment_notification/send_spec.rb index 6eeeed699..f54be912e 100644 --- a/spec/services/notifications/tag_adjustment_notification/send_spec.rb +++ b/spec/services/notifications/tag_adjustment_notification/send_spec.rb @@ -39,16 +39,4 @@ RSpec.describe Notifications::TagAdjustmentNotification::Send do described_class.call(tag_adjustment) end.to change(Notification, :count).by(1) end - - it "checks that article user last mod notification updates" do - expect do - described_class.call(tag_adjustment) - end.to change(tag_adjustment.article.user, :last_moderation_notification) - end - - it "updates the author's last_moderation_notification" do - original_last_moderation_notification_timestamp = user2.last_moderation_notification - Notification.send_tag_adjustment_notification_without_delay(tag_adjustment) - expect(user2.reload.last_moderation_notification).to be > original_last_moderation_notification_timestamp - end end