Fix missing avatars on topic list page

Fix missing avatars on topic list page.

Introduced in PERF: Fix N+1 for non-staff users when tagging is enabled. · discourse/discourse@b50fab2 · GitHub

From fe131c5ea27bfe72a5b0b1b6a51e61e59fa04d96 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Mon, 19 Nov 2018 14:54:40 +0800
Subject: [PATCH] Fix missing avatars on topic list page.

Introduced in https://github.com/discourse/discourse/commit/b50fab2d72c14ead753b9adaa149a6372f5d14dd

diff --git a/app/serializers/concerns/topic_tags_mixin.rb b/app/serializers/concerns/topic_tags_mixin.rb
index 56cdcd7..28f96a3 100644
--- a/app/serializers/concerns/topic_tags_mixin.rb
+++ b/app/serializers/concerns/topic_tags_mixin.rb
@@ -14,17 +14,11 @@ module TopicTagsMixin
     if scope.is_staff?
       tags
     else
-      tags - (@options[:hidden_tag_names] || hidden_tag_names)
+      tags - scope.hidden_tag_names
     end
   end
 
   def topic
     object.is_a?(Topic) ? object : object.topic
   end
-
-  private
-
-  def hidden_tag_names
-    @hidden_tag_names ||= DiscourseTagging.hidden_tag_names(scope)
-  end
 end
diff --git a/app/serializers/topic_list_serializer.rb b/app/serializers/topic_list_serializer.rb
index 8b55e04..6ee2785 100644
--- a/app/serializers/topic_list_serializer.rb
+++ b/app/serializers/topic_list_serializer.rb
@@ -9,27 +9,12 @@ class TopicListSerializer < ApplicationSerializer
              :per_page,
              :top_tags,
              :tags,
-             :shared_drafts,
-             :topics
+             :shared_drafts
 
+  has_many :topics, serializer: TopicListItemSerializer, embed: :objects
   has_many :shared_drafts, serializer: TopicListItemSerializer, embed: :objects
   has_many :tags, serializer: TagSerializer, embed: :objects
 
-  def topics
-    opts = {
-      scope: scope,
-      root: false
-    }
-
-    if SiteSetting.tagging_enabled && !scope.is_staff?
-      opts.merge!(hidden_tag_names: DiscourseTagging.hidden_tag_names(scope))
-    end
-
-    object.topics.map do |topic|
-      TopicListItemSerializer.new(topic, opts)
-    end
-  end
-
   def can_create_topic
     scope.can_create?(Topic)
   end
diff --git a/lib/guardian/tag_guardian.rb b/lib/guardian/tag_guardian.rb
index d444f20..8e40707 100644
--- a/lib/guardian/tag_guardian.rb
+++ b/lib/guardian/tag_guardian.rb
@@ -23,4 +23,14 @@ module TagGuardian
   def can_admin_tag_groups?
     is_staff? && SiteSetting.tagging_enabled
   end
+
+  def hidden_tag_names
+    @hidden_tag_names ||= begin
+      if SiteSetting.tagging_enabled && !is_staff?
+        DiscourseTagging.hidden_tag_names(self)
+      else
+        []
+      end
+    end
+  end
 end
diff --git a/spec/serializers/topic_list_serializer_spec.rb b/spec/serializers/topic_list_serializer_spec.rb
new file mode 100644
index 0000000..e92609a
--- /dev/null
+++ b/spec/serializers/topic_list_serializer_spec.rb
@@ -0,0 +1,23 @@
+require 'rails_helper'
+
+RSpec.describe TopicListSerializer do
+  let(:user) { Fabricate(:user) }
+
+  let(:topic) do
+    Fabricate(:topic).tap do |t|
+      t.allowed_user_ids = [t.user_id]
+    end
+  end
+
+  it 'should return the right payload' do
+    topic_list = TopicList.new(nil, user, [topic])
+
+    serialized = described_class.new(topic_list,
+      scope: Guardian.new(user)
+    ).as_json
+
+    expect(serialized[:users].first[:id]).to eq(topic.user_id)
+    expect(serialized[:primar_groups]).to eq([])
+    expect(serialized[:topic_list][:topics].first[:id]).to eq(topic.id)
+  end
+end

GitHub

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/missing-avatars-latest/102303/4