FIX: Show the last rendered user-tip (#29346)

…or a tip with the highest priority.

This regressed in 597ef11195 where we got rid of `next()` calls, so we'd render the first tip we encounter.

The commit also adds a test and updates existing ones.

app/assets/javascripts/discourse/app/services/user-tips.js

@@ -2,6 +2,7 @@ import Service, { service } from "@ember/service";
 import { TrackedSet } from "@ember-compat/tracked-built-ins";
 import { disableImplicitInjections } from "discourse/lib/implicit-injections";
 import { isTesting } from "discourse-common/config/environment";
+import discourseDebounce from "discourse-common/lib/debounce";
 
 @disableImplicitInjections
 export default class UserTips extends Service {
@@ -13,6 +14,10 @@ export default class UserTips extends Service {
   #shouldRenderSet = new TrackedSet();
 
   #updateRenderedId() {
+    if (this.isDestroying || this.isDestroyed) {
+      return;
+    }
+
     const tipsArray = [...this.#availableTips];
     if (tipsArray.find((tip) => tip.id === this.#renderedId)) {
       return;
@@ -21,11 +26,7 @@ export default class UserTips extends Service {
     const newId = tipsArray
       .sortBy("priority")
       .reverse()
-      .find((tip) => {
-        if (this.canSeeUserTip(tip.id)) {
-          return tip.id;
-        }
-      })?.id;
+      .find((tip) => this.canSeeUserTip(tip.id))?.id;
 
     if (this.#renderedId !== newId) {
       this.#shouldRenderSet.delete(this.#renderedId);
@@ -41,7 +42,7 @@ export default class UserTips extends Service {
   addAvailableTip(tip) {
     if (this.canSeeUserTip(tip.id) && !this._findAvailableTipById(tip.id)) {
       this.#availableTips.add(tip);
-      this.#updateRenderedId();
+      discourseDebounce(this, this.#updateRenderedId, 0);
     }
   }
 

app/assets/javascripts/discourse/tests/acceptance/user-tips-test.js

@@ -1,7 +1,7 @@
 import { visit } from "@ember/test-helpers";
 import { test } from "qunit";
 import pretender, { response } from "discourse/tests/helpers/create-pretender";
-import { acceptance, query } from "discourse/tests/helpers/qunit-helpers";
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
 import I18n from "discourse-i18n";
 
 acceptance("User Tips - first_notification", function (needs) {
@@ -11,9 +11,8 @@ acceptance("User Tips - first_notification", function (needs) {
   test("Shows first notification user tip", async function (assert) {
     this.siteSettings.enable_user_tips = true;
 
-    let requestsCount = 0;
     pretender.put("/u/eviltrout.json", () => {
-      requestsCount += 1;
+      assert.step("endpoint called");
 
       return response(200, {
         user: {
@@ -25,15 +24,13 @@ acceptance("User Tips - first_notification", function (needs) {
     });
 
     await visit("/t/internationalization-localization/280");
-    assert.equal(
-      query(".user-tip__title").textContent.trim(),
-      I18n.t("user_tips.first_notification.title")
-    );
+    assert
+      .dom(".user-tip__title")
+      .hasText(I18n.t("user_tips.first_notification.title"));
 
-    assert.strictEqual(
-      requestsCount,
-      1,
-      "Seeing the user tip updates the user option via a background request"
+    assert.verifySteps(
+      ["endpoint called"],
+      "seeing the user tip updates the user option via a background request"
     );
   });
 });
@@ -46,10 +43,9 @@ acceptance("User Tips - topic_timeline", function (needs) {
     this.siteSettings.enable_user_tips = true;
 
     await visit("/t/internationalization-localization/280");
-    assert.equal(
-      query(".user-tip__title").textContent.trim(),
-      I18n.t("user_tips.topic_timeline.title")
-    );
+    assert
+      .dom(".user-tip__title")
+      .hasText(I18n.t("user_tips.topic_timeline.title"));
   });
 });
 
@@ -61,10 +57,7 @@ acceptance("User Tips - post_menu", function (needs) {
     this.siteSettings.enable_user_tips = true;
 
     await visit("/t/internationalization-localization/280");
-    assert.equal(
-      query(".user-tip__title").textContent.trim(),
-      I18n.t("user_tips.post_menu.title")
-    );
+    assert.dom(".user-tip__title").hasText(I18n.t("user_tips.post_menu.title"));
   });
 });
 
@@ -77,10 +70,9 @@ acceptance("User Tips - topic_notification_levels", function (needs) {
 
     await visit("/t/internationalization-localization/280");
 
-    assert.equal(
-      query(".user-tip__title").textContent.trim(),
-      I18n.t("user_tips.topic_notification_levels.title")
-    );
+    assert
+      .dom(".user-tip__title")
+      .hasText(I18n.t("user_tips.topic_notification_levels.title"));
   });
 });
 
@@ -92,9 +84,8 @@ acceptance("User Tips - suggested_topics", function (needs) {
     this.siteSettings.enable_user_tips = true;
 
     await visit("/t/internationalization-localization/280");
-    assert.equal(
-      query(".user-tip__title").textContent.trim(),
-      I18n.t("user_tips.suggested_topics.title")
-    );
+    assert
+      .dom(".user-tip__title")
+      .hasText(I18n.t("user_tips.suggested_topics.title"));
   });
 });

app/assets/javascripts/discourse/tests/integration/components/user-tip-test.gjs

@@ -0,0 +1,24 @@
+import { getOwner } from "@ember/owner";
+import { render } from "@ember/test-helpers";
+import { module, test } from "qunit";
+import UserTip from "discourse/components/user-tip";
+import { setupRenderingTest } from "discourse/tests/helpers/component-test";
+import DTooltips from "float-kit/components/d-tooltips";
+
+module("Integration | Component | UserTip", function (hooks) {
+  setupRenderingTest(hooks);
+
+  test("shows the last user tip when there are no priorities", async function (assert) {
+    const site = getOwner(this).lookup("service:site");
+    site.user_tips = { foo: 1, bar: 2, baz: 3 };
+
+    await render(<template>
+      <UserTip @id="foo" @titleText="first tip" />
+      <UserTip @id="bar" @titleText="second tip" />
+      <UserTip @id="baz" @titleText="third tip" />
+      <DTooltips />
+    </template>);
+
+    assert.dom(".user-tip__title").hasText("third tip");
+  });
+});

spec/system/user_tips_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Homepage", type: :system do
+describe "User tips", type: :system do
   fab!(:admin)
   fab!(:user)
   fab!(:topics) { Fabricate.times(2, :post).map(&:topic) }