Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: digest emails should not include posts that are still in the edit grace period

This commit is contained in:
Neil Lalonde 2017-08-14 12:47:33 -04:00
parent 89e919ec42
commit 94d8f6d734
5 changed files with 148 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/mailers/user_notifications.rb
View File

@ -99,6 +99,7 @@ class UserNotifications < ActionMailer::Base
.where('posts.post_type = ?', Post.types[:regular])
.where('posts.deleted_at IS NULL AND posts.hidden = false AND posts.user_deleted = false')
.where("posts.post_number > ? AND posts.score > ?", 1, ScoreCalculator.default_score_weights[:like_score] * 5.0)
.where('posts.created_at < ?', (SiteSetting.editing_grace_period || 0).seconds.ago)
.limit(SiteSetting.digest_posts)
else
[]

1
app/models/topic.rb
View File

@ -337,6 +337,7 @@ class Topic < ActiveRecord::Base
.where(closed: false, archived: false)
.where("COALESCE(topic_users.notification_level, 1) <> ?", TopicUser.notification_levels[:muted])
.created_since(since)
.where('topics.created_at < ?', (SiteSetting.editing_grace_period || 0).seconds.ago)
.listable_topics
.includes(:category)

5
spec/jobs/user_email_spec.rb
View File

@ -80,7 +80,10 @@ describe Jobs::UserEmail do
context "email_log" do
before { Fabricate(:post) }
before do
SiteSetting.editing_grace_period = 0
Fabricate(:post)
end
it "creates an email log when the mail is sent (via Email::Sender)" do
last_emailed_at = user.last_emailed_at

30
spec/mailers/user_notifications_spec.rb
View File

