Merge pull request #3975 from techAPJ/category-user

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

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
     )

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
-