From d0719aed24d69df0f6ce7518658319a9f12cee20 Mon Sep 17 00:00:00 2001
From: Arpit Jalan <arpit@techapj.com>
Date: Tue, 26 Jan 2016 00:04:49 +0530
Subject: [PATCH] FIX: changing topic from one watched category to another
watched category makes topic 'new' again
---
app/models/category_user.rb | 6 ++++--
spec/models/category_user_spec.rb | 36 ++++++++++++++++++++++++++++++-
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/app/models/category_user.rb b/app/models/category_user.rb
index 2922b685888..e7138d91da6 100644
--- a/app/models/category_user.rb
+++ b/app/models/category_user.rb
@@ -23,7 +23,7 @@ class CategoryUser < ActiveRecord::Base
# we want to apply default of the new category
category_id = new_category.id
# 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
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
- def self.remove_default_from_topic(topic_id, level, reason)
+ def self.remove_default_from_topic(topic_id, category_id, level, reason)
sql = <<-SQL
DELETE FROM topic_users
WHERE topic_id = :topic_id
AND notifications_changed_at IS NULL
AND notification_level = :level
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
exec_sql(sql,
topic_id: topic_id,
+ category_id: category_id,
level: level,
reason: reason
)
diff --git a/spec/models/category_user_spec.rb b/spec/models/category_user_spec.rb
index 90fe0b1c5b3..9d7121c5998 100644
--- a/spec/models/category_user_spec.rb
+++ b/spec/models/category_user_spec.rb
@@ -80,6 +80,41 @@ describe CategoryUser do
expect(TopicUser.get(post.topic, user)).to be_blank
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
user = Fabricate(:user)
category = Fabricate(:category)
@@ -95,4 +130,3 @@ describe CategoryUser do
end
end
-