From 84b0a6414d15151113443c1d15199f6b98d86267 Mon Sep 17 00:00:00 2001 From: Ghassan Maslamani Date: Fri, 17 Jun 2022 19:32:57 +0300 Subject: [PATCH] FIX: double selecting replies (#17086) When selecting a post and its replies using the "select +replies" button, the action would push all ids, without checking if some were already selected. This change add a filter to remove ids that are already selected. This fixes https://meta.discourse.org/t/selecting-posts-replies-miscounts-the-number-of-posts/229242 Co-authored-by: @ZogStriP --- .../discourse/app/controllers/topic.js | 3 +- .../tests/unit/controllers/topic-test.js | 46 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 3832df9372a..2e28cca50f1 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -890,7 +890,8 @@ export default Controller.extend(bufferedProperty("model"), { selectReplies(post) { ajax(`/posts/${post.id}/reply-ids.json`).then((replies) => { const replyIds = replies.map((r) => r.id); - this.selectedPostIds.pushObjects([post.id, ...replyIds]); + const postIds = [...this.selectedPostIds, post.id, ...replyIds]; + this.set("selectedPostIds", [...new Set(postIds)]); this._forceRefreshPostStream(); }); }, diff --git a/app/assets/javascripts/discourse/tests/unit/controllers/topic-test.js b/app/assets/javascripts/discourse/tests/unit/controllers/topic-test.js index e7b66504a9d..de0517afb08 100644 --- a/app/assets/javascripts/discourse/tests/unit/controllers/topic-test.js +++ b/app/assets/javascripts/discourse/tests/unit/controllers/topic-test.js @@ -599,6 +599,52 @@ discourseModule("Unit | Controller | topic", function (hooks) { ); }); + test("selectReplies", async function (assert) { + pretender.get("/posts/1/reply-ids.json", () => { + return [ + 200, + { "Content-Type": "application/json" }, + [{ id: 2, level: 1 }], + ]; + }); + + let model = topicWithStream({ + posts: [{ id: 1 }, { id: 2 }], + }); + + const controller = this.getController("topic", { model }); + + controller.send("selectReplies", { id: 1 }); + await settled(); + + assert.strictEqual( + controller.get("selectedPostsCount"), + 2, + "It should select two, the post and its replies" + ); + + controller.send("togglePostSelection", { id: 1 }); + assert.strictEqual( + controller.get("selectedPostsCount"), + 1, + "It should be selecting one only " + ); + assert.strictEqual( + controller.get("selectedPostIds")[0], + 2, + "It should be selecting the reply id " + ); + + controller.send("selectReplies", { id: 1 }); + await settled(); + + assert.strictEqual( + controller.get("selectedPostsCount"), + 2, + "It should be selecting two, even if reply was already selected" + ); + }); + test("topVisibleChanged", function (assert) { let model = topicWithStream({ posts: [{ id: 1 }],