@ -106,7 +106,7 @@ describe UserNotifications do
context "with new topics" do
before do
Fabricate(:topic, user: Fabricate(:coding_horror))
Fabricate(:topic, user: Fabricate(:coding_horror), created_at: 1.hour.ago)
end
it "works" do
@ -127,8 +127,8 @@ describe UserNotifications do
end
it "excludes deleted topics and their posts" do
deleted = Fabricate(:topic, user: Fabricate(:user), title: "Delete this topic plz")
post = Fabricate(:post, topic: deleted, score: 100.0, post_number: 2, raw: "Your wish is my command")
deleted = Fabricate(:topic, user: Fabricate(:user), title: "Delete this topic plz", created_at: 1.hour.ago)
post = Fabricate(:post, topic: deleted, score: 100.0, post_number: 2, raw: "Your wish is my command", created_at: 1.hour.ago)
deleted.trash!
html = subject.html_part.body.to_s
expect(html).to_not include deleted.title
@ -136,10 +136,10 @@ describe UserNotifications do
end
it "excludes whispers and other post types that don't belong" do
t = Fabricate(:topic, user: Fabricate(:user), title: "Who likes the same stuff I like?")
whisper = Fabricate(:post, topic: t, score: 100.0, post_number: 2, raw: "You like weird stuff", post_type: Post.types[:whisper])
mod_action = Fabricate(:post, topic: t, score: 100.0, post_number: 3, raw: "This topic unlisted", post_type: Post.types[:moderator_action])
small_action = Fabricate(:post, topic: t, score: 100.0, post_number: 4, raw: "A small action", post_type: Post.types[:small_action])
t = Fabricate(:topic, user: Fabricate(:user), title: "Who likes the same stuff I like?", created_at: 1.hour.ago)
whisper = Fabricate(:post, topic: t, score: 100.0, post_number: 2, raw: "You like weird stuff", post_type: Post.types[:whisper], created_at: 1.hour.ago)
mod_action = Fabricate(:post, topic: t, score: 100.0, post_number: 3, raw: "This topic unlisted", post_type: Post.types[:moderator_action], created_at: 1.hour.ago)
small_action = Fabricate(:post, topic: t, score: 100.0, post_number: 4, raw: "A small action", post_type: Post.types[:small_action], created_at: 1.hour.ago)
html = subject.html_part.body.to_s
expect(html).to_not include whisper.raw
expect(html).to_not include mod_action.raw
@ -147,16 +147,24 @@ describe UserNotifications do
end
it "excludes deleted and hidden posts" do
t = Fabricate(:topic, user: Fabricate(:user), title: "Post objectionable stuff here")
deleted = Fabricate(:post, topic: t, score: 100.0, post_number: 2, raw: "This post is uncalled for", deleted_at: 5.minutes.ago)
hidden = Fabricate(:post, topic: t, score: 100.0, post_number: 3, raw: "Try to find this post", hidden: true, hidden_at: 5.minutes.ago, hidden_reason_id: Post.hidden_reasons[:flagged_by_tl3_user])
user_deleted = Fabricate(:post, topic: t, score: 100.0, post_number: 4, raw: "I regret this post", user_deleted: true)
t = Fabricate(:topic, user: Fabricate(:user), title: "Post objectionable stuff here", created_at: 1.hour.ago)
deleted = Fabricate(:post, topic: t, score: 100.0, post_number: 2, raw: "This post is uncalled for", deleted_at: 5.minutes.ago, created_at: 1.hour.ago)
hidden = Fabricate(:post, topic: t, score: 100.0, post_number: 3, raw: "Try to find this post", hidden: true, hidden_at: 5.minutes.ago, hidden_reason_id: Post.hidden_reasons[:flagged_by_tl3_user], created_at: 1.hour.ago)
user_deleted = Fabricate(:post, topic: t, score: 100.0, post_number: 4, raw: "I regret this post", user_deleted: true, created_at: 1.hour.ago)
html = subject.html_part.body.to_s
expect(html).to_not include deleted.raw
expect(html).to_not include hidden.raw
expect(html).to_not include user_deleted.raw
end
it "excludes posts that are newer than editing grace period" do
SiteSetting.editing_grace_period = 5.minutes
too_new = Fabricate(:topic, user: Fabricate(:user), title: "Oops I need to edit this", created_at: 1.minute.ago)
too_new_post = Fabricate(:post, user: too_new.user, topic: too_new, score: 100.0, post_number: 1, created_at: 1.minute.ago)
html = subject.html_part.body.to_s
expect(html).to_not include too_new.title
end
it "uses theme color" do
cs = Fabricate(:color_scheme, name: 'Fancy', color_scheme_colors: [
Fabricate(:color_scheme_color, name: 'header_primary', hex: 'F0F0F0'),

228
spec/models/topic_spec.rb
View File

@ -1327,113 +1327,131 @@ describe Topic do
describe 'for_digest' do
let(:user) { Fabricate.build(:user) }
it "returns none when there are no topics" do
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
context "no edit grace period" do
before do
SiteSetting.editing_grace_period = 0
end
it "returns none when there are no topics" do
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "doesn't return category topics" do
Fabricate(:category)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "returns regular topics" do
topic = Fabricate(:topic)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to eq([topic])
end
it "doesn't return topics from muted categories" do
user = Fabricate(:user)
category = Fabricate(:category)
Fabricate(:topic, category: category)
CategoryUser.set_notification_level_for_category(user, CategoryUser.notification_levels[:muted], category.id)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "doesn't return topics from suppressed categories" do
user = Fabricate(:user)
category = Fabricate(:category)
Fabricate(:topic, category: category)
SiteSetting.digest_suppress_categories = "#{category.id}"
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "doesn't return topics from TL0 users" do
new_user = Fabricate(:user, trust_level: 0)
Fabricate(:topic, user_id: new_user.id)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "returns topics from TL0 users if given include_tl0" do
new_user = Fabricate(:user, trust_level: 0)
topic = Fabricate(:topic, user_id: new_user.id)
expect(Topic.for_digest(user, 1.year.ago, top_order: true, include_tl0: true)).to eq([topic])
end
it "returns topics from TL0 users if enabled in preferences" do
new_user = Fabricate(:user, trust_level: 0)
topic = Fabricate(:topic, user_id: new_user.id)
u = Fabricate(:user)
u.user_option.include_tl0_in_digests = true
expect(Topic.for_digest(u, 1.year.ago, top_order: true)).to eq([topic])
end
it "doesn't return topics with only muted tags" do
user = Fabricate(:user)
tag = Fabricate(:tag)
TagUser.change(user.id, tag.id, TagUser.notification_levels[:muted])
Fabricate(:topic, tags: [tag])
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "returns topics with both muted and not muted tags" do
user = Fabricate(:user)
muted_tag, other_tag = Fabricate(:tag), Fabricate(:tag)
TagUser.change(user.id, muted_tag.id, TagUser.notification_levels[:muted])
topic = Fabricate(:topic, tags: [muted_tag, other_tag])
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to eq([topic])
end
it "returns topics with no tags too" do
user = Fabricate(:user)
muted_tag = Fabricate(:tag)
TagUser.change(user.id, muted_tag.id, TagUser.notification_levels[:muted])
topic1 = Fabricate(:topic, tags: [muted_tag])
topic2 = Fabricate(:topic, tags: [Fabricate(:tag), Fabricate(:tag)])
topic3 = Fabricate(:topic)
topics = Topic.for_digest(user, 1.year.ago, top_order: true)
expect(topics.size).to eq(2)
expect(topics).to contain_exactly(topic2, topic3)
end
it "sorts by category notification levels" do
category1, category2 = Fabricate(:category), Fabricate(:category)
2.times { |i| Fabricate(:topic, category: category1) }
topic1 = Fabricate(:topic, category: category2)
2.times { |i| Fabricate(:topic, category: category1) }
CategoryUser.create(user: user, category: category2, notification_level: CategoryUser.notification_levels[:watching])
for_digest = Topic.for_digest(user, 1.year.ago, top_order: true)
expect(for_digest.first).to eq(topic1)
end
it "sorts by topic notification levels" do
topics = []
3.times { |i| topics << Fabricate(:topic) }
user = Fabricate(:user)
TopicUser.create(user_id: user.id, topic_id: topics[0].id, notification_level: TopicUser.notification_levels[:tracking])
TopicUser.create(user_id: user.id, topic_id: topics[2].id, notification_level: TopicUser.notification_levels[:watching])
for_digest = Topic.for_digest(user, 1.year.ago, top_order: true).pluck(:id)
expect(for_digest).to eq([topics[2].id, topics[0].id, topics[1].id])
end
end
it "doesn't return category topics" do
Fabricate(:category)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
context "with editing_grace_period" do
before do
SiteSetting.editing_grace_period = 5.minutes
end
it "returns regular topics" do
topic = Fabricate(:topic)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to eq([topic])
end
it "doesn't return topics from muted categories" do
user = Fabricate(:user)
category = Fabricate(:category)
Fabricate(:topic, category: category)
CategoryUser.set_notification_level_for_category(user, CategoryUser.notification_levels[:muted], category.id)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "doesn't return topics from suppressed categories" do
user = Fabricate(:user)
category = Fabricate(:category)
Fabricate(:topic, category: category)
SiteSetting.digest_suppress_categories = "#{category.id}"
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "doesn't return topics from TL0 users" do
new_user = Fabricate(:user, trust_level: 0)
Fabricate(:topic, user_id: new_user.id)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "returns topics from TL0 users if given include_tl0" do
new_user = Fabricate(:user, trust_level: 0)
topic = Fabricate(:topic, user_id: new_user.id)
expect(Topic.for_digest(user, 1.year.ago, top_order: true, include_tl0: true)).to eq([topic])
end
it "returns topics from TL0 users if enabled in preferences" do
new_user = Fabricate(:user, trust_level: 0)
topic = Fabricate(:topic, user_id: new_user.id)
u = Fabricate(:user)
u.user_option.include_tl0_in_digests = true
expect(Topic.for_digest(u, 1.year.ago, top_order: true)).to eq([topic])
end
it "doesn't return topics with only muted tags" do
user = Fabricate(:user)
tag = Fabricate(:tag)
TagUser.change(user.id, tag.id, TagUser.notification_levels[:muted])
Fabricate(:topic, tags: [tag])
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "returns topics with both muted and not muted tags" do
user = Fabricate(:user)
muted_tag, other_tag = Fabricate(:tag), Fabricate(:tag)
TagUser.change(user.id, muted_tag.id, TagUser.notification_levels[:muted])
topic = Fabricate(:topic, tags: [muted_tag, other_tag])
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to eq([topic])
end
it "returns topics with no tags too" do
user = Fabricate(:user)
muted_tag = Fabricate(:tag)
TagUser.change(user.id, muted_tag.id, TagUser.notification_levels[:muted])
topic1 = Fabricate(:topic, tags: [muted_tag])
topic2 = Fabricate(:topic, tags: [Fabricate(:tag), Fabricate(:tag)])
topic3 = Fabricate(:topic)
topics = Topic.for_digest(user, 1.year.ago, top_order: true)
expect(topics.size).to eq(2)
expect(topics).to contain_exactly(topic2, topic3)
end
it "sorts by category notification levels" do
category1, category2 = Fabricate(:category), Fabricate(:category)
2.times { |i| Fabricate(:topic, category: category1) }
topic1 = Fabricate(:topic, category: category2)
2.times { |i| Fabricate(:topic, category: category1) }
CategoryUser.create(user: user, category: category2, notification_level: CategoryUser.notification_levels[:watching])
for_digest = Topic.for_digest(user, 1.year.ago, top_order: true)
expect(for_digest.first).to eq(topic1)
end
it "sorts by topic notification levels" do
topics = []
3.times { |i| topics << Fabricate(:topic) }
user = Fabricate(:user)
TopicUser.create(user_id: user.id, topic_id: topics[0].id, notification_level: TopicUser.notification_levels[:tracking])
TopicUser.create(user_id: user.id, topic_id: topics[2].id, notification_level: TopicUser.notification_levels[:watching])
for_digest = Topic.for_digest(user, 1.year.ago, top_order: true).pluck(:id)
expect(for_digest).to eq([topics[2].id, topics[0].id, topics[1].id])
it "excludes topics that are within the grace period" do
topic1 = Fabricate(:topic, created_at: 6.minutes.ago)
topic2 = Fabricate(:topic, created_at: 4.minutes.ago)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to eq([topic1])
end
end
end
@ -1441,7 +1459,7 @@ describe Topic do
describe 'secured' do
it 'can remove secure groups' do
category = Fabricate(:category, read_restricted: true)
Fabricate(:topic, category: category)
Fabricate(:topic, category: category, created_at: 1.day.ago)
expect(Topic.secured(Guardian.new(nil)).count).to eq(0)
expect(Topic.secured(Guardian.new(Fabricate(:admin))).count).to eq(2)