From 56aef3c082bac517ba48b0b225c7d1372b9ad688 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 4 Jul 2023 16:23:04 +1000
Subject: [PATCH] DEV: Fix or remove flaky chat specs (#22406)

* DEV: Fix flaky thread nav spec

When we transitioned from the chat thread panel under some conditions
the request for the thread would come back and realise the component
was destroyed, which was trying to do a transition to the channel
itself.

Now we check for the previous route here too and transition to the
correct route.

* DEV: Fix chat transcript spec relying on animation

The on-animation-end modifier is not reliable in system specs
because it fires instantly (we have disabled capybara animations)
so the showCopySuccess boolean can be mutated back to false straight
away.

Better to have a separate boolean tracked with a data-attr that we
can reliably inspect in the system spec.
---
 .../discourse/components/chat-thread.js         | 11 ++++++++++-
 .../components/chat/selection-manager.hbs       |  5 ++++-
 .../components/chat/selection-manager.js        |  7 +++++++
 .../discourse/components/chat/thread/header.js  | 17 +++++++++--------
 plugins/chat/spec/system/transcript_spec.rb     |  4 ++--
 5 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
index a471d27c71c..7cbb7b731e1 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
@@ -25,6 +25,7 @@ export default class ChatThreadPanel extends Component {
   @service chatThreadPaneSubscriptionsManager;
   @service appEvents;
   @service capabilities;
+  @service chatHistory;
 
   @tracked loading;
   @tracked uploadDropZone;
@@ -168,7 +169,15 @@ export default class ChatThreadPanel extends Component {
           this._selfDeleted ||
           this.args.thread.channel.id !== result.meta.channel_id
         ) {
-          this.router.transitionTo("chat.channel", "-", result.meta.channel_id);
+          if (this.chatHistory.previousRoute?.name === "chat.channel.index") {
+            this.router.transitionTo(
+              "chat.channel",
+              "-",
+              result.meta.channel_id
+            );
+          } else {
+            this.router.transitionTo("chat.channel.threads");
+          }
         }
 
         const [messages, meta] = this.afterFetchCallback(
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.hbs
index 986efe55de3..7468f8b4ada 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.hbs
@@ -1,4 +1,7 @@
-<div class="chat-selection-management">
+<div
+  class="chat-selection-management"
+  data-last-copy-successful={{this.lastCopySuccessful}}
+>
   <div class="chat-selection-management__buttons">
     <DButton
       @id="chat-quote-btn"
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js b/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js
index 86d3ca66f00..2c4a2e93428 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js
@@ -15,7 +15,12 @@ export default class ChatSelectionManager extends Component {
   @service site;
   @service("chat-api") api;
 
+  // NOTE: showCopySuccess is used to display the message which animates
+  // after a delay. The on-animation-end helper is not really usable in
+  // system specs because it fires straight away, so we use lastCopySuccessful
+  // with a data attr instead so it's not instantly mutated.
   @tracked showCopySuccess = false;
+  @tracked lastCopySuccessful = false;
 
   get enableMove() {
     return this.args.enableMove ?? false;
@@ -88,6 +93,7 @@ export default class ChatSelectionManager extends Component {
   @action
   async copyMessages() {
     try {
+      this.lastCopySuccessful = false;
       this.showCopySuccess = false;
 
       if (!isTesting()) {
@@ -96,6 +102,7 @@ export default class ChatSelectionManager extends Component {
       }
 
       this.showCopySuccess = true;
+      this.lastCopySuccessful = true;
     } catch (error) {
       popupAjaxError(error);
     }
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.js b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.js
index e47df4a5859..3ddce5f9eb0 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.js
@@ -18,20 +18,21 @@ export default class ChatThreadHeader extends Component {
   @tracked persistedNotificationLevel = true;
 
   get backLink() {
+    let route;
+
     if (
       this.chatHistory.previousRoute?.name === "chat.channel.index" &&
       this.site.mobileView
     ) {
-      return {
-        route: "chat.channel.index",
-        models: this.args.channel.routeModels,
-      };
+      route = "chat.channel.index";
     } else {
-      return {
-        route: "chat.channel.threads",
-        models: [],
-      };
+      route = "chat.channel.threads";
     }
+
+    return {
+      route,
+      models: this.args.channel.routeModels,
+    };
   }
 
   get label() {
diff --git a/plugins/chat/spec/system/transcript_spec.rb b/plugins/chat/spec/system/transcript_spec.rb
index 93d30c37451..d75e376e3cc 100644
--- a/plugins/chat/spec/system/transcript_spec.rb
+++ b/plugins/chat/spec/system/transcript_spec.rb
@@ -20,7 +20,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
     messages = Array.wrap(messages)
     messages.each { |message| channel_page.messages.select(message) }
     channel_page.selection_management.copy
-    expect(page).to have_selector(".chat-selection-management__copy-success")
+    expect(page).to have_css(".chat-selection-management[data-last-copy-successful]")
     clip_text = cdp.read_clipboard
     expect(clip_text.chomp).to eq(generate_transcript(messages, current_user))
     clip_text
@@ -116,7 +116,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
         channel_page.selection_management.cancel
         channel_page.send_message(clip_text)
 
-        expect(page).to have_selector(".chat-message", count: 2)
+        expect(page).to have_css(".chat-message", count: 2)
         expect(page).to have_css(".chat-transcript")
       end
     end