From 0d8fd9ace60ad676af0cfb58d65191821a77e8a9 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 6 Aug 2021 20:53:23 +0530 Subject: [PATCH] FIX: do not show flair bg color if flair is not visible (#13969) follow up to https://github.com/discourse/discourse/commit/fe3e18f9814d94cf5ca19891262b9376861ce3d0 --- app/models/group.rb | 27 +++++++++++++------------ spec/components/user_lookup_spec.rb | 3 ++- spec/models/group_spec.rb | 6 ++++-- spec/requests/groups_controller_spec.rb | 8 +++++--- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 93ff94b7a07..f766f9c372c 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 65c5f2c9e0d..e96cf117335 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 5f989c0cb0a..3d9bacb095c 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 fd30fe8084b..d933a7d4819 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)