From d4bfd441ba07113b97e343220083b913e9c5582b Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 31 May 2023 19:32:06 +0530 Subject: [PATCH] FEATURE: display PM participant group names in the topics list. (#21677) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After this change, we can view all participant group names on the topic list page. Co-authored-by: Régis Hanol --- .../app/components/topic-list-item.js | 13 +++++++- .../app/templates/list/participant-groups.hbr | 8 +++++ .../app/templates/list/topic-list-item.hbr | 3 ++ .../stylesheets/common/base/_topic-list.scss | 13 ++++++++ app/models/topic.rb | 7 ++++- app/models/topic_list.rb | 5 +++ .../topic_participant_groups_summary.rb | 31 +++++++++++++++++++ app/serializers/topic_list_item_serializer.rb | 11 ++++++- config/locales/client.en.yml | 1 + lib/group_lookup.rb | 18 +++++++++++ lib/topic_query.rb | 8 ++++- lib/topic_query/private_message_lists.rb | 9 +++--- spec/lib/group_lookup_spec.rb | 21 +++++++++++++ .../topic_participant_groups_summary_spec.rb | 17 ++++++++++ 14 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/templates/list/participant-groups.hbr create mode 100644 app/models/topic_participant_groups_summary.rb create mode 100644 lib/group_lookup.rb create mode 100644 spec/lib/group_lookup_spec.rb create mode 100644 spec/models/topic_participant_groups_summary_spec.rb diff --git a/app/assets/javascripts/discourse/app/components/topic-list-item.js b/app/assets/javascripts/discourse/app/components/topic-list-item.js index f0dfb2277e1..9f176eca95e 100644 --- a/app/assets/javascripts/discourse/app/components/topic-list-item.js +++ b/app/assets/javascripts/discourse/app/components/topic-list-item.js @@ -3,7 +3,7 @@ import discourseComputed, { observes, } from "discourse-common/utils/decorators"; import Component from "@ember/component"; -import DiscourseURL from "discourse/lib/url"; +import DiscourseURL, { groupPath } from "discourse/lib/url"; import I18n from "I18n"; import { RUNTIME_OPTIONS } from "discourse-common/lib/raw-handlebars-helpers"; import { alias } from "@ember/object/computed"; @@ -115,6 +115,17 @@ export default Component.extend({ nodeClassList.toggle("read", !data.show_indicator); }, + @discourseComputed("topic.participant_groups") + participantGroups(groupNames) { + if (!groupNames) { + return []; + } + + return groupNames.map((name) => { + return { name, url: groupPath(name) }; + }); + }, + @discourseComputed("topic.id") unreadIndicatorChannel(topicId) { return `/private-messages/unread-indicator/${topicId}`; diff --git a/app/assets/javascripts/discourse/app/templates/list/participant-groups.hbr b/app/assets/javascripts/discourse/app/templates/list/participant-groups.hbr new file mode 100644 index 00000000000..3e910ed67f8 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/list/participant-groups.hbr @@ -0,0 +1,8 @@ +
+{{#each groups as |group|}} + + {{d-icon 'users'}} + {{group.name}} + +{{/each}} +
diff --git a/app/assets/javascripts/discourse/app/templates/list/topic-list-item.hbr b/app/assets/javascripts/discourse/app/templates/list/topic-list-item.hbr index dc1e35ac5e5..5bf3090c087 100644 --- a/app/assets/javascripts/discourse/app/templates/list/topic-list-item.hbr +++ b/app/assets/javascripts/discourse/app/templates/list/topic-list-item.hbr @@ -40,6 +40,9 @@ {{/unless}} {{/unless}} {{discourse-tags topic mode="list" tagsForUser=tagsForUser}} + {{#if participantGroups}} + {{raw "list/participant-groups" groups=participantGroups}} + {{/if}} {{raw "list/action-list" topic=topic postNumbers=topic.liked_post_numbers className="likes" icon="heart"}} {{#if expandPinned}} diff --git a/app/assets/stylesheets/common/base/_topic-list.scss b/app/assets/stylesheets/common/base/_topic-list.scss index dfadbd1a2b6..5816c244411 100644 --- a/app/assets/stylesheets/common/base/_topic-list.scss +++ b/app/assets/stylesheets/common/base/_topic-list.scss @@ -263,6 +263,19 @@ .discourse-tag.box { margin-right: 0.25em; } + .participant-groups { + margin-left: 0.5em; + font-weight: normal; + font-size: var(--font-down-1); + + > a { + color: var(--primary-medium); + border: solid 1px var(--primary-low); + padding: 0 0.3em 0.07em; + border-radius: 0.6em; + margin-right: 0.25em; + } + } } .topic-featured-link { diff --git a/app/models/topic.rb b/app/models/topic.rb index 6396acea035..29241514031 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -28,7 +28,7 @@ class Topic < ActiveRecord::Base def_delegator :notifier, :mute!, :notify_muted! def_delegator :notifier, :toggle_mute, :toggle_mute - attr_accessor :allowed_user_ids, :tags_changed, :includes_destination_category + attr_accessor :allowed_user_ids, :allowed_group_ids, :tags_changed, :includes_destination_category def self.max_fancy_title_length 400 @@ -293,6 +293,7 @@ class Topic < ActiveRecord::Base attr_accessor :posters # TODO: can replace with posters_summary once we remove old list code attr_accessor :participants + attr_accessor :participant_groups attr_accessor :topic_list attr_accessor :meta_data attr_accessor :include_last_poster @@ -1274,6 +1275,10 @@ class Topic < ActiveRecord::Base @participants_summary ||= TopicParticipantsSummary.new(self, options).summary end + def participant_groups_summary(options = {}) + @participant_groups_summary ||= TopicParticipantGroupsSummary.new(self, options).summary + end + def make_banner!(user, bannered_until = nil) if bannered_until bannered_until = diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb index b4be8c367fd..de50712dc79 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -97,12 +97,15 @@ class TopicList # Create a lookup for all the user ids we need user_ids = [] + group_ids = [] @topics.each do |ft| user_ids << ft.user_id << ft.last_post_user_id << ft.featured_user_ids << ft.allowed_user_ids + group_ids |= (ft.allowed_group_ids || []) end user_ids = TopicList.preload_user_ids(@topics, user_ids, self) user_lookup = UserLookup.new(user_ids) + group_lookup = GroupLookup.new(group_ids) @topics.each do |ft| ft.user_data = @topic_lookup[ft.id] if @topic_lookup.present? @@ -120,6 +123,8 @@ class TopicList ft.posters = ft.posters_summary(user_lookup: user_lookup) ft.participants = ft.participants_summary(user_lookup: user_lookup, user: @current_user) + ft.participant_groups = + ft.participant_groups_summary(group_lookup: group_lookup, group: @opts[:group]) ft.topic_list = self end diff --git a/app/models/topic_participant_groups_summary.rb b/app/models/topic_participant_groups_summary.rb new file mode 100644 index 00000000000..6ee323fed71 --- /dev/null +++ b/app/models/topic_participant_groups_summary.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# This is used on a topic page +class TopicParticipantGroupsSummary + attr_reader :topic, :options + + def initialize(topic, options = {}) + @topic = topic + @options = options + @group = options[:group] + end + + def summary + group_participants.compact + end + + def group_participants + return [] if group_ids.blank? + group_ids.map { |id| group_lookup[id] } + end + + def group_ids + ids = topic.allowed_group_ids + ids = ids - [@group.id] if @group.present? + ids + end + + def group_lookup + @group_lookup ||= options[:group_lookup] || GroupLookup.new(group_ids) + end +end diff --git a/app/serializers/topic_list_item_serializer.rb b/app/serializers/topic_list_item_serializer.rb index 3d43c0848a8..f6189a23f99 100644 --- a/app/serializers/topic_list_item_serializer.rb +++ b/app/serializers/topic_list_item_serializer.rb @@ -14,11 +14,16 @@ class TopicListItemSerializer < ListableTopicSerializer :liked_post_numbers, :featured_link, :featured_link_root_domain, - :allowed_user_count + :allowed_user_count, + :participant_groups has_many :posters, serializer: TopicPosterSerializer, embed: :objects has_many :participants, serializer: TopicPosterSerializer, embed: :objects + def include_participant_groups? + object.private_message? + end + def posters object.posters || object.posters_summary || [] end @@ -44,6 +49,10 @@ class TopicListItemSerializer < ListableTopicSerializer object.participants_summary || [] end + def participant_groups + object.participant_groups_summary || [] + end + def include_liked_post_numbers? include_post_action? :like end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d1ef7242b9c..225e87179d6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2968,6 +2968,7 @@ en: read_more_in_category: "Want to read more? Browse other topics in %{categoryLink} or view latest topics." read_more: "Want to read more? Browse all categories or view latest topics." unread_indicator: "No member has read the last post of this topic yet." + participant_groups: "Participant groups" # This string uses the ICU Message Format. See https://meta.discourse.org/t/7035 for translation guidelines. # diff --git a/lib/group_lookup.rb b/lib/group_lookup.rb new file mode 100644 index 00000000000..cd942a80c0a --- /dev/null +++ b/lib/group_lookup.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class GroupLookup + def initialize(group_ids = []) + @group_ids = group_ids.flatten.compact.uniq + end + + # Lookup a group by id + def [](group_id) + group_names[group_id] + end + + private + + def group_names + @group_names ||= Group.where(id: @group_ids).pluck(:id, :name).to_h + end +end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index ddb15e1ce40..75cbe36d3a5 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -498,7 +498,13 @@ class TopicQuery end topics.each do |t| - t.allowed_user_ids = filter == :private_messages ? t.allowed_users.map { |u| u.id } : [] + if filter == :private_messages + t.allowed_user_ids = t.allowed_users.map { |u| u.id } + t.allowed_group_ids = t.allowed_groups.map { |g| g.id } + else + t.allowed_user_ids = [] + t.allowed_group_ids = [] + end end list = TopicList.new(filter, @user, topics, options.merge(@options)) diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index 87228950b49..24db40ada31 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -71,7 +71,7 @@ class TopicQuery list = list.where("gm.id IS NULL") publish_read_state = !!group.publish_read_state list = append_read_state(list, group) if publish_read_state - create_list(:private_messages, { publish_read_state: publish_read_state }, list) + create_list(:private_messages, { publish_read_state: publish_read_state, group: group }, list) end def list_private_messages_group_archive(user) @@ -84,7 +84,7 @@ class TopicQuery publish_read_state = !!group.publish_read_state list = append_read_state(list, group) if publish_read_state - create_list(:private_messages, { publish_read_state: publish_read_state }, list) + create_list(:private_messages, { publish_read_state: publish_read_state, group: group }, list) end def list_private_messages_group_new(user) @@ -92,14 +92,14 @@ class TopicQuery list = remove_dismissed(list, user) publish_read_state = !!group.publish_read_state list = append_read_state(list, group) if publish_read_state - create_list(:private_messages, { publish_read_state: publish_read_state }, list) + create_list(:private_messages, { publish_read_state: publish_read_state, group: group }, list) end def list_private_messages_group_unread(user) list = filter_private_messages_unread(user, :group) publish_read_state = !!group.publish_read_state list = append_read_state(list, group) if publish_read_state - create_list(:private_messages, { publish_read_state: publish_read_state }, list) + create_list(:private_messages, { publish_read_state: publish_read_state, group: group }, list) end def list_private_messages_warnings(user) @@ -259,6 +259,7 @@ class TopicQuery Topic .private_messages .includes(:allowed_users) + .includes(:allowed_groups) .joins( "LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})", ) diff --git a/spec/lib/group_lookup_spec.rb b/spec/lib/group_lookup_spec.rb new file mode 100644 index 00000000000..d782ecb6f4d --- /dev/null +++ b/spec/lib/group_lookup_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.describe GroupLookup do + fab!(:group) { Fabricate(:group) } + + describe "#[]" do + before { @group_lookup = GroupLookup.new([group.id, nil]) } + + it "returns nil if group_id does not exists" do + expect(@group_lookup[0]).to eq(nil) + end + + it "returns nil if group_id is nil" do + expect(@group_lookup[nil]).to eq(nil) + end + + it "returns name if group_id exists" do + expect(@group_lookup[group.id]).to eq(group.name) + end + end +end diff --git a/spec/models/topic_participant_groups_summary_spec.rb b/spec/models/topic_participant_groups_summary_spec.rb new file mode 100644 index 00000000000..d88c1874684 --- /dev/null +++ b/spec/models/topic_participant_groups_summary_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec.describe TopicParticipantGroupsSummary do + describe "#summary" do + fab!(:group1) { Fabricate(:group) } + fab!(:group2) { Fabricate(:group) } + fab!(:group3) { Fabricate(:group) } + + let(:topic) { Fabricate(:private_message_topic) } + + it "must contain the name of allowed groups" do + topic.allowed_group_ids = [group1.id, group2.id, group3.id] + expect(described_class.new(topic, group: group1).summary).to eq([group2.name, group3.name]) + expect(described_class.new(topic, group: group2).summary).to eq([group1.name, group3.name]) + end + end +end