maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make reads controller async and sanitize notification titles (#1158)

Browse source
This commit is contained in:
Andy Zhao 2018-11-19 19:33:00 -05:00 • committed by Ben Halpern
parent a1010201f2
commit 044de6a8b5
7 changed files with 67 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
app/labor/notification_counter.rb
View file

@ -11,5 +11,4 @@ class NotificationCounter
def set_to_zero
@user.notifications.where(read?: false).update_all(read?: true)
end
handle_asynchronously :set_to_zero
end

2
app/views/notifications/_aggregated_reactions.html.erb
View file

@ -35,7 +35,7 @@
reacted to
<a href="<%= reactable_data["path"] %>" class="notification-comment-reacted-link">
<%# your article/comment or the actual title of the article/comment %>
<%= reactable_data["title"].blank? ? "your #{reactable_data["class"]["name"].downcase}" : reactable_data["title"] %>
<%= reactable_data["title"].blank? ? "your #{reactable_data["class"]["name"].downcase}" : sanitize(reactable_data["title"]) %>
</a>
<span class="reaction-images">
with

2
app/views/notifications/_article.html.erb
View file

@ -15,7 +15,7 @@
<a href="<%= json_data["article"]["path"] %>">
<div class="notification-new-post">
<div class="notification-new-post-title">
<%= json_data["article"]["title"] %>
<%= sanitize(json_data["article"]["title"]) %>
</div>
<div class="notification-new-post-tags">
<% json_data["article"]["cached_tag_list_array"].each do |tag| %>

2
app/views/notifications/_badgeachievement.html.erb
View file

@ -2,7 +2,7 @@
<div class="content notification-content badge-content">
<% cache "activity-badge-reward-#{json_data["badge_achievement"]["badge_id"]}" do %>
<div class="badge-title">
You received the <strong><%= json_data["badge_achievement"]["badge"]["title"] %></strong> badge
You received the <strong><%= sanitize(json_data["badge_achievement"]["badge"]["title"]) %></strong> badge
</div>
<p class="badge-description">
<%= json_data["badge_achievement"]["badge"]["description"] %>

6
app/views/notifications/_comment.html.erb
View file

@ -11,7 +11,7 @@
<a href="<%= json_data["user"]["path"] %>"><%= json_data["user"]["name"] %></a>
commented on
<a href="<%= json_data["comment"]["commentable"]["path"] %>">
<%= json_data["comment"]["commentable"]["title"] %>
<%= sanitize(json_data["comment"]["commentable"]["title"]) %>
</a>
<%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "default" %>
<% elsif notification.action == "Moderation" %>
@ -20,7 +20,7 @@
<br/><br>
<b>Here's the article for context:
<a href="<%= json_data["comment"]["commentable"]["path"] %>">
<%= json_data["comment"]["commentable"]["title"] %>
<%= sanitize(json_data["comment"]["commentable"]["title"]) %>
</a></b>
<%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "moderation" %>
<br>
@ -29,7 +29,7 @@
<a href="<%= json_data["user"]["path"] %>"><%= json_data["user"]["name"] %></a>
wrote their first comment on:
<a href="<%= json_data["comment"]["commentable"]["path"] %>">
<%= json_data["comment"]["commentable"]["title"] %>
<%= sanitize(json_data["comment"]["commentable"]["title"]) %>
</a>
<%= render "notifications/activity/activities/shared/comment_box", activity: activity, context: "moderation" %>
Give them their first reply! 🎉

2
app/views/notifications/_reaction.html.erb
View file

@ -27,7 +27,7 @@
<strong>
<a href="<%= first_notification.json_data["reaction"]["reactable"]["path"] %>" class="notification-comment-reacted-link">
<%# title is blank when it's a comment with only an image, for example %>
<%= first_notification.json_data["reaction"]["reactable"]["title"].blank? ? "your #{first_notification.json_data["reaction"]["reactable_type"].downcase}" : first_notification.json_data["reaction"]["reactable"]["title"] %>
<%= first_notification.json_data["reaction"]["reactable"]["title"].blank? ? "your #{first_notification.json_data["reaction"]["reactable_type"].downcase}" : sanitize(first_notification.json_data["reaction"]["reactable"]["title"]) %>
</a>
</strong>
<span class="reaction-images">

106
spec/requests/notifications_spec.rb
View file

@ -4,13 +4,13 @@ RSpec.describe "NotificationsIndex", type: :request do
let(:user) { create(:user) }
describe "GET notifications" do
xit "renders page with the proper heading" do
it "renders page with the proper heading" do
get "/notifications"
expect(response.body).to include("Notifications")
end
context "when signed out" do
xit "renders the signup cue" do
it "renders the signup cue" do
get "/notifications"
expect(response.body).to include "<div class=\"signup-cue"
end
@ -19,7 +19,7 @@ RSpec.describe "NotificationsIndex", type: :request do
context "when signed in" do
before { sign_in user }
xit "does not render the signup cue" do
it "does not render the signup cue" do
get "/notifications"
expect(response.body).not_to include "Create your account"
end
@ -36,44 +36,44 @@ RSpec.describe "NotificationsIndex", type: :request do
follow_instances.each { |follow| Notification.send_new_follower_notification_without_delay(follow) }
end
xit "renders the proper message for a single notification" do
it "renders the proper message for a single notification" do
mock_follow_notifications(1)
get "/notifications"
follow_message = "#{User.last.name}</a> followed you!"
expect(response.body).to include follow_message
end
xit "renders the proper message for two notifications in the same day" do
it "renders the proper message for two notifications in the same day" do
mock_follow_notifications(2)
get "/notifications"
follow_message = "#{CGI.escapeHTML(User.last.name)}</a> and\n <a href=\"/#{CGI.escapeHTML(User.second_to_last.username)}\">#{CGI.escapeHTML(User.second_to_last.name)}</a> followed you!"
expect(response.body).to include CGI.escapeHTML(follow_message)
end
xit "renders the proper message for three or more notifications in the same day" do
it "renders the proper message for three or more notifications in the same day" do
mock_follow_notifications(rand(3..10))
get "/notifications"
follow_message = "others followed you!"
expect(response.body).to include follow_message
end
xit "groups two notifications on the same day" do
it "groups two notifications on the same day" do
mock_follow_notifications(2)
get "/notifications"
notifications = controller.instance_variable_get(:@notifications)
grouped_notifications = controller.instance_variable_get(:@notifications)[0].grouped_notifications
# only one notification object containing a group of notifications
expect(notifications.count).to eq 1
expect(grouped_notifications.count).to eq 2
end
xit "groups three or more notifications on the same day" do
it "groups three or more notifications on the same day" do
amount = rand(3..10)
mock_follow_notifications(amount)
get "/notifications"
notifications = controller.instance_variable_get(:@notifications)
expect(notifications.count).to eq 1
grouped_notifications = controller.instance_variable_get(:@notifications)[0].grouped_notifications
expect(grouped_notifications.count).to eq amount
end
xit "does not group notifications that occur on different days" do
it "does not group notifications that occur on different days" do
mock_follow_notifications(2)
Notification.last.update(created_at: Notification.last.created_at - 1.day)
get "/notifications"
@ -83,8 +83,9 @@ RSpec.describe "NotificationsIndex", type: :request do
end
context "when a user has new reaction notifications" do
let(:article1) { create(:article, user_id: user.id) }
let(:article2) { create(:article, user_id: user.id) }
let(:article1) { create(:article, user_id: user.id) }
let(:article2) { create(:article, user_id: user.id) }
let(:special_characters_article) { create(:article, user_id: user.id, title: "Nothing like good ol' blah blah blah") }
before do
sign_in user
@ -104,28 +105,28 @@ RSpec.describe "NotificationsIndex", type: :request do
reactions.each { |reaction| Notification.send_reaction_notification_without_delay(reaction) }
end
xit "renders the proper message for a single public reaction" do
it "renders the proper message for a single public reaction" do
mock_heart_reaction_notifications(1, %w(like unicorn))
get "/notifications"
message = "#{CGI.escapeHTML(User.last.name)}</strong></a> reacted to"
expect(response.body).to include message
end
xit "renders the proper message for a single private reaction" do
it "renders the proper message for a single private reaction" do
mock_heart_reaction_notifications(1, %w(readinglist))
get "/notifications"
message = "Someone reacted to"
expect(response.body).to include message
end
xit "renders the proper message for two or more public reactions" do
it "renders the proper message for two or more public reactions" do
mock_heart_reaction_notifications(2, %w(like unicorn))
get "/notifications"
message = "#{User.last.name}</a> and <a href=\"/#{CGI.escapeHTML(User.second_to_last.username)}\">#{CGI.escapeHTML(User.second_to_last.name)}</a>\n reacted to"
expect(response.body).to include CGI.escapeHTML(message)
end
xit "renders the proper message for two or more reactions where at least one is private" do
it "renders the proper message for two or more reactions where at least one is private" do
mock_heart_reaction_notifications(1, %w(readinglist))
mock_heart_reaction_notifications(1, %w(unicorn like))
get "/notifications"
@ -133,29 +134,29 @@ RSpec.describe "NotificationsIndex", type: :request do
expect(response.body).to include CGI.escapeHTML(message)
end
xit "renders the proper message for multiple public reactions" do
it "renders the proper message for multiple public reactions" do
mock_heart_reaction_notifications(3, %w(unicorn like))
get "/notifications"
message = "#{User.last.name}</a> and 2 others\n reacted to"
expect(response.body).to include CGI.escapeHTML(message)
end
xit "properly groups two notifications that have the same day and reactable" do
it "properly groups two notifications that have the same day and reactable" do
mock_heart_reaction_notifications(2, %w(unicorn like readinglist))
get "/notifications"
notifications = controller.instance_variable_get(:@notifications)
expect(notifications.count).to eq 1
grouped_notifications = controller.instance_variable_get(:@notifications)[0].grouped_notifications
expect(grouped_notifications.count).to eq 2
end
xit "properly groups three or more notifications that have the same day and reactable" do
it "properly groups three or more notifications that have the same day and reactable" do
amount = rand(3..10)
mock_heart_reaction_notifications(amount, %w(unicorn like readinglist))
get "/notifications"
notifications = controller.instance_variable_get(:@notifications)
expect(notifications.count).to eq 1
grouped_notifications = controller.instance_variable_get(:@notifications)[0].grouped_notifications
expect(grouped_notifications.count).to eq amount
end
xit "does not group notifications that are on different days but same reactable" do
it "does not group notifications that are on different days but have the same reactable" do
mock_heart_reaction_notifications(2, %w(unicorn like readinglist))
Notification.last.update(created_at: Notification.last.created_at - 1.day)
get "/notifications"
@ -163,13 +164,26 @@ RSpec.describe "NotificationsIndex", type: :request do
expect(notifications.count).to eq 2
end
xit "does not group notifications that are on the same day but different reactables" do
it "does not group notifications that are on the same day but have different reactables" do
mock_heart_reaction_notifications(1, %w(unicorn like readinglist), article1)
mock_heart_reaction_notifications(1, %w(unicorn like readinglist), article2)
get "/notifications"
notifications = controller.instance_variable_get(:@notifications)
expect(notifications.count).to eq 2
end
it "properly renders reactable titles", focus: true do
mock_heart_reaction_notifications(1, %w(unicorn like readinglist), special_characters_article)
get "/notifications"
expect(response.body).to include special_characters_article.title
end
it "properly renders reactable titles for multiple reactions", focus: true do
amount = rand(3..10)
mock_heart_reaction_notifications(amount, %w(unicorn like readinglist), special_characters_article)
get "/notifications"
expect(response.body).to include special_characters_article.title
end
end
context "when a user has a new comment notification" do
@ -183,19 +197,19 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
xit "renders the correct message" do
it "renders the correct message" do
expect(response.body).to include "commented on"
end
xit "does not render the moderation message" do
it "does not render the moderation message" do
expect(response.body).not_to include "As a trusted member"
end
xit "renders the original article's title" do
expect(response.body).to include article.title
it "renders the original article's title" do
expect(response.body).to include CGI.escapeHTML(article.title)
end
xit "renders the comment's processed HTML" do
it "renders the comment's processed HTML" do
expect(response.body).to include CGI.escapeHTML(comment.processed_html)
end
end
@ -213,15 +227,15 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
xit "renders the proper message" do
it "renders the proper message" do
expect(response.body).to include "As a trusted member"
end
xit "renders the article's title" do
it "renders the article's title" do
expect(response.body).to include CGI.escapeHTML(article.title)
end
xit "renders the comment's processed HTML" do
it "renders the comment's processed HTML" do
expect(response.body).to include CGI.escapeHTML(comment.processed_html)
end
end
@ -232,7 +246,7 @@ RSpec.describe "NotificationsIndex", type: :request do
sign_in user
end
xit "renders the welcome notification" do
it "renders the welcome notification" do
broadcast = create(:broadcast, :onboarding)
Notification.send_welcome_notification_without_delay(user.id)
get "/notifications"
@ -249,20 +263,20 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
xit "renders the proper message with the badge's title" do
it "renders the proper message with the badge's title" do
message = "You received the <strong>#{Badge.first.title}"
expect(response.body).to include CGI.escapeHTML(message)
end
xit "renders the rewarding context message" do
it "renders the rewarding context message" do
expect(response.body).to include CGI.escapeHTML(user.badge_achievements.first.rewarding_context_message)
end
xit "renders the badge's description" do
it "renders the badge's description" do
expect(response.body).to include CGI.escapeHTML(Badge.first.description)
end
xit "renders the CHECK YOUR PROFILE button" do
it "renders the CHECK YOUR PROFILE button" do
expect(response.body).to include "CHECK YOUR PROFILE"
end
end
@ -289,11 +303,11 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
xit "renders the proper message" do
it "renders the proper message" do
expect(response.body).to include "mentioned you in a comment"
end
xit "renders the processed HTML of the comment where they were mentioned" do
it "renders the processed HTML of the comment where they were mentioned" do
expect(response.body).to include CGI.escapeHTML(comment.processed_html)
end
end
@ -309,15 +323,15 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
xit "renders the proper message" do
it "renders the proper message" do
expect(response.body).to include "made a new post:"
end
xit "renders the article's title" do
it "renders the article's title" do
expect(response.body).to include CGI.escapeHTML(article.title)
end
xit "renders the author's name" do
it "renders the author's name" do
expect(response.body).to include CGI.escapeHTML(article.user.name)
end
end