DEV: Pass messageId as a dynamic segment instead of a query param (#20013)

* DEV: Rnemae channel path to just c

Also swap the channel id and channel slug params to be consistent with core.

* linting

* channel_path

* Drop slugify helper and channel route without slug

* Request slug and route models through the channel model if possible

* DEV: Pass messageId as a dynamic segment instead of a query param

* Ensure change is backwards-compatible

* drop query param from oneboxes

* Correctly extract channelId from routes

* Better route organization using siblings for regular and near-message

* Ensures sessions are unique even when using parallelism

* prevents didReceiveAttrs to clear input mid test

* we disable animations in capybara so sometimes the message was barely showing

* adds wait

* ensures finished loading

* is it causing more harm than good?

* this check is slowing things for no reason

* actually target the button

* more resilient select chat message

* apply similar fix to bookmark

* fix

---------

Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>

plugins/chat/app/jobs/regular/chat_notify_mentioned.rb

@@ -65,7 +65,7 @@ module Jobs
         username: @creator.username,
         tag: Chat::ChatNotifier.push_notification_tag(:mention, @chat_channel.id),
         excerpt: @chat_message.push_notification_excerpt,
-        post_url: "#{@chat_channel.relative_url}?messageId=#{@chat_message.id}",
+        post_url: "#{@chat_channel.relative_url}/#{@chat_message.id}",
       }
 
       translation_prefix =

plugins/chat/assets/javascripts/discourse/chat-route-map.js

@@ -5,17 +5,16 @@ export default function () {
       path: "/channel/:channelId/:channelTitle",
     });
 
