From d0719aed24d69df0f6ce7518658319a9f12cee20 Mon Sep 17 00:00:00 2001 From: Arpit Jalan 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 -