FIX: do not show flair bg color if flair is not visible (#13969)

FIX: do not show flair bg color if flair is not visible (#13969)

follow up to https://github.com/discourse/discourse/commit/fe3e18f9814d94cf5ca19891262b9376861ce3d0

diff --git a/app/models/group.rb b/app/models/group.rb
index 93ff94b..f766f9c 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -803,19 +803,20 @@ class Group < ActiveRecord::Base
   end
 
   def flair_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
+    return if members_visibility_level != Group.visibility_levels[:public]
+    return if visibility_level != Group.visibility_levels[:public]
+
+    return flair_icon if flair_type == :icon
+    return upload_cdn_path(flair_upload.url) if flair_type == :image
+
+    nil
+  end
+
+  def flair_bg_color
+    return if members_visibility_level != Group.visibility_levels[:public]
+    return if visibility_level != Group.visibility_levels[:public]
+
+    read_attribute(:flair_bg_color)
   end
 
   [:muted, :regular, :tracking, :watching, :watching_first_post].each do |level|
diff --git a/spec/components/user_lookup_spec.rb b/spec/components/user_lookup_spec.rb
index 65c5f2c..e96cf11 100644
--- a/spec/components/user_lookup_spec.rb
+++ b/spec/components/user_lookup_spec.rb
@@ -55,7 +55,7 @@ describe UserLookup do
   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!(:group) { Fabricate(:group, name: "flair_group", flair_icon: "icon", flair_bg_color: "40E0D0", visibility_level: Group.visibility_levels[:public], members_visibility_level: Group.visibility_levels[:public]) }
     fab!(:user2) { Fabricate(:user, flair_group: group) }
 
     before do
@@ -79,6 +79,7 @@ describe UserLookup do
       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")
+      expect(user_lookup_group.flair_bg_color).to eq("40E0D0")
     end
   end
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 5f989c0..3d9bacb 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1310,10 +1310,12 @@ describe Group do
   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])
+    public_group = Fabricate(:group, flair_icon: "icon", flair_bg_color: "40E0D0", visibility_level: Group.visibility_levels[:public], members_visibility_level: Group.visibility_levels[:public])
+    private_group = Fabricate(:group, flair_icon: "icon", flair_bg_color: "40E0D0", 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)
+    expect(public_group.flair_bg_color).to eq("40E0D0")
+    expect(private_group.flair_bg_color).to eq(nil)
   end
 end
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index fd30fe8..d933a7d 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -723,7 +723,7 @@ describe GroupsController do
 
         group.reload
 
-        expect(group.flair_bg_color).to eq('FFF')
+        expect(group.flair_bg_color).to eq(nil)
         expect(group.flair_color).to eq('BBB')
         expect(group.flair_url).to eq(nil)
         expect(group.bio_raw).to eq('testing')
@@ -834,9 +834,10 @@ describe GroupsController do
         expect(response.status).to eq(200)
 
         group.reload
-        expect(group.flair_bg_color).to eq('FFF')
+        expect(group.flair_bg_color).to eq(nil)
         expect(group.flair_color).to eq('BBB')
         expect(group.flair_icon).to eq('fa-adjust')
+        expect(group.flair_url).to eq(nil)
         expect(group.name).to eq('admins')
         expect(group.visibility_level).to eq(1)
         expect(group.mentionable_level).to eq(1)
@@ -1005,9 +1006,10 @@ describe GroupsController do
         expect(response.status).to eq(200)
 
         group.reload
-        expect(group.flair_bg_color).to eq('FFF')
+        expect(group.flair_bg_color).to eq(nil)
         expect(group.flair_color).to eq('BBB')
         expect(group.flair_icon).to eq('fa-adjust')
+        expect(group.flair_url).to eq(nil)
         expect(group.name).to eq('trust_level_4')
         expect(group.mentionable_level).to eq(1)
         expect(group.messageable_level).to eq(1)

GitHub sha: 0d8fd9ace60ad676af0cfb58d65191821a77e8a9

This commit appears in #13969 which was approved by eviltrout. It was merged by techAPJ.