From fe3e18f9814d94cf5ca19891262b9376861ce3d0 Mon Sep 17 00:00:00 2001
From: Arpit Jalan <arpit@techapj.com>
Date: Mon, 2 Aug 2021 06:21:00 +0530
Subject: [PATCH] FIX: do not show private group flair on user avatars (#13872)

Meta ref: https://meta.discourse.org/t/visible-flair-for-invisible-groups-is-that-on-purpose/167674
---
 app/models/group.rb                     | 15 ++++++++-----
 app/serializers/site_serializer.rb      |  4 +++-
 lib/user_lookup.rb                      |  2 +-
 spec/components/user_lookup_spec.rb     | 28 +++++++++++++++++++++++++
 spec/models/group_spec.rb               |  8 +++++++
 spec/requests/groups_controller_spec.rb |  2 +-
 6 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index 4832b9a7743..5cab53bdc4e 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -803,11 +803,16 @@ class Group < ActiveRecord::Base
   end
 
   def flair_url
-    case flair_type
-    when :icon
-      flair_icon
-    when :image
-      upload_cdn_path(flair_upload.url)
+    if members_visibility_level == Group.visibility_levels[:public] &&
+      visibility_level == Group.visibility_levels[:public]
+      case flair_type
+      when :icon
+        flair_icon
+      when :image
+        upload_cdn_path(flair_upload.url)
+      else
+        nil
+      end
     else
       nil
     end
diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb
index c7d8d5d66c2..fba550565df 100644
--- a/app/serializers/site_serializer.rb
+++ b/app/serializers/site_serializer.rb
@@ -63,7 +63,7 @@ class SiteSerializer < ApplicationSerializer
   def groups
     cache_anon_fragment("group_names") do
       object.groups.order(:name)
-        .select(:id, :name, :flair_icon, :flair_upload_id, :flair_bg_color, :flair_color)
+        .select(:id, :name, :flair_icon, :flair_upload_id, :flair_bg_color, :flair_color, :visibility_level, :members_visibility_level)
         .map do |g|
           {
             id: g.id,
@@ -71,6 +71,8 @@ class SiteSerializer < ApplicationSerializer
             flair_url: g.flair_url,
             flair_bg_color: g.flair_bg_color,
             flair_color: g.flair_color,
+            visibility_level: g.visibility_level,
+            members_visibility_level: g.members_visibility_level,
           }
         end.as_json
     end
diff --git a/lib/user_lookup.rb b/lib/user_lookup.rb
index 590c6e27545..3ed0da73a32 100644
--- a/lib/user_lookup.rb
+++ b/lib/user_lookup.rb
@@ -6,7 +6,7 @@ class UserLookup
   end
 
   def self.group_lookup_columns
-    @group_lookup_columns ||= %i{id name flair_icon flair_upload_id flair_bg_color flair_color}
+    @group_lookup_columns ||= %i{id name flair_icon flair_upload_id flair_bg_color flair_color visibility_level members_visibility_level}
   end
 
   def initialize(user_ids = [])
diff --git a/spec/components/user_lookup_spec.rb b/spec/components/user_lookup_spec.rb
index b9fce92e67f..65c5f2c9e0d 100644
--- a/spec/components/user_lookup_spec.rb
+++ b/spec/components/user_lookup_spec.rb
@@ -53,4 +53,32 @@ describe UserLookup do
       expect(user_lookup_group.name).to eq("testgroup")
     end
   end
+
+  describe '#flair_groups' do
+    fab!(:group) { Fabricate(:group, name: "flair_group", flair_icon: "icon", visibility_level: Group.visibility_levels[:public], members_visibility_level: Group.visibility_levels[:public]) }
+    fab!(:user2) { Fabricate(:user, flair_group: group) }
+
+    before do
+      @user_lookup = UserLookup.new([user.id, user2.id, nil])
+    end
+
+    it 'returns nil if user_id does not exists' do
+      expect(@user_lookup.flair_groups[0]).to eq(nil)
+    end
+
+    it 'returns nil if user_id is nil' do
+      expect(@user_lookup.flair_groups[nil]).to eq(nil)
+    end
+
+    it 'returns nil if user has no flair group' do
+      expect(@user_lookup.flair_groups[user.id]).to eq(nil)
+    end
+
+    it 'returns group if user has flair group' do
+      user_lookup_group = @user_lookup.flair_groups[user2.id]
+      expect(user_lookup_group).to eq(group)
+      expect(user_lookup_group.name).to eq("flair_group")
+      expect(user_lookup_group.flair_url).to eq("icon")
+    end
+  end
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 5da7ff6448f..2cd66c8ee93 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1307,4 +1307,12 @@ describe Group do
       expect(Group.find_by_email("nope@test.com")).to eq(nil)
     end
   end
+
+  it "fetches flair_url based on group visibility" do
+    public_group = Fabricate(:group, flair_icon: "icon", visibility_level: Group.visibility_levels[:public], members_visibility_level: Group.visibility_levels[:public])
+    private_group = Fabricate(:group, flair_icon: "icon", visibility_level: Group.visibility_levels[:logged_on_users], members_visibility_level: Group.visibility_levels[:public])
+
+    expect(public_group.flair_url).to eq("icon")
+    expect(private_group.flair_url).to eq(nil)
+  end
 end
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index 9d6236580b7..fd30fe8084b 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -725,7 +725,7 @@ describe GroupsController do
 
         expect(group.flair_bg_color).to eq('FFF')
         expect(group.flair_color).to eq('BBB')
-        expect(group.flair_url).to eq('fa-adjust')
+        expect(group.flair_url).to eq(nil)
         expect(group.bio_raw).to eq('testing')
         expect(group.full_name).to eq('awesome team')
         expect(group.public_admission).to eq(true)