FIX: Change notification_level back to tracking only if set by plugin.

FIX: Change notification_level back to tracking only if set by plugin.

From 7663268584776a5aa8620f0109507949c4d8d525 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Tue, 4 Sep 2018 15:30:53 +0800
Subject: [PATCH] FIX: Change notification_level back to tracking only if set
 by plugin.


diff --git a/lib/topic_assigner.rb b/lib/topic_assigner.rb
index 20c43b6..9b262ff 100644
--- a/lib/topic_assigner.rb
+++ b/lib/topic_assigner.rb
@@ -232,13 +232,21 @@ SQL
 
       post.publish_change_to_clients!(:revised, reload_topic: true)
 
-      TopicUser.change(
-        assigned_to_id,
-        @topic.id,
-        notification_level: TopicUser.notification_levels[:tracking],
+      if TopicUser.exists?(
+        user_id: assigned_to_id,
+        topic: @topic,
+        notification_level: TopicUser.notification_levels[:watching],
         notifications_reason_id: TopicUser.notification_reasons[:plugin_changed]
       )
 
+        TopicUser.change(
+          assigned_to_id,
+          @topic.id,
+          notification_level: TopicUser.notification_levels[:tracking],
+          notifications_reason_id: TopicUser.notification_reasons[:plugin_changed]
+        )
+      end
+
       assigned_user = User.find_by(id: assigned_to_id)
       MessageBus.publish(
         "/staff/topic-assignment",
diff --git a/spec/lib/topic_assigner_spec.rb b/spec/lib/topic_assigner_spec.rb
index d4a9791..30cde5b 100644
--- a/spec/lib/topic_assigner_spec.rb
+++ b/spec/lib/topic_assigner_spec.rb
@@ -49,6 +49,22 @@ RSpec.describe TopicAssigner do
         .to eq(TopicUser.notification_levels[:tracking])
     end
 
+    it 'does not update notification level if it is not set by the plugin' do
+      assigner.assign(moderator)
+
+      expect(TopicUser.find_by(user: moderator).notification_level)
+        .to eq(TopicUser.notification_levels[:watching])
+
+      TopicUser.change(moderator.id, topic.id,
+        notification_level: TopicUser.notification_levels[:muted]
+      )
+
+      assigner.unassign
+
+      expect(TopicUser.find_by(user: moderator, topic: topic).notification_level)
+        .to eq(TopicUser.notification_levels[:muted])
+    end
+
     it "can unassign all a user's topics at once" do
       assigner.assign(moderator)
       TopicAssigner.unassign_all(moderator, moderator)

GitHub

@SamSaffron This should fix it. However, I noticed that if another plugin alters the notification level it might render this inaccurate but we’ll either need to allow plugins to extend TopicUser.notification_reasons or add a column that holds a custom reason.

I think this is fine for now! thanks