From fc1fd1b41689694b3916dc4e10605eb9b8bb89b7 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 7 Sep 2021 12:30:40 +0800 Subject: [PATCH] FEATURE: Display new/unread count in browse more messages for PMs. (#14188) In order to include the new/unread count in the browse more message under suggested topics, a couple of technical changes have to be made. 1. `PrivateMessageTopicTrackingState` is now auto-injected which is similar to how it is done for `TopicTrackingState`. This is done so we don't have to attempt to pass the `PrivateMessageTopicTrackingState` object multiple levels down into the suggested-topics component. While the object is auto-injected, we only fetch the initial state and start tracking when the relevant private messages routes has been hit and only when a private message's suggested topics is loaded. This is done as we do not want to add the extra overhead of fetching the inital state to all page loads but instead wait till the private messages routes are hit. 2. Previously, we would stop tracking once the `user-private-messages` route has been deactivated. However, that is not ideal since navigating out of the route and back means we send an API call to the server each time. Since `PrivateMessageTopicTrackingState` is kept in sync cheaply via messageBus, we can just continue to track the state even if the user has navigated away from the relevant stages. --- .../app/components/suggested-topics.js | 81 +++++++- .../app/controllers/user-topics-list.js | 11 +- .../discourse/app/models/post-stream.js | 21 +- .../private-message-topic-tracking-state.js | 83 +++++--- .../inject-discourse-objects.js | 18 +- .../routes/build-private-messages-route.js | 2 - .../discourse/app/routes/topic-from-params.js | 8 + .../app/routes/user-private-messages.js | 40 +--- .../acceptance/user-private-messages-test.js | 195 ++++++++++++++---- app/serializers/current_user_serializer.rb | 4 +- app/serializers/suggested_topics_mixin.rb | 19 ++ config/locales/client.en.yml | 32 +++ lib/topic_view.rb | 11 +- .../current_user_serializer_spec.rb | 4 +- .../serializers/topic_view_serializer_spec.rb | 28 +++ 15 files changed, 423 insertions(+), 134 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/suggested-topics.js b/app/assets/javascripts/discourse/app/components/suggested-topics.js index f601c1d84d2..02616fdaa73 100644 --- a/app/assets/javascripts/discourse/app/components/suggested-topics.js +++ b/app/assets/javascripts/discourse/app/components/suggested-topics.js @@ -5,6 +5,7 @@ import Site from "discourse/models/site"; import { categoryBadgeHTML } from "discourse/helpers/category-link"; import discourseComputed from "discourse-common/utils/decorators"; import getURL from "discourse-common/lib/get-url"; +import { iconHTML } from "discourse-common/lib/icon-library"; export default Component.extend({ tagName: "", @@ -18,13 +19,68 @@ export default Component.extend({ } }), - @discourseComputed("topic", "topicTrackingState.messageCount") + @discourseComputed( + "topic", + "pmTopicTrackingState.isTracking", + "pmTopicTrackingState.statesModificationCounter", + "topicTrackingState.messageCount" + ) browseMoreMessage(topic) { - // TODO decide what to show for pms - if (topic.get("isPrivateMessage")) { - return; - } + return topic.isPrivateMessage + ? this._privateMessageBrowseMoreMessage(topic) + : this._topicBrowseMoreMessage(topic); + }, + _privateMessageBrowseMoreMessage(topic) { + const username = this.currentUser.username; + const suggestedGroupName = topic.suggested_group_name; + const inboxFilter = suggestedGroupName ? "group" : "user"; + + const unreadCount = this.pmTopicTrackingState.lookupCount("unread", { + inboxFilter: inboxFilter, + groupName: suggestedGroupName, + }); + + const newCount = this.pmTopicTrackingState.lookupCount("new", { + inboxFilter: inboxFilter, + groupName: suggestedGroupName, + }); + + if (unreadCount + newCount > 0) { + const hasBoth = unreadCount > 0 && newCount > 0; + + if (suggestedGroupName) { + return I18n.messageFormat("user.messages.read_more_group_pm_MF", { + BOTH: hasBoth, + UNREAD: unreadCount, + NEW: newCount, + username: username, + groupName: suggestedGroupName, + groupLink: this._groupLink(username, suggestedGroupName), + basePath: getURL(""), + }); + } else { + return I18n.messageFormat("user.messages.read_more_personal_pm_MF", { + BOTH: hasBoth, + UNREAD: unreadCount, + NEW: newCount, + username, + basePath: getURL(""), + }); + } + } else if (suggestedGroupName) { + return I18n.t("user.messages.read_more_in_group", { + groupLink: this._groupLink(username, suggestedGroupName), + }); + } else { + return I18n.t("user.messages.read_more", { + basePath: getURL(""), + username, + }); + } + }, + + _topicBrowseMoreMessage(topic) { const opts = { latestLink: `${I18n.t( "topic.view_latest_topics" @@ -50,8 +106,13 @@ export default Component.extend({ ""; } - const unreadTopics = this.topicTrackingState.countUnread(); - const newTopics = this.currentUser ? this.topicTrackingState.countNew() : 0; + let unreadTopics = 0; + let newTopics = 0; + + if (this.currentUser) { + unreadTopics = this.topicTrackingState.countUnread(); + newTopics = this.topicTrackingState.countNew(); + } if (newTopics + unreadTopics > 0) { const hasBoth = unreadTopics > 0 && newTopics > 0; @@ -71,4 +132,10 @@ export default Component.extend({ return I18n.t("topic.read_more", opts); } }, + + _groupLink(username, groupName) { + return `${iconHTML("users")} ${groupName}`; + }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js index 6800f060d3c..7e95ed10997 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js +++ b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js @@ -18,7 +18,6 @@ export default Controller.extend(BulkTopicSelection, { showPosters: false, channel: null, tagsForUser: null, - pmTopicTrackingState: null, incomingCount: reads("pmTopicTrackingState.newIncoming.length"), @discourseComputed("emptyState", "model.topics.length", "incomingCount") @@ -46,15 +45,11 @@ export default Controller.extend(BulkTopicSelection, { }, subscribe() { - this.pmTopicTrackingState?.trackIncoming( - this.inbox, - this.filter, - this.group - ); + this.pmTopicTrackingState.trackIncoming(this.inbox, this.filter); }, unsubscribe() { - this.pmTopicTrackingState?.resetTracking(); + this.pmTopicTrackingState.resetIncomingTracking(); }, @action @@ -85,7 +80,7 @@ export default Controller.extend(BulkTopicSelection, { @action showInserted() { this.model.loadBefore(this.pmTopicTrackingState.newIncoming); - this.pmTopicTrackingState.resetTracking(); + this.pmTopicTrackingState.resetIncomingTracking(); return false; }, }); diff --git a/app/assets/javascripts/discourse/app/models/post-stream.js b/app/assets/javascripts/discourse/app/models/post-stream.js index a28a4833212..62b96e788f9 100644 --- a/app/assets/javascripts/discourse/app/models/post-stream.js +++ b/app/assets/javascripts/discourse/app/models/post-stream.js @@ -1076,9 +1076,7 @@ export default RestModel.extend({ const store = this.store; return ajax(url, { data }).then((result) => { - if (result.suggested_topics) { - this.set("topic.suggested_topics", result.suggested_topics); - } + this._setSuggestedTopics(result); const posts = get(result, "post_stream.posts"); @@ -1124,9 +1122,7 @@ export default RestModel.extend({ data, headers, }).then((result) => { - if (result.suggested_topics) { - this.set("topic.suggested_topics", result.suggested_topics); - } + this._setSuggestedTopics(result); const posts = get(result, "post_stream.posts"); @@ -1245,4 +1241,17 @@ export default RestModel.extend({ } } }, + + _setSuggestedTopics(result) { + if (!result.suggested_topics) { + return; + } + + this.topic.setProperties({ + suggested_topics: result.suggested_topics, + suggested_group_name: result.suggested_group_name, + }); + + this.pmTopicTrackingState.startTracking(); + }, }); diff --git a/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js b/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js index 4fcb8f96925..de465563897 100644 --- a/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js @@ -1,4 +1,7 @@ import EmberObject from "@ember/object"; +import { ajax } from "discourse/lib/ajax"; +import { on } from "discourse-common/utils/decorators"; +import { popupAjaxError } from "discourse/lib/ajax-error"; import { ARCHIVE_FILTER, INBOX_FILTER, @@ -15,20 +18,33 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ filter: null, activeGroup: null, - startTracking(data) { + @on("init") + _setup() { this.states = new Map(); + this.statesModificationCounter = 0; + this.isTracking = false; this.newIncoming = []; - this._loadStates(data); - this.establishChannels(); }, - establishChannels() { + startTracking() { + if (this.isTracking) { + return; + } + + this._establishChannels(); + + this._loadInitialState().finally(() => { + this.set("isTracking", true); + }); + }, + + _establishChannels() { this.messageBus.subscribe( - this._userChannel(this.user.id), + this._userChannel(), this._processMessage.bind(this) ); - this.user.groupsWithMessages?.forEach((group) => { + this.currentUser.groupsWithMessages?.forEach((group) => { this.messageBus.subscribe( this._groupChannel(group.id), this._processMessage.bind(this) @@ -36,26 +52,21 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ }); }, - stopTracking() { - this.messageBus.unsubscribe(this._userChannel(this.user.id)); - - this.user.groupsWithMessages?.forEach((group) => { - this.messageBus.unsubscribe(this._groupChannel(group.id)); - }); - }, - - lookupCount(type) { + lookupCount(type, opts = {}) { const typeFilterFn = type === "new" ? this._isNew : this._isUnread; + const inbox = opts.inboxFilter || this.inbox; let filterFn; - if (this.inbox === "user") { + if (inbox === "user") { filterFn = this._isPersonal.bind(this); - } else if (this.inbox === "group") { + } else if (inbox === "group") { filterFn = this._isGroup.bind(this); } return Array.from(this.states.values()).filter((topic) => { - return typeFilterFn(topic) && (!filterFn || filterFn(topic)); + return ( + typeFilterFn(topic) && (!filterFn || filterFn(topic, opts.groupName)) + ); }).length; }, @@ -63,14 +74,14 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ this.setProperties({ inbox, filter, activeGroup: group }); }, - resetTracking() { + resetIncomingTracking() { if (this.inbox) { this.set("newIncoming", []); } }, - _userChannel(userId) { - return `${this.CHANNEL_PREFIX}/user/${userId}`; + _userChannel() { + return `${this.CHANNEL_PREFIX}/user/${this.currentUser.id}`; }, _groupChannel(groupId) { @@ -95,9 +106,9 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ }, _isPersonal(topic) { - const groups = this.user.groups; + const groups = this.currentUser?.groups; - if (groups.length === 0) { + if (!groups || groups.length === 0) { return true; } @@ -106,10 +117,10 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ }); }, - _isGroup(topic) { - return this.user.groups.some((group) => { + _isGroup(topic, activeGroupName) { + return this.currentUser.groups.some((group) => { return ( - group.name === this.activeGroup.name && + group.name === (activeGroupName || this.activeGroup.name) && topic.group_ids?.includes(group.id) ); }); @@ -182,14 +193,24 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ } }, - _loadStates(data) { - (data || []).forEach((topic) => { - this._modifyState(topic.topic_id, topic); - }); + _loadInitialState() { + return ajax( + `/u/${this.currentUser.username}/private-message-topic-tracking-state` + ) + .then((pmTopicTrackingStateData) => { + pmTopicTrackingStateData.forEach((topic) => { + this._modifyState(topic.topic_id, topic, { skipIncrement: true }); + }); + }) + .catch(popupAjaxError); }, - _modifyState(topicId, data) { + _modifyState(topicId, data, opts = {}) { this.states.set(topicId, data); + + if (!opts.skipIncrement) { + this.incrementProperty("statesModificationCounter"); + } }, }); diff --git a/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js b/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js index b3e19dba60d..10fb0706835 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js @@ -1,6 +1,7 @@ import TopicTrackingState, { startTracking, } from "discourse/models/topic-tracking-state"; +import PrivateMessageTopicTrackingState from "discourse/models/private-message-topic-tracking-state"; import DiscourseLocation from "discourse/lib/discourse-location"; import KeyValueStore from "discourse/lib/key-value-store"; import MessageBus from "message-bus-client"; @@ -50,19 +51,30 @@ export default { app.register("current-user:main", currentUser, { instantiate: false }); app.currentUser = currentUser; - ALL_TARGETS.forEach((t) => - app.inject(t, "topicTrackingState", "topic-tracking-state:main") - ); + ALL_TARGETS.forEach((t) => { + app.inject(t, "topicTrackingState", "topic-tracking-state:main"); + app.inject(t, "pmTopicTrackingState", "pm-topic-tracking-state:main"); + }); const topicTrackingState = TopicTrackingState.create({ messageBus: MessageBus, siteSettings, currentUser, }); + app.register("topic-tracking-state:main", topicTrackingState, { instantiate: false, }); + const pmTopicTrackingState = PrivateMessageTopicTrackingState.create({ + messageBus: MessageBus, + currentUser, + }); + + app.register("pm-topic-tracking-state:main", pmTopicTrackingState, { + instantiate: false, + }); + const site = Site.current(); app.register("site:main", site, { instantiate: false }); diff --git a/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js b/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js index 846408a7cc7..65a3cddd621 100644 --- a/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js @@ -61,8 +61,6 @@ export default (inboxType, path, filter) => { filter: filter, group: null, inbox: inboxType, - pmTopicTrackingState: - userPrivateMessagesController.pmTopicTrackingState, emptyState: this.emptyState(), }); diff --git a/app/assets/javascripts/discourse/app/routes/topic-from-params.js b/app/assets/javascripts/discourse/app/routes/topic-from-params.js index f97c7c882ea..e2918e1f420 100644 --- a/app/assets/javascripts/discourse/app/routes/topic-from-params.js +++ b/app/assets/javascripts/discourse/app/routes/topic-from-params.js @@ -34,6 +34,14 @@ export default DiscourseRoute.extend({ }); }, + afterModel() { + const topic = this.modelFor("topic"); + + if (topic.isPrivateMessage && topic.suggested_topics) { + this.pmTopicTrackingState.startTracking(); + } + }, + deactivate() { this._super(...arguments); this.controllerFor("topic").unsubscribe(); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages.js b/app/assets/javascripts/discourse/app/routes/user-private-messages.js index 9c0673865e0..a80933010ca 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages.js @@ -1,9 +1,6 @@ import Composer from "discourse/models/composer"; import DiscourseRoute from "discourse/routes/discourse"; import Draft from "discourse/models/draft"; -import { ajax } from "discourse/lib/ajax"; -import { popupAjaxError } from "discourse/lib/ajax-error"; -import PrivateMessageTopicTrackingState from "discourse/models/private-message-topic-tracking-state"; export default DiscourseRoute.extend({ renderTemplate() { @@ -11,36 +8,15 @@ export default DiscourseRoute.extend({ }, model() { - const user = this.modelFor("user"); - return ajax(`/u/${user.username}/private-message-topic-tracking-state`) - .then((pmTopicTrackingStateData) => { - return { - user, - pmTopicTrackingStateData, - }; - }) - .catch((e) => { - popupAjaxError(e); - return { user }; - }); + return this.modelFor("user"); + }, + + afterModel() { + return this.pmTopicTrackingState.startTracking(); }, setupController(controller, model) { - const user = model.user; - - const pmTopicTrackingState = PrivateMessageTopicTrackingState.create({ - messageBus: controller.messageBus, - user, - }); - - pmTopicTrackingState.startTracking(model.pmTopicTrackingStateData); - - controller.setProperties({ - model: user, - pmTopicTrackingState, - }); - - this.set("pmTopicTrackingState", pmTopicTrackingState); + controller.set("model", model); if (this.currentUser) { const composerController = this.controllerFor("composer"); @@ -58,10 +34,6 @@ export default DiscourseRoute.extend({ } }, - deactivate() { - this.pmTopicTrackingState.stopTracking(); - }, - actions: { refresh() { this.refresh(); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js index d1891a8dead..d785f1b09f0 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js @@ -10,6 +10,7 @@ import { } from "discourse/tests/helpers/qunit-helpers"; import selectKit from "discourse/tests/helpers/select-kit-helper"; import { PERSONAL_INBOX } from "discourse/controllers/user-private-messages"; +import { fixturesByUrl } from "discourse/tests/helpers/create-pretender"; acceptance( "User Private Messages - user with no group messages", @@ -47,7 +48,11 @@ acceptance( let fetchUserNew; let fetchedGroupNew; - needs.user(); + needs.user({ + id: 5, + username: "charlie", + groups: [{ id: 14, name: "awesome_group", has_messages: true }], + }); needs.site({ can_tag_pms: true, @@ -60,6 +65,12 @@ acceptance( }); needs.pretender((server, helper) => { + server.get("/t/13.json", () => { + const response = { ...fixturesByUrl["/t/12/1.json"] }; + response.suggested_group_name = "awesome_group"; + return helper.response(response); + }); + server.get("/topics/private-messages-all/:username.json", () => { return helper.response({ topic_list: { @@ -159,46 +170,86 @@ acceptance( }); }); - const publishUnreadToMessageBus = function (group_ids) { - publishToMessageBus("/private-message-topic-tracking-state/user/5", { - topic_id: Math.random(), - message_type: "unread", - payload: { - last_read_post_number: 1, - highest_post_number: 2, - notification_level: 2, - group_ids: group_ids || [], - }, - }); - }; - - const publishNewToMessageBus = function (group_ids) { - publishToMessageBus("/private-message-topic-tracking-state/user/5", { - topic_id: Math.random(), - message_type: "new_topic", - payload: { - last_read_post_number: null, - highest_post_number: 1, - group_ids: group_ids || [], - }, - }); - }; - - const publishArchiveToMessageBus = function () { - publishToMessageBus("/private-message-topic-tracking-state/user/5", { - topic_id: Math.random(), - message_type: "archive", - }); - }; - - const publishGroupArchiveToMessageBus = function (group_ids) { + const publishUnreadToMessageBus = function (opts = {}) { publishToMessageBus( - `/private-message-topic-tracking-state/group/${group_ids[0]}`, + `/private-message-topic-tracking-state/user/${opts.userId || 5}`, + { + topic_id: Math.random(), + message_type: "unread", + payload: { + last_read_post_number: 1, + highest_post_number: 2, + notification_level: 2, + group_ids: opts.groupIds || [], + }, + } + ); + }; + + const publishNewToMessageBus = function (opts = {}) { + publishToMessageBus( + `/private-message-topic-tracking-state/user/${opts.userId || 5}`, + { + topic_id: Math.random(), + message_type: "new_topic", + payload: { + last_read_post_number: null, + highest_post_number: 1, + group_ids: opts.groupIds || [], + }, + } + ); + }; + + const publishArchiveToMessageBus = function (userId) { + publishToMessageBus( + `/private-message-topic-tracking-state/user/${userId || 5}`, + { + topic_id: Math.random(), + message_type: "archive", + } + ); + }; + + const publishGroupArchiveToMessageBus = function (groupIds) { + publishToMessageBus( + `/private-message-topic-tracking-state/group/${groupIds[0]}`, { topic_id: Math.random(), message_type: "group_archive", payload: { - group_ids: group_ids, + group_ids: groupIds, + }, + } + ); + }; + + const publishGroupUnreadToMessageBus = function (groupIds) { + publishToMessageBus( + `/private-message-topic-tracking-state/group/${groupIds[0]}`, + { + topic_id: Math.random(), + message_type: "unread", + payload: { + last_read_post_number: 1, + highest_post_number: 2, + notification_level: 2, + group_ids: groupIds || [], + }, + } + ); + }; + + const publishGroupNewToMessageBus = function (groupIds) { + publishToMessageBus( + `/private-message-topic-tracking-state/group/${groupIds[0]}`, + { + topic_id: Math.random(), + message_type: "new_topic", + payload: { + last_read_post_number: null, + highest_post_number: 1, + group_ids: groupIds || [], }, } ); @@ -332,8 +383,8 @@ acceptance( test("incoming unread messages while viewing group unread", async function (assert) { await visit("/u/charlie/messages/group/awesome_group/unread"); - publishUnreadToMessageBus([14]); - publishNewToMessageBus([14]); + publishUnreadToMessageBus({ groupIds: [14] }); + publishNewToMessageBus({ groupIds: [14] }); await visit("/u/charlie/messages/group/awesome_group/unread"); // wait for re-render @@ -544,6 +595,74 @@ acceptance( "does not display the tags filter" ); }); + + test("suggested messages without new or unread", async function (assert) { + await visit("/t/12"); + + assert.equal( + query(".suggested-topics-message").innerText.trim(), + "Want to read more? Browse other messages in personal messages.", + "displays the right browse more message" + ); + }); + + test("suggested messages with new and unread", async function (assert) { + await visit("/t/12"); + + publishNewToMessageBus({ userId: 5 }); + + await visit("/t/12"); // await re-render + + assert.equal( + query(".suggested-topics-message").innerText.trim(), + "There is 1 new message remaining, or browse other personal messages", + "displays the right browse more message" + ); + + publishUnreadToMessageBus({ userId: 5 }); + + await visit("/t/12"); // await re-render + + assert.equal( + query(".suggested-topics-message").innerText.trim(), + "There is 1 unread and 1 new message remaining, or browse other personal messages", + "displays the right browse more message" + ); + }); + + test("suggested messages for group messages without new or unread", async function (assert) { + await visit("/t/13"); + + assert.equal( + query(".suggested-topics-message").innerText.trim(), + "Want to read more? Browse other messages in awesome_group.", + "displays the right browse more message" + ); + }); + + test("suggested messages for group messages with new and unread", async function (assert) { + await visit("/t/13"); + + publishGroupNewToMessageBus([14]); + + await visit("/t/13"); // await re-render + + assert.equal( + query(".suggested-topics-message").innerText.trim(), + "There is 1 new message remaining, or browse other messages in awesome_group", + "displays the right browse more message" + ); + + publishGroupUnreadToMessageBus([14]); + + await visit("/t/13"); // await re-render + + assert.equal( + query(".suggested-topics-message").innerText.trim(), + "There is 1 unread and 1 new message remaining, or browse other messages in awesome_group", + "displays the right browse more message" + ); + }); } ); diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index b0fd9fbe708..5bd1e7e0157 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -69,8 +69,8 @@ class CurrentUserSerializer < BasicUserSerializer def groups owned_group_ids = GroupUser.where(user_id: id, owner: true).pluck(:group_id).to_set - object.visible_groups.pluck(:id, :name).map do |id, name| - group = { id: id, name: name } + object.visible_groups.pluck(:id, :name, :has_messages).map do |id, name, has_messages| + group = { id: id, name: name, has_messages: has_messages } group[:owner] = true if owned_group_ids.include?(id) group end diff --git a/app/serializers/suggested_topics_mixin.rb b/app/serializers/suggested_topics_mixin.rb index cfd8f23ce18..cf0de7b4a10 100644 --- a/app/serializers/suggested_topics_mixin.rb +++ b/app/serializers/suggested_topics_mixin.rb @@ -4,6 +4,7 @@ module SuggestedTopicsMixin def self.included(klass) klass.attributes :related_messages klass.attributes :suggested_topics + klass.attributes :suggested_group_name end def include_related_messages? @@ -16,6 +17,24 @@ module SuggestedTopicsMixin object.next_page.nil? && object.suggested_topics&.topics end + def include_suggested_group_name? + return false unless include_suggested_topics? + object.topic.private_message? && scope.user + end + + def suggested_group_name + return if object.topic.topic_allowed_users.exists?(user_id: scope.user.id) + + if object.topic_allowed_group_ids.present? + Group.joins(:group_users) + .where( + "group_users.group_id IN (?) AND group_users.user_id = ?", + object.topic_allowed_group_ids, scope.user.id + ) + .pluck_first(:name) + end + end + def related_messages object.related_messages.topics.map do |t| SuggestedTopicSerializer.new(t, scope: scope, root: false) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f531052cad7..846e7512348 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1204,6 +1204,38 @@ en: failed_to_move: "Failed to move selected messages (perhaps your network is down)" tags: "Tags" warnings: "Official Warnings" + read_more_in_group: "Want to read more? Browse other messages in %{groupLink}." + read_more: "Want to read more? Browse other messages in personal messages." + + read_more_group_pm_MF: "There { + UNREAD, plural, + =0 {} + one { + is # unread + } other { + are # unread + } + } { + NEW, plural, + =0 {} + one { {BOTH, select, true{and } false {is } other{}} # new message} + other { {BOTH, select, true{and } false {are } other{}} # new messages} + } remaining, or browse other messages in {groupLink}" + + read_more_personal_pm_MF: "There { + UNREAD, plural, + =0 {} + one { + is # unread + } other { + are # unread + } + } { + NEW, plural, + =0 {} + one { {BOTH, select, true{and } false {is } other{}} # new message} + other { {BOTH, select, true{and } false {are } other{}} # new messages} + } remaining, or browse other personal messages" preferences_nav: account: "Account" diff --git a/lib/topic_view.rb b/lib/topic_view.rb index e97ea551453..0e72a2b14b0 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -464,11 +464,18 @@ class TopicView end end + def topic_allowed_group_ids + @topic_allowed_group_ids ||= begin + @topic.allowed_groups.map(&:id) + end + end + def group_allowed_user_ids return @group_allowed_user_ids unless @group_allowed_user_ids.nil? - group_ids = @topic.allowed_groups.map(&:id) - @group_allowed_user_ids = Set.new(GroupUser.where(group_id: group_ids).pluck('distinct user_id')) + @group_allowed_user_ids = GroupUser + .where(group_id: topic_allowed_group_ids) + .pluck('distinct user_id') end def category_group_moderator_user_ids diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index 52049da3f95..3b63aff05e8 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -135,7 +135,9 @@ RSpec.describe CurrentUserSerializer do public_group.save! payload = serializer.as_json - expect(payload[:groups]).to eq([{ id: public_group.id, name: public_group.name }]) + expect(payload[:groups]).to contain_exactly( + { id: public_group.id, name: public_group.name, has_messages: false } + ) end end diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb index cc4eabbf2fa..02a126060a9 100644 --- a/spec/serializers/topic_view_serializer_spec.rb +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -151,6 +151,34 @@ describe TopicViewSerializer do end end + describe '#suggested_group_name' do + fab!(:pm) { Fabricate(:private_message_post).topic } + fab!(:group) { Fabricate(:group) } + + it 'is nil for a regular topic' do + json = serialize_topic(topic, user) + + expect(json[:suggested_group_name]).to eq(nil) + end + + it 'is nil if user is an allowed user of the private message' do + pm.allowed_users << user + + json = serialize_topic(pm, user) + + expect(json[:suggested_group_name]).to eq(nil) + end + + it 'returns the right group name if user is part of allowed group in the private message' do + pm.allowed_groups << group + group.add(user) + + json = serialize_topic(pm, user) + + expect(json[:suggested_group_name]).to eq(group.name) + end + end + describe 'when tags added to private message topics' do fab!(:moderator) { Fabricate(:moderator) } fab!(:tag) { Fabricate(:tag) }