Expand @-mention functionality to posts (#13367)

* Initial work for @-mention notifications from posts

* Revert article.published changes to article updater, add clarifying comments

* Extract article preview into reusable partial for notification views

* Clean up Article Updater

* Address + remove some FIXMEs

* Add a whole buncha specs for @-mention functionality in posts YAY

* Refactor create all spec to use shared examples, add clarifying comments

* Add guard clause to create all service

* Update new mention and notifiable action specs

* Some additional cleanup

* Add specs + shared examples to SendEmailNotificationWorker spec

* Use aggregate_failures where applicable

Co-authored-by: Michael Kohl <citizen428@dev.to>

* Cleanup and address code review comments

* Add MentionDecorator + relevant specs

* Address comments/issues flagged by @rhymes

* Optimize plucking user_ids when checking for article followers

Co-authored-by: Michael Kohl <citizen428@dev.to>
This commit is contained in:
Vaidehi Joshi 2021-05-12 07:33:33 -07:00 committed by GitHub
parent cdcb3b9be3
commit 8f0c24c167
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 608 additions and 236 deletions

View file

@ -0,0 +1,10 @@
class MentionDecorator < ApplicationDecorator
def formatted_mentionable_type
# Articles are colloquially referred to as "posts".
mentionable_type == "Article" ? "post" : mentionable_type.downcase
end
def mentioned_by_blocked_user?
mentionable_type == "User" && UserBlock.blocking?(mentionable_id, user_id)
end
end

View file

@ -35,9 +35,11 @@ class NotifyMailer < ApplicationMailer
@mentioner = User.find(@mention.mentionable.user_id)
@mentionable = @mention.mentionable
@mentionable_type = @mention.decorate.formatted_mentionable_type
@unsubscribe = generate_unsubscribe_token(@user.id, :email_mention_notifications)
mail(to: @user.email, subject: "#{@mentioner.name} just mentioned you!")
mail(to: @user.email, subject: "#{@mentioner.name} just mentioned you in their #{@mentionable_type}")
end
def unread_notifications_email

View file

@ -31,6 +31,7 @@ class Article < ApplicationRecord
# The date that we began limiting the number of user mentions in an article.
MAX_USER_MENTION_LIVE_AT = Time.utc(2021, 4, 7).freeze
has_many :mentions, as: :mentionable, inverse_of: :mentionable, dependent: :destroy
has_many :comments, as: :commentable, inverse_of: :commentable, dependent: :nullify
has_many :html_variant_successes, dependent: :nullify
has_many :html_variant_trials, dependent: :nullify

View file

@ -40,6 +40,18 @@ class Notification < ApplicationRecord
Notifications::NewFollowerWorker.new.perform(follow_data, is_read)
end
def send_to_mentioned_users_and_followers(notifiable, _action = nil)
return unless notifiable.is_a?(Article) && notifiable.published?
# We need to create associated mentions inline because they need to exist _before_ creating any
# other Article-related notifications. This ensures that users will not receive a second notification for the
# post being published if they have already received an initial notification about being @-mentioned in the post.
Mentions::CreateAll.call(notifiable)
# Kicks off a worker to send any notifications about the post being published, if necessary.
Notification.send_to_followers(notifiable, "Published")
end
def send_to_followers(notifiable, action = nil)
Notifications::NotifiableActionWorker.perform_async(notifiable.id, notifiable.class.name, action)
end
@ -70,11 +82,17 @@ class Notification < ApplicationRecord
end
def send_mention_notification(mention)
return if mention.mentionable_type == "User" && UserBlock.blocking?(mention.mentionable_id, mention.user_id)
return if MentionDecorator.new(mention).mentioned_by_blocked_user?
Notifications::MentionWorker.perform_async(mention.id)
end
def send_mention_notification_without_delay(mention)
return if MentionDecorator.new(mention).mentioned_by_blocked_user?
Notifications::NewMention::Send.call(mention) if mention
end
def send_welcome_notification(receiver_id, broadcast_id)
Notifications::WelcomeNotificationWorker.perform_async(receiver_id, broadcast_id)
end

View file

@ -16,10 +16,12 @@ module Articles
article = save_article
if article.persisted?
# Subscribe author to notifications for all comments on their article.
NotificationSubscription.create(user: user, notifiable_id: article.id, notifiable_type: "Article",
config: "all_comments")
Notification.send_to_followers(article, "Published") if article.published?
# Send notifications to any mentioned users, followed by any users who follow the article's author.
Notification.send_to_mentioned_users_and_followers(article) if article.published?
dispatch_event(article)
end

View file

@ -16,7 +16,8 @@ module Articles
def call
user.rate_limiter.check_limit!(:article_update)
was_published = article.published
# Grab the state of the article's "publish" status before making any further updates to it.
was_previously_published = article.published
# updated edited time only if already published and not edited by an admin
update_edited_at = article.user == user && article.published
@ -27,21 +28,33 @@ module Articles
if success
user.rate_limiter.track_limit_by_action(:article_update)
# send notification only the first time an article is published
send_notification = article.published && article.saved_change_to_published_at.present?
Notification.send_to_followers(article, "Published") if send_notification
if article.published && article.saved_change_to_published_at.present?
# The first time that an article is published, we want to send notifications to any mentioned users first,
# and then send notifications to any users who follow the article's author so as to avoid double mentions.
Notification.send_to_mentioned_users_and_followers(article)
elsif article.published
# If the article has already been published and is only being updated, then we need to create
# mentions and send notifications to mentioned users inline via the Mentions::CreateAll service.
Mentions::CreateAll.call(article)
end
# remove related notifications if unpublished
# Remove any associated notifications if Article is unpublished
if article.saved_changes["published"] == [true, false]
Notification.remove_all_by_action_without_delay(notifiable_ids: article.id, notifiable_type: "Article",
action: "Published")
if article.comments.exists?
Notification.remove_all(notifiable_ids: article.comments.ids,
notifiable_type: "Comment")
end
if article.mentions.exists?
Notification.remove_all(notifiable_ids: article.comments.ids,
notifiable_type: "Article")
end
end
# don't send only if article keeps being unpublished
dispatch_event(article) if article.published || was_published
# Do not notify if the article was previously already in a published state or is continually unpublished.
dispatch_event(article) if article.published || was_previously_published
end
Result.new(success: success, article: article.decorate)
end

View file

@ -1,4 +1,7 @@
module Mentions
# This class creates mentions + associated notifications for Articles and Comments.
# This class will check to see if there are any @-mentions in the post, and will
# create the associated mentions inline if necessary.
class CreateAll
def initialize(notifiable)
@notifiable = notifiable
@ -9,7 +12,6 @@ module Mentions
end
def call
# Only works for comments right now.
mentioned_users = users_mentioned_in_text_excluding_author
delete_mentions_removed_from_notifiable_text(mentioned_users)
@ -48,7 +50,7 @@ module Mentions
end
def reject_notifiable_author(users)
users.reject { |user| authored_by?(user, @notifiable) }
users.reject { |user| authored_by?(user, notifiable) }
end
def authored_by?(user, notifiable)
@ -56,21 +58,34 @@ module Mentions
end
def delete_mentions_removed_from_notifiable_text(users)
mentions = @notifiable.mentions.where.not(user_id: users).destroy_all
mentions = notifiable.mentions.where.not(user_id: users).destroy_all
Notification.remove_all(notifiable_ids: mentions.map(&:id), notifiable_type: "Mention") if mentions.present?
end
def user_has_comment_notifications?(user)
user.notifications.exists?(notifiable_id: @notifiable.id)
user.notifications.exists?(notifiable_id: notifiable.id, notifiable_type: "Comment")
end
def create_mention_for(user)
return if user_has_comment_notifications?(user)
# Do not create additional notifications for being mentioned in a comment.
if notifiable.is_a?(Comment) && user_has_comment_notifications?(user)
return
end
# The mentionable_type is the model that created the mention, the user is the user to be mentioned.
mention = Mention.create(user_id: user.id, mentionable_id: notifiable.id,
mentionable_type: notifiable.class.name)
# If notifiable is an Article, we need to create the notification for the mention immediately so
# that the notification exists in the database before we attempt to create other Article-related notifications.
# However, if notifiable is a Comment, we can create the notification for the mention in the background.
case notifiable
when Article
Notification.send_mention_notification_without_delay(mention)
when Comment
Notification.send_mention_notification(mention)
end
mention = Mention.create(user_id: user.id, mentionable_id: @notifiable.id,
mentionable_type: @notifiable.class.name)
# mentionable_type = model that created the mention, user = user to be mentioned
Notification.send_mention_notification(mention)
mention
end

View file

@ -3,6 +3,7 @@ module Notifications
class Send
delegate :user_data, to: Notifications
delegate :comment_data, to: Notifications
delegate :article_data, to: Notifications
def initialize(mention)
@mention = mention
@ -28,7 +29,14 @@ module Notifications
def json_data
data = { user: user_data(mention.mentionable.user) }
data[:comment] = comment_data(mention.mentionable) if mention.mentionable_type == "Comment"
case mention.mentionable_type
when "Comment"
data[:comment] = comment_data(mention.mentionable)
when "Article"
data[:article] = article_data(mention.mentionable)
end
data
end
end

View file

@ -25,7 +25,16 @@ module Notifications
json_data[:organization] = organization_data(notifiable.organization) if notifiable.organization_id
notifications_attributes = []
notifiable.followers.sort_by(&:updated_at).last(10_000).reverse_each do |follower|
# If a user was mentioned in the article, they will have already received a mention.
# We explicitly need to exclude them from the article_followers array if they already
# have a mention in order to avoid sending a user multiple notifications for one article.
user_ids_with_article_mentions = notifiable.mentions&.pluck(:user_id)
article_followers = notifiable.followers.reject do |follower|
user_ids_with_article_mentions.include?(follower.id)
end
article_followers.sort_by(&:updated_at).last(10_000).reverse_each do |follower|
now = Time.current
notifications_attributes.push(
user_id: follower.id,

View file

@ -1,26 +1,31 @@
<head>
<style type="text/css">
img {
max-width:100%;
height:auto;
border:none
}
</style>
<style type="text/css">
img {
max-width:100%;
height:auto;
border:none
}
</style>
</head>
<h2 style="font-size:25px;">
<a href="<%= user_url(@mentioner) %>"><%= @mentioner.name %></a> just mentioned you in their <%= @mention.mentionable_type.downcase %>!
<a href="<%= user_url(@mentioner) %>"><%= @mentioner.name %></a> just mentioned you in their <%= @mentionable_type %>
</h2>
<table>
<tr>
<td style='color:#c3c3c3;font-size:14px;padding-bottom:8px'>
</td>
</tr>
<tr>
<td style='padding:20px;padding-bottom:30px;border-radius:3px;font-size:19px;background:#f4f4f4'>
<%= @mentionable.processed_html.html_safe %>
<a style="background:#3c7dc1;color:white;padding:5px 14px;border-radius:3px;text-decoration:none;display:inline-block;margin-top:4px;" href='<%= app_url(@mention.mentionable.path) %>'>View on <%= community_name %></a>
</td>
</tr>
</table>
<% if @mentionable.is_a?(Comment) %>
<table>
<tr>
<td style='color:#c3c3c3;font-size:14px;padding-bottom:8px'>
</td>
</tr>
<tr>
<td style='padding:20px;padding-bottom:30px;border-radius:3px;font-size:19px;background:#f4f4f4'>
<%= @mentionable.processed_html.html_safe %>
<a style="background:#3c7dc1;color:white;padding:5px 14px;border-radius:3px;text-decoration:none;display:inline-block;margin-top:4px;" href='<%= app_url(@mention.mentionable.path) %>'>View on <%= community_name %></a>
</td>
</tr>
</table>
<% else %>
<a style="background:#3c7dc1;color:white;padding:5px 14px;border-radius:3px;text-decoration:none;display:inline-block;margin-top:4px;" href='<%= app_url(@mention.mentionable.path) %>'>Read their post on <%= community_name %></a>
<% end %>

View file

@ -21,40 +21,7 @@
<p class="lh-tight"><small class="fs-s color-base-60"><%= time_ago_in_words json_data["article"]["published_at"] %> ago</small></p>
</header>
<article class="notification__preview crayons-card">
<a href="<%= json_data["article"]["path"] %>" class="crayons-link block notification__preview__inner">
<h3 class="notification__preview__title"><%= h(json_data["article"]["title"]) %></h3>
<div class="-ml-1">
<% json_data["article"]["cached_tag_list_array"].each do |tag| %>
<span class="crayons-tag"><span class="crayons-tag__prefix">#</span><%= tag %></span>
<% end %>
</div>
</a>
<% cache "activity-published-article-reactions-#{@last_user_reaction}-#{json_data['article']['updated_at']}-#{json_data['article']['id']}" do %>
<footer class="comment-actions notification__actions">
<button
class="crayons-btn crayons-btn--ghost crayons-btn--icon-left crayons-btn--s reaction-like reaction-button <%= Reaction.cached_any_reactions_for?(notification.mocked_object("article"), current_user, "like") ? "reacted" : "" %>"
data-reactable-id="<%= json_data["article"]["id"] %>"
data-category="like"
data-reactable-type="Article">
<%= inline_svg_tag("small-heart.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: "Favorite heart button") %>
<%= inline_svg_tag("small-heart-filled.svg", aria: true, class: "crayons-icon reaction-icon--like reaction-icon reacted", title: "Favorite heart button") %>
<span class="hidden m:inline-block">Like</span>
</button>
<button
class="ml-auto crayons-btn crayons-btn--ghost crayons-btn--icon-right crayons-btn--s reaction-readinglist reaction-button readinglist-button <%= Reaction.cached_any_reactions_for?(notification.mocked_object("article"), current_user, "readinglist") ? "reacted" : "" %>"
data-reactable-id="<%= json_data["article"]["id"] %>"
data-category="readinglist"
data-reactable-type="Article">
<span class="reaction-button-text hidden m:inline-block">Save</span>
<%= inline_svg_tag("small-save.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: "Save button") %>
<%= inline_svg_tag("small-save-filled.svg", aria: true, class: "crayons-icon reaction-icon--readinglist reaction-icon reacted", title: "Save button") %>
</button>
</footer>
<% end %>
</article>
<%= render "notifications/shared/article_preview", json_data: json_data, notification: notification, context: "default" %>
</div>
<% end %>
</div>

View file

@ -5,9 +5,26 @@
<% end %>
<div class="notification__content">
<h2 class="fs-base fw-normal mb-4">
<a href="<%= json_data["user"]["path"] %>" class="crayons-link fw-bold"><%= json_data["user"]["name"] %></a> mentioned you in a comment
</h2>
<%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "default" %>
<% if notification.json_data["article"] %>
<header class="mb-4">
<h2 class="fs-base fw-normal">
<a href="<%= json_data["user"]["path"] %>" class="crayons-link fw-bold">
<%= json_data["user"]["name"] %>
</a>
mentioned you in a post
<% if json_data["organization"] %>
under <a href="<%= json_data["organization"]["path"] %>" class="crayons-link fw-bold"><%= json_data["organization"]["name"] %></a>
<% end %>
</h2>
<p class="lh-tight"><small class="fs-s color-base-60"><%= time_ago_in_words json_data["article"]["published_at"] %> ago</small></p>
</header>
<%= render "notifications/shared/article_preview", json_data: json_data, notification: notification, context: "default" %>
<% elsif notification.json_data["comment"] %>
<h2 class="fs-base fw-normal mb-4">
<a href="<%= json_data["user"]["path"] %>" class="crayons-link fw-bold"><%= json_data["user"]["name"] %></a> mentioned you in a comment
</h2>
<%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "default" %>
<% end %>
</div>
</div>

View file

@ -0,0 +1,34 @@
<article class="notification__preview crayons-card">
<a href="<%= json_data["article"]["path"] %>" class="crayons-link block notification__preview__inner">
<h3 class="notification__preview__title"><%= h(json_data["article"]["title"]) %></h3>
<div class="-ml-1">
<% json_data["article"]["cached_tag_list_array"].each do |tag| %>
<span class="crayons-tag"><span class="crayons-tag__prefix">#</span><%= tag %></span>
<% end %>
</div>
</a>
<% cache "activity-published-article-reactions-#{@last_user_reaction}-#{json_data['article']['updated_at']}-#{json_data['article']['id']}" do %>
<footer class="comment-actions notification__actions">
<button
class="crayons-btn crayons-btn--ghost crayons-btn--icon-left crayons-btn--s reaction-like reaction-button <%= Reaction.cached_any_reactions_for?(notification.mocked_object("article"), current_user, "like") ? "reacted" : "" %>"
data-reactable-id="<%= json_data["article"]["id"] %>"
data-category="like"
data-reactable-type="Article">
<%= inline_svg_tag("small-heart.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: "Favorite heart button") %>
<%= inline_svg_tag("small-heart-filled.svg", aria: true, class: "crayons-icon reaction-icon--like reaction-icon reacted", title: "Favorite heart button") %>
<span class="hidden m:inline-block">Like</span>
</button>
<button
class="ml-auto crayons-btn crayons-btn--ghost crayons-btn--icon-right crayons-btn--s reaction-readinglist reaction-button readinglist-button <%= Reaction.cached_any_reactions_for?(notification.mocked_object("article"), current_user, "readinglist") ? "reacted" : "" %>"
data-reactable-id="<%= json_data["article"]["id"] %>"
data-category="readinglist"
data-reactable-type="Article">
<span class="reaction-button-text hidden m:inline-block">Save</span>
<%= inline_svg_tag("small-save.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: "Save button") %>
<%= inline_svg_tag("small-save-filled.svg", aria: true, class: "crayons-icon reaction-icon--readinglist reaction-icon reacted", title: "Save button") %>
</button>
</footer>
<% end %>
</article>

View file

@ -1,4 +1,5 @@
module Mentions
# This worker is currently only used to create mentions on comments.
class CreateAllWorker
include Sidekiq::Worker
sidekiq_options queue: :default, retry: 10

View file

@ -0,0 +1,35 @@
require "rails_helper"
RSpec.describe MentionDecorator, type: :decorator do
let(:user) { create(:user) }
describe "#formatted_mentionable_type" do
let(:article) { create(:article) }
let(:comment) { create(:comment, user_id: user.id, commentable: article) }
it "returns the correct mentionable type for mentions on articles" do
mention = create(:mention, mentionable: article, user: user).decorate
expect(mention.decorate.formatted_mentionable_type).to eq("post")
end
it "returns the correct mentionable type for mentions on comments" do
mention = create(:mention, mentionable: comment, user: user).decorate
expect(mention.decorate.formatted_mentionable_type).to eq("comment")
end
end
describe "#mentioned_by_blocked_user?" do
let(:blocked_user) { create(:user) }
let(:mention) { create(:mention, mentionable: user, user: blocked_user) }
it "returns true if mentioned user has blocked the mentioner" do
create(:user_block, blocker: user, blocked: blocked_user, config: "default")
expect(mention.decorate.mentioned_by_blocked_user?).to be(true)
end
it "returns false if mentioned user has not blocked the mentioner" do
expect(mention.decorate.mentioned_by_blocked_user?).to be(false)
end
end
end

View file

@ -48,21 +48,36 @@ RSpec.describe NotifyMailer, type: :mailer do
end
describe "#new_mention_email" do
let(:mention) { create(:mention, user: user2, mentionable: comment) }
let(:email) { described_class.with(mention: mention).new_mention_email }
context "when mentioning in a comment" do
let(:comment_mention) { create(:mention, user: user2, mentionable: comment) }
let(:email) { described_class.with(mention: comment_mention).new_mention_email }
it "renders proper subject" do
expect(email.subject).to eq("#{comment.user.name} just mentioned you!")
it "renders proper subject and receiver", :aggregate_failures do
expect(email.subject).to eq("#{comment.user.name} just mentioned you in their comment")
expect(email.to).to eq([user2.email])
end
it "renders proper sender", :aggregate_failures do
expect(email.from).to eq([SiteConfig.email_addresses[:default]])
expected_from = "#{SiteConfig.community_name} <#{SiteConfig.email_addresses[:default]}>"
expect(email["from"].value).to eq(expected_from)
end
end
it "renders proper sender" do
expect(email.from).to eq([SiteConfig.email_addresses[:default]])
expected_from = "#{SiteConfig.community_name} <#{SiteConfig.email_addresses[:default]}>"
expect(email["from"].value).to eq(expected_from)
end
context "when mentioning in an article" do
let(:article_mention) { create(:mention, user: user2, mentionable: article) }
let(:email) { described_class.with(mention: article_mention).new_mention_email }
it "renders proper receiver" do
expect(email.to).to eq([user2.email])
it "renders proper subject and receiver", :aggregate_failures do
expect(email.subject).to eq("#{article.user.name} just mentioned you in their post")
expect(email.to).to eq([user2.email])
end
it "renders proper sender", :aggregate_failures do
expect(email.from).to eq([SiteConfig.email_addresses[:default]])
expected_from = "#{SiteConfig.community_name} <#{SiteConfig.email_addresses[:default]}>"
expect(email["from"].value).to eq(expected_from)
end
end
end

View file

@ -13,11 +13,16 @@ class NotifyMailerPreview < ActionMailer::Preview
NotifyMailer.with(user: User.last).unread_notifications_email
end
def new_mention_email
def new_comment_mention_email
mention = Mention.find_or_create_by(user: User.find(1), mentionable: Comment.find(1))
NotifyMailer.with(mention: mention).new_mention_email
end
def new_article_mention_email
mention = Mention.find_or_create_by(user: User.find(1), mentionable: Article.find(1))
NotifyMailer.with(mention: mention).new_mention_email
end
def video_upload_complete_email
NotifyMailer.with(article: Article.last).video_upload_complete_email
end

View file

@ -19,6 +19,7 @@ RSpec.describe Article, type: :model do
it { is_expected.to belong_to(:user) }
it { is_expected.to have_many(:comments).dependent(:nullify) }
it { is_expected.to have_many(:mentions).dependent(:destroy) }
it { is_expected.to have_many(:html_variant_successes).dependent(:nullify) }
it { is_expected.to have_many(:html_variant_trials).dependent(:nullify) }
it { is_expected.to have_many(:notification_subscriptions).dependent(:destroy) }

View file

@ -392,6 +392,66 @@ RSpec.describe Notification, type: :model do
end
end
describe "send_to_mentioned_users_and_followers" do
let!(:mention_markdown) { "Hello there, @#{user2.username}!" }
context "when the notifiable is an article from a user" do
before do
article.update!(body_markdown: mention_markdown)
user2.follow(user)
end
it "sends a single notification to mentioned user", :aggregate_failures do
expect do
sidekiq_perform_enqueued_jobs do
described_class.send_to_mentioned_users_and_followers(article)
end
end.to change(user2.notifications, :count).from(0).to(1)
expect(user2.notifications.first.notifiable_type).to eq("Mention")
end
it "sends a notification to the organization's followers (who were not mentioned)", :aggregate_failures do
user3.follow(user)
expect do
sidekiq_perform_enqueued_jobs do
described_class.send_to_mentioned_users_and_followers(article)
end
end.to change(user3.notifications, :count).from(0).to(1)
expect(user3.notifications.first.notifiable_type).to eq("Article")
end
end
context "when the notifiable is an article from an organization" do
let(:org_article) { create(:article, organization: organization, user: user) }
before do
org_article.update!(body_markdown: mention_markdown)
user2.follow(user)
end
it "sends a single notification to mentioned user", :aggregate_failures do
expect do
sidekiq_perform_enqueued_jobs do
described_class.send_to_mentioned_users_and_followers(org_article)
end
end.to change(user2.notifications, :count).from(0).to(1)
expect(user2.notifications.first.notifiable_type).to eq("Mention")
end
it "sends a notification to the organization's followers (who were not mentioned)", :aggregate_failures do
user3.follow(organization)
expect do
sidekiq_perform_enqueued_jobs do
described_class.send_to_mentioned_users_and_followers(org_article)
end
end.to change(user3.notifications, :count).from(0).to(1)
expect(user3.notifications.first.notifiable_type).to eq("Article")
end
end
end
describe "#send_to_followers" do
context "when the notifiable is an article from a user" do
it "sends a notification to the author's followers" do

View file

@ -22,6 +22,14 @@ RSpec.describe "NotificationsIndex", type: :request do
expect(response.body).to include article.path
end
def renders_authors_name(article)
expect(response.body).to include CGI.escapeHTML(article.user.name)
end
def renders_article_published_at(article)
expect(response.body).to include time_ago_in_words(article.published_at)
end
def renders_comments_html(comment)
expect(response.body).to include comment.processed_html
end
@ -645,7 +653,7 @@ RSpec.describe "NotificationsIndex", type: :request do
end
end
context "when a user has a new mention notification" do
context "when a user has a new comment mention notification" do
let(:user2) { create(:user) }
let(:article) { create(:article, user_id: user.id) }
let(:comment) do
@ -673,14 +681,36 @@ RSpec.describe "NotificationsIndex", type: :request do
end
end
context "when a user has a new article notification" do
context "when a user has a new article mention notification" do
let(:user2) { create(:user) }
let(:article) { create(:article, user_id: user2.id) }
let(:mention) { create(:mention, mentionable: article, user: user) }
before do
article.update!(body_markdown: "Hello, @#{user.username}!")
sidekiq_perform_enqueued_jobs do
Notification.send_mention_notification(mention)
end
sign_in user
get "/notifications"
end
it "renders the proper message" do
expect(response.body).to include "mentioned you in a post"
renders_article_path(article)
renders_authors_name(article)
renders_article_published_at(article)
end
end
context "when a user has a new article created notification" do
let(:user2) { create(:user) }
let(:article) { create(:article, user_id: user.id) }
before do
user2.follow(user)
sidekiq_perform_enqueued_jobs do
Notification.send_to_followers(article, "Published")
Notification.send_to_mentioned_users_and_followers(article)
end
sign_in user2
get "/notifications"
@ -693,14 +723,6 @@ RSpec.describe "NotificationsIndex", type: :request do
renders_article_published_at(article)
end
def renders_authors_name(article)
expect(response.body).to include CGI.escapeHTML(article.user.name)
end
def renders_article_published_at(article)
expect(response.body).to include time_ago_in_words(article.published_at)
end
it "renders the reaction as previously reacted if it was reacted on" do
Reaction.create(user: user2, reactable: article, category: "like")
get "/notifications"
@ -720,7 +742,7 @@ RSpec.describe "NotificationsIndex", type: :request do
before do
user2.follow(user)
sidekiq_perform_enqueued_jobs do
Notification.send_to_followers(article, "Published")
Notification.send_to_mentioned_users_and_followers(article)
end
sign_in admin
end

View file

@ -26,6 +26,13 @@ RSpec.describe Articles::Creator, type: :service do
end
end
it "delegates to the Mentions::CreateAll service" do
valid_attributes[:published] = true
allow(Mentions::CreateAll).to receive(:call)
article = described_class.call(user, valid_attributes)
expect(Mentions::CreateAll).to have_received(:call).with(article)
end
it "creates a notification subscription" do
expect do
described_class.call(user, valid_attributes)
@ -76,6 +83,12 @@ RSpec.describe Articles::Creator, type: :service do
end
end
it "doesn't delegate to the Mentions::CreateAll service" do
allow(Mentions::CreateAll).to receive(:call)
article = described_class.call(user, invalid_body_attributes)
expect(Mentions::CreateAll).not_to have_received(:call).with(article)
end
it "doesn't create a notification subscription" do
expect do
described_class.call(user, invalid_body_attributes)

View file

@ -49,24 +49,50 @@ RSpec.describe Articles::Updater, type: :service do
end
describe "notifications" do
it "sends notifications when an article was published" do
attributes[:published] = true
sidekiq_assert_enqueued_with(job: Notifications::NotifiableActionWorker) do
context "when an article is updated and published the first time" do
before { attributes[:published] = true }
it "enqueues a job to send a notification" do
sidekiq_assert_enqueued_with(job: Notifications::NotifiableActionWorker) do
described_class.call(user, draft, attributes)
end
end
it "delegates to the Mentions::CreateAll service to create mentions" do
allow(Mentions::CreateAll).to receive(:call)
described_class.call(user, draft, attributes)
expect(Mentions::CreateAll).to have_received(:call).with(draft)
end
end
it "doesn't send when an article was unpublished" do
attributes[:published] = false
sidekiq_assert_not_enqueued_with(job: Notifications::NotifiableActionWorker) do
context "when an article is being updated and has already been published" do
it "doesn't enqueue a job to send a notification" do
attributes[:published] = true
sidekiq_assert_not_enqueued_with(job: Notifications::NotifiableActionWorker) do
described_class.call(user, article, attributes)
end
end
it "delegates to the Mentions::CreateAll service to create mentions" do
allow(Mentions::CreateAll).to receive(:call)
described_class.call(user, article, attributes)
expect(Mentions::CreateAll).to have_received(:call).with(article)
end
end
it "doesn't send when an article went from published to published" do
attributes[:published] = true
sidekiq_assert_not_enqueued_with(job: Notifications::NotifiableActionWorker) do
context "when an article is unpublished" do
before { attributes[:published] = false }
it "doesn't send any notifications" do
sidekiq_assert_not_enqueued_with(job: Notifications::NotifiableActionWorker) do
described_class.call(user, article, attributes)
end
end
it "doesn't delegate to the Mentions::CreateAll service to create mentions" do
allow(Mentions::CreateAll).to receive(:call)
described_class.call(user, article, attributes)
expect(Mentions::CreateAll).not_to have_received(:call).with(article)
end
end
end

View file

@ -1,88 +1,139 @@
require "rails_helper"
RSpec.describe Mentions::CreateAll, type: :service do
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:article) { create(:article, user_id: user.id) }
let(:comment) { create(:comment, user_id: user2.id, commentable: article) }
let(:comment2) do
create(
:comment,
body_markdown: "Hello @#{user.username}, you are cool.",
user_id: user2.id,
commentable: article,
)
RSpec.shared_examples "valid notifiable and no mentions" do
it "does not create mentions if a user is not mentioned" do
set_markdown_and_save(notifiable, markdown)
described_class.call(notifiable)
expect(Mention.all.size).to eq(0)
end
it "creates mention if there is a user mentioned and if the user doenst own the comment" do
comment.body_markdown = "Hello @#{user.username}, you are cool."
comment.save
described_class.call(comment)
it "creates a mention if notifiable is updated to include mention", :aggregate_failures do
set_markdown_and_save(notifiable, markdown)
described_class.call(notifiable)
expect(Mention.all.size).to eq(0)
set_markdown_and_save(notifiable, mention_markdown)
described_class.call(notifiable)
expect(Mention.all.size).to eq(1)
end
end
RSpec.shared_examples "valid notifiable and has mentions" do
it "creates mention if there is a user mentioned and if the user doesn't own notifiable" do
set_markdown_and_save(notifiable, mention_markdown)
described_class.call(notifiable)
expect(Mention.all.size).to eq(1)
end
it "deletes mention if deleted from comment" do
comment.body_markdown = "Hello @#{user.username}, you are cool."
comment.save
described_class.call(comment)
it "deletes mention if deleted from notifiable", :aggregate_failures do
set_markdown_and_save(notifiable, mention_markdown)
described_class.call(notifiable)
expect(Mention.all.size).to eq(1)
comment.body_markdown = "Hello, you are cool."
comment.save
described_class.call(comment)
set_markdown_and_save(notifiable, "Hello, you are cool.")
described_class.call(notifiable)
expect(Mention.all.size).to eq(0)
end
it "creates one mention even if multiple mentions of same user" do
comment.body_markdown = "Hello @#{user.username} @#{user.username} @#{user.username}, you rock."
comment.save
described_class.call(comment)
set_markdown_and_save(notifiable, "Hello @#{user.username} @#{user.username} @#{user.username}, you rock.")
described_class.call(notifiable)
expect(Mention.all.size).to eq(1)
end
it "ignores mentions when embedded within a comment liquid tag" do
markdown = "Check out this comment: {% comment #{comment2.id_code_generated} %}"
comment_with_liquid_tag = create(:comment, user_id: user2.id, commentable: article, body_markdown: markdown)
comment_with_liquid_tag.save
described_class.call(comment_with_liquid_tag)
expect(Mention.all.size).to eq(0)
end
it "creates multiple mentions for multiple users" do
user2 = create(:user)
comment.body_markdown = "Hello @#{user.username} @#{user2.username}, you are cool."
comment.save
described_class.call(comment)
user3 = create(:user)
set_markdown_and_save(notifiable, "Hello @#{user.username} @#{user3.username}, you are cool.")
described_class.call(notifiable)
expect(Mention.all.size).to eq(2)
end
it "deletes one of multiple mentions if one of multiple is deleted" do
user2 = create(:user)
comment.body_markdown = "Hello @#{user.username} @#{user2.username}, you are cool."
comment.save
described_class.call(comment)
it "deletes one of multiple mentions if one of multiple is deleted", :aggregate_failures do
user3 = create(:user)
set_markdown_and_save(notifiable, "Hello @#{user.username} @#{user3.username}, you are cool.")
described_class.call(notifiable)
expect(Mention.all.size).to eq(2)
comment.body_markdown = "Hello @#{user2.username}, you are cool."
comment.save
described_class.call(comment)
set_markdown_and_save(notifiable, "Hello @#{user3.username}, you are cool.")
described_class.call(notifiable)
expect(Mention.all.size).to eq(1)
end
it "creates a mention on creation of comment" do
described_class.call(comment2)
it "creates a mention on creation of notifiable" do
described_class.call(notifiable)
expect(Mention.all.size).to eq(1)
end
it "does not create a mention without valid mentionable" do
comment2.update_column(:body_markdown, "")
described_class.call(comment2)
expect(Mention.all.size).to eq(0)
end
it "does not create a mention when the user mentions themselves" do
comment.body_markdown = "Me, Myself and I @#{user2.username}"
comment.save
described_class.call(comment)
set_markdown_and_save(notifiable, "Me, Myself and I @#{user2.username}")
described_class.call(notifiable)
expect(Mention.all.size).to eq(0)
end
end
RSpec.shared_examples "valid notifiable with embedded mentions" do
it "does not create a mention" do
comment = create(:comment, user_id: user2.id, commentable: article, body_markdown: "Hi there, @#{user.username}")
liquid_tag_markdown = "Check out this comment: {% comment #{comment.id_code_generated} %}"
notifiable.update_column(:body_markdown, liquid_tag_markdown)
described_class.call(notifiable)
expect(Mention.all.size).to eq(0)
end
end
RSpec.shared_examples "invalid notifiable and has mentions" do
it "does not create a mention without valid mentionable" do
notifiable.update_column(:body_markdown, "")
described_class.call(notifiable)
expect(Mention.all.size).to eq(0)
end
end
RSpec.describe Mentions::CreateAll, type: :service do
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:article) { create(:article, user_id: user.id) }
let(:markdown) { "Hello, you are cool." }
let(:mention_markdown) { "Hello @#{user.username}, you are cool." }
def set_markdown_and_save(notifiable, markdown)
notifiable.update(body_markdown: markdown)
end
it_behaves_like "valid notifiable and no mentions" do
let(:notifiable) { create(:comment, body_markdown: markdown, user_id: user2.id, commentable: article) }
end
it_behaves_like "valid notifiable and has mentions" do
let(:notifiable) { create(:comment, body_markdown: mention_markdown, user_id: user2.id, commentable: article) }
end
it_behaves_like "valid notifiable with embedded mentions" do
# Explicitly use markdown here without a mention to test that embedded mentions do not trigger a notification.
let(:notifiable) { create(:comment, body_markdown: markdown, user_id: user2.id, commentable: article) }
end
it_behaves_like "invalid notifiable and has mentions" do
let(:notifiable) { create(:comment, body_markdown: mention_markdown, user_id: user2.id, commentable: article) }
end
it_behaves_like "valid notifiable and no mentions" do
let(:notifiable) { create(:article, user_id: user2.id) }
end
it_behaves_like "valid notifiable and has mentions" do
let(:notifiable) { create(:article, user_id: user2.id) }
before { notifiable.update!(body_markdown: mention_markdown) }
end
it_behaves_like "valid notifiable with embedded mentions" do
let(:notifiable) { create(:article, user_id: user2.id) }
end
it_behaves_like "invalid notifiable and has mentions" do
let(:notifiable) { create(:article, user_id: user2.id) }
end
end

View file

@ -1,9 +1,7 @@
require "rails_helper"
RSpec.describe Notifications::NewMention::Send, type: :service do
let(:user) { create(:user) }
let(:comment) { create(:comment, commentable: create(:article)) }
let(:mention) { create(:mention, mentionable: comment, user: user) }
RSpec.shared_examples "mentionable" do
let(:mention) { create(:mention, mentionable: mentionable, user: user) }
it "creates a mention notification" do
expect do
@ -11,15 +9,29 @@ RSpec.describe Notifications::NewMention::Send, type: :service do
end.to change(Notification, :count).by(1)
end
it "creates a correct mention notification" do
it "creates a correct mention notification", :aggregate_failures do
notification = described_class.call(mention)
mentionable_type = mentionable.class.to_s.downcase
expect(notification.user_id).to eq(user.id)
expect(notification.notifiable).to eq(mention)
expect(notification.json_data["comment"]["path"]).to eq(comment.path)
expect(notification.json_data[mentionable_type]["path"]).to eq(mentionable.path)
end
it "sends from proper mentioner" do
notification = described_class.call(mention)
expect(notification.json_data["user"]["id"]).to eq(comment.user_id)
expect(notification.json_data["user"]["id"]).to eq(mentionable.user_id)
end
end
RSpec.describe Notifications::NewMention::Send, type: :service do
let(:user) { create(:user) }
let!(:article) { create(:article) }
it_behaves_like "mentionable" do
let(:mentionable) { create(:comment, commentable: article) }
end
it_behaves_like "mentionable" do
let(:mentionable) { article }
end
end

View file

@ -8,51 +8,71 @@ RSpec.describe Notifications::NotifiableAction::Send, type: :service do
let(:user2) { create(:user) }
let(:user3) { create(:user) }
before do
user2.follow(user)
user3.follow(organization)
end
context "when following a user or organization" do
before do
user2.follow(user)
user3.follow(organization)
end
it "creates notifications" do
expect do
it "creates notifications" do
expect do
described_class.call(article, "Published")
end.to change(Notification, :count).by(2)
end
it "creates a correct user notification", :aggregate_failures do
described_class.call(article, "Published")
end.to change(Notification, :count).by(2)
notifications = Notification.where(user_id: user2.id, notifiable_id: article.id, notifiable_type: "Article")
expect(notifications.size).to eq(1)
notification = notifications.first
expect(notification.action).to eq("Published")
expect(notification.json_data["article"]["id"]).to eq(article.id)
expect(notification.json_data["user"]["id"]).to eq(user.id)
expect(notification.json_data["user"]["username"]).to eq(user.username)
end
it "creates a correct organization notification", :aggregate_failures do
described_class.call(article, "Published")
notifications = Notification.where(user_id: user3.id, notifiable_id: article.id, notifiable_type: "Article")
expect(notifications.size).to eq(1)
notification = notifications.first
expect(notification.action).to eq("Published")
expect(notification.json_data["article"]["id"]).to eq(article.id)
expect(notification.json_data["user"]["id"]).to eq(user.id)
expect(notification.json_data["organization"]["id"]).to eq(organization.id)
expect(notification.json_data["organization"]["name"]).to eq(organization.name)
end
it "does not create a notification if the follower has muted the user" do
user2.follows.first.update(subscription_status: "none")
user3.stop_following(organization)
described_class.call(article, "Published")
expect(Notification.count).to eq(0)
end
it "doesn't fail if the notification already exists" do
notification = create(:notification, user: user2, action: "Published", notifiable: article)
result = described_class.call(article, "Published")
ids = result.to_a.map { |r| r["id"] }
expect(ids).to include(notification.id)
end
end
it "creates a correct user notification" do
described_class.call(article, "Published")
notifications = Notification.where(user_id: user2.id, notifiable_id: article.id, notifiable_type: "Article")
expect(notifications.size).to eq(1)
notification = notifications.first
expect(notification.action).to eq("Published")
expect(notification.json_data["article"]["id"]).to eq(article.id)
expect(notification.json_data["user"]["id"]).to eq(user.id)
expect(notification.json_data["user"]["username"]).to eq(user.username)
end
context "when following a user or organization and being mentioned in an article" do
it "does not create a notification when following a user" do
user2.follow(user)
create(:mention, mentionable: article, user: user2)
it "creates a correct organization notification" do
described_class.call(article, "Published")
notifications = Notification.where(user_id: user3.id, notifiable_id: article.id, notifiable_type: "Article")
expect(notifications.size).to eq(1)
notification = notifications.first
expect(notification.action).to eq("Published")
expect(notification.json_data["article"]["id"]).to eq(article.id)
expect(notification.json_data["user"]["id"]).to eq(user.id)
expect(notification.json_data["organization"]["id"]).to eq(organization.id)
expect(notification.json_data["organization"]["name"]).to eq(organization.name)
end
described_class.call(article, "Published")
expect(Notification.count).to eq(0)
end
it "does not create a notification if the follower has muted the user" do
user2.follows.first.update(subscription_status: "none")
user3.stop_following(organization)
described_class.call(article, "Published")
expect(Notification.count).to eq 0
end
it "does not create a notification when following an organization" do
user3.follow(organization)
create(:mention, mentionable: article, user: user3)
it "doesn't fail if the notification already exists" do
notification = create(:notification, user: user2, action: "Published", notifiable: article)
result = described_class.call(article, "Published")
ids = result.to_a.map { |r| r["id"] }
expect(ids).to include(notification.id)
described_class.call(article, "Published")
expect(Notification.count).to eq(0)
end
end
end

View file

@ -1,32 +1,42 @@
require "rails_helper"
RSpec.shared_examples "a valid mentionable" do
context "with a mention" do
it "calls on NotifyMailer" do
worker.perform(mention.id) do
expect(NotifyMailer).to have_received(:new_mention_email).with(mention)
end
end
end
context "without a mention" do
it "does not error" do
expect { worker.perform(nil) }.not_to raise_error
end
it "does not call NotifyMailer" do
worker.perform(nil) do
expect(NotifyMailer).not_to have_received(:new_mention_email)
end
end
end
end
RSpec.describe Mentions::SendEmailNotificationWorker, type: :worker do
include_examples "#enqueues_on_correct_queue", "default", 1
describe "#perform" do
let(:worker) { subject }
let(:user) { create(:user) }
let(:mention) { create(:mention, user_id: user.id, mentionable_id: comment.id, mentionable_type: "Comment") }
let(:comment) { create(:comment, user_id: user.id, commentable: create(:article)) }
let(:worker) { subject }
let(:user) { create(:user) }
let(:article) { create(:article) }
let(:comment) { create(:comment, user_id: user.id, commentable: article) }
context "with mention" do
it "calls on NotifyMailer" do
worker.perform(mention.id) do
expect(NotifyMailer).to have_received(:new_mention_email).with(mention)
end
end
it_behaves_like "a valid mentionable" do
let(:mention) { create(:mention, user_id: user.id, mentionable_id: comment.id, mentionable_type: "Comment") }
end
context "without a mention" do
it "does not error" do
expect { worker.perform(nil) }.not_to raise_error
end
it "does not call NotifyMailer" do
worker.perform(nil) do
expect(NotifyMailer).not_to have_received(:new_mention_email)
end
end
it_behaves_like "a valid mentionable" do
let(:mention) { create(:mention, user_id: user.id, mentionable_id: article.id, mentionable_type: "Article") }
end
end
end