From f5e41f0627fa000f20882aeaaca099a429d752b5 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Thu, 23 May 2024 11:17:43 +0200
Subject: [PATCH] FIX: correctly generate pm path on user (#27152)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this fix when generating a pm path leading to a group messages inbox we would blindly take the first group of the pm, however, it's possible our current user doesn't have access to this group.

This commit will now try to find the first group the user has access to, and generate a path to this group’s inbox.
---
 .../javascripts/discourse/app/models/user.js  | 20 +++++-----
 .../tests/acceptance/personal-message-test.js |  2 +-
 .../discourse/tests/unit/models/user-test.js  | 38 +++++++++++++++++++
 3 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js
index d72a03cf32d..3c80f8474f7 100644
--- a/app/assets/javascripts/discourse/app/models/user.js
+++ b/app/assets/javascripts/discourse/app/models/user.js
@@ -324,19 +324,21 @@ export default class User extends RestModel.extend(Evented) {
   }
 
   pmPath(topic) {
-    const userId = this.id;
     const username = this.username_lower;
-
-    const details = topic && topic.get("details");
-    const allowedUsers = details && details.get("allowed_users");
-    const groups = details && details.get("allowed_groups");
+    const details = topic.details;
+    const allowedUsers = details?.allowed_users;
+    const groups = details?.allowed_groups;
 
     // directly targeted so go to inbox
-    if (!groups || (allowedUsers && allowedUsers.findBy("id", userId))) {
+    if (!groups || allowedUsers?.findBy("id", this.id)) {
       return userPath(`${username}/messages`);
-    } else {
-      if (groups && groups[0]) {
-        return userPath(`${username}/messages/group/${groups[0].name}`);
+    } else if (groups) {
+      const firstAllowedGroup = groups.find((allowedGroup) =>
+        this.groups.some((userGroup) => userGroup.id === allowedGroup.id)
+      );
+
+      if (firstAllowedGroup) {
+        return userPath(`${username}/messages/group/${firstAllowedGroup.name}`);
       }
     }
   }
diff --git a/app/assets/javascripts/discourse/tests/acceptance/personal-message-test.js b/app/assets/javascripts/discourse/tests/acceptance/personal-message-test.js
index 6779c2ac557..99a62db494c 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/personal-message-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/personal-message-test.js
@@ -16,7 +16,7 @@ import I18n from "discourse-i18n";
 import selectKit from "../helpers/select-kit-helper";
 
 acceptance("Personal Message", function (needs) {
-  needs.user();
+  needs.user({ id: 1 });
 
   test("suggested messages", async function (assert) {
     await visit("/t/pm-for-testing/12");
diff --git a/app/assets/javascripts/discourse/tests/unit/models/user-test.js b/app/assets/javascripts/discourse/tests/unit/models/user-test.js
index 93acc37e470..4bfd0f88810 100644
--- a/app/assets/javascripts/discourse/tests/unit/models/user-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/models/user-test.js
@@ -194,4 +194,42 @@ module("Unit | Model | user", function (hooks) {
       "_clearStatusTimerId wasn't set"
     );
   });
+
+  test("pmPath", function (assert) {
+    const store = getOwner(this).lookup("service:store");
+    const user = store.createRecord("user", {
+      id: 1,
+      username: "eviltrout",
+      groups: [],
+    });
+    const topic = store.createRecord("topic", { id: 1, details: {} });
+
+    assert.strictEqual(
+      user.pmPath(topic),
+      `/u/${user.username_lower}/messages`,
+      "user is in no groups and not directly allowed on the topic"
+    );
+
+    const group1 = store.createRecord("group", { id: 1, name: "group1" });
+    const group2 = store.createRecord("group", { id: 2, name: "group2" });
+    topic.details = {
+      allowed_users: [user],
+      allowed_groups: [group1, group2],
+    };
+    user.groups = [group2];
+
+    assert.strictEqual(
+      user.pmPath(topic),
+      `/u/${user.username_lower}/messages`,
+      "user is in one group (not the first one) and allowed on the topic"
+    );
+
+    topic.details.allowed_users = [];
+
+    assert.strictEqual(
+      user.pmPath(topic),
+      `/u/${user.username_lower}/messages/group/${group2.name}`,
+      "user is in one group (not the first one) and not allowed on the topic"
+    );
+  });
 });