From b23862a8fce4ca1ec7cdc5a155d878784eed0736 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 30 Jun 2021 08:57:39 +0800 Subject: [PATCH 1/3] FEATURE: New and Unread messages for user personal messages. --- .../discourse/app/controllers/topic.js | 8 +- .../app/controllers/user-private-messages.js | 93 ++++- .../discourse/app/lib/cached-topic-list.js | 9 +- .../discourse/app/routes/app-route-map.js | 11 +- .../build-private-messages-group-route.js | 82 +++++ .../routes/build-private-messages-route.js | 21 +- .../routes/user-private-messages-archive.js | 6 +- .../user-private-messages-group-archive.js | 49 +-- .../routes/user-private-messages-group-new.js | 3 + .../user-private-messages-group-unread.js | 3 + .../app/routes/user-private-messages-group.js | 43 +-- .../app/routes/user-private-messages-index.js | 2 +- .../app/routes/user-private-messages-new.js | 7 + .../user-private-messages-personal-archive.js | 3 + .../user-private-messages-personal-new.js | 7 + .../user-private-messages-personal-sent.js | 3 + .../user-private-messages-personal-unread.js | 7 + .../routes/user-private-messages-personal.js | 3 + .../app/routes/user-private-messages-sent.js | 6 +- .../routes/user-private-messages-unread.js | 7 + .../app/routes/user-private-messages.js | 39 +- .../discourse/app/templates/user/messages.hbs | 175 +++++---- .../acceptance/user-private-messages-test.js | 174 +++++++++ .../discourse/tests/acceptance/user-test.js | 5 - .../discourse/tests/fixtures/user-fixtures.js | 27 ++ .../tests/helpers/create-pretender.js | 14 +- app/assets/stylesheets/common/base/user.scss | 1 + app/assets/stylesheets/desktop/user.scss | 36 +- app/assets/stylesheets/mobile/user.scss | 33 ++ app/controllers/list_controller.rb | 39 +- app/controllers/tags_controller.rb | 12 +- app/models/tag.rb | 6 +- app/models/topic_tracking_state.rb | 5 +- config/locales/client.en.yml | 6 +- config/routes.rb | 16 +- lib/topic_query.rb | 142 +------- lib/topic_query/private_message_lists.rb | 272 ++++++++++++++ spec/components/topic_query_spec.rb | 82 ----- .../topic_query/private_message_lists_spec.rb | 336 ++++++++++++++++++ spec/models/topic_tracking_state_spec.rb | 18 +- spec/requests/list_controller_spec.rb | 49 +-- spec/requests/tags_controller_spec.rb | 19 +- 42 files changed, 1407 insertions(+), 472 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-group-new.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-group-unread.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-new.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-archive.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-new.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-sent.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-unread.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal.js create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-unread.js create mode 100644 app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js create mode 100644 lib/topic_query/private_message_lists.rb create mode 100644 spec/lib/topic_query/private_message_lists_spec.rb diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index ae881ebfaf4a2a..f48dda98231990 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -154,10 +154,14 @@ export default Controller.extend(bufferedProperty("model"), { showCategoryChooser: not("model.isPrivateMessage"), gotoInbox(name) { - let url = userPath(this.get("currentUser.username_lower") + "/messages"); + let url = userPath(`${this.get("currentUser.username_lower")}/messages`); + if (name) { - url = url + "/group/" + name; + url = `${url}/group/${name}`; + } else { + url = `${url}/personal`; } + DiscourseURL.routeTo(url); }, diff --git a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js index 22f99d27bf706d..1fdd985abbcfb8 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js @@ -3,26 +3,117 @@ import { action } from "@ember/object"; import { alias, and, equal } from "@ember/object/computed"; import discourseComputed from "discourse-common/utils/decorators"; import { VIEW_NAME_WARNINGS } from "discourse/routes/user-private-messages-warnings"; +import I18n from "I18n"; + +export const PERSONAL_INBOX = "__personal_inbox__"; +const ALL_INBOX = "__all_inbox__"; export default Controller.extend({ + queryParams: ["tag"], user: controller(), pmView: false, viewingSelf: alias("user.viewingSelf"), isGroup: equal("pmView", "groups"), + group: null, + groupFilter: alias("group.name"), currentPath: alias("router._router.currentPath"), pmTaggingEnabled: alias("site.can_tag_pms"), - tagId: null, + tag: null, showNewPM: and("user.viewingSelf", "currentUser.can_send_private_messages"), + @discourseComputed("inboxes") + sectionClass(inboxes) { + const defaultClass = "user-secondary-navigation user-messages"; + + return inboxes.length + ? `${defaultClass} user-messages-inboxes` + : defaultClass; + }, + + @discourseComputed("pmView") + isPersonalInbox(pmView) { + return pmView && pmView.startsWith("personal"); + }, + + @discourseComputed("isPersonalInbox", "group.name") + isAllInbox(isPersonalInbox, groupName) { + return !this.isPersonalInbox && !groupName; + }, + + @discourseComputed("isPersonalInbox", "group.name") + selectedInbox(isPersonalInbox, groupName) { + if (groupName) { + return groupName; + } + + return isPersonalInbox ? PERSONAL_INBOX : ALL_INBOX; + }, + @discourseComputed("viewingSelf", "pmView", "currentUser.admin") showWarningsWarning(viewingSelf, pmView, isAdmin) { return pmView === VIEW_NAME_WARNINGS && !viewingSelf && !isAdmin; }, + @discourseComputed("tags") + tagsContent(tags) { + return tags.map((tag) => { + return { id: tag.id, name: tag.text }; + }); + }, + + @discourseComputed("model.groups", "tags") + inboxes(groups) { + const groupsWithMessages = groups?.filter((group) => { + return group.has_messages; + }); + + if (!groupsWithMessages || groupsWithMessages.length === 0) { + return []; + } + + const inboxes = []; + + inboxes.push({ + id: ALL_INBOX, + name: I18n.t("user.messages.all"), + }); + + inboxes.push({ + id: PERSONAL_INBOX, + name: I18n.t("user.messages.personal"), + icon: "envelope", + }); + + groupsWithMessages.forEach((group) => { + inboxes.push({ id: group.name, name: group.name, icon: "users" }); + }); + + return inboxes; + }, + @action changeGroupNotificationLevel(notificationLevel) { this.group.setNotification(notificationLevel, this.get("user.model.id")); }, + + @action + updateInbox(inbox) { + const queryParams = {}; + + if (this.tag) { + queryParams.tag = this.tag; + } + + if (inbox === ALL_INBOX) { + this.transitionToRoute("userPrivateMessages.index", { queryParams }); + } else if (inbox === PERSONAL_INBOX) { + this.transitionToRoute("userPrivateMessages.personal", { queryParams }); + } else { + this.transitionToRoute("userPrivateMessages.group", inbox, { + queryParams, + }); + } + }, }); diff --git a/app/assets/javascripts/discourse/app/lib/cached-topic-list.js b/app/assets/javascripts/discourse/app/lib/cached-topic-list.js index 9ff608c2e649da..4a42a03e230d1a 100644 --- a/app/assets/javascripts/discourse/app/lib/cached-topic-list.js +++ b/app/assets/javascripts/discourse/app/lib/cached-topic-list.js @@ -1,6 +1,11 @@ -export function findOrResetCachedTopicList(session, filter) { +export function findOrResetCachedTopicList(session, filter, params) { const lastTopicList = session.get("topicList"); - if (lastTopicList && lastTopicList.filter === filter) { + + if ( + lastTopicList && + lastTopicList.filter === filter && + lastTopicList.params.tag === params?.tag + ) { return lastTopicList; } else { session.setProperties({ diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index 9bbb446410bc0b..42eb98b4540862 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -140,11 +140,20 @@ export default function () { "userPrivateMessages", { path: "/messages", resetNamespace: true }, function () { - this.route("sent"); + this.route("new"); + this.route("unread"); this.route("archive"); + this.route("sent"); + this.route("personal"); + this.route("personalSent", { path: "personal/sent" }); + this.route("personalNew", { path: "personal/new" }); + this.route("personalUnread", { path: "personal/unread" }); + this.route("personalArchive", { path: "personal/archive" }); this.route("warnings"); this.route("group", { path: "group/:name" }); this.route("groupArchive", { path: "group/:name/archive" }); + this.route("groupNew", { path: "group/:name/new" }); + this.route("groupUnread", { path: "group/:name/unread" }); this.route("tags"); this.route("tagsShow", { path: "tags/:id" }); } diff --git a/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js b/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js new file mode 100644 index 00000000000000..f8b973c4d6145c --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js @@ -0,0 +1,82 @@ +import I18n from "I18n"; +import createPMRoute from "discourse/routes/build-private-messages-route"; +import { findOrResetCachedTopicList } from "discourse/lib/cached-topic-list"; + +export default (viewName, channel) => { + return createPMRoute("groups", "private-messages-groups").extend({ + groupName: null, + + titleToken() { + const groupName = this.groupName; + + if (groupName) { + let title = groupName.capitalize(); + + if (viewName !== "index") { + title = `${title} ${I18n.t("user.messages." + viewName)}`; + } + + return [title, I18n.t(`user.private_messages`)]; + } + }, + + model(params) { + const username = this.modelFor("user").get("username_lower"); + let filter = `topics/private-messages-group/${username}/${params.name}`; + + if (viewName !== "index") { + filter = `${filter}/${viewName}`; + } + + const tag = this.paramsFor("user-private-messages").tag; + const filterParams = {}; + + if (tag) { + filterParams.tag = tag; + } + + const lastTopicList = findOrResetCachedTopicList( + this.session, + filter, + filterParams + ); + + return lastTopicList + ? lastTopicList + : this.store.findFiltered("topicList", { + filter, + params: filterParams, + }); + }, + + afterModel(model) { + const filters = model.get("filter").split("/"); + let groupName; + + if (viewName !== "index") { + groupName = filters[filters.length - 2]; + } else { + groupName = filters.pop(); + } + + const group = this.modelFor("user") + .get("groups") + .filterBy("name", groupName)[0]; + + this.setProperties({ groupName: groupName, group }); + }, + + setupController() { + this._super.apply(this, arguments); + this.controllerFor("user-private-messages").set("group", this.group); + + if (channel) { + this.controllerFor("user-topics-list").subscribe( + `/private-messages/group/${this.get( + "groupName" + ).toLowerCase()}/${channel}` + ); + } + }, + }); +}; 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 b62178ec0ac33b..0b59ce0ab04e72 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 @@ -23,10 +23,26 @@ export default (viewName, path, channel) => { model() { const filter = "topics/" + path + "/" + this.modelFor("user").get("username_lower"); - const lastTopicList = findOrResetCachedTopicList(this.session, filter); + + const tag = this.paramsFor("user-private-messages").tag; + const params = {}; + + if (tag) { + params.tag = tag; + } + + const lastTopicList = findOrResetCachedTopicList( + this.session, + filter, + params + ); + return lastTopicList ? lastTopicList - : this.store.findFiltered("topicList", { filter }); + : this.store.findFiltered("topicList", { + filter, + params, + }); }, setupController() { @@ -49,6 +65,7 @@ export default (viewName, path, channel) => { this.controllerFor("user-private-messages").setProperties({ archive: false, pmView: viewName, + group: null, }); this.searchService.set("contextType", "private_messages"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-archive.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-archive.js index 81981c9863163d..d12d153a5bbee4 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-archive.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-archive.js @@ -1,3 +1,7 @@ import createPMRoute from "discourse/routes/build-private-messages-route"; -export default createPMRoute("archive", "private-messages-archive", "archive"); +export default createPMRoute( + "archive", + "private-messages-all-archive", + "archive" +); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-group-archive.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-group-archive.js index e1c743452f6b10..42b0d95c916d24 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-group-archive.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-group-archive.js @@ -1,48 +1,3 @@ -import I18n from "I18n"; -import createPMRoute from "discourse/routes/build-private-messages-route"; -import { findOrResetCachedTopicList } from "discourse/lib/cached-topic-list"; +import createPMRoute from "discourse/routes/build-private-messages-group-route"; -export default createPMRoute("groups", "private-messages-groups").extend({ - groupName: null, - - titleToken() { - const groupName = this.groupName; - - if (groupName) { - return [ - `${groupName.capitalize()} ${I18n.t("user.messages.archive")}`, - I18n.t("user.private_messages"), - ]; - } - }, - - model(params) { - const username = this.modelFor("user").get("username_lower"); - const filter = `topics/private-messages-group/${username}/${params.name}/archive`; - const lastTopicList = findOrResetCachedTopicList(this.session, filter); - return lastTopicList - ? lastTopicList - : this.store.findFiltered("topicList", { filter }); - }, - - afterModel(model) { - const split = model.get("filter").split("/"); - const groupName = split[split.length - 2]; - this.set("groupName", groupName); - const group = this.modelFor("user") - .get("groups") - .filterBy("name", groupName)[0]; - this.controllerFor("user-private-messages").set("group", group); - }, - - setupController(controller, model) { - this._super.apply(this, arguments); - const split = model.get("filter").split("/"); - const group = split[split.length - 2]; - this.controllerFor("user-private-messages").set("groupFilter", group); - this.controllerFor("user-private-messages").set("archive", true); - this.controllerFor("user-topics-list").subscribe( - `/private-messages/group/${group}/archive` - ); - }, -}); +export default createPMRoute("archive", "archive"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-group-new.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-group-new.js new file mode 100644 index 00000000000000..25d9693cbed2b2 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-group-new.js @@ -0,0 +1,3 @@ +import createPMRoute from "discourse/routes/build-private-messages-group-route"; + +export default createPMRoute("new", null /* no message bus notifications */); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-group-unread.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-group-unread.js new file mode 100644 index 00000000000000..9bee9aa9994313 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-group-unread.js @@ -0,0 +1,3 @@ +import createPMRoute from "discourse/routes/build-private-messages-group-route"; + +export default createPMRoute("unread", null /* no message bus notifications */); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-group.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-group.js index 022d12de160239..ea93a1fa9192c2 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-group.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-group.js @@ -1,42 +1,3 @@ -import I18n from "I18n"; -import createPMRoute from "discourse/routes/build-private-messages-route"; -import { findOrResetCachedTopicList } from "discourse/lib/cached-topic-list"; +import createPMRoute from "discourse/routes/build-private-messages-group-route"; -export default createPMRoute("groups", "private-messages-groups").extend({ - groupName: null, - - titleToken() { - const groupName = this.groupName; - if (groupName) { - return [groupName.capitalize(), I18n.t("user.private_messages")]; - } - }, - - model(params) { - const username = this.modelFor("user").get("username_lower"); - const filter = `topics/private-messages-group/${username}/${params.name}`; - const lastTopicList = findOrResetCachedTopicList(this.session, filter); - return lastTopicList - ? lastTopicList - : this.store.findFiltered("topicList", { filter }); - }, - - afterModel(model) { - const groupName = model.get("filter").split("/").pop(); - this.set("groupName", groupName); - const group = this.modelFor("user") - .get("groups") - .filterBy("name", groupName)[0]; - this.controllerFor("user-private-messages").set("group", group); - }, - - setupController(controller, model) { - this._super.apply(this, arguments); - const group = model.get("filter").split("/").pop(); - this.controllerFor("user-private-messages").set("groupFilter", group); - this.controllerFor("user-private-messages").set("archive", false); - this.controllerFor("user-topics-list").subscribe( - `/private-messages/group/${group}` - ); - }, -}); +export default createPMRoute("index", "inbox"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-index.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-index.js index 322f8ace40f110..e973160ef8fa68 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-index.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-index.js @@ -1,3 +1,3 @@ import createPMRoute from "discourse/routes/build-private-messages-route"; -export default createPMRoute("index", "private-messages", "inbox"); +export default createPMRoute("index", "private-messages-all", "inbox"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-new.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-new.js new file mode 100644 index 00000000000000..b6f538f6e5044e --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-new.js @@ -0,0 +1,7 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute( + "new", + "private-messages-all-new", + null /* no message bus notifications */ +); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-archive.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-archive.js new file mode 100644 index 00000000000000..1999282a5cee71 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-archive.js @@ -0,0 +1,3 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute("personal", "private-messages-archive", "archive"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-new.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-new.js new file mode 100644 index 00000000000000..e1bb8d75fab579 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-new.js @@ -0,0 +1,7 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute( + "personal", + "private-messages-new", + null /* no message bus notifications */ +); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-sent.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-sent.js new file mode 100644 index 00000000000000..a4c68b3a015679 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-sent.js @@ -0,0 +1,3 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute("personal", "private-messages-sent", "sent"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-unread.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-unread.js new file mode 100644 index 00000000000000..24dd211d32ec85 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-unread.js @@ -0,0 +1,7 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute( + "personal", + "private-messages-unread", + null /* no message bus notifications */ +); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal.js new file mode 100644 index 00000000000000..7a13e5d5211af6 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-personal.js @@ -0,0 +1,3 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute("personal", "private-messages", "inbox"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-sent.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-sent.js index b3fa314e3ddef3..6ed2c2d14bebb5 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-sent.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-sent.js @@ -1,3 +1,7 @@ import createPMRoute from "discourse/routes/build-private-messages-route"; -export default createPMRoute("sent", "private-messages-sent", "sent"); +export default createPMRoute( + "sent", + "private-messages-all-sent", + null /* no message bus notifications */ +); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-unread.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-unread.js new file mode 100644 index 00000000000000..29b1aba2a8ed9e --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-unread.js @@ -0,0 +1,7 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export default createPMRoute( + "unread", + "private-messages-all-unread", + null /* no message bus notifications */ +); 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 bedaa51e073e2c..28080200159bdb 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages.js @@ -1,20 +1,51 @@ import Composer from "discourse/models/composer"; +import Tag from "discourse/models/tag"; 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"; export default DiscourseRoute.extend({ + queryParams: { + tag: { + refreshModel: true, + }, + }, + renderTemplate() { this.render("user/messages"); }, model() { - return this.modelFor("user"); + const user = this.modelFor("user"); + const model = { user, tags: [] }; + + if (this.site.can_tag_pms) { + return ajax(`/tags/personal_messages/${user.username_lower}`, { + data: { limit: this.siteSettings.max_tags_in_filter_list }, + }) + .then((result) => { + model.tags = result.tags.map((tag) => Tag.create(tag)); + return model; + }) + .catch((e) => { + popupAjaxError(e); + return model; + }); + } else { + return model; + } }, - setupController(controller, user) { - const composerController = this.controllerFor("composer"); - controller.set("model", user); + setupController(controller, model) { + controller.setProperties({ + model: model.user, + tags: model.tags, + }); + if (this.currentUser) { + const composerController = this.controllerFor("composer"); + Draft.get("new_private_message").then((data) => { if (data.draft) { composerController.open({ diff --git a/app/assets/javascripts/discourse/app/templates/user/messages.hbs b/app/assets/javascripts/discourse/app/templates/user/messages.hbs index 5a845eafdc210e..a9aade483286ab 100644 --- a/app/assets/javascripts/discourse/app/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/messages.hbs @@ -1,81 +1,134 @@ -{{#d-section class="user-secondary-navigation" pageClass="user-messages"}} +{{#d-section class=sectionClass pageClass="user-messages"}} + {{#if inboxes.length}} + {{combo-box + content=inboxes + classNames="user-messages-inboxes-drop" + value=selectedInbox + onChange=(action "updateInbox") + options=(hash + filterable=true + ) + }} + {{/if}} + {{#unless site.mobileView}} - {{#if showNewPM}} - {{d-button class="btn-primary new-private-message" action=(route-action "composePrivateMessage") icon="envelope" label="user.new_private_message"}} + {{#if (and pmTaggingEnabled tagsContent)}} + {{combo-box + content=tagsContent + value=tag + classNames="user-messages-tags-drop" + options=(hash + filterable=true + none="tagging.selector_all_tags" + ) + }} {{/if}} {{/unless}} {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list"}} -
  • - {{#link-to "userPrivateMessages.index" model}} - {{i18n "user.messages.inbox"}} - {{/link-to}} -
  • -
  • - {{#link-to "userPrivateMessages.sent" model}} - {{i18n "user.messages.sent"}} - {{/link-to}} -
  • -
  • - {{#link-to "userPrivateMessages.archive" model}} - {{i18n "user.messages.archive"}} - {{/link-to}} -
  • - {{plugin-outlet name="user-messages-nav" connectorTagName="li" args=(hash model=model)}} - {{#each model.groups as |group|}} - {{#if group.has_messages}} -
  • - {{#link-to "userPrivateMessages.group" group.name}} - {{d-icon "users"}} - {{capitalize-string group.name}} - {{/link-to}} -
  • -
  • - {{#link-to "userPrivateMessages.groupArchive" group.name}} - {{i18n "user.messages.archive"}} - {{/link-to}} -
  • - {{/if}} - {{/each}} + {{#if isAllInbox}} +
  • + {{#link-to "userPrivateMessages.index" model (query-params tag=tag)}} + {{i18n "user.messages.latest"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.sent" model (query-params tag=tag)}} + {{i18n "user.messages.sent"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.new" model (query-params tag=tag)}} + {{i18n "user.messages.new"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.unread" model (query-params tag=tag)}} + {{i18n "user.messages.unread"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.archive" model (query-params tag=tag)}} + {{i18n "user.messages.archive"}} + {{/link-to}} +
  • + {{plugin-outlet name="user-messages-nav" connectorTagName="li" args=(hash model=model)}} + {{/if}} - {{#if pmTaggingEnabled}} + {{#if group}} +
  • + {{#link-to "userPrivateMessages.group" group.name (query-params tag=tag)}} + {{i18n "user.messages.latest"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.groupNew" group.name (query-params tag=tag)}} + {{i18n "user.messages.new"}} + {{/link-to}} +
  • - {{#link-to "userPrivateMessages.tags" model}} - {{i18n "user.messages.tags"}} + {{#link-to "userPrivateMessages.groupUnread" group.name (query-params tag=tag)}} + {{i18n "user.messages.unread"}} {{/link-to}}
  • - {{#if tagId}} -
  • - {{#link-to "userPrivateMessages.tagsShow" tagId}} - {{tagId}} - {{/link-to}} +
  • + {{#link-to "userPrivateMessages.groupArchive" group.name (query-params tag=tag)}} + {{i18n "user.messages.archive"}} + {{/link-to}} +
  • + {{/if}} + + {{#if isPersonalInbox}} +
  • + {{#link-to "userPrivateMessages.personal" model (query-params tag=tag)}} + {{i18n "user.messages.latest"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.personalSent" model (query-params tag=tag)}} + {{i18n "user.messages.sent"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.personalNew" model (query-params tag=tag)}} + {{i18n "user.messages.new"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.personalUnread" model (query-params tag=tag)}} + {{i18n "user.messages.unread"}} + {{/link-to}} +
  • +
  • + {{#link-to "userPrivateMessages.personalArchive" model (query-params tag=tag)}} + {{i18n "user.messages.archive"}} + {{/link-to}} +
  • + {{/if}} + + {{#unless mobileView}} + {{#if group}} +
  • + {{group-notifications-button + value=group.group_user.notification_level + onChange=(action "changeGroupNotificationLevel") + }}
  • {{/if}} - {{/if}} - {{/mobile-nav}} -{{/d-section}} -
    -
    - {{#if site.mobileView}} {{#if showNewPM}} - {{d-button - class="btn-primary new-private-message" - action=(route-action "composePrivateMessage") - icon="envelope" - label="user.new_private_message"}} +
  • + {{d-button class="btn-primary new-private-message" action=(route-action "composePrivateMessage") icon="envelope" label="user.new_private_message"}} +
  • {{/if}} - {{/if}} + {{/unless}} + {{/mobile-nav}} +{{/d-section}} - {{#if isGroup}} - {{group-notifications-button - value=group.group_user.notification_level - onChange=(action "changeGroupNotificationLevel") - }} - {{/if}} -
    +
    {{#if showWarningsWarning}}
    {{html-safe (i18n "admin.user.warnings_list_warning")}}
    {{/if}} + {{outlet}}
    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 new file mode 100644 index 00000000000000..54fb5adf833092 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js @@ -0,0 +1,174 @@ +import { + acceptance, + count, + exists, +} from "discourse/tests/helpers/qunit-helpers"; +import { visit } from "@ember/test-helpers"; +import { test } from "qunit"; +import selectKit from "discourse/tests/helpers/select-kit-helper"; +import { PERSONAL_INBOX } from "discourse/controllers/user-private-messages"; + +acceptance( + "User Private Messages - user with no group messages", + function (needs) { + needs.user(); + + test("viewing messages", async function (assert) { + await visit("/u/eviltrout/messages"); + + assert.equal(count(".topic-list-item"), 1, "displays the topic list"); + + assert.ok( + !exists(".user-messages-inboxes-drop"), + "does not display inboxes dropdown" + ); + + assert.ok( + !exists(".user-messages-tags-drop"), + "does not display tags dropdown" + ); + + assert.ok( + !exists(".group-notifications-button"), + "displays the group notifications button" + ); + }); + } +); + +acceptance("User Private Messages - with PM tagging enabled", function (needs) { + needs.user(); + + needs.site({ + can_tag_pms: true, + }); + + needs.pretender((server, helper) => { + server.get("/tags/personal_messages/:username", () => { + return helper.response({ + tags: [ + { id: "tag1", text: "tag1", count: 1 }, + { id: "tag2", text: "tag2", count: 2 }, + ], + }); + }); + + server.get("/topics/private-messages-all/:username.json", (request) => { + let topicList; + + if (request.queryParams?.tag) { + topicList = { + topics: [ + { id: 1, posters: [] }, + { id: 2, posters: [] }, + ], + }; + } else { + topicList = { + topics: [ + { id: 1, posters: [] }, + { id: 2, posters: [] }, + { id: 3, posters: [] }, + ], + }; + } + + return helper.response({ topic_list: topicList }); + }); + }); + + test("viewing messages", async function (assert) { + await visit("/u/charlie/messages"); + + assert.equal(count(".topic-list-item"), 3, "displays the right topic list"); + + assert.ok(exists(".user-messages-tags-drop"), "displays tags dropdown"); + + await selectKit(".user-messages-tags-drop").expand(); + await selectKit(".user-messages-tags-drop").selectRowByValue("tag1"); + + assert.equal(count(".topic-list-item"), 2, "displays the right topic list"); + }); +}); + +acceptance( + "User Private Messages - user with group messages", + function (needs) { + needs.user(); + + needs.pretender((server, helper) => { + server.get("/topics/private-messages-all/:username.json", () => { + return helper.response({ + topic_list: { + topics: [ + { id: 1, posters: [] }, + { id: 2, posters: [] }, + { id: 3, posters: [] }, + ], + }, + }); + }); + + server.get( + "/topics/private-messages-group/:username/:group_name.json", + () => { + return helper.response({ + topic_list: { + topics: [ + { id: 1, posters: [] }, + { id: 2, posters: [] }, + ], + }, + }); + } + ); + }); + + test("viewing messages", async function (assert) { + await visit("/u/charlie/messages"); + + assert.equal( + count(".topic-list-item"), + 3, + "displays the right topic list" + ); + + assert.ok( + exists(".user-messages-inboxes-drop"), + "displays inboxes dropdown" + ); + + await selectKit(".user-messages-inboxes-drop").expand(); + await selectKit(".user-messages-inboxes-drop").selectRowByValue( + PERSONAL_INBOX + ); + + assert.equal( + count(".topic-list-item"), + 1, + "displays the right topic list" + ); + + assert.ok( + !exists(".user-messages-tags-drop"), + "does not display tags dropdown" + ); + + await selectKit(".user-messages-inboxes-drop").expand(); + await selectKit(".user-messages-inboxes-drop").selectRowByValue( + "awesome_group" + ); + + assert.equal( + count(".topic-list-item"), + 2, + "displays the right topic list" + ); + + assert.ok( + exists(".group-notifications-button"), + "displays the group notifications button" + ); + }); + } +); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-test.js index 371c8be8f36c2e..6c8c2acfcbc9d0 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-test.js @@ -38,11 +38,6 @@ acceptance("User Routes", function (needs) { assert.ok($("body.user-invites-page").length, "has the body class"); }); - test("Messages", async function (assert) { - await visit("/u/eviltrout/messages"); - assert.ok($("body.user-messages-page").length, "has the body class"); - }); - test("Notifications", async function (assert) { await visit("/u/eviltrout/notifications"); assert.ok($("body.user-notifications-page").length, "has the body class"); diff --git a/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js index 7c23b4631fdeda..1ff14845c8f802 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js @@ -2648,6 +2648,33 @@ export default { default_notification_level: 3, membership_request_template: null, }, + { + id: 14, + automatic: false, + name: "awesome_group", + display_name: "awesome_group", + user_count: 3, + mentionable_level: 0, + messageable_level: 0, + visibility_level: 0, + automatic_membership_email_domains: null, + primary_group: false, + title: null, + grant_trust_level: null, + incoming_email: null, + has_messages: true, + flair_url: null, + flair_bg_color: null, + flair_color: null, + bio_raw: null, + bio_cooked: null, + public_admission: false, + public_exit: false, + allow_membership_requests: false, + full_name: null, + default_notification_level: 3, + membership_request_template: null, + }, ], group_users: [ { group_id: 10, user_id: 5, notification_level: 3 }, diff --git a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js index f6167df6577354..123eaa9c217c5a 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js @@ -207,12 +207,14 @@ export function applyDefaultHandlers(pretender) { }); }); - pretender.get("/topics/private-messages/eviltrout.json", () => { - return response(fixturesByUrl["/topics/private-messages/eviltrout.json"]); - }); - - pretender.get("/topics/private-messages-warnings/eviltrout.json", () => { - return response(fixturesByUrl["/topics/private-messages/eviltrout.json"]); + [ + "/topics/private-messages-all/:username.json", + "/topics/private-messages/:username.json", + "/topics/private-messages-warnings/eviltrout.json", + ].forEach((url) => { + pretender.get(url, () => { + return response(fixturesByUrl["/topics/private-messages/eviltrout.json"]); + }); }); pretender.get("/topics/feature_stats.json", () => { diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index 4bdf5f39c48e01..5c1e478ee518a7 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/app/assets/stylesheets/common/base/user.scss @@ -31,6 +31,7 @@ .user-content { min-width: 100%; } + .user-additional-controls + .user-content, .user-secondary-navigation + .user-content { grid-column-start: 2; diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index de3568b37f6dd6..d323c587b72554 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -6,10 +6,6 @@ margin-top: 10px; } } - - .show-mores { - position: absolute; - } } .form-horizontal .control-group.category { @@ -20,8 +16,15 @@ font-size: 1.5em; text-align: center; } + .user-secondary-navigation { min-width: 150px; + + .combo-box { + width: 100%; + margin-top: 1px; + } + .nav-stacked { background-color: transparent; @@ -47,6 +50,27 @@ } } } + + &.user-messages { + .user-messages-inboxes-drop, + .user-messages-tags-drop { + padding: 13px 13px 0 0; + + .select-kit-selected-name { + overflow: hidden; + } + } + + .nav-stacked { + a { + padding-left: 0; + } + + .new-private-message { + margin-top: 13px; + } + } + } } .user-content { @@ -226,6 +250,10 @@ table.user-invite-list { } } +.user-messages { + margin-right: 0.2em; +} + .user-preferences { padding-top: 10px; padding-left: 30px; diff --git a/app/assets/stylesheets/mobile/user.scss b/app/assets/stylesheets/mobile/user.scss index a955b9aeba74bd..3cf1607eceaf70 100644 --- a/app/assets/stylesheets/mobile/user.scss +++ b/app/assets/stylesheets/mobile/user.scss @@ -30,6 +30,39 @@ grid-row-start: 3; grid-column-start: 1; } + + .user-messages.user-messages-inboxes { + grid-row-start: 2; + grid-row-end: 3; + grid-column-start: 1; + grid-column-end: 3; + display: grid; + grid-template-columns: 1fr 1fr; + grid-template-rows: auto auto auto; + grid-column-gap: 16px; + + .user-messages-inboxes-drop { + grid-row-start: 1; + grid-row-end: 2; + grid-column-start: 1; + grid-column-end: 2; + + .select-kit-header { + padding: 8px 10px; + + .caret-icon { + color: var(--primary-medium); + } + } + } + + .messages-nav { + grid-row-start: 1; + grid-row-end: 2; + grid-column-start: 2; + grid-column-end: 3; + } + } } .user-main { diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 1fc891ea76758e..a6e0c7e4d20831 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -146,35 +146,28 @@ def group_topics end def self.generate_message_route(action) - case action - when :private_messages_tag - define_method("#{action}") do - raise Discourse::NotFound if !guardian.can_tag_pms? - message_route(action) - end - when :private_messages_group, :private_messages_group_archive - define_method("#{action}") do - group = Group.find_by("LOWER(name) = ?", params[:group_name].downcase) - raise Discourse::NotFound if !group - raise Discourse::NotFound unless guardian.can_see_group_messages?(group) - - message_route(action) - end - else - define_method("#{action}") do - message_route(action) - end + define_method action do + message_route(action) end end def message_route(action) target_user = fetch_user_from_params({ include_inactive: current_user.try(:staff?) }, [:user_stat, :user_option]) + case action + when :private_messages_tag + Discourse.deprecate("The '/private-messages-tags/:username/:tag_id.json' has been deprecated and will be removed in the Discourse 2.9 release.") + raise Discourse::NotFound if !guardian.can_tag_pms? when :private_messages_warnings guardian.ensure_can_see_warnings!(target_user) + when :private_messages_group, :private_messages_group_archive + group = Group.find_by("LOWER(name) = ?", params[:group_name].downcase) + raise Discourse::NotFound if !group + raise Discourse::NotFound unless guardian.can_see_group_messages?(group) else guardian.ensure_can_see_private_messages!(target_user.id) end + list_opts = build_topic_list_options list = generate_list_for(action.to_s, target_user, list_opts) url_prefix = "topics" @@ -187,11 +180,19 @@ def message_route(action) private_messages private_messages_sent private_messages_unread + private_messages_new private_messages_archive private_messages_group + private_messages_group_new + private_messages_group_unread private_messages_group_archive - private_messages_tag private_messages_warnings + private_messages_all + private_messages_all_sent + private_messages_all_unread + private_messages_all_new + private_messages_all_archive + private_messages_tag }.each do |action| generate_message_route(action) end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 912a60c9c770fc..9cfec7163a3561 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -301,7 +301,17 @@ def personal_messages allowed_user = fetch_user_from_params raise Discourse::NotFound if allowed_user.blank? raise Discourse::NotFound if current_user.id != allowed_user.id && !@guardian.is_admin? - pm_tags = Tag.pm_tags(guardian: guardian, allowed_user: allowed_user) + + opts = { + guardian: guardian, + allowed_user: allowed_user + } + + if params[:limit] + opts[:limit] = params[:limit] + end + + pm_tags = Tag.pm_tags(opts) render json: { tags: pm_tags } end diff --git a/app/models/tag.rb b/app/models/tag.rb index 44c23734a05a6d..f489747138d694 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -136,16 +136,16 @@ def self.pm_tags(limit: 1000, guardian: nil, allowed_user: nil) WHERE topic_tags.topic_id IN ( SELECT topic_id FROM topic_allowed_users - WHERE user_id = #{user_id} + WHERE user_id = #{user_id.to_i} UNION SELECT tg.topic_id FROM topic_allowed_groups tg - JOIN group_users gu ON gu.user_id = #{user_id} + JOIN group_users gu ON gu.user_id = #{user_id.to_i} AND gu.group_id = tg.group_id ) GROUP BY tags.name ORDER BY count DESC - LIMIT #{limit} + LIMIT #{limit.to_i} SQL end diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 77c57662fd92fd..15199c9745c924 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -545,8 +545,9 @@ def self.publish_private_message(topic, archive_user_id: nil, group_user_ids = group.users.pluck(:id) next if group_user_ids.blank? group_channels = [] - group_channels << "/private-messages/group/#{group.name.downcase}" - group_channels << "#{group_channels.first}/archive" if group_archive + channel_prefix = "/private-messages/group/#{group.name.downcase}" + group_channels << "#{channel_prefix}/inbox" + group_channels << "#{channel_prefix}/archive" if group_archive group_channels.each { |channel| channels[channel] = group_user_ids } end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ad3949804f6947..9bfbeab182b90c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1171,9 +1171,13 @@ en: rejected_posts: "rejected posts" messages: - all: "All" + all: "all inboxes" inbox: "Inbox" + personal: "Personal" + latest: "Latest" sent: "Sent" + unread: "Unread" + new: "New" archive: "Archive" groups: "My Groups" move_to_inbox: "Move to Inbox" diff --git a/config/routes.rb b/config/routes.rb index f48ad3d5ea9c77..4dd2c39c82f9d8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -447,9 +447,10 @@ get "#{root_path}/:username/private-messages/:filter" => "user_actions#private_messages", constraints: { username: RouteFormat.username } get "#{root_path}/:username/messages" => "user_actions#private_messages", constraints: { username: RouteFormat.username } get "#{root_path}/:username/messages/:filter" => "user_actions#private_messages", constraints: { username: RouteFormat.username } + get "#{root_path}/:username/messages/personal" => "user_actions#private_messages", constraints: { username: RouteFormat.username } + get "#{root_path}/:username/messages/personal/:filter" => "user_actions#private_messages", constraints: { username: RouteFormat.username } get "#{root_path}/:username/messages/group/:group_name" => "user_actions#private_messages", constraints: { username: RouteFormat.username, group_name: RouteFormat.username } - get "#{root_path}/:username/messages/group/:group_name/archive" => "user_actions#private_messages", constraints: { username: RouteFormat.username, group_name: RouteFormat.username } - get "#{root_path}/:username/messages/tags/:tag_id" => "user_actions#private_messages", constraints: StaffConstraint.new + get "#{root_path}/:username/messages/group/:group_name/:filter" => "user_actions#private_messages", constraints: { username: RouteFormat.username, group_name: RouteFormat.username } get "#{root_path}/:username.json" => "users#show", constraints: { username: RouteFormat.username }, defaults: { format: :json } get({ "#{root_path}/:username" => "users#show", constraints: { username: RouteFormat.username } }.merge(index == 1 ? { as: 'user' } : {})) put "#{root_path}/:username" => "users#update", constraints: { username: RouteFormat.username }, defaults: { format: :json } @@ -764,17 +765,28 @@ scope "/topics", username: RouteFormat.username do get "created-by/:username" => "list#topics_by", as: "topics_by", defaults: { format: :json } + get "private-messages-all/:username" => "list#private_messages_all", as: "topics_private_messages_all", defaults: { format: :json } + get "private-messages-all-sent/:username" => "list#private_messages_all_sent", as: "topics_private_messages_all_sent", defaults: { format: :json } + get "private-messages-all-new/:username" => "list#private_messages_all_new", as: "topics_private_messages_all_new", defaults: { format: :json } + get "private-messages-all-unread/:username" => "list#private_messages_all_unread", as: "topics_private_messages_all_unread", defaults: { format: :json } + get "private-messages-all-archive/:username" => "list#private_messages_all_archive", as: "topics_private_messages_all_archive", defaults: { format: :json } get "private-messages/:username" => "list#private_messages", as: "topics_private_messages", defaults: { format: :json } get "private-messages-sent/:username" => "list#private_messages_sent", as: "topics_private_messages_sent", defaults: { format: :json } get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive", defaults: { format: :json } get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread", defaults: { format: :json } + + # Deprecated: Remove after Discourse 2.9 has been released get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", defaults: { format: :json } + + get "private-messages-new/:username" => "list#private_messages_new", as: "topics_private_messages_new", defaults: { format: :json } get "private-messages-warnings/:username" => "list#private_messages_warnings", as: "topics_private_messages_warnings", defaults: { format: :json } get "groups/:group_name" => "list#group_topics", as: "group_topics", group_name: RouteFormat.username scope "/private-messages-group/:username", group_name: RouteFormat.username do get ":group_name.json" => "list#private_messages_group", as: "topics_private_messages_group" get ":group_name/archive.json" => "list#private_messages_group_archive", as: "topics_private_messages_group_archive" + get ":group_name/new.json" => "list#private_messages_group_new", as: "topics_private_messages_group_new" + get ":group_name/unread.json" => "list#private_messages_group_unread", as: "topics_private_messages_group_unread" end end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 82b76cf28a9e60..f41db2f86d3f43 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -6,6 +6,8 @@ # class TopicQuery + include PrivateMessageLists + PG_MAX_INT ||= 2147483647 DEFAULT_PER_PAGE_COUNT ||= 30 @@ -55,6 +57,7 @@ def self.public_valid_options q f group_name + tag tags match_all_tags no_subcategories @@ -293,12 +296,6 @@ def list_topics_by(user) end end - def not_archived(list, user) - list.joins("LEFT JOIN user_archived_messages um - ON um.user_id = #{user.id.to_i} AND um.topic_id = topics.id") - .where('um.user_id IS NULL') - end - def list_group_topics(group) list = default_results.where(" topics.user_id IN ( @@ -309,79 +306,6 @@ def list_group_topics(group) create_list(:group_topics, {}, list) end - def list_private_messages(user) - list = private_messages_for(user, :user) - - list = not_archived(list, user) - .where('NOT (topics.participant_count = 1 AND topics.user_id = ? AND topics.moderator_posts_count = 0)', user.id) - - create_list(:private_messages, {}, list) - end - - def list_private_messages_archive(user) - list = private_messages_for(user, :user) - list = list.joins(:user_archived_messages).where('user_archived_messages.user_id = ?', user.id) - create_list(:private_messages, {}, list) - end - - def list_private_messages_sent(user) - list = private_messages_for(user, :user) - list = list.where('EXISTS ( - SELECT 1 FROM posts - WHERE posts.topic_id = topics.id AND - posts.user_id = ? - )', user.id) - list = not_archived(list, user) - create_list(:private_messages, {}, list) - end - - def list_private_messages_unread(user) - list = private_messages_for(user, :user) - - list = TopicQuery.unread_filter( - list, - staff: user.staff? - ) - - first_unread_pm_at = UserStat.where(user_id: user.id).pluck_first(:first_unread_pm_at) - list = list.where("topics.updated_at >= ?", first_unread_pm_at) if first_unread_pm_at - create_list(:private_messages, {}, list) - end - - def list_private_messages_group(user) - list = private_messages_for(user, :group) - group = Group.where('name ilike ?', @options[:group_name]).select(:id, :publish_read_state).first - publish_read_state = !!group&.publish_read_state - list = list.joins("LEFT JOIN group_archived_messages gm ON gm.topic_id = topics.id AND - gm.group_id = #{group&.id&.to_i}") - list = list.where("gm.id IS NULL") - list = append_read_state(list, group) if publish_read_state - create_list(:private_messages, { publish_read_state: publish_read_state }, list) - end - - def list_private_messages_group_archive(user) - list = private_messages_for(user, :group) - group_id = Group.where('name ilike ?', @options[:group_name]).pluck_first(:id) - list = list.joins("JOIN group_archived_messages gm ON gm.topic_id = topics.id AND - gm.group_id = #{group_id.to_i}") - create_list(:private_messages, {}, list) - end - - def list_private_messages_tag(user) - list = private_messages_for(user, :all) - list = list.joins("JOIN topic_tags tt ON tt.topic_id = topics.id - JOIN tags t ON t.id = tt.tag_id AND t.name = '#{@options[:tags][0]}'") - create_list(:private_messages, {}, list) - end - - def list_private_messages_warnings(user) - list = private_messages_for(user, :user) - list = list.where('topics.subtype = ?', TopicSubtype.moderator_warning) - # Exclude official warnings that the user created, instead of received - list = list.where('topics.user_id <> ?', user.id) - create_list(:private_messages, {}, list) - end - def list_category_topic_ids(category) query = default_results(category: category.id) pinned_ids = query.where('topics.pinned_at IS NOT NULL AND topics.category_id = ?', category.id).limit(nil).order('pinned_at DESC').pluck(:id) @@ -590,50 +514,6 @@ def per_page_setting DEFAULT_PER_PAGE_COUNT end - def private_messages_for(user, type) - options = @options - options.reverse_merge!(per_page: per_page_setting) - - result = Topic.includes(:allowed_users) - result = result.includes(:tags) if SiteSetting.tagging_enabled - - if type == :group - result = result.joins( - "INNER JOIN topic_allowed_groups tag ON tag.topic_id = topics.id AND tag.group_id IN (SELECT id FROM groups WHERE LOWER(name) = '#{PG::Connection.escape_string(@options[:group_name].downcase)}')" - ) - - unless user.admin? - result = result.joins("INNER JOIN group_users gu ON gu.group_id = tag.group_id AND gu.user_id = #{user.id.to_i}") - end - elsif type == :user - result = result.where("topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = #{user.id.to_i})") - elsif type == :all - result = result.where("topics.id IN ( - SELECT topic_id - FROM topic_allowed_users - WHERE user_id = #{user.id.to_i} - UNION ALL - SELECT topic_id FROM topic_allowed_groups - WHERE group_id IN ( - SELECT group_id FROM group_users WHERE user_id = #{user.id.to_i} - ) - )") - end - - result = result.joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})") - .order("topics.bumped_at DESC") - .private_messages - - result = result.limit(options[:per_page]) unless options[:limit] == false - result = result.visible if options[:visible] || @user.nil? || @user.regular? - - if options[:page] - offset = options[:page].to_i * options[:per_page] - result = result.offset(offset) if offset > 0 - end - result - end - def apply_shared_drafts(result, category_id, options) # PERF: avoid any penalty if there are no shared drafts enabled @@ -955,7 +835,7 @@ def remove_muted_categories(list, user, opts = nil) list end - def remove_muted_tags(list, user, opts = nil) + def remove_muted_tags(list, user, opts = {}) if !SiteSetting.tagging_enabled || SiteSetting.remove_muted_tags_from_latest == 'never' return list end @@ -1149,18 +1029,4 @@ def suggested_ordering(result, options) result.order('topics.bumped_at DESC') end - - private - - def append_read_state(list, group) - group_id = group&.id - return list if group_id.nil? - - selected_values = list.select_values.empty? ? ['topics.*'] : list.select_values - selected_values << "COALESCE(tg.last_read_post_number, 0) AS last_read_post_number" - - list - .joins("LEFT OUTER JOIN topic_groups tg ON topics.id = tg.topic_id AND tg.group_id = #{group_id}") - .select(*selected_values) - end end diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb new file mode 100644 index 00000000000000..cd525cc43ce470 --- /dev/null +++ b/lib/topic_query/private_message_lists.rb @@ -0,0 +1,272 @@ +# frozen_string_literal: true + +class TopicQuery + module PrivateMessageLists + def list_private_messages_all(user) + list = private_messages_for(user, :all) + list = filter_archived(list, user, archived: false) + create_list(:private_messages, {}, list) + end + + def list_private_messages_all_sent(user) + list = private_messages_for(user, :all) + + list = list.where(<<~SQL, user.id) + EXISTS ( + SELECT 1 FROM posts + WHERE posts.topic_id = topics.id AND posts.user_id = ? + ) + SQL + + list = filter_archived(list, user, archived: false) + create_list(:private_messages, {}, list) + end + + def list_private_messages_all_archive(user) + list = private_messages_for(user, :all) + list = filter_archived(list, user, archived: true) + create_list(:private_messages, {}, list) + end + + def list_private_messages_all_new(user) + list_private_messages_new(user, :all) + end + + def list_private_messages_all_unread(user) + list_private_messages_unread(user, :all) + end + + def list_private_messages(user) + list = private_messages_for(user, :user) + list = not_archived(list, user) + create_list(:private_messages, {}, list) + end + + def list_private_messages_archive(user) + list = private_messages_for(user, :user) + list = list.joins(:user_archived_messages).where('user_archived_messages.user_id = ?', user.id) + create_list(:private_messages, {}, list) + end + + def list_private_messages_sent(user) + list = private_messages_for(user, :user) + + list = list.where(<<~SQL, user.id) + EXISTS ( + SELECT 1 FROM posts + WHERE posts.topic_id = topics.id AND posts.user_id = ? + ) + SQL + + list = not_archived(list, user) + create_list(:private_messages, {}, list) + end + + def list_private_messages_new(user, type = :user) + list = TopicQuery.new_filter( + private_messages_for(user, type), + treat_as_new_topic_start_date: user.user_option.treat_as_new_topic_start_date + ) + + list = remove_muted_tags(list, user) + + create_list(:private_messages, {}, list) + end + + def list_private_messages_unread(user, type = :user) + list = TopicQuery.unread_filter( + private_messages_for(user, type), + staff: user.staff? + ) + + first_unread_pm_at = UserStat + .where(user_id: user.id) + .pluck_first(:first_unread_pm_at) + + if first_unread_pm_at + list = list.where("topics.updated_at >= ?", first_unread_pm_at) + end + + create_list(:private_messages, {}, list) + end + + def list_private_messages_group(user) + list = private_messages_for(user, :group) + + list = list.joins(<<~SQL) + LEFT JOIN group_archived_messages gm + ON gm.topic_id = topics.id AND gm.group_id = #{group.id.to_i} + SQL + + 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) + end + + def list_private_messages_group_archive(user) + list = private_messages_for(user, :group) + + list = list.joins(<<~SQL) + INNER JOIN group_archived_messages gm + ON gm.topic_id = topics.id AND gm.group_id = #{group.id.to_i} + SQL + + 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) + end + + def list_private_messages_group_new(user) + list = TopicQuery.new_filter( + private_messages_for(user, :group), + treat_as_new_topic_start_date: user.user_option.treat_as_new_topic_start_date + ) + + 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) + end + + def list_private_messages_group_unread(user) + list = TopicQuery.unread_filter( + private_messages_for(user, :group), + staff: user.staff? + ) + + first_unread_pm_at = UserStat + .where(user_id: user.id) + .pluck_first(:first_unread_pm_at) + + if first_unread_pm_at + list = list.where("topics.updated_at >= ?", first_unread_pm_at) + end + + 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) + end + + def list_private_messages_warnings(user) + list = private_messages_for(user, :user) + list = list.where('topics.subtype = ?', TopicSubtype.moderator_warning) + # Exclude official warnings that the user created, instead of received + list = list.where('topics.user_id <> ?', user.id) + create_list(:private_messages, {}, list) + end + + def private_messages_for(user, type) + options = @options + options.reverse_merge!(per_page: per_page_setting) + + result = Topic.includes(:allowed_users) + result = result.includes(:tags) if tagging_enabled? + + if type == :group + result = result.joins( + "INNER JOIN topic_allowed_groups tag ON tag.topic_id = topics.id AND tag.group_id IN (SELECT id FROM groups WHERE LOWER(name) = '#{PG::Connection.escape_string(@options[:group_name].downcase)}')" + ) + + unless user.admin? + result = result.joins("INNER JOIN group_users gu ON gu.group_id = tag.group_id AND gu.user_id = #{user.id.to_i}") + end + elsif type == :user + result = result.where("topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = #{user.id.to_i})") + elsif type == :all + result = result.where("topics.id IN ( + SELECT topic_id + FROM topic_allowed_users + WHERE user_id = #{user.id.to_i} + UNION ALL + SELECT topic_id FROM topic_allowed_groups + WHERE group_id IN ( + SELECT group_id FROM group_users WHERE user_id = #{user.id.to_i} + ) + )") + end + + result = result.joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})") + .order("topics.bumped_at DESC") + .private_messages + + if @options[:tag] && tagging_enabled? + tag_id = Tag.where("lower(name) = ?", @options[:tag]).pluck_first(:id) + + if tag_id + result = result.joins(<<~SQL) + INNER JOIN topic_tags + ON topic_tags.topic_id = topics.id + AND topic_tags.tag_id = #{tag_id.to_i} + SQL + end + end + + result = result.limit(options[:per_page]) unless options[:limit] == false + result = result.visible if options[:visible] || @user.nil? || @user.regular? + + if options[:page] + offset = options[:page].to_i * options[:per_page] + result = result.offset(offset) if offset > 0 + end + result + end + + def list_private_messages_tag(user) + list = private_messages_for(user, :all) + list = list.joins("JOIN topic_tags tt ON tt.topic_id = topics.id + JOIN tags t ON t.id = tt.tag_id AND t.name = '#{@options[:tags][0]}'") + create_list(:private_messages, {}, list) + end + + private + + def append_read_state(list, group) + group_id = group.id + return list if group_id.nil? + + selected_values = list.select_values.empty? ? ['topics.*'] : list.select_values + selected_values << "COALESCE(tg.last_read_post_number, 0) AS last_read_post_number" + + list + .joins("LEFT OUTER JOIN topic_groups tg ON topics.id = tg.topic_id AND tg.group_id = #{group_id}") + .select(*selected_values) + end + + def filter_archived(list, user, archived: true) + list = list.joins(<<~SQL) + LEFT JOIN group_archived_messages gm ON gm.topic_id = topics.id + LEFT JOIN user_archived_messages um + ON um.user_id = #{user.id.to_i} + AND um.topic_id = topics.id + SQL + + list = + if archived + list.where("um.user_id IS NOT NULL OR gm.topic_id IS NOT NULL") + else + list.where("um.user_id IS NULL AND gm.topic_id IS NULL") + end + + list + end + + def not_archived(list, user) + list.joins("LEFT JOIN user_archived_messages um + ON um.user_id = #{user.id.to_i} AND um.topic_id = topics.id") + .where('um.user_id IS NULL') + end + + def group + @group ||= begin + Group + .where('name ilike ?', @options[:group_name]) + .select(:id, :publish_read_state) + .first + end + end + + def tagging_enabled? + @guardian.can_tag_pms? + end + end +end diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index b4cc1efa27fd4f..744ccd62a7d5bd 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -1067,7 +1067,6 @@ def suggested_for(topic) expect(TopicQuery.new(user, tags: [tag.name]).list_private_messages_tag(user).topics).to eq([private_message]) end - end end @@ -1193,75 +1192,6 @@ def suggested_for(topic) end end - describe '#list_private_messages_group' do - fab!(:group) { Fabricate(:group) } - - let!(:group_message) do - Fabricate(:private_message_topic, - allowed_groups: [group], - topic_allowed_users: [ - Fabricate.build(:topic_allowed_user, user: Fabricate(:user)), - ] - ) - end - - before do - group.add(creator) - end - - it 'should return the right list for a group user' do - topics = TopicQuery.new(nil, group_name: group.name) - .list_private_messages_group(creator) - .topics - - expect(topics).to contain_exactly(group_message) - end - - it 'should return the right list for an admin not part of the group' do - group.update!(name: group.name.capitalize) - - topics = TopicQuery.new(nil, group_name: group.name.upcase) - .list_private_messages_group(Fabricate(:admin)) - .topics - - expect(topics).to contain_exactly(group_message) - end - - it "should not allow a moderator not part of the group to view the group's messages" do - topics = TopicQuery.new(nil, group_name: group.name) - .list_private_messages_group(Fabricate(:moderator)) - .topics - - expect(topics).to eq([]) - end - - it "should not allow a user not part of the group to view the group's messages" do - topics = TopicQuery.new(nil, group_name: group.name) - .list_private_messages_group(Fabricate(:user)) - .topics - - expect(topics).to eq([]) - end - - context "Calculating minimum unread count for a topic" do - before { group.update!(publish_read_state: true) } - - let(:listed_message) do - TopicQuery.new(nil, group_name: group.name) - .list_private_messages_group(creator) - .topics.first - end - - it 'returns the last read post number' do - topic_group = TopicGroup.create!( - topic: group_message, group: group, last_read_post_number: 10 - ) - - expect(listed_message.last_read_post_number).to eq(topic_group.last_read_post_number) - end - end - end - context "shared drafts" do fab!(:category) { Fabricate(:category_with_definition) } fab!(:shared_drafts_category) { Fabricate(:category_with_definition) } @@ -1349,16 +1279,4 @@ def suggested_for(topic) end end end - - describe '#list_private_messages' do - it "includes topics with moderator posts" do - private_message_topic = Fabricate(:private_message_post, user: user).topic - - expect(TopicQuery.new(user).list_private_messages(user).topics).to be_empty - - private_message_topic.add_moderator_post(admin, "Thank you for your flag") - - expect(TopicQuery.new(user).list_private_messages(user).topics).to eq([private_message_topic]) - end - end end diff --git a/spec/lib/topic_query/private_message_lists_spec.rb b/spec/lib/topic_query/private_message_lists_spec.rb new file mode 100644 index 00000000000000..264465c6397866 --- /dev/null +++ b/spec/lib/topic_query/private_message_lists_spec.rb @@ -0,0 +1,336 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe TopicQuery::PrivateMessageLists do + fab!(:user) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + + fab!(:group) do + Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap do |g| + g.add(user_2) + end + end + + fab!(:group_message) do + create_post( + user: user, + target_group_names: [group.name], + archetype: Archetype.private_message + ).topic + end + + fab!(:private_message) do + create_post( + user: user, + target_usernames: [user_2.username], + archetype: Archetype.private_message + ).topic + end + + describe '#private_messages_for' do + context "tag option" do + before do + SiteSetting.allow_staff_to_tag_pms = true + user.update!(admin: true) + end + + fab!(:tag) do + Fabricate(:tag).tap do |t| + private_message.tags << t + end + end + + let(:topic_query) { TopicQuery.new(user, tag: tag.name) } + + it 'returns a list of all private messages for a given tag' do + topics = topic_query.private_messages_for(user, :user) + + expect(topics).to contain_exactly(private_message) + end + + it 'returns the right list of private messages when tagging is disabled' do + SiteSetting.tagging_enabled = false + + topics = topic_query.private_messages_for(user, :user) + + expect(topics).to contain_exactly(private_message, group_message) + end + end + end + + describe '#list_private_messages_all' do + it 'returns a list of all private messages that a user has access to' do + topics = TopicQuery.new(nil).list_private_messages_all(user).topics + + expect(topics).to contain_exactly(group_message, private_message) + end + + it 'does not include user or group archived messages' do + UserArchivedMessage.archive!(user.id, group_message) + UserArchivedMessage.archive!(user.id, private_message) + + topics = TopicQuery.new(nil).list_private_messages_all(user).topics + + expect(topics).to eq([]) + + GroupArchivedMessage.archive!(user_2.id, group_message) + + topics = TopicQuery.new(nil).list_private_messages_all(user_2).topics + + expect(topics).to contain_exactly(private_message) + end + end + + describe '#list_private_messages_all_sent' do + it 'returns a list of all private messages that a user has sent' do + topics = TopicQuery.new(nil).list_private_messages_all_sent(user_2).topics + + expect(topics).to eq([]) + + create_post(user: user_2, topic: private_message) + + topics = TopicQuery.new(nil).list_private_messages_all_sent(user_2).topics + + expect(topics).to contain_exactly(private_message) + + create_post(user: user_2, topic: group_message) + + topics = TopicQuery.new(nil).list_private_messages_all_sent(user_2).topics + + expect(topics).to contain_exactly(private_message, group_message) + end + + it 'does not include user or group archived messages' do + create_post(user: user_2, topic: private_message) + create_post(user: user_2, topic: group_message) + + UserArchivedMessage.archive!(user_2.id, private_message) + GroupArchivedMessage.archive!(user_2.id, group_message) + + topics = TopicQuery.new(nil).list_private_messages_all_sent(user_2).topics + + expect(topics).to eq([]) + end + end + + describe '#list_private_messages_all_archive' do + it 'returns a list of all private messages that has been archived' do + UserArchivedMessage.archive!(user_2.id, private_message) + GroupArchivedMessage.archive!(user_2.id, group_message) + + topics = TopicQuery.new(nil).list_private_messages_all_archive(user_2).topics + + expect(topics).to contain_exactly(private_message, group_message) + end + end + + describe '#list_private_messages_all_new' do + it 'returns a list of new private messages' do + topics = TopicQuery.new(nil).list_private_messages_all_new(user_2).topics + + expect(topics).to contain_exactly(private_message, group_message) + + TopicUser.find_by(user: user_2, topic: group_message).update!( + last_read_post_number: 1 + ) + + topics = TopicQuery.new(nil).list_private_messages_all_new(user_2).topics + + expect(topics).to contain_exactly(private_message) + end + end + + describe '#list_private_messages_all_unread' do + it 'returns a list of unread private messages' do + topics = TopicQuery.new(nil).list_private_messages_all_unread(user_2).topics + + expect(topics).to eq([]) + + TopicUser.find_by(user: user_2, topic: group_message).update!( + last_read_post_number: 1 + ) + + create_post(user: user, topic: group_message) + + topics = TopicQuery.new(nil).list_private_messages_all_unread(user_2).topics + + expect(topics).to contain_exactly(group_message) + end + end + + describe '#list_private_messages' do + it 'returns a list of all private messages that a user has access to' do + topics = TopicQuery.new(nil).list_private_messages(user_2).topics + + expect(topics).to contain_exactly(private_message) + end + end + + describe '#list_private_messages_group' do + it 'should return the right list for a group user' do + group.add(user_2) + + topics = TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group(user_2) + .topics + + expect(topics).to contain_exactly(group_message) + end + + it 'should return the right list for an admin not part of the group' do + group.update!(name: group.name.capitalize) + + topics = TopicQuery.new(nil, group_name: group.name.upcase) + .list_private_messages_group(Fabricate(:admin)) + .topics + + expect(topics).to contain_exactly(group_message) + end + + it "should not allow a moderator not part of the group to view the group's messages" do + topics = TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group(Fabricate(:moderator)) + .topics + + expect(topics).to eq([]) + end + + it "should not allow a user not part of the group to view the group's messages" do + topics = TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group(Fabricate(:user)) + .topics + + expect(topics).to eq([]) + end + + context "Calculating minimum unread count for a topic" do + before do + group.update!(publish_read_state: true) + group.add(user) + end + + let(:listed_message) do + TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group(user) + .topics.first + end + + it 'returns the last read post number' do + topic_group = TopicGroup.create!( + topic: group_message, group: group, last_read_post_number: 10 + ) + + expect(listed_message.last_read_post_number).to eq(topic_group.last_read_post_number) + end + end + end + + describe '#list_private_messages_group_new' do + it 'returns a list of new private messages for a group that user is a part of' do + topics = TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group_new(user_2) + .topics + + expect(topics).to contain_exactly(group_message) + end + end + + describe '#list_private_messages_group_unread' do + it 'returns a list of unread private messages for a group that user is a part of' do + topics = TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group_unread(user_2) + .topics + + expect(topics).to eq([]) + + TopicUser.find_by(user: user_2, topic: group_message).update!( + last_read_post_number: 1 + ) + + create_post(user: user, topic: group_message) + + topics = TopicQuery.new(nil, group_name: group.name) + .list_private_messages_group_unread(user_2) + .topics + + expect(topics).to contain_exactly(group_message) + end + end + + describe '#list_private_messages_unread' do + fab!(:user) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + + fab!(:pm) do + create_post( + user: user, + target_usernames: [user_2.username], + archetype: Archetype.private_message + ).topic + end + + fab!(:pm_2) do + create_post( + user: user, + target_usernames: [user_2.username], + archetype: Archetype.private_message + ).topic + end + + fab!(:pm_3) do + create_post( + user: user, + target_usernames: [user_2.username], + archetype: Archetype.private_message + ).topic + end + + it 'returns a list of private messages with unread posts that user is at least tracking' do + freeze_time 1.minute.from_now do + create_post(user: user_2, topic_id: pm.id) + create_post(user: user_2, topic_id: pm_3.id) + end + + TopicUser.find_by(user: user, topic: pm_3).update!( + notification_level: TopicUser.notification_levels[:regular] + ) + + expect(TopicQuery.new(user).list_private_messages_unread(user).topics) + .to contain_exactly(pm) + end + end + + describe '#list_private_messages_new' do + fab!(:user) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + + fab!(:pm) do + create_post( + user: user, + target_usernames: [user_2.username], + archetype: Archetype.private_message + ).topic + end + + it 'returns a list of new private messages' do + expect(TopicQuery.new(user_2).list_private_messages_new(user_2).topics) + .to contain_exactly(pm) + end + + it 'returns a list of new private messages accounting for muted tags' do + tag = Fabricate(:tag) + + pm.tags << tag + + TagUser.create!( + tag: tag, + user: user_2, + notification_level: TopicUser.notification_levels[:muted] + ) + + expect(TopicQuery.new(user_2).list_private_messages_new(user_2).topics) + .to eq([]) + end + end +end diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index 26e3d2c26641d2..85133188c37136 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -205,8 +205,8 @@ expect(messages.map(&:channel)).to contain_exactly( '/private-messages/inbox', - "/private-messages/group/#{group1.name}", - "/private-messages/group/#{group2.name}" + "/private-messages/group/#{group1.name}/inbox", + "/private-messages/group/#{group2.name}/inbox" ) message = messages.find do |m| @@ -218,7 +218,7 @@ [group1, group2].each do |group| message = messages.find do |m| - m.channel == "/private-messages/group/#{group.name}" + m.channel == "/private-messages/group/#{group.name}/inbox" end expect(message.data["topic_id"]).to eq(private_message_topic.id) @@ -237,9 +237,9 @@ expect(messages.map(&:channel)).to contain_exactly( '/private-messages/inbox', - "/private-messages/group/#{group1.name}", + "/private-messages/group/#{group1.name}/inbox", "/private-messages/group/#{group1.name}/archive", - "/private-messages/group/#{group2.name}", + "/private-messages/group/#{group2.name}/inbox", "/private-messages/group/#{group2.name}/archive", ) @@ -249,11 +249,9 @@ expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id)) [group1, group2].each do |group| - group_channel = "/private-messages/group/#{group.name}" - [ - group_channel, - "#{group_channel}/archive" + "/private-messages/group/#{group.name}/inbox", + "/private-messages/group/#{group.name}/archive" ].each do |channel| message = messages.find { |m| m.channel == channel } expect(message.data["topic_id"]).to eq(private_message_topic.id) @@ -291,7 +289,7 @@ expected_channels = [ '/private-messages/inbox', '/private-messages/sent', - "/private-messages/group/#{group.name}" + "/private-messages/group/#{group.name}/inbox" ] expect(messages.map(&:channel)).to contain_exactly(*expected_channels) diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 9bd7c6f8596e76..1a324ae4a68f93 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -136,40 +136,17 @@ end describe "filter private messages by tag" do - let(:user) { Fabricate(:user) } - let(:moderator) { Fabricate(:moderator) } - let(:admin) { Fabricate(:admin) } - let(:tag) { Fabricate(:tag) } - let(:private_message) { Fabricate(:private_message_topic, user: admin) } - - before do - SiteSetting.tagging_enabled = true - SiteSetting.allow_staff_to_tag_pms = true - Fabricate(:topic_tag, tag: tag, topic: private_message) - end + fab!(:tag) { Fabricate(:tag) } - it 'should fail for non-staff users' do - sign_in(user) - get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" - expect(response.status).to eq(404) + fab!(:private_message) do + Fabricate(:private_message_topic, user: admin).tap { |t| t.tags << tag } end - it 'should fail for staff users if disabled' do - SiteSetting.allow_staff_to_tag_pms = false - - [moderator, admin].each do |user| - sign_in(user) - get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" - expect(response.status).to eq(404) - end - end + fab!(:private_message_2) { Fabricate(:private_message_topic, user: admin) } - it 'should be success for staff users' do - [moderator, admin].each do |user| - sign_in(user) - get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" - expect(response.status).to eq(200) - end + before do + SiteSetting.tagging_enabled = true + SiteSetting.allow_staff_to_tag_pms = true end it 'should work for tag with unicode name' do @@ -177,10 +154,16 @@ Fabricate(:topic_tag, tag: unicode_tag, topic: private_message) sign_in(admin) - get "/topics/private-messages-tags/#{admin.username}/#{UrlHelper.encode_component(unicode_tag.name)}.json" + + get "/topics/private-messages-all/#{admin.username}.json", params: { + tag: unicode_tag.name + } + expect(response.status).to eq(200) - expect(response.parsed_body["topic_list"]["topics"].first["id"]) - .to eq(private_message.id) + + topics = response.parsed_body["topic_list"]["topics"] + + expect(topics.map { |t| t["id"] }).to contain_exactly(private_message.id) end end diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index fecf09f2ebdb99..77b1bcd6d39085 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -473,7 +473,7 @@ it "can't see pm tags" do get "/tags/personal_messages/#{regular_user.username}.json" - expect(response).not_to be_successful + expect(response.status).to eq(403) end end @@ -485,7 +485,7 @@ it "can't see pm tags for regular user" do get "/tags/personal_messages/#{regular_user.username}.json" - expect(response).not_to be_successful + expect(response.status).to eq(404) end it "can see their own pm tags" do @@ -496,6 +496,21 @@ tag = response.parsed_body['tags'] expect(tag[0]["id"]).to eq('test') end + + it 'accepts a limit on number of tags to return' do + Fabricate(:tag, topics: [personal_message], name: 'test2') + Fabricate(:tag, topics: [personal_message], name: 'test3') + + get "/tags/personal_messages/#{moderator.username}.json", params: { + limit: 2 + } + + expect(response.status).to eq(200) + + tags = response.parsed_body["tags"] + + expect(tags.map { |t| t["id"] }).to contain_exactly("test", "test2") + end end context "as an admin" do From 3c9c15f42ab006a3e9f28c289b020186a68bc3c0 Mon Sep 17 00:00:00 2001 From: awesomerobot Date: Wed, 28 Jul 2021 14:26:35 -0400 Subject: [PATCH 2/3] UX: spacing adjustments, mobile nav refactor --- .../discourse/app/templates/user/messages.hbs | 77 ++++++++-------- .../stylesheets/common/components/navs.scss | 3 +- app/assets/stylesheets/desktop/user.scss | 43 +++++++-- app/assets/stylesheets/mobile/user.scss | 87 +++++++++++++++---- 4 files changed, 147 insertions(+), 63 deletions(-) diff --git a/app/assets/javascripts/discourse/app/templates/user/messages.hbs b/app/assets/javascripts/discourse/app/templates/user/messages.hbs index a9aade483286ab..8122b1b557f9e7 100644 --- a/app/assets/javascripts/discourse/app/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/messages.hbs @@ -1,30 +1,36 @@ {{#d-section class=sectionClass pageClass="user-messages"}} {{#if inboxes.length}} +
    + {{combo-box + content=inboxes + classNames="user-messages-inboxes-drop" + value=selectedInbox + onChange=(action "updateInbox") + options=(hash + filterable=true + ) + }} + {{#if (and group site.mobileView)}} + {{group-notifications-button + value=group.group_user.notification_level + onChange=(action "changeGroupNotificationLevel") + }} + {{/if}} +
    + {{/if}} + + {{#if (and pmTaggingEnabled tagsContent)}} {{combo-box - content=inboxes - classNames="user-messages-inboxes-drop" - value=selectedInbox - onChange=(action "updateInbox") + content=tagsContent + value=tag + classNames="user-messages-tags-drop" options=(hash filterable=true + none="tagging.selector_all_tags" ) }} {{/if}} - {{#unless site.mobileView}} - {{#if (and pmTaggingEnabled tagsContent)}} - {{combo-box - content=tagsContent - value=tag - classNames="user-messages-tags-drop" - options=(hash - filterable=true - none="tagging.selector_all_tags" - ) - }} - {{/if}} - {{/unless}} - {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list"}} {{#if isAllInbox}}
  • @@ -105,26 +111,27 @@ {{/link-to}}
  • {{/if}} - - {{#unless mobileView}} - {{#if group}} -
  • - {{group-notifications-button - value=group.group_user.notification_level - onChange=(action "changeGroupNotificationLevel") - }} -
  • - {{/if}} - - {{#if showNewPM}} -
  • - {{d-button class="btn-primary new-private-message" action=(route-action "composePrivateMessage") icon="envelope" label="user.new_private_message"}} -
  • - {{/if}} - {{/unless}} {{/mobile-nav}} {{/d-section}} +{{#if (and site.mobileView showNewPM)}} + {{d-button class="btn-primary new-private-message" action=(route-action "composePrivateMessage") icon="envelope" label="user.new_private_message"}} +{{/if}} + +{{#unless site.mobileView}} +
    + {{#if group}} + {{group-notifications-button + value=group.group_user.notification_level + onChange=(action "changeGroupNotificationLevel") + }} + {{/if}} + {{#if showNewPM}} + {{d-button class="btn-primary new-private-message" action=(route-action "composePrivateMessage") icon="envelope" label="user.new_private_message"}} + {{/if}} +
    +{{/unless}} +
    {{#if showWarningsWarning}}
    {{html-safe (i18n "admin.user.warnings_list_warning")}}
    diff --git a/app/assets/stylesheets/common/components/navs.scss b/app/assets/stylesheets/common/components/navs.scss index 7763081b4b7cba..c1d7f391736755 100644 --- a/app/assets/stylesheets/common/components/navs.scss +++ b/app/assets/stylesheets/common/components/navs.scss @@ -66,12 +66,10 @@ .nav-stacked { @extend %nav; padding: 0; - overflow: hidden; background: var(--primary-low); li { border-bottom: 1px solid var(--primary-low); - position: relative; &:last-of-type { border-bottom: 0; @@ -89,6 +87,7 @@ line-height: $line-height-small; cursor: pointer; color: var(--primary); + @include ellipsis; &.active { color: var(--secondary); diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index d323c587b72554..880d142da8a5a3 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -22,11 +22,14 @@ .combo-box { width: 100%; - margin-top: 1px; + &:not(:last-of-type) { + margin-bottom: 0.875em; + } } .nav-stacked { background-color: transparent; + margin: 0; li { border-bottom: none; @@ -51,10 +54,23 @@ } } + .select-kit + .messages-nav { + margin-top: 1em; + } + + .inboxes-controls { + margin-bottom: 0.75em; + } + &.user-messages { + --left-padding: 0.8em; .user-messages-inboxes-drop, .user-messages-tags-drop { - padding: 13px 13px 0 0; + padding: 0 1em 0 0; + + .select-kit-header { + padding-left: var(--left-padding); + } .select-kit-selected-name { overflow: hidden; @@ -63,15 +79,18 @@ .nav-stacked { a { - padding-left: 0; - } - - .new-private-message { - margin-top: 13px; + padding-left: calc( + var(--left-padding) - 1px + ); // 1px accounts for border on select-kit elements above } } } } +.user-additional-controls { + button { + margin-bottom: 1em; + } +} .user-content { padding-bottom: 12px; @@ -250,6 +269,16 @@ table.user-invite-list { } } +.user-messages-page { + .topic-list th { + padding-top: 4px; + } + + .show-mores { + position: absolute; + } +} + .user-messages { margin-right: 0.2em; } diff --git a/app/assets/stylesheets/mobile/user.scss b/app/assets/stylesheets/mobile/user.scss index 3cf1607eceaf70..fcd29a50a1322a 100644 --- a/app/assets/stylesheets/mobile/user.scss +++ b/app/assets/stylesheets/mobile/user.scss @@ -4,8 +4,7 @@ display: grid; grid-template-columns: 1fr 1fr; grid-template-rows: auto auto auto; - grid-row-gap: 20px; - grid-column-gap: 16px; + grid-gap: 16px; .user-primary-navigation { grid-column-start: 1; grid-row-start: 1; @@ -31,38 +30,83 @@ grid-column-start: 1; } + // specific to messages + .user-messages.user-messages-inboxes { grid-row-start: 2; - grid-row-end: 3; grid-column-start: 1; grid-column-end: 3; + display: grid; grid-template-columns: 1fr 1fr; - grid-template-rows: auto auto auto; - grid-column-gap: 16px; + gap: 16px; - .user-messages-inboxes-drop { - grid-row-start: 1; - grid-row-end: 2; + + .user-additional-controls { + grid-row-start: 2; grid-column-start: 1; - grid-column-end: 2; + } + } - .select-kit-header { - padding: 8px 10px; + .inboxes-controls { + display: flex; + } - .caret-icon { - color: var(--primary-medium); - } + .user-messages-inboxes-drop { + padding: 0; + flex: 1 1 auto; + .select-kit-header { + padding: 8px 10px; + + .caret-icon { + color: var(--primary-medium); } } + } - .messages-nav { - grid-row-start: 1; - grid-row-end: 2; - grid-column-start: 2; - grid-column-end: 3; + .messages-nav { + grid-column-start: 2; + grid-column-end: 3; + grid-row-start: 1; + } + + .user-messages-tags-drop { + justify-self: start; + margin-bottom: -1.5em; // pulls the topic list below closer + .select-kit-header { + border: none; + justify-self: start; + padding-left: 10px; + } + &.single-select.is-expanded .select-kit-header { + outline: none; } } + + .new-private-message { + grid-row-start: 1; + grid-column-start: 2; + } + + .group-notifications-button { + margin-left: 8px; + + .select-kit-header { + height: 100%; + + .selected-name .name { + display: none; + } + } + } +} + +.user-messages-page { + .paginated-topics-list { + margin-top: 0; + } + .show-mores { + margin-top: 0.5em; + } } .user-main { @@ -199,6 +243,10 @@ flex: 1 1 25%; margin-left: auto; + .btn { + margin-bottom: 16px; + } + ul { margin: 0; display: flex; @@ -256,6 +304,7 @@ .user-main .collapsed-info.about .details { display: flex; + margin-bottom: 16px; .user-profile-avatar { margin: 0; flex: 0 0 auto; From 34e199db6532530b857109e7028606acf1790615 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 29 Jul 2021 15:03:49 +0800 Subject: [PATCH 3/3] Revert back to old tagging behavior. Too much effort to properly implement a tag filter now. Plus that is not our priority. --- .../app/controllers/user-private-messages.js | 37 ++++----- .../discourse/app/lib/cached-topic-list.js | 8 +- .../build-private-messages-group-route.js | 18 +---- .../routes/build-private-messages-route.js | 18 +---- .../app/routes/user-private-messages.js | 35 +------- .../discourse/app/templates/user/messages.hbs | 61 +++++++------- .../acceptance/user-private-messages-test.js | 79 +++++-------------- app/assets/stylesheets/desktop/user.scss | 3 +- app/assets/stylesheets/mobile/user.scss | 13 --- app/controllers/list_controller.rb | 1 - app/controllers/tags_controller.rb | 12 +-- config/routes.rb | 3 - lib/topic_query.rb | 1 - lib/topic_query/private_message_lists.rb | 18 +---- .../topic_query/private_message_lists_spec.rb | 31 -------- spec/requests/list_controller_spec.rb | 49 ++++++++---- spec/requests/tags_controller_spec.rb | 15 ---- 17 files changed, 113 insertions(+), 289 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js index 1fdd985abbcfb8..bcdece0d7e9fdc 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js @@ -9,7 +9,6 @@ export const PERSONAL_INBOX = "__personal_inbox__"; const ALL_INBOX = "__all_inbox__"; export default Controller.extend({ - queryParams: ["tag"], user: controller(), pmView: false, @@ -19,10 +18,21 @@ export default Controller.extend({ groupFilter: alias("group.name"), currentPath: alias("router._router.currentPath"), pmTaggingEnabled: alias("site.can_tag_pms"), - tag: null, + tagId: null, showNewPM: and("user.viewingSelf", "currentUser.can_send_private_messages"), + @discourseComputed("inboxes", "isAllInbox") + displayGlobalFilters(inboxes, isAllInbox) { + if (inboxes.length === 0) { + return true; + } + if (inboxes.length && isAllInbox) { + return true; + } + return false; + }, + @discourseComputed("inboxes") sectionClass(inboxes) { const defaultClass = "user-secondary-navigation user-messages"; @@ -56,14 +66,7 @@ export default Controller.extend({ return pmView === VIEW_NAME_WARNINGS && !viewingSelf && !isAdmin; }, - @discourseComputed("tags") - tagsContent(tags) { - return tags.map((tag) => { - return { id: tag.id, name: tag.text }; - }); - }, - - @discourseComputed("model.groups", "tags") + @discourseComputed("model.groups") inboxes(groups) { const groupsWithMessages = groups?.filter((group) => { return group.has_messages; @@ -100,20 +103,12 @@ export default Controller.extend({ @action updateInbox(inbox) { - const queryParams = {}; - - if (this.tag) { - queryParams.tag = this.tag; - } - if (inbox === ALL_INBOX) { - this.transitionToRoute("userPrivateMessages.index", { queryParams }); + this.transitionToRoute("userPrivateMessages.index"); } else if (inbox === PERSONAL_INBOX) { - this.transitionToRoute("userPrivateMessages.personal", { queryParams }); + this.transitionToRoute("userPrivateMessages.personal"); } else { - this.transitionToRoute("userPrivateMessages.group", inbox, { - queryParams, - }); + this.transitionToRoute("userPrivateMessages.group", inbox); } }, }); diff --git a/app/assets/javascripts/discourse/app/lib/cached-topic-list.js b/app/assets/javascripts/discourse/app/lib/cached-topic-list.js index 4a42a03e230d1a..38148314d3ce06 100644 --- a/app/assets/javascripts/discourse/app/lib/cached-topic-list.js +++ b/app/assets/javascripts/discourse/app/lib/cached-topic-list.js @@ -1,11 +1,7 @@ -export function findOrResetCachedTopicList(session, filter, params) { +export function findOrResetCachedTopicList(session, filter) { const lastTopicList = session.get("topicList"); - if ( - lastTopicList && - lastTopicList.filter === filter && - lastTopicList.params.tag === params?.tag - ) { + if (lastTopicList && lastTopicList.filter === filter) { return lastTopicList; } else { session.setProperties({ diff --git a/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js b/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js index f8b973c4d6145c..d1aa356eebf65e 100644 --- a/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js @@ -28,25 +28,11 @@ export default (viewName, channel) => { filter = `${filter}/${viewName}`; } - const tag = this.paramsFor("user-private-messages").tag; - const filterParams = {}; - - if (tag) { - filterParams.tag = tag; - } - - const lastTopicList = findOrResetCachedTopicList( - this.session, - filter, - filterParams - ); + const lastTopicList = findOrResetCachedTopicList(this.session, filter); return lastTopicList ? lastTopicList - : this.store.findFiltered("topicList", { - filter, - params: filterParams, - }); + : this.store.findFiltered("topicList", { filter }); }, afterModel(model) { 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 0b59ce0ab04e72..2eed30a80da0a6 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 @@ -24,25 +24,11 @@ export default (viewName, path, channel) => { const filter = "topics/" + path + "/" + this.modelFor("user").get("username_lower"); - const tag = this.paramsFor("user-private-messages").tag; - const params = {}; - - if (tag) { - params.tag = tag; - } - - const lastTopicList = findOrResetCachedTopicList( - this.session, - filter, - params - ); + const lastTopicList = findOrResetCachedTopicList(this.session, filter); return lastTopicList ? lastTopicList - : this.store.findFiltered("topicList", { - filter, - params, - }); + : this.store.findFiltered("topicList", { filter }); }, setupController() { 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 28080200159bdb..34e8b488c1d840 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages.js @@ -1,47 +1,18 @@ import Composer from "discourse/models/composer"; -import Tag from "discourse/models/tag"; 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"; export default DiscourseRoute.extend({ - queryParams: { - tag: { - refreshModel: true, - }, - }, - renderTemplate() { this.render("user/messages"); }, model() { - const user = this.modelFor("user"); - const model = { user, tags: [] }; - - if (this.site.can_tag_pms) { - return ajax(`/tags/personal_messages/${user.username_lower}`, { - data: { limit: this.siteSettings.max_tags_in_filter_list }, - }) - .then((result) => { - model.tags = result.tags.map((tag) => Tag.create(tag)); - return model; - }) - .catch((e) => { - popupAjaxError(e); - return model; - }); - } else { - return model; - } + return this.modelFor("user"); }, - setupController(controller, model) { - controller.setProperties({ - model: model.user, - tags: model.tags, - }); + setupController(controller, user) { + controller.set("model", user); if (this.currentUser) { const composerController = this.controllerFor("composer"); diff --git a/app/assets/javascripts/discourse/app/templates/user/messages.hbs b/app/assets/javascripts/discourse/app/templates/user/messages.hbs index 8122b1b557f9e7..3ac8f46b6592fc 100644 --- a/app/assets/javascripts/discourse/app/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/messages.hbs @@ -19,66 +19,53 @@ {{/if}} - {{#if (and pmTaggingEnabled tagsContent)}} - {{combo-box - content=tagsContent - value=tag - classNames="user-messages-tags-drop" - options=(hash - filterable=true - none="tagging.selector_all_tags" - ) - }} - {{/if}} - {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list"}} {{#if isAllInbox}}
  • - {{#link-to "userPrivateMessages.index" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.index" model}} {{i18n "user.messages.latest"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.sent" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.sent" model}} {{i18n "user.messages.sent"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.new" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.new" model}} {{i18n "user.messages.new"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.unread" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.unread" model}} {{i18n "user.messages.unread"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.archive" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.archive" model}} {{i18n "user.messages.archive"}} {{/link-to}}
  • - {{plugin-outlet name="user-messages-nav" connectorTagName="li" args=(hash model=model)}} {{/if}} {{#if group}}
  • - {{#link-to "userPrivateMessages.group" group.name (query-params tag=tag)}} + {{#link-to "userPrivateMessages.group" group.name}} {{i18n "user.messages.latest"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.groupNew" group.name (query-params tag=tag)}} + {{#link-to "userPrivateMessages.groupNew" group.name}} {{i18n "user.messages.new"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.groupUnread" group.name (query-params tag=tag)}} + {{#link-to "userPrivateMessages.groupUnread" group.name}} {{i18n "user.messages.unread"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.groupArchive" group.name (query-params tag=tag)}} + {{#link-to "userPrivateMessages.groupArchive" group.name}} {{i18n "user.messages.archive"}} {{/link-to}}
  • @@ -86,31 +73,51 @@ {{#if isPersonalInbox}}
  • - {{#link-to "userPrivateMessages.personal" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.personal" model}} {{i18n "user.messages.latest"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.personalSent" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.personalSent" model}} {{i18n "user.messages.sent"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.personalNew" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.personalNew" model}} {{i18n "user.messages.new"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.personalUnread" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.personalUnread" model}} {{i18n "user.messages.unread"}} {{/link-to}}
  • - {{#link-to "userPrivateMessages.personalArchive" model (query-params tag=tag)}} + {{#link-to "userPrivateMessages.personalArchive" model}} {{i18n "user.messages.archive"}} {{/link-to}}
  • {{/if}} + + {{#if displayGlobalFilters}} + {{#if pmTaggingEnabled}} +
  • + {{#link-to "userPrivateMessages.tags" model}} + {{i18n "user.messages.tags"}} + {{/link-to}} + + {{#if tagId}} +
  • + {{#link-to "userPrivateMessages.tagsShow" tagId}} + {{tagId}} + {{/link-to}} +
  • + {{/if}} + + {{/if}} + + {{plugin-outlet name="user-messages-nav" connectorTagName="li" args=(hash model=model)}} + {{/if}} {{/mobile-nav}} {{/d-section}} 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 54fb5adf833092..b73c982025c63a 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 @@ -13,6 +13,10 @@ acceptance( function (needs) { needs.user(); + needs.site({ + can_tag_pms: true, + }); + test("viewing messages", async function (assert) { await visit("/u/eviltrout/messages"); @@ -23,10 +27,7 @@ acceptance( "does not display inboxes dropdown" ); - assert.ok( - !exists(".user-messages-tags-drop"), - "does not display tags dropdown" - ); + assert.ok(exists(".messages-nav .tags"), "displays the tags filter"); assert.ok( !exists(".group-notifications-button"), @@ -36,66 +37,15 @@ acceptance( } ); -acceptance("User Private Messages - with PM tagging enabled", function (needs) { - needs.user(); - - needs.site({ - can_tag_pms: true, - }); - - needs.pretender((server, helper) => { - server.get("/tags/personal_messages/:username", () => { - return helper.response({ - tags: [ - { id: "tag1", text: "tag1", count: 1 }, - { id: "tag2", text: "tag2", count: 2 }, - ], - }); - }); - - server.get("/topics/private-messages-all/:username.json", (request) => { - let topicList; - - if (request.queryParams?.tag) { - topicList = { - topics: [ - { id: 1, posters: [] }, - { id: 2, posters: [] }, - ], - }; - } else { - topicList = { - topics: [ - { id: 1, posters: [] }, - { id: 2, posters: [] }, - { id: 3, posters: [] }, - ], - }; - } - - return helper.response({ topic_list: topicList }); - }); - }); - - test("viewing messages", async function (assert) { - await visit("/u/charlie/messages"); - - assert.equal(count(".topic-list-item"), 3, "displays the right topic list"); - - assert.ok(exists(".user-messages-tags-drop"), "displays tags dropdown"); - - await selectKit(".user-messages-tags-drop").expand(); - await selectKit(".user-messages-tags-drop").selectRowByValue("tag1"); - - assert.equal(count(".topic-list-item"), 2, "displays the right topic list"); - }); -}); - acceptance( "User Private Messages - user with group messages", function (needs) { needs.user(); + needs.site({ + can_tag_pms: true, + }); + needs.pretender((server, helper) => { server.get("/topics/private-messages-all/:username.json", () => { return helper.response({ @@ -138,6 +88,8 @@ acceptance( "displays inboxes dropdown" ); + assert.ok(exists(".messages-nav .tags"), "displays the tags filter"); + await selectKit(".user-messages-inboxes-drop").expand(); await selectKit(".user-messages-inboxes-drop").selectRowByValue( PERSONAL_INBOX @@ -150,8 +102,8 @@ acceptance( ); assert.ok( - !exists(".user-messages-tags-drop"), - "does not display tags dropdown" + !exists(".messages-nav .tags"), + "does not display the tags filter" ); await selectKit(".user-messages-inboxes-drop").expand(); @@ -169,6 +121,11 @@ acceptance( exists(".group-notifications-button"), "displays the group notifications button" ); + + assert.ok( + !exists(".messages-nav .tags"), + "does not display the tags filter" + ); }); } ); diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index 880d142da8a5a3..b23fa9d4864300 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -64,8 +64,7 @@ &.user-messages { --left-padding: 0.8em; - .user-messages-inboxes-drop, - .user-messages-tags-drop { + .user-messages-inboxes-drop { padding: 0 1em 0 0; .select-kit-header { diff --git a/app/assets/stylesheets/mobile/user.scss b/app/assets/stylesheets/mobile/user.scss index fcd29a50a1322a..47ff10bddbf97d 100644 --- a/app/assets/stylesheets/mobile/user.scss +++ b/app/assets/stylesheets/mobile/user.scss @@ -69,19 +69,6 @@ grid-row-start: 1; } - .user-messages-tags-drop { - justify-self: start; - margin-bottom: -1.5em; // pulls the topic list below closer - .select-kit-header { - border: none; - justify-self: start; - padding-left: 10px; - } - &.single-select.is-expanded .select-kit-header { - outline: none; - } - } - .new-private-message { grid-row-start: 1; grid-column-start: 2; diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index a6e0c7e4d20831..34c505bb194ddc 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -156,7 +156,6 @@ def message_route(action) case action when :private_messages_tag - Discourse.deprecate("The '/private-messages-tags/:username/:tag_id.json' has been deprecated and will be removed in the Discourse 2.9 release.") raise Discourse::NotFound if !guardian.can_tag_pms? when :private_messages_warnings guardian.ensure_can_see_warnings!(target_user) diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 9cfec7163a3561..912a60c9c770fc 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -301,17 +301,7 @@ def personal_messages allowed_user = fetch_user_from_params raise Discourse::NotFound if allowed_user.blank? raise Discourse::NotFound if current_user.id != allowed_user.id && !@guardian.is_admin? - - opts = { - guardian: guardian, - allowed_user: allowed_user - } - - if params[:limit] - opts[:limit] = params[:limit] - end - - pm_tags = Tag.pm_tags(opts) + pm_tags = Tag.pm_tags(guardian: guardian, allowed_user: allowed_user) render json: { tags: pm_tags } end diff --git a/config/routes.rb b/config/routes.rb index 4dd2c39c82f9d8..fd833b8bf51466 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -774,10 +774,7 @@ get "private-messages-sent/:username" => "list#private_messages_sent", as: "topics_private_messages_sent", defaults: { format: :json } get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive", defaults: { format: :json } get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread", defaults: { format: :json } - - # Deprecated: Remove after Discourse 2.9 has been released get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", defaults: { format: :json } - get "private-messages-new/:username" => "list#private_messages_new", as: "topics_private_messages_new", defaults: { format: :json } get "private-messages-warnings/:username" => "list#private_messages_warnings", as: "topics_private_messages_warnings", defaults: { format: :json } get "groups/:group_name" => "list#group_topics", as: "group_topics", group_name: RouteFormat.username diff --git a/lib/topic_query.rb b/lib/topic_query.rb index f41db2f86d3f43..44b821dd57c71d 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -57,7 +57,6 @@ def self.public_valid_options q f group_name - tag tags match_all_tags no_subcategories diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index cd525cc43ce470..28534735ada1d1 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -160,7 +160,7 @@ def private_messages_for(user, type) options.reverse_merge!(per_page: per_page_setting) result = Topic.includes(:allowed_users) - result = result.includes(:tags) if tagging_enabled? + result = result.includes(:tags) if SiteSetting.tagging_enabled if type == :group result = result.joins( @@ -189,18 +189,6 @@ def private_messages_for(user, type) .order("topics.bumped_at DESC") .private_messages - if @options[:tag] && tagging_enabled? - tag_id = Tag.where("lower(name) = ?", @options[:tag]).pluck_first(:id) - - if tag_id - result = result.joins(<<~SQL) - INNER JOIN topic_tags - ON topic_tags.topic_id = topics.id - AND topic_tags.tag_id = #{tag_id.to_i} - SQL - end - end - result = result.limit(options[:per_page]) unless options[:limit] == false result = result.visible if options[:visible] || @user.nil? || @user.regular? @@ -264,9 +252,5 @@ def group .first end end - - def tagging_enabled? - @guardian.can_tag_pms? - end end end diff --git a/spec/lib/topic_query/private_message_lists_spec.rb b/spec/lib/topic_query/private_message_lists_spec.rb index 264465c6397866..8d37b1d17ddca2 100644 --- a/spec/lib/topic_query/private_message_lists_spec.rb +++ b/spec/lib/topic_query/private_message_lists_spec.rb @@ -28,37 +28,6 @@ ).topic end - describe '#private_messages_for' do - context "tag option" do - before do - SiteSetting.allow_staff_to_tag_pms = true - user.update!(admin: true) - end - - fab!(:tag) do - Fabricate(:tag).tap do |t| - private_message.tags << t - end - end - - let(:topic_query) { TopicQuery.new(user, tag: tag.name) } - - it 'returns a list of all private messages for a given tag' do - topics = topic_query.private_messages_for(user, :user) - - expect(topics).to contain_exactly(private_message) - end - - it 'returns the right list of private messages when tagging is disabled' do - SiteSetting.tagging_enabled = false - - topics = topic_query.private_messages_for(user, :user) - - expect(topics).to contain_exactly(private_message, group_message) - end - end - end - describe '#list_private_messages_all' do it 'returns a list of all private messages that a user has access to' do topics = TopicQuery.new(nil).list_private_messages_all(user).topics diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 1a324ae4a68f93..9bd7c6f8596e76 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -136,17 +136,40 @@ end describe "filter private messages by tag" do - fab!(:tag) { Fabricate(:tag) } - - fab!(:private_message) do - Fabricate(:private_message_topic, user: admin).tap { |t| t.tags << tag } - end - - fab!(:private_message_2) { Fabricate(:private_message_topic, user: admin) } + let(:user) { Fabricate(:user) } + let(:moderator) { Fabricate(:moderator) } + let(:admin) { Fabricate(:admin) } + let(:tag) { Fabricate(:tag) } + let(:private_message) { Fabricate(:private_message_topic, user: admin) } before do SiteSetting.tagging_enabled = true SiteSetting.allow_staff_to_tag_pms = true + Fabricate(:topic_tag, tag: tag, topic: private_message) + end + + it 'should fail for non-staff users' do + sign_in(user) + get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" + expect(response.status).to eq(404) + end + + it 'should fail for staff users if disabled' do + SiteSetting.allow_staff_to_tag_pms = false + + [moderator, admin].each do |user| + sign_in(user) + get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" + expect(response.status).to eq(404) + end + end + + it 'should be success for staff users' do + [moderator, admin].each do |user| + sign_in(user) + get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" + expect(response.status).to eq(200) + end end it 'should work for tag with unicode name' do @@ -154,16 +177,10 @@ Fabricate(:topic_tag, tag: unicode_tag, topic: private_message) sign_in(admin) - - get "/topics/private-messages-all/#{admin.username}.json", params: { - tag: unicode_tag.name - } - + get "/topics/private-messages-tags/#{admin.username}/#{UrlHelper.encode_component(unicode_tag.name)}.json" expect(response.status).to eq(200) - - topics = response.parsed_body["topic_list"]["topics"] - - expect(topics.map { |t| t["id"] }).to contain_exactly(private_message.id) + expect(response.parsed_body["topic_list"]["topics"].first["id"]) + .to eq(private_message.id) end end diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index 77b1bcd6d39085..782c5f2e943ba4 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -496,21 +496,6 @@ tag = response.parsed_body['tags'] expect(tag[0]["id"]).to eq('test') end - - it 'accepts a limit on number of tags to return' do - Fabricate(:tag, topics: [personal_message], name: 'test2') - Fabricate(:tag, topics: [personal_message], name: 'test3') - - get "/tags/personal_messages/#{moderator.username}.json", params: { - limit: 2 - } - - expect(response.status).to eq(200) - - tags = response.parsed_body["tags"] - - expect(tags.map { |t| t["id"] }).to contain_exactly("test", "test2") - end end context "as an admin" do