From 6a9af7c82f2902a996e25548c173e7183b3e8db3 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Tue, 22 Oct 2024 22:41:29 +0200 Subject: [PATCH] FIX: Show the last rendered user-tip (#29346) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …or a tip with the highest priority. This regressed in 597ef1119536693ef0bc7fe799ff485047f9f6f7 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. --- .../discourse/app/services/user-tips.js | 13 +++--- .../tests/acceptance/user-tips-test.js | 45 ++++++++----------- .../integration/components/user-tip-test.gjs | 24 ++++++++++ spec/system/user_tips_spec.rb | 2 +- 4 files changed, 50 insertions(+), 34 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/integration/components/user-tip-test.gjs diff --git a/app/assets/javascripts/discourse/app/services/user-tips.js b/app/assets/javascripts/discourse/app/services/user-tips.js index 569a1ab985d..9084741cec6 100644 --- a/app/assets/javascripts/discourse/app/services/user-tips.js +++ b/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); } } diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-tips-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-tips-test.js index 6a43d0ea92c..31307ac276b 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-tips-test.js +++ b/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")); }); }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-tip-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/user-tip-test.gjs new file mode 100644 index 00000000000..227384c98cf --- /dev/null +++ b/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(); + + assert.dom(".user-tip__title").hasText("third tip"); + }); +}); diff --git a/spec/system/user_tips_spec.rb b/spec/system/user_tips_spec.rb index def803b7b31..02497edfc0d 100644 --- a/spec/system/user_tips_spec.rb +++ b/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) }