FIX: changing topic from one watched category to another watched category makes topic 'new' again

This commit is contained in:
Arpit Jalan 2016-01-26 00:04:49 +05:30
parent 0a95fee68f
commit d0719aed24
2 changed files with 39 additions and 3 deletions

View file

@ -23,7 +23,7 @@ class CategoryUser < ActiveRecord::Base
# we want to apply default of the new category # we want to apply default of the new category
category_id = new_category.id category_id = new_category.id
# remove defaults from previous category # remove defaults from previous category
remove_default_from_topic(topic.id, TopicUser.notification_levels[:"#{s}ing"], TopicUser.notification_reasons[:"auto_#{s}_category"]) remove_default_from_topic(topic.id, category_id, TopicUser.notification_levels[:"#{s}ing"], TopicUser.notification_reasons[:"auto_#{s}_category"])
end end
apply_default_to_topic(topic.id, category_id, TopicUser.notification_levels[:"#{s}ing"], TopicUser.notification_reasons[:"auto_#{s}_category"]) apply_default_to_topic(topic.id, category_id, TopicUser.notification_levels[:"#{s}ing"], TopicUser.notification_reasons[:"auto_#{s}_category"])
@ -76,17 +76,19 @@ class CategoryUser < ActiveRecord::Base
) )
end end
def self.remove_default_from_topic(topic_id, level, reason) def self.remove_default_from_topic(topic_id, category_id, level, reason)
sql = <<-SQL sql = <<-SQL
DELETE FROM topic_users DELETE FROM topic_users
WHERE topic_id = :topic_id WHERE topic_id = :topic_id
AND notifications_changed_at IS NULL AND notifications_changed_at IS NULL
AND notification_level = :level AND notification_level = :level
AND notifications_reason_id = :reason AND notifications_reason_id = :reason
AND NOT EXISTS(SELECT 1 FROM category_users WHERE category_users.category_id = :category_id AND category_users.notification_level = :level AND category_users.user_id = topic_users.user_id)
SQL SQL
exec_sql(sql, exec_sql(sql,
topic_id: topic_id, topic_id: topic_id,
category_id: category_id,
level: level, level: level,
reason: reason reason: reason
) )

View file

@ -80,6 +80,41 @@ describe CategoryUser do
expect(TopicUser.get(post.topic, user)).to be_blank expect(TopicUser.get(post.topic, user)).to be_blank
end end
it "does not delete TopicUser record when topic category is changed, and new category has same notification level" do
# this is done so as to maintain topic notification state when topic category is changed and the new category has same notification level for the user
# see: https://meta.discourse.org/t/changing-topic-from-one-watched-category-to-another-watched-category-makes-topic-new-again/36517/15
user = Fabricate(:user)
watched_category_1 = Fabricate(:category)
watched_category_2 = Fabricate(:category)
CategoryUser.create!(user: user, category: watched_category_1, notification_level: CategoryUser.notification_levels[:watching])
CategoryUser.create!(user: user, category: watched_category_2, notification_level: CategoryUser.notification_levels[:watching])
post = create_post(category: watched_category_1)
tu = TopicUser.get(post.topic, user)
expect(tu.notification_level).to eq TopicUser.notification_levels[:watching]
# Now, change the topic's category
post.topic.change_category_to_id(watched_category_2.id)
expect(TopicUser.get(post.topic, user)).to eq tu
end
it "deletes TopicUser record when topic category is changed, and new category has different notification level" do
user = Fabricate(:user)
watched_category = Fabricate(:category)
tracked_category = Fabricate(:category)
CategoryUser.create!(user: user, category: watched_category, notification_level: CategoryUser.notification_levels[:watching])
CategoryUser.create!(user: user, category: tracked_category, notification_level: CategoryUser.notification_levels[:tracking])
post = create_post(category: watched_category)
tu = TopicUser.get(post.topic, user)
expect(tu.notification_level).to eq TopicUser.notification_levels[:watching]
# Now, change the topic's category
post.topic.change_category_to_id(tracked_category.id)
expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:tracking]
end
it "is destroyed when a user is deleted" do it "is destroyed when a user is deleted" do
user = Fabricate(:user) user = Fabricate(:user)
category = Fabricate(:category) category = Fabricate(:category)
@ -95,4 +130,3 @@ describe CategoryUser do
end end
end end