From 2987901043fa5d320b75987712bd2868b61cf8e2 Mon Sep 17 00:00:00 2001
From: Sam Saffron <sam.saffron@gmail.com>
Date: Wed, 24 Jun 2020 17:07:52 +1000
Subject: [PATCH] FIX: skip category notification_level unless scoped

#b19dcac2 improved the serializer so it sends default notification
levels to users to work around cases where a category edit would
would result in clients being left with invalid notification state

Unfortunately this did not address the root issue.

When we edit categories we publish state to multiple users this
means that the serializer is executed unscoped with no user.

The client already handles this case per:

https://github.com/discourse/discourse/blob/dcad720a4ce0aa67e94299ff8370fafd20cc41b2/app/assets/javascripts/discourse/app/models/site.js#L119-L119

If a property is not shipped to it, it will leave it alone on the
existing category.


This fix ensures that these wide category info updates do not
include notification state to avoid corruption of local state.
---
 app/serializers/category_serializer.rb       |  4 ++++
 spec/models/category_spec.rb                 | 18 ++++++++++++++++++
 spec/serializers/category_serializer_spec.rb |  4 ++--
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb
index 583ee138651..6fb52855cd3 100644
--- a/app/serializers/category_serializer.rb
+++ b/app/serializers/category_serializer.rb
@@ -81,6 +81,10 @@ class CategorySerializer < SiteCategorySerializer
     scope && scope.can_edit?(object)
   end
 
+  def include_notification_level?
+    scope && scope.user
+  end
+
   def notification_level
     user = scope && scope.user
     object.notification_level ||
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index f122673c0a9..cab079d3cc0 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -1134,6 +1134,24 @@ describe Category do
     end
   end
 
+  describe "messageBus" do
+    it "does not publish notification level when publishing to /categories" do
+      category = Fabricate(:category)
+      category.name = "Amazing category"
+      messages = MessageBus.track_publish("/categories") do
+        category.save!
+      end
+
+      expect(messages.length).to eq(1)
+      message = messages.first
+
+      category_hash = message.data[:categories].first
+
+      expect(category_hash[:name]).to eq(category.name)
+      expect(category_hash.key?(:notification_level)).to eq(false)
+    end
+  end
+
   describe "#ensure_consistency!" do
     it "creates category topic" do
 
diff --git a/spec/serializers/category_serializer_spec.rb b/spec/serializers/category_serializer_spec.rb
index e2a3399990b..9e81726f555 100644
--- a/spec/serializers/category_serializer_spec.rb
+++ b/spec/serializers/category_serializer_spec.rb
@@ -29,9 +29,9 @@ describe CategorySerializer do
     expect(json[:custom_fields]).to be_present
   end
 
-  it "includes the default notification level" do
+  it "does not include the default notification level when there is no user" do
     json = described_class.new(category, scope: Guardian.new, root: false).as_json
-    expect(json[:notification_level]).to eq(CategoryUser.default_notification_level)
+    expect(json.key?(:notification_level)).to eq(false)
   end
 
   describe "user notification level" do