From 9d5da2b383765becb824a8f3ff3665abc8e527fa Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 28 Sep 2021 11:58:04 +0800 Subject: [PATCH] PERF: Revert all inboxes from messages route. (#14445) The all inboxes was introduced in 016efeadf6f242e04daf5ef8e18c2ca708a1392d but we decided to roll it back for performance reasons. The main performance challenge here is that PG has to basically loop through all the PMs that a user is allowed to view before being able to order by `Topic#bumped_at`. The all inboxes was not planned as part of the new/unread filter so we've decided not to tackle the performance issue for the upcoming release. Follow-up to 016efeadf6f242e04daf5ef8e18c2ca708a1392d --- .../discourse/app/controllers/topic.js | 2 - .../app/controllers/user-private-messages.js | 81 +-------- .../discourse/app/routes/app-route-map.js | 5 - .../routes/user-private-messages-archive.js | 4 +- .../app/routes/user-private-messages-index.js | 2 +- .../app/routes/user-private-messages-new.js | 2 +- .../user-private-messages-personal-archive.js | 9 - .../user-private-messages-personal-new.js | 6 - .../user-private-messages-personal-sent.js | 3 - .../user-private-messages-personal-unread.js | 6 - .../routes/user-private-messages-personal.js | 5 - .../app/routes/user-private-messages-sent.js | 2 +- .../routes/user-private-messages-unread.js | 6 +- .../discourse/app/templates/user/messages.hbs | 159 ++++++------------ .../acceptance/user-private-messages-test.js | 85 ++-------- app/assets/stylesheets/desktop/user.scss | 11 -- app/assets/stylesheets/mobile/user.scss | 19 --- app/controllers/list_controller.rb | 9 +- config/locales/client.en.yml | 12 +- config/routes.rb | 7 - lib/topic_query/private_message_lists.rb | 34 ---- .../topic_query/private_message_lists_spec.rb | 110 ------------ 22 files changed, 83 insertions(+), 496 deletions(-) delete mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-archive.js delete mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-new.js delete mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-sent.js delete mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal-unread.js delete mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-personal.js diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 8f83a5e8805..97ff99bd83d 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -158,8 +158,6 @@ export default Controller.extend(bufferedProperty("model"), { if (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 2ff2c1fc95e..b6e1f9a2f28 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js @@ -6,7 +6,6 @@ import { VIEW_NAME_WARNINGS } from "discourse/routes/user-private-messages-warni import I18n from "I18n"; export const PERSONAL_INBOX = "__personal_inbox__"; -const ALL_INBOX = "__all_inbox__"; export default Controller.extend({ user: controller(), @@ -22,56 +21,17 @@ export default Controller.extend({ 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"; - - return inboxes.length - ? `${defaultClass} user-messages-inboxes` - : defaultClass; - }, - - @discourseComputed("pmView") - isPersonalInbox(pmView) { - return pmView && pmView.startsWith("user"); - }, - - @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("pmTopicTrackingState.newIncoming.[]", "selectedInbox") + @discourseComputed("pmTopicTrackingState.newIncoming.[]", "group") newLinkText() { return this._linkText("new"); }, - @discourseComputed("selectedInbox", "pmTopicTrackingState.newIncoming.[]") + @discourseComputed("pmTopicTrackingState.newIncoming.[]", "group") unreadLinkText() { return this._linkText("unread"); }, @@ -86,45 +46,8 @@ export default Controller.extend({ } }, - @discourseComputed("model.groupsWithMessages") - inboxes(groupsWithMessages) { - 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) { - if (inbox === ALL_INBOX) { - this.transitionToRoute("userPrivateMessages.index"); - } else if (inbox === PERSONAL_INBOX) { - this.transitionToRoute("userPrivateMessages.personal"); - } else { - this.transitionToRoute("userPrivateMessages.group", inbox); - } - }, }); 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 42eb98b4540..b4c778fdfac 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -144,11 +144,6 @@ export default function () { 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" }); 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 9861501713e..a6128847f7c 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 @@ -3,7 +3,7 @@ import createPMRoute, { } from "discourse/routes/build-private-messages-route"; export default createPMRoute( - "all", - "private-messages-all-archive", + "user", + "private-messages-archive", ARCHIVE_FILTER ); 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 aab9f5a98ba..fd0a1f8c10e 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 @@ -2,4 +2,4 @@ import createPMRoute, { INBOX_FILTER, } from "discourse/routes/build-private-messages-route"; -export default createPMRoute("all", "private-messages-all", INBOX_FILTER); +export default createPMRoute("user", "private-messages", INBOX_FILTER); 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 index cb064b20c2f..5e0ab10824b 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-new.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-new.js @@ -3,4 +3,4 @@ import { default as createPMRoute, } from "discourse/routes/build-private-messages-route"; -export default createPMRoute("all", "private-messages-all-new", NEW_FILTER); +export default createPMRoute("user", "private-messages-new", NEW_FILTER); 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 deleted file mode 100644 index a6128847f7c..00000000000 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-archive.js +++ /dev/null @@ -1,9 +0,0 @@ -import createPMRoute, { - ARCHIVE_FILTER, -} from "discourse/routes/build-private-messages-route"; - -export default createPMRoute( - "user", - "private-messages-archive", - ARCHIVE_FILTER -); 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 deleted file mode 100644 index 5e0ab10824b..00000000000 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-new.js +++ /dev/null @@ -1,6 +0,0 @@ -import { - NEW_FILTER, - default as createPMRoute, -} from "discourse/routes/build-private-messages-route"; - -export default createPMRoute("user", "private-messages-new", NEW_FILTER); 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 deleted file mode 100644 index 0c318870ac6..00000000000 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-sent.js +++ /dev/null @@ -1,3 +0,0 @@ -import createPMRoute from "discourse/routes/build-private-messages-route"; - -export default createPMRoute("user", "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 deleted file mode 100644 index 689fff4687c..00000000000 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal-unread.js +++ /dev/null @@ -1,6 +0,0 @@ -import { - UNREAD_FILTER, - default as createPMRoute, -} from "discourse/routes/build-private-messages-route"; - -export default createPMRoute("user", "private-messages-unread", UNREAD_FILTER); 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 deleted file mode 100644 index fd0a1f8c10e..00000000000 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-personal.js +++ /dev/null @@ -1,5 +0,0 @@ -import createPMRoute, { - INBOX_FILTER, -} from "discourse/routes/build-private-messages-route"; - -export default createPMRoute("user", "private-messages", INBOX_FILTER); 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 d7a69c493ca..0c318870ac6 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,3 @@ import createPMRoute from "discourse/routes/build-private-messages-route"; -export default createPMRoute("all", "private-messages-all-sent", "sent"); +export default createPMRoute("user", "private-messages-sent", "sent"); 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 index d8919152dbf..689fff4687c 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-unread.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-unread.js @@ -3,8 +3,4 @@ import { default as createPMRoute, } from "discourse/routes/build-private-messages-route"; -export default createPMRoute( - "all", - "private-messages-all-unread", - UNREAD_FILTER -); +export default createPMRoute("user", "private-messages-unread", UNREAD_FILTER); diff --git a/app/assets/javascripts/discourse/app/templates/user/messages.hbs b/app/assets/javascripts/discourse/app/templates/user/messages.hbs index 248ab54c8ec..0acbfd7e276 100644 --- a/app/assets/javascripts/discourse/app/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/messages.hbs @@ -1,142 +1,92 @@ -{{#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}} - +{{#d-section class="user-secondary-navigation" pageClass="user-messages"}} {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list"}} - {{#if isAllInbox}} -
  • - {{#link-to "userPrivateMessages.index" model}} - {{i18n "user.messages.latest"}} - {{/link-to}} -
  • -
  • +
  • + {{#link-to "userPrivateMessages.index" model}} + {{i18n "user.messages.inbox"}} + {{/link-to}} +
  • + + {{#unless group}} +
  • {{#link-to "userPrivateMessages.sent" model}} {{i18n "user.messages.sent"}} {{/link-to}}
  • {{#if viewingSelf}} -
  • +
  • {{#link-to "userPrivateMessages.new" model class="new"}} {{newLinkText}} {{/link-to}}
  • -
  • + +
  • {{#link-to "userPrivateMessages.unread" model class="unread"}} {{unreadLinkText}} {{/link-to}}
  • {{/if}} -
  • +
  • {{#link-to "userPrivateMessages.archive" model}} {{i18n "user.messages.archive"}} {{/link-to}}
  • - {{/if}} + {{/unless}} - {{#if group}} -
  • - {{#link-to "userPrivateMessages.group" group.name}} - {{i18n "user.messages.latest"}} - {{/link-to}} -
  • - - {{#if viewingSelf}} + {{#each model.groups as |group|}} + {{#if group.has_messages}}
  • - {{#link-to "userPrivateMessages.groupNew" group.name class="new"}} - {{newLinkText}} + {{#link-to "userPrivateMessages.group" group.name}} + {{d-icon "users"}} + {{capitalize-string group.name}} {{/link-to}}
  • -
  • - {{#link-to "userPrivateMessages.groupUnread" group.name class="unread"}} - {{unreadLinkText}} - {{/link-to}} -
  • - {{/if}} -
  • - {{#link-to "userPrivateMessages.groupArchive" group.name}} - {{i18n "user.messages.archive"}} - {{/link-to}} -
  • - {{/if}} - - {{#if isPersonalInbox}} -
  • - {{#link-to "userPrivateMessages.personal" model}} - {{i18n "user.messages.latest"}} - {{/link-to}} -
  • -
  • - {{#link-to "userPrivateMessages.personalSent" model}} - {{i18n "user.messages.sent"}} - {{/link-to}} -
  • - - {{#if viewingSelf}} -
  • - {{#link-to "userPrivateMessages.personalNew" model class="new"}} - {{newLinkText}} - {{/link-to}} -
  • -
  • - {{#link-to "userPrivateMessages.personalUnread" model class="unread"}} - {{unreadLinkText}} - {{/link-to}} -
  • - {{/if}} - -
  • - {{#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}} + {{#if (eq groupFilter group.name)}} + {{#if viewingSelf}}
  • - {{#link-to "userPrivateMessages.tagsShow" tagId}} - {{tagId}} + {{#link-to "userPrivateMessages.groupNew" group.name class="new"}} + {{newLinkText}} + {{/link-to}} +
  • + +
  • + {{#link-to "userPrivateMessages.groupUnread" group.name class="unread"}} + {{unreadLinkText}} {{/link-to}}
  • {{/if}} - - {{/if}} - {{plugin-outlet name="user-messages-nav" connectorTagName="li" args=(hash model=model)}} +
  • + {{#link-to "userPrivateMessages.groupArchive" group.name}} + {{i18n "user.messages.archive"}} + {{/link-to}} +
  • + {{/if}} + {{/if}} + {{/each}} + + {{#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)}} {{/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}} @@ -145,6 +95,7 @@ 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}} 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 99772ca01cb..1de9f52bc10 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 @@ -9,8 +9,6 @@ import { query, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; -import selectKit from "discourse/tests/helpers/select-kit-helper"; -import { PERSONAL_INBOX } from "discourse/controllers/user-private-messages"; import { fixturesByUrl } from "discourse/tests/helpers/create-pretender"; acceptance( @@ -27,13 +25,6 @@ acceptance( 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(".messages-nav .tags"), "displays the tags filter"); - assert.ok( !exists(".group-notifications-button"), "displays the group notifications button" @@ -88,7 +79,7 @@ acceptance( return helper.response(response); }); - server.get("/topics/private-messages-all/:username.json", () => { + server.get("/topics/private-messages/:username.json", () => { return helper.response({ topic_list: { topics: [ @@ -101,9 +92,6 @@ acceptance( }); [ - "/topics/private-messages-all-new/:username.json", - "/topics/private-messages-all-unread/:username.json", - "/topics/private-messages-all-archive/:username.json", "/topics/private-messages-new/:username.json", "/topics/private-messages-unread/:username.json", "/topics/private-messages-archive/:username.json", @@ -297,10 +285,8 @@ acceptance( ); }); - test("incoming group archive message on all and archive filter", async function (assert) { + test("incoming group archive message on inbox and archive filter", async function (assert) { for (const url of [ - "/u/charlie/messages", - "/u/charlie/messages/archive", "/u/charlie/messages/group/awesome_group", "/u/charlie/messages/group/awesome_group/archive", ]) { @@ -317,8 +303,8 @@ acceptance( } for (const url of [ - "/u/charlie/messages/personal", - "/u/charlie/messages/personal/archive", + "/u/charlie/messages", + "/u/charlie/messages/archive", ]) { await visit(url); @@ -410,20 +396,6 @@ acceptance( await visit("/u/charlie/messages/unread"); - assert.equal( - query(".messages-nav li a.unread").innerText.trim(), - I18n.t("user.messages.unread_with_count", { count: 1 }), - "displays the right count" - ); - - assert.equal( - query(".messages-nav li a.new").innerText.trim(), - I18n.t("user.messages.new_with_count", { count: 1 }), - "displays the right count" - ); - - await visit("/u/charlie/messages/personal/unread"); - assert.equal( query(".messages-nav li a.unread").innerText.trim(), I18n.t("user.messages.unread"), @@ -467,7 +439,7 @@ acceptance( }); test("dismissing personal unread messages", async function (assert) { - await visit("/u/charlie/messages/personal/unread"); + await visit("/u/charlie/messages/unread"); assert.equal( count(".topic-list-item"), @@ -504,7 +476,7 @@ acceptance( ); }); - test("dismissing all new messages", async function (assert) { + test("dismissing new messages", async function (assert) { await visit("/u/charlie/messages/new"); publishNewToMessageBus({ topicId: 1, userId: 5 }); @@ -533,7 +505,7 @@ acceptance( }); test("dismissing personal new messages", async function (assert) { - await visit("/u/charlie/messages/personal/new"); + await visit("/u/charlie/messages/new"); assert.equal( count(".topic-list-item"), @@ -577,33 +549,7 @@ acceptance( "displays the right topic list" ); - assert.ok( - exists(".user-messages-inboxes-drop"), - "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 - ); - - assert.equal( - count(".topic-list-item"), - 1, - "displays the right topic list" - ); - - assert.ok( - !exists(".messages-nav .tags"), - "does not display the tags filter" - ); - - await selectKit(".user-messages-inboxes-drop").expand(); - await selectKit(".user-messages-inboxes-drop").selectRowByValue( - "awesome_group" - ); + await visit("/u/charlie/messages/group/awesome_group"); assert.equal( count(".topic-list-item"), @@ -615,11 +561,6 @@ acceptance( exists(".group-notifications-button"), "displays the group notifications button" ); - - assert.ok( - !exists(".messages-nav .tags"), - "does not display the tags filter" - ); }); test("suggested messages without new or unread", async function (assert) { @@ -722,11 +663,11 @@ acceptance("User Private Messages - user with no messages", function (needs) { }; const apiUrls = [ - "/topics/private-messages-all/:username.json", - "/topics/private-messages-all-sent/:username.json", - "/topics/private-messages-all-new/:username.json", - "/topics/private-messages-all-unread/:username.json", - "/topics/private-messages-all-archive/:username.json", + "/topics/private-messages/:username.json", + "/topics/private-messages-sent/:username.json", + "/topics/private-messages-new/:username.json", + "/topics/private-messages-unread/:username.json", + "/topics/private-messages-archive/:username.json", ]; apiUrls.forEach((url) => { diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index 48205251922..4fea447637f 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -60,17 +60,6 @@ &.user-messages { --left-padding: 0.8em; - .user-messages-inboxes-drop { - padding: 0 1em 0 0; - - .select-kit-header { - padding-left: var(--left-padding); - } - - .select-kit-selected-name { - overflow: hidden; - } - } .nav-stacked { a { diff --git a/app/assets/stylesheets/mobile/user.scss b/app/assets/stylesheets/mobile/user.scss index 50fe7b87a11..ae32c03884b 100644 --- a/app/assets/stylesheets/mobile/user.scss +++ b/app/assets/stylesheets/mobile/user.scss @@ -41,15 +41,6 @@ grid-template-columns: 1fr 1fr; gap: 16px; - &.user-messages-inboxes { - // when there are multiple inboxes - .messages-nav { - grid-column-start: 2; - grid-column-end: 3; - grid-row-start: 1; - } - } - + .user-additional-controls { grid-row-start: 2; grid-column-start: 1; @@ -60,16 +51,6 @@ display: flex; } - .user-messages-inboxes-drop { - padding: 0; - flex: 1 1 auto; - .select-kit-header { - .caret-icon { - color: var(--primary-medium); - } - } - } - .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 7c7e3d24926..75d3ff1318d 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -158,9 +158,7 @@ class ListController < ApplicationController when :private_messages_unread, :private_messages_new, :private_messages_group_new, - :private_messages_group_unread, - :private_messages_all_new, - :private_messages_all_unread + :private_messages_group_unread raise Discourse::NotFound if target_user.id != current_user.id when :private_messages_tag @@ -194,11 +192,6 @@ class ListController < ApplicationController private_messages_group_unread private_messages_group_archive 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) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 76774756710..df6177059fa 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1218,7 +1218,7 @@ en: tags: "Tags" warnings: "Official Warnings" read_more_in_group: "Want to read more? Browse other messages in %{groupLink}." - read_more: "Want to read more? Browse other messages in personal messages." + read_more: "Want to read more? Browse other messages in personal messages." read_more_group_pm_MF: "There { UNREAD, plural, @@ -1239,16 +1239,16 @@ en: UNREAD, plural, =0 {} one { - is # unread + is # unread } other { - are # unread + are # unread } } { NEW, plural, =0 {} - one { {BOTH, select, true{and } false {is } other{}} # new message} - other { {BOTH, select, true{and } false {are } other{}} # new messages} - } remaining, or browse other personal messages" + one { {BOTH, select, true{and } false {is } other{}} # new message} + other { {BOTH, select, true{and } false {are } other{}} # new messages} + } remaining, or browse other personal messages" preferences_nav: account: "Account" diff --git a/config/routes.rb b/config/routes.rb index 5a4b246d1a4..adc5edcea69 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -449,8 +449,6 @@ Discourse::Application.routes.draw do 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/:filter" => "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 @@ -777,11 +775,6 @@ Discourse::Application.routes.draw do 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 } diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index c056d5946bf..949f67d125c 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -2,40 +2,6 @@ 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) diff --git a/spec/lib/topic_query/private_message_lists_spec.rb b/spec/lib/topic_query/private_message_lists_spec.rb index a5108ebf171..a2bf68eb5e9 100644 --- a/spec/lib/topic_query/private_message_lists_spec.rb +++ b/spec/lib/topic_query/private_message_lists_spec.rb @@ -44,116 +44,6 @@ describe TopicQuery::PrivateMessageLists do ).topic 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!(group.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!(group.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!(group.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 - before do - TopicUser.find_by(user: user_2, topic: group_message).update!( - last_read_post_number: 1 - ) - - create_post(user: user, topic: group_message) - end - - it 'returns a list of unread private messages' do - topics = TopicQuery.new(nil).list_private_messages_all_unread(user_2).topics - - expect(topics).to contain_exactly(group_message) - end - - it 'takes into account first_unread_pm_at optimization' do - user_2.user_stat.update!(first_unread_pm_at: group_message.created_at + 1.day) - - GroupUser.find_by(user: user_2, group: group).update!( - first_unread_pm_at: group_message.created_at - 1.day - ) - - 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