-    this.route(
+    this.route("channel", { path: "/c/:channelTitle/:channelId" }, function () {
-      "channel",
+      this.route("from-params", { path: "/" });
-      { path: "/c/:channelTitle/:channelId/" },
+      this.route("near-message", { path: "/:messageId" });
-      function () {
+
-        this.route("info", { path: "/info" }, function () {
+      this.route("info", { path: "/info" }, function () {
-          this.route("about", { path: "/about" });
+        this.route("about", { path: "/about" });
-          this.route("members", { path: "/members" });
+        this.route("members", { path: "/members" });
-          this.route("settings", { path: "/settings" });
+        this.route("settings", { path: "/settings" });
-        });
+      });
-      }
+    });
-    );
 
     this.route("draft-channel", { path: "/draft-channel" });
     this.route("browse", { path: "/browse" }, function () {

plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs

@@ -10,7 +10,7 @@
   >
     <div class="chat-channel-card__header">
       <LinkTo
-        @route="chat.channel"
+        @route="chat.channel.from-params"
         @models={{@channel.routeModels}}
         class="chat-channel-card__name-container"
       >

plugins/chat/assets/javascripts/discourse/components/chat-channel-row.hbs

@@ -1,5 +1,5 @@
 <LinkTo
-  @route="chat.channel"
+  @route="chat.channel.from-params"
   @models={{@channel.routeModels}}
   class={{concat-class
     "chat-channel-row"

plugins/chat/assets/javascripts/discourse/components/chat-composer.js

@@ -223,6 +223,7 @@ export default Component.extend(TextareaTextManipulation, {
     this._super(...arguments);
 
     if (
+      !this.value &&
       !this.editingMessage &&
       this.draft &&
       this.chatChannel?.canModifyMessages(this.currentUser)

plugins/chat/assets/javascripts/discourse/components/chat-drawer.js

@@ -211,17 +211,6 @@ export default Component.extend({
       URL || this.chatStateManager.lastKnownChatURL
     );
 
-    let highlightCb = null;
-
-    if (route.queryParams.messageId) {
-      highlightCb = () => {
-        this.appEvents.trigger(
-          "chat-live-pane:highlight-message",
-          route.queryParams.messageId
-        );
-      };
-    }
-
     switch (route.name) {
       case "chat":
         this.set("view", LIST_VIEW);
@@ -231,25 +220,42 @@ export default Component.extend({
         this.set("view", DRAFT_CHANNEL_VIEW);
         this.appEvents.trigger("chat:float-toggled", false);
         return;
-      case "chat.channel":
+      case "chat.channel.from-params":
-        return this._openChannel(route, highlightCb);
+        return this._openChannel(
+          route.parent.params.channelId,
+          this._highlightCb(route.queryParams.messageId)
+        );
+      case "chat.channel.near-message":
+        return this._openChannel(
+          route.parent.params.channelId,
+          this._highlightCb(route.params.messageId)
+        );
       case "chat.channel-legacy":
-        return this._openChannel(route, highlightCb);
+        return this._openChannel(
+          route.params.channelId,
+          this._highlightCb(route.queryParams.messageId)
+        );
     }
   },
 
-  _openChannel(route, afterRenderFunc = null) {
+  _highlightCb(messageId) {
-    return this.chatChannelsManager
+    if (messageId) {
-      .find(route.params.channelId)
+      return () => {
-      .then((channel) => {
+        this.appEvents.trigger("chat-live-pane:highlight-message", messageId);
-        this.chat.setActiveChannel(channel);
+      };
-        this.set("view", CHAT_VIEW);
+    }
-        this.appEvents.trigger("chat:float-toggled", false);
+  },
 
-        if (afterRenderFunc) {
+  _openChannel(channelId, afterRenderFunc = null) {
-          schedule("afterRender", afterRenderFunc);
+    return this.chatChannelsManager.find(channelId).then((channel) => {
-        }
+      this.chat.setActiveChannel(channel);
-      });
+      this.set("view", CHAT_VIEW);
+      this.appEvents.trigger("chat:float-toggled", false);
+
+      if (afterRenderFunc) {
+        schedule("afterRender", afterRenderFunc);
+      }
+    });
   },
 
   @action

plugins/chat/assets/javascripts/discourse/components/chat-message.js

@@ -786,7 +786,7 @@ export default Component.extend({
 
     const { protocol, host } = window.location;
     let url = getURL(
-      `/chat/c/-/${this.details.chat_channel_id}?messageId=${this.message.id}`
+      `/chat/c/-/${this.details.chat_channel_id}/${this.message.id}`
     );
     url = url.indexOf("/") === 0 ? protocol + "//" + host + url : url;
     clipboardCopy(url);

plugins/chat/assets/javascripts/discourse/components/chat-selection-manager.hbs

@@ -1,4 +1,9 @@
-<div class="chat-selection-management">
+<div
+  class={{concat-class
+    "chat-selection-management"
+    (if this.chatCopySuccess "chat-copy-success")
+  }}
+>
   <div class="chat-selection-management-buttons">
     <DButton
       @id="chat-quote-btn"
@@ -44,9 +49,9 @@
     />
   </div>
 
-  {{#if this.showChatQuoteSuccess}}
+  {{#if this.showChatCopySuccess}}
     <div class="chat-selection-message">
       {{i18n "chat.quote.copy_success"}}
     </div>
   {{/if}}
-</div>
+</div>

plugins/chat/assets/javascripts/discourse/components/chat-selection-manager.js

@@ -16,7 +16,8 @@ export default class AdminCustomizeColorsShowController extends Component {
   tagName = "";
   chatChannel = null;
   selectedMessageIds = null;
-  showChatQuoteSuccess = false;
+  chatCopySuccess = false;
+  showChatCopySuccess = false;
   cancelSelecting = null;
   canModerate = false;
 
@@ -100,12 +101,15 @@ export default class AdminCustomizeColorsShowController extends Component {
   @action
   async copyMessages() {
     try {
+      this.set("chatCopySuccess", false);
+
       if (!isTesting()) {
         // clipboard API throws errors in tests
         await clipboardCopyAsync(this.generateQuote);
+        this.set("chatCopySuccess", true);
       }
 
-      this.set("showChatQuoteSuccess", true);
+      this.set("showChatCopySuccess", true);
 
       schedule("afterRender", () => {
         const element = document.querySelector(".chat-selection-message");
@@ -114,7 +118,7 @@ export default class AdminCustomizeColorsShowController extends Component {
             return;
           }
 
-          this.set("showChatQuoteSuccess", false);
+          this.set("showChatCopySuccess", false);
         });
       });
     } catch (error) {

plugins/chat/assets/javascripts/discourse/components/reviewable-chat-message.hbs

@@ -1,8 +1,11 @@
 <div class="flagged-post-header">
   <LinkTo
-    @route="chat.channel"
+    @route="chat.channel.near-message"
-    @models={{this.chatChannel.routeModels}}
+    @models={{array
-    @query={{hash messageId=@reviewable.target_id}}
+      this.chatChannel.slugifiedTitle
+      this.chatChannel.id
+      @reviewable.target_id
+    }}
   >
     <ChatChannelTitle @channel={{this.chatChannel}} />
   </LinkTo>

plugins/chat/assets/javascripts/discourse/controllers/chat-channel.js

@@ -5,6 +5,7 @@ import { inject as service } from "@ember/service";
 export default class ChatChannelController extends Controller {
   @service chat;
 
+  // Backwards-compatibility
   queryParams = ["messageId"];
 
   @action

plugins/chat/assets/javascripts/discourse/helpers/format-chat-date.js

@@ -14,9 +14,7 @@ registerUnbound("format-chat-date", function (message, details, mode) {
   let url = "";
 
   if (details) {
-    url = getURL(
+    url = getURL(`/chat/c/-/${details.chat_channel_id}/${message.id}`);
-      `/chat/c/-/${details.chat_channel_id}?messageId=${message.id}`
-    );
   }
 
   let title = date.format(I18n.t("dates.long_with_year"));

plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js

@@ -52,7 +52,7 @@ export default {
             }
 
             get route() {
-              return "chat.channel";
+              return "chat.channel.from-params";
             }
 
             get models() {
@@ -215,7 +215,7 @@ export default {
             }
 
             get route() {
-              return "chat.channel";
+              return "chat.channel.from-params";
             }
 
             get models() {

plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js

@@ -26,7 +26,7 @@ export default {
                 });
                 return `/chat/c/${slug || "-"}/${
                   this.notification.data.chat_channel_id
-                }?messageId=${this.notification.data.chat_message_id}`;
+                }/${this.notification.data.chat_message_id}`;
               }
 
               get linkTitle() {
@@ -61,7 +61,7 @@ export default {
                 });
                 return `/chat/c/${slug || "-"}/${
                   this.notification.data.chat_channel_id
-                }?messageId=${this.notification.data.chat_message_id}`;
+                }/${this.notification.data.chat_message_id}`;
               }
 
               get linkTitle() {

plugins/chat/assets/javascripts/discourse/routes/chat-channel-from-params.js

@@ -0,0 +1,18 @@
+import DiscourseRoute from "discourse/routes/discourse";
+import { inject as service } from "@ember/service";
+
+export default class ChatChannelFromParamsRoute extends DiscourseRoute {
+  @service router;
+
+  async model() {
+    return this.modelFor("chat-channel");
+  }
+
+  afterModel(model) {
+    const { channelTitle } = this.paramsFor("chat.channel");
+
+    if (channelTitle !== model.slugifiedTitle) {
+      this.router.replaceWith("chat.channel.from-params", ...model.routeModels);
+    }
+  }
+}

plugins/chat/assets/javascripts/discourse/routes/chat-channel-legacy.js

@@ -9,8 +9,13 @@ export default class ChatChannelLegacyRoute extends DiscourseRoute {
       this.routeName
     );
 
-    this.router.replaceWith("chat.channel", channelTitle, channelId, {
+    this.router.replaceWith(
-      queryParams: { messageId },
+      "chat.channel.from-params",
-    });
+      channelTitle,
+      channelId,
+      {
+        queryParams: { messageId },
+      }
+    );
   }
 }

plugins/chat/assets/javascripts/discourse/routes/chat-channel-near-message.js

@@ -0,0 +1,41 @@
+import DiscourseRoute from "discourse/routes/discourse";
+import { inject as service } from "@ember/service";
+import { action } from "@ember/object";
+import { schedule } from "@ember/runloop";
+
+export default class ChatChannelNearMessage extends DiscourseRoute {
+  @service chat;
+  @service router;
+
+  async model() {
+    return this.modelFor("chat-channel");
+  }
+
+  afterModel(model) {
+    const { messageId } = this.paramsFor(this.routeName);
+    const { channelTitle } = this.paramsFor("chat.channel");
+
+    if (channelTitle !== model.slugifiedTitle) {
+      this.router.replaceWith(
+        "chat.channel.near-message",
+        ...model.routeModels,
+        messageId
+      );
+    }
+  }
+
+  @action
+  didTransition() {
+    this.controllerFor("chat-channel").set("messageId", null);
+
+    const { messageId } = this.paramsFor(this.routeName);
+    const { channelId } = this.paramsFor("chat.channel");
+
+    if (channelId && messageId) {
+      schedule("afterRender", () => {
+        this.chat.openChannelAtMessage(channelId, messageId);
+      });
+    }
+    return true;
+  }
+}

plugins/chat/assets/javascripts/discourse/routes/chat-channel.js

@@ -1,12 +1,10 @@
 import DiscourseRoute from "discourse/routes/discourse";
 import { inject as service } from "@ember/service";
-import { action } from "@ember/object";
-import { schedule } from "@ember/runloop";
 
 export default class ChatChannelRoute extends DiscourseRoute {
+  @service chatChannelsManager;
   @service chat;
   @service router;
-  @service chatChannelsManager;
 
   async model(params) {
     return this.chatChannelsManager.find(params.channelId);
@@ -15,24 +13,15 @@ export default class ChatChannelRoute extends DiscourseRoute {
   afterModel(model) {
     this.chat.setActiveChannel(model);
 
-    const { channelTitle, messageId } = this.paramsFor(this.routeName);
+    const { messageId } = this.paramsFor(this.routeName);
 
-    if (channelTitle !== model.slugifiedTitle) {
+    // messageId query param backwards-compatibility
-      this.router.replaceWith("chat.channel.index", ...model.routeModels, {
+    if (messageId) {
-        queryParams: { messageId },
+      this.router.replaceWith(
-      });
+        "chat.channel.near-message",
+        ...model.routeModels,
+        messageId
+      );
     }
   }
-
-  @action
-  didTransition() {
-    const { channelId, messageId } = this.paramsFor(this.routeName);
-    if (channelId && messageId) {
-      schedule("afterRender", () => {
-        this.chat.openChannelAtMessage(channelId, messageId);
-        this.controller.set("messageId", null); // clear the query param
-      });
-    }
-    return true;
-  }
 }

plugins/chat/assets/javascripts/discourse/routes/chat-message.js

@@ -10,12 +10,10 @@ export default class ChatMessageRoute extends DiscourseRoute {
     return ajax(`/chat/message/${params.messageId}.json`)
       .then((response) => {
         this.transitionTo(
-          "chat.channel",
+          "chat.channel.near-message",
           response.chat_channel_title,
           response.chat_channel_id,
-          {
+          params.messageId
-            queryParams: { messageId: params.messageId },
-          }
         );
       })
       .catch(() => this.replaceWith("/404"));

plugins/chat/assets/javascripts/discourse/routes/chat.js

@@ -21,8 +21,9 @@ export default class ChatRoute extends DiscourseRoute {
     }
 
     const INTERCEPTABLE_ROUTES = [
-      "chat.channel.index",
       "chat.channel",
+      "chat.channel.from-params",
+      "chat.channel.near-message",
       "chat.channel-legacy",
       "chat",
       "chat.index",
@@ -37,10 +38,7 @@ export default class ChatRoute extends DiscourseRoute {
       transition.abort();
 
       let URL = transition.intent.url;
-      if (
+      if (transition.targetName.startsWith("chat.channel")) {
-        transition.targetName.startsWith("chat.channel") ||
-        transition.targetName.startsWith("chat.channel-legacy")
-      ) {
         URL ??= this.router.urlFor(
           transition.targetName,
           ...transition.intent.contexts

plugins/chat/assets/javascripts/discourse/services/chat.js

@@ -277,7 +277,8 @@ export default class Chat extends Service {
 
   async _openFoundChannelAtMessage(channel, messageId = null) {
     if (
-      this.router.currentRouteName === "chat.channel.index" &&
+      (this.router.currentRouteName === "chat.channel.from-params" ||
+        this.router.currentRouteName === "chat.channel.near-message") &&
       this.activeChannel?.id === channel.id
     ) {
       this.setActiveChannel(channel);
@@ -292,14 +293,18 @@ export default class Chat extends Service {
       this.site.mobileView ||
       this.chatStateManager.isFullPagePreferred
     ) {
-      const queryParams = messageId ? { messageId } : {};
+      if (messageId) {
-
+        return this.router.transitionTo(
-      return this.router.transitionTo(
+          "chat.channel.near-message",
-        "chat.channel",
+          ...channel.routeModels,
-        channel.slugifiedTitle,
+          messageId
-        channel.id,
+        );
-        { queryParams }
+      } else {
-      );
+        return this.router.transitionTo(
+          "chat.channel.from-params",
+          ...channel.routeModels
+        );
+      }
     } else {
       this._fireOpenFloatAppEvent(channel, messageId);
       return Promise.resolve();

plugins/chat/assets/javascripts/discourse/templates/chat-channel-info.hbs

@@ -12,7 +12,7 @@
           </LinkTo>
         {{else}}
           <LinkTo
-            @route="chat.channel"
+            @route="chat.channel.from-params"
             @models={{this.model.routeModels}}
             class="chat-full-page-header__back-btn no-text btn-flat btn"
             title={{i18n "chat.channel_info.back_to_channel"}}

plugins/chat/assets/javascripts/discourse/templates/chat-channel-near-message.hbs

@@ -0,0 +1 @@
+<FullPageChat />

plugins/chat/assets/javascripts/discourse/widgets/chat-invitation-notification-item.js

@@ -38,7 +38,7 @@ createWidgetFrom(DefaultNotificationItem, "chat-invitation-notification-item", {
       title: data.chat_channel_title,
       slug: data.chat_channel_slug,
     });
-    return `/chat/c/${slug || "-"}/${data.chat_channel_id}?messageId=${
+    return `/chat/c/${slug || "-"}/${data.chat_channel_id}/${
       data.chat_message_id
     }`;
   },

plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js

@@ -48,7 +48,7 @@ const chatNotificationItem = {
       title: data.chat_channel_title,
       slug: data.chat_channel_slug,
     });
-    return `/chat/c/${slug || "-"}/${data.chat_channel_id}?messageId=${
+    return `/chat/c/${slug || "-"}/${data.chat_channel_id}/${
       data.chat_message_id
     }`;
   },

plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js

@@ -122,7 +122,7 @@ const chatTranscriptRule = {
     } else {
       let linkToken = state.push("link_open", "a", 1);
       linkToken.attrs = [
-        ["href", `${channelLink}?messageId=${messageIdStart}`],
+        ["href", `${channelLink}/${messageIdStart}`],
         ["title", messageTimeStart],
       ];
 

plugins/chat/plugin.rb

@@ -265,16 +265,8 @@ after_initialize do
 
   if Oneboxer.respond_to?(:register_local_handler)
     Oneboxer.register_local_handler("chat/chat") do |url, route|
-      queryParams =
+      if route[:message_id].present?
-        begin
+        message = ChatMessage.find_by(id: route[:message_id])
-          CGI.parse(URI.parse(url).query)
-        rescue StandardError
-          {}
-        end
-      messageId = queryParams["messageId"]&.first
-
-      if messageId.present?
-        message = ChatMessage.find_by(id: messageId)
         next if !message
 
         chat_channel = message.chat_channel
@@ -334,16 +326,8 @@ after_initialize do
 
   if InlineOneboxer.respond_to?(:register_local_handler)
     InlineOneboxer.register_local_handler("chat/chat") do |url, route|
-      queryParams =
+      if route[:message_id].present?
-        begin
+        message = ChatMessage.find_by(id: route[:message_id])
-          CGI.parse(URI.parse(url).query)
-        rescue StandardError
-          {}
-        end
-      messageId = queryParams["messageId"]&.first
-
-      if messageId.present?
-        message = ChatMessage.find_by(id: messageId)
         next if !message
 
         chat_channel = message.chat_channel
@@ -655,6 +639,7 @@ after_initialize do
 
     base_c_route = "/c/:channel_title/:channel_id"
     get base_c_route => "chat#respond", :as => "channel"
+    get "#{base_c_route}/:message_id" => "chat#respond"
 
     %w[info info/about info/members info/settings].each do |route|
       get "#{base_c_route}/#{route}" => "chat#respond"

plugins/chat/spec/integration/post_chat_quote_spec.rb

@@ -41,7 +41,7 @@ describe "chat bbcode quoting in posts" do
       <div class="chat-transcript-username">
       martin</div>
       <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234?messageId=2321" title="2022-01-25T05:40:39Z"></a>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
       </div>
       </div>
       <div class="chat-transcript-messages">
@@ -63,7 +63,7 @@ describe "chat bbcode quoting in posts" do
       <div class="chat-transcript-username">
       martin</div>
       <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234?messageId=2321" title="2022-01-25T05:40:39Z"></a>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
       </div>
       <a class="chat-transcript-channel" href="/chat/c/-/1234">
       #Cool Cats Club</a>
@@ -87,7 +87,7 @@ describe "chat bbcode quoting in posts" do
       <div class="chat-transcript-username">
       martin</div>
       <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234?messageId=2321" title="2022-01-25T05:40:39Z"></a>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
       </div>
       <a class="chat-transcript-channel" href="/chat/c/-/1234">
       #Cool Cats Club</a>
@@ -137,7 +137,7 @@ describe "chat bbcode quoting in posts" do
       <div class="chat-transcript-username">
       martin</div>
       <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234?messageId=2321" title="2022-01-25T05:40:39Z"></a>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
       </div>
       <a class="chat-transcript-channel" href="/chat/c/-/1234">
       #Cool Cats Club</a>
@@ -242,7 +242,7 @@ martin</div>
 <div class="chat-transcript-username">
 #{message1.user.username}</div>
 <div class="chat-transcript-datetime">
-<a href="/chat/c/-/#{channel.id}?messageId=#{message1.id}" title="#{message1.created_at.iso8601}"></a>
+<a href="/chat/c/-/#{channel.id}/#{message1.id}" title="#{message1.created_at.iso8601}"></a>
 </div>
 <a class="chat-transcript-channel" href="/chat/c/-/#{channel.id}">
 ##{channel.name}</a>
@@ -256,7 +256,7 @@ martin</div>
 <div class="chat-transcript-username">
 #{message2.user.username}</div>
 <div class="chat-transcript-datetime">
-<a href="/chat/c/-/#{channel.id}?messageId=#{message2.id}" title="#{message1.created_at.iso8601}"></a>
+<a href="/chat/c/-/#{channel.id}/#{message2.id}" title="#{message1.created_at.iso8601}"></a>
 </div>
 <a class="chat-transcript-channel" href="/chat/c/-/#{channel.id}">
 ##{channel.name}</a>

plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb

@@ -215,7 +215,7 @@ describe Jobs::ChatNotifyMentioned do
       )
       expect(desktop_notification.data[:excerpt]).to eq(message.push_notification_excerpt)
       expect(desktop_notification.data[:post_url]).to eq(
-        "/chat/c/#{public_channel.slug}/#{public_channel.id}?messageId=#{message.id}",
+        "/chat/c/#{public_channel.slug}/#{public_channel.id}/#{message.id}",
       )
     end
 
@@ -229,7 +229,7 @@ describe Jobs::ChatNotifyMentioned do
           username: user_1.username,
           tag: Chat::ChatNotifier.push_notification_tag(:mention, public_channel.id),
           excerpt: message.push_notification_excerpt,
-          post_url: "/chat/c/#{public_channel.slug}/#{public_channel.id}?messageId=#{message.id}",
+          post_url: "/chat/c/#{public_channel.slug}/#{public_channel.id}/#{message.id}",
           translated_title: payload_translated_title,
         },
       )

plugins/chat/spec/models/chat_message_spec.rb

@@ -138,7 +138,7 @@ describe ChatMessage do
         <div class="chat-transcript-username">
         chatbbcodeuser</div>
         <div class="chat-transcript-datetime">
-        <a href="/chat/c/-/#{chat_channel.id}?messageId=#{msg1.id}" title="#{msg1.created_at.iso8601}"></a>
+        <a href="/chat/c/-/#{chat_channel.id}/#{msg1.id}" title="#{msg1.created_at.iso8601}"></a>
         </div>
         </div>
         <div class="chat-transcript-messages">

plugins/chat/spec/plugin_spec.rb

@@ -155,10 +155,9 @@ describe Chat do
       end
 
       it "renders messages" do
-        results =
+        results = InlineOneboxer.new(["#{chat_url}/#{chat_message.id}"], skip_cache: true).process
-          InlineOneboxer.new(["#{chat_url}?messageId=#{chat_message.id}"], skip_cache: true).process
         expect(results).to be_present
-        expect(results[0][:url]).to eq("#{chat_url}?messageId=#{chat_message.id}")
+        expect(results[0][:url]).to eq("#{chat_url}/#{chat_message.id}")
         expect(results[0][:title]).to eq(
           "Message ##{chat_message.id} by #{chat_message.user.username} – ##{chat_channel.name}",
         )
@@ -197,7 +196,7 @@ describe Chat do
       end
 
       it "renders messages" do
-        expect(Oneboxer.preview("#{chat_url}?messageId=#{chat_message.id}")).to match_html <<~HTML
+        expect(Oneboxer.preview("#{chat_url}/#{chat_message.id}")).to match_html <<~HTML
           <div class="chat-transcript" data-message-id="#{chat_message.id}" data-username="#{user.username}" data-datetime="#{chat_message.created_at.iso8601}" data-channel-name="#{chat_channel.name}" data-channel-id="#{chat_channel.id}">
           <div class="chat-transcript-user">
             <div class="chat-transcript-user-avatar">
@@ -207,7 +206,7 @@ describe Chat do
             </div>
             <div class="chat-transcript-username">#{user.username}</div>
               <div class="chat-transcript-datetime">
-                <a href="#{chat_url}?messageId=#{chat_message.id}" title="#{chat_message.created_at}">#{chat_message.created_at}</a>
+                <a href="#{chat_url}/#{chat_message.id}" title="#{chat_message.created_at}">#{chat_message.created_at}</a>
               </div>
               <a class="chat-transcript-channel" href="/chat/c/-/#{chat_channel.id}">
                 <span class="category-chat-badge" style="color: ##{chat_channel.chatable.color}">

plugins/chat/spec/system/bookmark_message_spec.rb

@@ -52,8 +52,14 @@ RSpec.describe "Bookmark message", type: :system, js: true do
   context "when mobile", mobile: true do
     it "allows to bookmark a message" do
       chat.visit_channel(category_channel_1)
+      expect(channel).to have_no_loading_skeleton
 
-      channel.message_by_id(message_1.id).click(delay: 0.5)
+      i = 0.5
+      try_until_success(timeout: 20) do
+        channel.message_by_id(message_1.id).click(delay: i)
+        first(".bookmark-btn")
+        i += 0.1
+      end
       find(".bookmark-btn").click
 
       bookmark_modal.fill_name("Check this out later")

plugins/chat/spec/system/navigating_to_message_spec.rb

@@ -26,7 +26,7 @@ RSpec.describe "Navigating to message", type: :system, js: true do
         Fabricate(
           :post,
           topic: topic_1,
-          raw: "<a href=\"/chat/c/-/#{channel_1.id}?messageId=#{first_message.id}\">#{link}</a>",
+          raw: "<a href=\"/chat/c/-/#{channel_1.id}/#{first_message.id}\">#{link}</a>",
         )
       end
 
@@ -45,7 +45,7 @@ RSpec.describe "Navigating to message", type: :system, js: true do
         Fabricate(
           :chat_message,
           chat_channel: channel_1,
-          message: "[#{link}](/chat/c/-/#{channel_1.id}?messageId=#{first_message.id})",
+          message: "[#{link}](/chat/c/-/#{channel_1.id}/#{first_message.id})",
         )
       end
 
@@ -77,7 +77,7 @@ RSpec.describe "Navigating to message", type: :system, js: true do
         Fabricate(
           :chat_message,
           chat_channel: channel_2,
-          message: "[#{link}](/chat/c/-/#{channel_1.id}?messageId=#{first_message.id})",
+          message: "[#{link}](/chat/c/-/#{channel_1.id}/#{first_message.id})",
         )
         channel_2.add(current_user)
       end
@@ -94,7 +94,7 @@ RSpec.describe "Navigating to message", type: :system, js: true do
 
     context "when navigating directly to a message link" do
       it "highglights the correct message" do
-        visit("/chat/c/-/#{channel_1.id}?messageId=#{first_message.id}")
+        visit("/chat/c/-/#{channel_1.id}/#{first_message.id}")
 
         expect(page).to have_css(
           ".chat-message-container.highlighted[data-id='#{first_message.id}']",
@@ -111,7 +111,7 @@ RSpec.describe "Navigating to message", type: :system, js: true do
         Fabricate(
           :post,
           topic: topic_1,
-          raw: "<a href=\"/chat/c/-/#{channel_1.id}?messageId=#{first_message.id}\">#{link}</a>",
+          raw: "<a href=\"/chat/c/-/#{channel_1.id}/#{first_message.id}\">#{link}</a>",
         )
       end
 
@@ -130,7 +130,7 @@ RSpec.describe "Navigating to message", type: :system, js: true do
         Fabricate(
           :chat_message,
           chat_channel: channel_1,
-          message: "[#{link}](/chat/c/-/#{channel_1.id}?messageId=#{first_message.id})",
+          message: "[#{link}](/chat/c/-/#{channel_1.id}/#{first_message.id})",
         )
       end
 

plugins/chat/spec/system/transcript_spec.rb

@@ -26,12 +26,12 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do
   end
 
   def select_message_mobile(message)
-    if page.has_css?(".chat-message-container.selecting-messages")
+    i = 0.5
-      chat_channel_page.message_by_id(message.id).find(".chat-message-selector").click
+    try_until_success(timeout: 20) do
-    else
+      chat_channel_page.message_by_id(message.id).click(delay: i)
-      chat_channel_page.message_by_id(message.id).click(delay: 0.5)
+      first(".chat-message-action-item[data-id=\"selectMessage\"]")
-      find(".chat-message-action-item[data-id=\"selectMessage\"]").click
     end
+    find(".chat-message-action-item[data-id=\"selectMessage\"] button").click
   end
 
   def cdp_allow_clipboard_access!
@@ -62,15 +62,15 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do
     selector =
       case button
       when "quote"
-        "#chat-quote-btn"
+        "chat-quote-btn"
       when "copy"
-        "#chat-copy-btn"
+        "chat-copy-btn"
       when "cancel"
-        "#chat-cancel-selection-btn"
+        "chat-cancel-selection-btn"
       when "move"
-        "#chat-move-to-channel-btn"
+        "chat-move-to-channel-btn"
       end
-    within(".chat-selection-management-buttons") { find(selector).click }
+    find_button(selector, disabled: false, wait: 5).click
   end
 
   def copy_messages_to_clipboard(messages)
@@ -78,7 +78,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do
     messages.each { |message| select_message_desktop(message) }
     expect(chat_channel_page).to have_selection_management
     click_selection_button("copy")
-    expect(page).to have_content("Chat quote copied to clipboard")
+    expect(page).to have_selector(".chat-copy-success")
     clip_text = read_clipboard
     expect(clip_text.chomp).to eq(generate_transcript(messages, current_user))
     clip_text
@@ -227,6 +227,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do
       it "first navigates to the channel's category before opening the topic composer with the quote prefilled",
          mobile: true do
         chat_page.visit_channel(chat_channel_1)
+
         expect(chat_channel_page).to have_no_loading_skeleton
 
         select_message_mobile(message_1)

plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb

@@ -59,7 +59,7 @@ RSpec.describe "User menu notifications | sidebar", type: :system, js: true do
           end
           expect(find("#quick-access-chat-notifications")).to have_link(
             I18n.t("js.notifications.popup.direct_message_chat_mention.direct"),
-            href: "/chat/c/#{other_user.username}/#{dm_channel_1.id}?messageId=#{message.id}",
+            href: "/chat/c/#{other_user.username}/#{dm_channel_1.id}/#{message.id}",
           )
         end
       end
@@ -100,7 +100,7 @@ RSpec.describe "User menu notifications | sidebar", type: :system, js: true do
               identifier: "@#{group.name}",
               channel: channel_1.name,
             ),
-            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}?messageId=#{message.id}",
+            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}/#{message.id}",
           )
         end
       end
@@ -126,7 +126,7 @@ RSpec.describe "User menu notifications | sidebar", type: :system, js: true do
 
           expect(find("#quick-access-chat-notifications")).to have_link(
             I18n.t("js.notifications.popup.chat_mention.direct", channel: channel_1.name),
-            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}?messageId=#{message.id}",
+            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}/#{message.id}",
           )
         end
       end
@@ -153,7 +153,7 @@ RSpec.describe "User menu notifications | sidebar", type: :system, js: true do
               identifier: "@all",
               channel: channel_1.name,
             ),
-            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}?messageId=#{message.id}",
+            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}/#{message.id}",
           )
         end
       end

plugins/chat/spec/system/visit_channel_spec.rb

@@ -69,7 +69,7 @@ RSpec.describe "Visit channel", type: :system, js: true do
 
       context "when loading a non existing message of a channel" do
         it "shows an error" do
-          visit("/chat/c/-/#{category_channel_1.id}?messageId=-999")
+          visit("/chat/c/-/#{category_channel_1.id}/-999")
 
           expect(page).to have_content(I18n.t("not_found"))
         end

plugins/chat/test/javascripts/unit/helpers/format-chat-date-test.js

@@ -12,9 +12,6 @@ module("Discourse Chat | Unit | Helpers | format-chat-date", function (hooks) {
     this.set("message", { id: 1 });
     await render(hbs`{{format-chat-date this.message this.details}}`);
 
-    assert.equal(
+    assert.equal(query(".chat-time").getAttribute("href"), "/chat/c/-/1/1");
-      query(".chat-time").getAttribute("href"),
-      "/chat/c/-/1?messageId=1"
-    );
   });
 });

plugins/chat/test/javascripts/widgets/chat-invitation-notification-item-test.js

@@ -45,7 +45,7 @@ module(
         query(".chat-invitation a").getAttribute("href"),
         `/chat/c/${slugifyChannel({
           title: data.chat_channel_title,
-        })}/${data.chat_channel_id}?messageId=${data.chat_message_id}`
+        })}/${data.chat_channel_id}/${data.chat_message_id}`
       );
     });
   }

plugins/chat/test/javascripts/widgets/chat-mention-notification-item-test.js

@@ -54,7 +54,7 @@ module(
         query(".chat-invitation a").getAttribute("href"),
         `/chat/c/${slugifyChannel({
           title: data.chat_channel_title,
-        })}/${data.chat_channel_id}?messageId=${data.chat_message_id}`
+        })}/${data.chat_channel_id}/${data.chat_message_id}`
       );
     });
   }
@@ -93,7 +93,7 @@ module(
         query(".chat-invitation a").getAttribute("href"),
         `/chat/c/${slugifyChannel({
           title: data.chat_channel_title,
-        })}/${data.chat_channel_id}?messageId=${data.chat_message_id}`
+        })}/${data.chat_channel_id}/${data.chat_message_id}`
       );
     });
   }
@@ -132,7 +132,7 @@ module(
         query(".chat-invitation a").getAttribute("href"),
         `/chat/c/${slugifyChannel({
           title: data.chat_channel_title,
-        })}/${data.chat_channel_id}?messageId=${data.chat_message_id}`
+        })}/${data.chat_channel_id}/${data.chat_message_id}`
       );
     });
   }

spec/rails_helper.rb

@@ -245,7 +245,7 @@ RSpec.configure do |config|
     end
 
     Capybara.threadsafe = true
-    Capybara.disable_animation = true
+    Capybara.disable_animation = false
 
     Capybara.configure do |capybara_config|
       capybara_config.server_host = "localhost"

spec/support/system_helpers.rb

@@ -32,7 +32,9 @@ module SystemHelpers
     start ||= Time.zone.now
     backoff ||= frequency
     yield
-  rescue RSpec::Expectations::ExpectationNotMetError
+  rescue RSpec::Expectations::ExpectationNotMetError,
+         Capybara::ExpectationNotMet,
+         Capybara::ElementNotFound
     raise if Time.zone.now >= start + timeout.seconds
     sleep backoff
     backoff += frequency
@@ -67,8 +69,15 @@ module SystemHelpers
 
     ENV["TZ"] = timezone
 
-    Capybara.using_session(timezone) { freeze_time(&example) }
+    using_session(timezone) { freeze_time(&example) }
 
     ENV["TZ"] = previous_browser_timezone
   end
+
+  # When using parallelism, Capybara's `using_session` method can cause
+  # intermittent failures as two sessions can be created with the same name
+  # in different tests and be run at the same time.
+  def using_session(name, &block)
+    Capybara.using_session(name.to_s + self.method_name, &block)
+  end
 end