FIX: Do not display twice a user who changed vote (#13284)

FIX: Do not display twice a user who changed vote (#13284)

  • FIX: Fetch last page again if incomplete

The next fetched page number used to increase continuously even if the last page was incomplete and fetching it again could have new voters.

  • FIX: Do not display twice a user who changed vote

A user could appear under two voting options when they changed their vote because pressing the Load More Voters button updated only the current option.

diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6
index 1a47c24..554c4ef 100644
--- a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6
+++ b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6
@@ -14,6 +14,8 @@ import { relativeAge } from "discourse/lib/formatter";
 import round from "discourse/lib/round";
 import showModal from "discourse/lib/show-modal";
 
+const FETCH_VOTERS_COUNT = 25;
+
 function optionHtml(option) {
   const $node = $(`<span>${option.html}</span>`);
 
@@ -106,14 +108,14 @@ createWidget("discourse-poll-load-more", {
   },
 
   click() {
-    const { state } = this;
+    const { state, attrs } = this;
 
     if (state.loading) {
       return;
     }
 
     state.loading = true;
-    return this.sendWidgetAction("loadMore").finally(
+    return this.sendWidgetAction("fetchVoters", attrs.optionId).finally(
       () => (state.loading = false)
     );
   },
@@ -131,63 +133,18 @@ createWidget("discourse-poll-voters", {
     };
   },
 
-  fetchVoters() {
-    const { attrs, state } = this;
-
-    if (state.loaded === "loading") {
-      return;
-    }
-    state.loaded = "loading";
-
-    return _fetchVoters({
-      post_id: attrs.postId,
-      poll_name: attrs.pollName,
-      option_id: attrs.optionId,
-      page: state.page,
-    }).then((result) => {
-      state.loaded = "loaded";
-      state.page += 1;
-
-      const newVoters =
-        attrs.pollType === "number"
-          ? result.voters
-          : result.voters[attrs.optionId];
-
-      const existingVoters = new Set(
-        state.voters.map((voter) => voter.username)
-      );
-
-      newVoters.forEach((voter) => {
-        if (!existingVoters.has(voter.username)) {
-          existingVoters.add(voter.username);
-          state.voters.push(voter);
-        }
-      });
-
-      this.scheduleRerender();
-    });
-  },
-
-  loadMore() {
-    return this.fetchVoters();
-  },
-
-  html(attrs, state) {
-    if (attrs.voters && state.loaded === "new") {
-      state.voters = attrs.voters;
-    }
-
-    const contents = state.voters.map((user) => {
-      return h("li", [
+  html(attrs) {
+    const contents = attrs.voters.map((user) =>
+      h("li", [
         avatarFor("tiny", {
           username: user.username,
           template: user.avatar_template,
         }),
         " ",
-      ]);
-    });
+      ])
+    );
 
-    if (state.voters.length < attrs.totalVotes) {
+    if (attrs.voters.length < attrs.totalVotes) {
       contents.push(this.attach("discourse-poll-load-more", attrs));
     }
 
@@ -203,14 +160,56 @@ createWidget("discourse-poll-standard-results", {
     return { loaded: false };
   },
 
-  fetchVoters() {
+  fetchVoters(optionId) {
     const { attrs, state } = this;
 
+    if (!state.page) {
+      state.page = {};
+    }
+
+    if (!state.page[optionId]) {
+      state.page[optionId] = 1;
+    }
+
     return _fetchVoters({
       post_id: attrs.post.id,
       poll_name: attrs.poll.get("name"),
+      option_id: optionId,
+      page: state.page[optionId],
+      limit: FETCH_VOTERS_COUNT,
     }).then((result) => {
-      state.voters = result.voters;
+      if (!state.voters[optionId]) {
+        state.voters[optionId] = [];
+      }
+
+      const voters = state.voters[optionId];
+      const newVoters = result.voters[optionId];
+
+      // remove users who changed their vote
+      const newVotersSet = new Set(newVoters.map((voter) => voter.username));
+      Object.keys(state.voters).forEach((otherOptionId) => {
+        if (optionId !== otherOptionId) {
+          state.voters[otherOptionId] = state.voters[otherOptionId].filter(
+            (voter) => !newVotersSet.has(voter.username)
+          );
+        }
+      });
+
+      const votersSet = new Set(voters.map((voter) => voter.username));
+      let count = 0;
+      newVoters.forEach((voter) => {
+        if (!votersSet.has(voter.username)) {
+          voters.push(voter);
+          count++;
+        }
+      });
+
+      // request next page in the future only if a complete set was
+      // returned this time
+      if (count >= FETCH_VOTERS_COUNT) {
+        state.page[optionId]++;
+      }
+
       this.scheduleRerender();
     });
   },
@@ -295,14 +294,42 @@ createWidget("discourse-poll-number-results", {
     return { loaded: false };
   },
 
-  fetchVoters() {
+  fetchVoters(optionId) {
     const { attrs, state } = this;
 
+    if (!state.page) {
+      state.page = 1;
+    }
+
     return _fetchVoters({
       post_id: attrs.post.id,
       poll_name: attrs.poll.get("name"),
+      option_id: optionId,
+      page: state.page,
+      limit: FETCH_VOTERS_COUNT,
     }).then((result) => {
-      state.voters = result.voters;
+      if (!state.voters) {
+        state.voters = [];
+      }
+
+      const voters = state.voters;
+      const newVoters = result.voters;
+
+      const votersSet = new Set(voters.map((voter) => voter.username));
+      let count = 0;
+      newVoters.forEach((voter) => {
+        if (!votersSet.has(voter.username)) {
+          voters.push(voter);
+          count++;
+        }
+      });
+
+      // request next page in the future only if a complete set was
+      // returned this time
+      if (count >= FETCH_VOTERS_COUNT) {
+        state.page++;
+      }
+
       this.scheduleRerender();
     });
   },
diff --git a/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6 b/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6
new file mode 100644
index 0000000..06956b6
--- /dev/null
+++ b/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6
@@ -0,0 +1,626 @@
+import {
+  acceptance,
+  publishToMessageBus,
+} from "discourse/tests/helpers/qunit-helpers";
+import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
+
+acceptance("Poll results", function (needs) {
+  needs.user();
+  needs.settings({ poll_enabled: true });
+  needs.hooks.beforeEach(() => {
+    clearPopupMenuOptionsCallback();
+  });
+
+  needs.pretender((server, helper) => {
+    server.get("/posts/by_number/134/1", () => {
+      return helper.response({
+        id: 156,
+        name: null,
+        username: "bianca",
+        avatar_template: "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png",
+        created_at: "2021-06-08T21:56:55.166Z",
+        cooked:
+          '\u003cdiv class="poll" data-poll-status="open" data-poll-public="true" data-poll-results="always" data-poll-charttype="bar" data-poll-type="regular" data-poll-name="poll"\u003e\n\u003cdiv\u003e\n\u003cdiv class="poll-container"\u003e\n\u003cul\u003e\n\u003cli data-poll-option-id="db753fe0bc4e72869ac1ad8765341764"\u003eOption \u003cspan class="hashtag"\u003e#1\u003c/span\u003e\n\u003c/li\u003e\n\u003cli data-poll-option-id="d8c22ff912e03740d9bc19e133e581e0"\u003eOption \u003cspan class="hashtag"\u003e#2\u003c/span\u003e\n\u003c/li\u003e\n\u003c/ul\u003e\n\u003c/div\u003e\n\u003cdiv class="poll-info"\u003e\n\u003cp\u003e\n\u003cspan class="info-number"\u003e0\u003c/span\u003e\n\u003cspan class="info-label"\u003evoters\u003c/span\u003e\n\u003c/p\u003e\n\u003c/div\u003e\n\u003c/div\u003e\n\u003c/div\u003e',
+        post_number: 1,
+        post_type: 1,
+        updated_at: "2021-06-08T21:59:16.444Z",
+        reply_count: 0,
+        reply_to_post_number: null,
+        quote_count: 0,
+        incoming_link_count: 0,
+        reads: 2,
+        readers_count: 1,
+        score: 0,
+        yours: true,
+        topic_id: 134,
+        topic_slug: "load-more-poll-voters",
+        display_username: null,
+        primary_group_name: null,
+        primary_group_flair_url: null,
+        primary_group_flair_bg_color: null,
+        primary_group_flair_color: null,
+        version: 1,
+        can_edit: true,
+        can_delete: false,
+        can_recover: false,
+        can_wiki: true,
+        title_is_group: false,

[... diff too long, it was truncated ...]

GitHub sha: f490a8d39a12ed2c6b125b3380f08b2d1d43ee7f

This commit appears in #13284 which was approved by eviltrout and ZogStriP. It was merged by nbianca.