From 69eecf92d0886d9f1e92ea00c8b1bbf7801dab0c Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 7 Jun 2023 10:15:39 +1000 Subject: [PATCH] FIX: Use a default hashtag icon color for user with no permission (#21825) One user can create a post or chat message with a hashtag they have permission to use, but then when other users look at that post they will see an empty space next to the hashtag because they do not have the permission to load the colors in CSS classes for the related category. This fixes the issue by adding a default color with a special CSS class if the user doesn't have permission to see the linked channel/category on the hashtag. --- .../app/lib/hashtag-types/category.js | 6 ++- .../common/components/hashtag.scss | 8 ++++ .../discourse/lib/hashtag-types/channel.js | 6 ++- .../spec/system/hashtag_autocomplete_spec.rb | 40 +++++++++++++++++++ spec/system/hashtag_autocomplete_spec.rb | 17 ++++++++ spec/system/page_objects/pages/topic.rb | 6 ++- 6 files changed, 79 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js b/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js index ffb8f3001d4..e37a8e6617b 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js @@ -31,6 +31,10 @@ export default class CategoryHashtagType extends HashtagTypeBase { } generateIconHTML(hashtag) { - return ``; + const hashtagId = parseInt(hashtag.id, 10); + const colorCssClass = !this.preloadedData.mapBy("id").includes(hashtagId) + ? "hashtag-missing" + : `hashtag-color--${this.type}-${hashtag.id}`; + return ``; } } diff --git a/app/assets/stylesheets/common/components/hashtag.scss b/app/assets/stylesheets/common/components/hashtag.scss index ab328819ddb..9c9825d9d28 100644 --- a/app/assets/stylesheets/common/components/hashtag.scss +++ b/app/assets/stylesheets/common/components/hashtag.scss @@ -25,6 +25,10 @@ a.hashtag { .d-icon { font-size: var(--font-down-1); margin: 0 0.2em 0 0.1em; + + &.hashtag-missing { + color: var(--primary-medium); + } } img.emoji { @@ -43,6 +47,10 @@ a.hashtag { height: 9px; margin-right: 5px; display: inline-block; + + &.hashtag-missing { + background-color: var(--primary-medium); + } } } diff --git a/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js b/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js index 5e49cb37e31..f49216bbb38 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js +++ b/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js @@ -25,8 +25,12 @@ export default class ChannelHashtagType extends HashtagTypeBase { } generateIconHTML(hashtag) { + const hashtagId = parseInt(hashtag.id, 10); + const colorCssClass = !this.preloadedData.mapBy("id").includes(hashtagId) + ? "hashtag-missing" + : `hashtag-color--${this.type}-${hashtag.id}`; return iconHTML(hashtag.icon, { - class: `hashtag-color--${this.type}-${hashtag.id}`, + class: colorCssClass, }); } } diff --git a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb index 2973832dd67..d7d3c7d5ec4 100644 --- a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb +++ b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb @@ -80,4 +80,44 @@ describe "Using #hashtag autocompletion to search for and lookup channels", razed HTML end + + context "when a user cannot access the category for a cooked channel hashtag" do + fab!(:admin) { Fabricate(:admin) } + fab!(:manager_group) { Fabricate(:group, name: "Managers") } + fab!(:private_category) do + Fabricate(:private_category, name: "Management", slug: "management", group: manager_group) + end + fab!(:admin_group_user) { Fabricate(:group_user, user: admin, group: manager_group) } + fab!(:management_channel) do + Fabricate(:chat_channel, chatable: private_category, slug: "management") + end + fab!(:post_with_private_category) do + Fabricate( + :post, + topic: topic, + raw: "this is a secret #management::channel channel", + user: admin, + ) + end + fab!(:message_with_private_channel) do + Fabricate( + :chat_message, + chat_channel: channel1, + user: admin, + message: "this is a secret #management channel", + ) + end + + before { management_channel.add(admin) } + + it "shows a default color and css class for the channel icon in a post" do + topic_page.visit_topic(topic, post_number: post_with_private_category.post_number) + expect(page).to have_css(".hashtag-cooked .hashtag-missing") + end + + it "shows a default color and css class for the channel icon in a channel" do + chat_page.visit_channel(channel1) + expect(page).to have_css(".hashtag-cooked .hashtag-missing") + end + end end diff --git a/spec/system/hashtag_autocomplete_spec.rb b/spec/system/hashtag_autocomplete_spec.rb index cc7d40a94df..da22f0728fd 100644 --- a/spec/system/hashtag_autocomplete_spec.rb +++ b/spec/system/hashtag_autocomplete_spec.rb @@ -93,4 +93,21 @@ describe "Using #hashtag autocompletion to search for and lookup categories and cooltag HTML end + + context "when a user cannot access the category for a hashtag cooked in another post" do + fab!(:admin) { Fabricate(:admin) } + fab!(:manager_group) { Fabricate(:group, name: "Managers") } + fab!(:private_category) do + Fabricate(:private_category, name: "Management", slug: "management", group: manager_group) + end + fab!(:admin_group_user) { Fabricate(:group_user, user: admin, group: manager_group) } + fab!(:post_with_private_category) do + Fabricate(:post, topic: topic, raw: "this is a secret #management category", user: admin) + end + + it "shows a default color and css class for the category icon square" do + topic_page.visit_topic(topic, post_number: post_with_private_category.post_number) + expect(page).to have_css(".hashtag-cooked .hashtag-missing") + end + end end diff --git a/spec/system/page_objects/pages/topic.rb b/spec/system/page_objects/pages/topic.rb index dbc56fe3d40..15b7e23c926 100644 --- a/spec/system/page_objects/pages/topic.rb +++ b/spec/system/page_objects/pages/topic.rb @@ -8,8 +8,10 @@ module PageObjects @fast_edit_component = PageObjects::Components::FastEditor.new end - def visit_topic(topic) - page.visit "/t/#{topic.id}" + def visit_topic(topic, post_number: nil) + url = "/t/#{topic.id}" + url += "/#{post_number}" if post_number + page.visit url self end