From b2dc32f41c3e4cc1a7542931bfed617eaaa7a156 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Thu, 19 Dec 2024 13:02:41 +0100 Subject: [PATCH] FIX: An off-by-one error in glimmer topic list (#30372) `findIndex` returns -1 when no element is found, but the `start` boundary can't be less than 0. --- .../app/components/topic-list/list.gjs | 5 +- .../components/topic-list-test.gjs | 71 +++++++++++++++++++ .../integration/components/topic-list-test.js | 62 ---------------- 3 files changed, 75 insertions(+), 63 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/integration/components/topic-list-test.gjs delete mode 100644 app/assets/javascripts/discourse/tests/integration/components/topic-list-test.js diff --git a/app/assets/javascripts/discourse/app/components/topic-list/list.gjs b/app/assets/javascripts/discourse/app/components/topic-list/list.gjs index 99bccf33f8f..316985ae9d7 100644 --- a/app/assets/javascripts/discourse/app/components/topic-list/list.gjs +++ b/app/assets/javascripts/discourse/app/components/topic-list/list.gjs @@ -143,7 +143,10 @@ export default class TopicList extends Component { // work backwards // this is more efficient cause we keep appending to list - const start = topics.findIndex((topic) => !topic.get("pinned")); + const start = Math.max( + topics.findIndex((topic) => !topic.get("pinned")), + 0 + ); let lastVisitedTopic, topic; for (let i = topics.length - 1; i >= start; i--) { diff --git a/app/assets/javascripts/discourse/tests/integration/components/topic-list-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/topic-list-test.gjs new file mode 100644 index 00000000000..2fda07c5d58 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/topic-list-test.gjs @@ -0,0 +1,71 @@ +import { getOwner } from "@ember/owner"; +import { click, render } from "@ember/test-helpers"; +import { module, test } from "qunit"; +import HbrTopicList from "discourse/components/topic-list"; +import TopicList from "discourse/components/topic-list/list"; +import BulkSelectHelper from "discourse/lib/bulk-select-helper"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; + +module("Integration | Component | topic-list", function (hooks) { + setupRenderingTest(hooks); + + test("bulk select", async function (assert) { + const bulkSelectHelper = new BulkSelectHelper(this); + const store = getOwner(this).lookup("service:store"); + const topics = [ + store.createRecord("topic", { id: 24234 }), + store.createRecord("topic", { id: 24235 }), + ]; + + await render(); + + assert.strictEqual(bulkSelectHelper.selected.length, 0, "defaults to 0"); + + await click("button.bulk-select"); + assert.true(bulkSelectHelper.bulkSelectEnabled, "bulk select is enabled"); + + await click("button.bulk-select-all"); + assert.strictEqual( + bulkSelectHelper.selected.length, + 2, + "clicking Select All selects all loaded topics" + ); + assert.true( + bulkSelectHelper.autoAddTopicsToBulkSelect, + "clicking Select All turns on the autoAddTopicsToBulkSelect flag" + ); + + await click("button.bulk-clear-all"); + assert.strictEqual( + bulkSelectHelper.selected.length, + 0, + "clicking Clear All deselects all topics" + ); + assert.false( + bulkSelectHelper.autoAddTopicsToBulkSelect, + "clicking Clear All turns off the autoAddTopicsToBulkSelect flag" + ); + }); + + test("renders a list of all-pinned topics", async function (assert) { + const currentUser = getOwner(this).lookup("service:current-user"); + currentUser.set("previous_visit_at", +new Date()); + const store = getOwner(this).lookup("service:store"); + const topics = [ + store.createRecord("topic", { id: 24234, pinned: true }), + store.createRecord("topic", { id: 24235, pinned: true }), + ]; + + await render(); + + assert.dom(".topic-status .d-icon-thumbtack").exists({ count: 2 }); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/integration/components/topic-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/topic-list-test.js deleted file mode 100644 index ef60c841945..00000000000 --- a/app/assets/javascripts/discourse/tests/integration/components/topic-list-test.js +++ /dev/null @@ -1,62 +0,0 @@ -import { getOwner } from "@ember/owner"; -import { click, render } from "@ember/test-helpers"; -import { hbs } from "ember-cli-htmlbars"; -import { module, test } from "qunit"; -import BulkSelectHelper from "discourse/lib/bulk-select-helper"; -import { setupRenderingTest } from "discourse/tests/helpers/component-test"; - -module("Integration | Component | topic-list", function (hooks) { - setupRenderingTest(hooks); - - test("bulk select", async function (assert) { - const store = getOwner(this).lookup("service:store"); - this.setProperties({ - topics: [ - store.createRecord("topic", { id: 24234 }), - store.createRecord("topic", { id: 24235 }), - ], - bulkSelectHelper: new BulkSelectHelper(this), - }); - - await render(hbs` - - `); - - assert.strictEqual( - this.bulkSelectHelper.selected.length, - 0, - "defaults to 0" - ); - await click("button.bulk-select"); - assert.true( - this.bulkSelectHelper.bulkSelectEnabled, - "bulk select is enabled" - ); - - await click("button.bulk-select-all"); - assert.strictEqual( - this.bulkSelectHelper.selected.length, - 2, - "clicking Select All selects all loaded topics" - ); - assert.true( - this.bulkSelectHelper.autoAddTopicsToBulkSelect, - "clicking Select All turns on the autoAddTopicsToBulkSelect flag" - ); - - await click("button.bulk-clear-all"); - assert.strictEqual( - this.bulkSelectHelper.selected.length, - 0, - "clicking Clear All deselects all topics" - ); - assert.false( - this.bulkSelectHelper.autoAddTopicsToBulkSelect, - "clicking Clear All turns off the autoAddTopicsToBulkSelect flag" - ); - }); -});