From f490a8d39a12ed2c6b125b3380f08b2d1d43ee7f Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Fri, 18 Jun 2021 18:56:54 +0300 Subject: [PATCH] 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. --- .../javascripts/widgets/discourse-poll.js.es6 | 141 ++-- .../acceptance/poll-results-test.js.es6 | 626 ++++++++++++++++++ 2 files changed, 710 insertions(+), 57 deletions(-) create mode 100644 plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6 diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 index 1a47c2488e7..554c4efe9ab 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 = $(`${option.html}`); @@ -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 00000000000..06956b676aa --- /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, + bookmarked: false, + raw: + "[poll type=regular results=always public=true chartType=bar]\n* Option #1\n* Option #2\n[/poll]", + actions_summary: [ + { id: 3, can_act: true }, + { id: 4, can_act: true }, + { id: 8, can_act: true }, + { id: 7, can_act: true }, + ], + moderator: false, + admin: true, + staff: true, + user_id: 1, + hidden: false, + trust_level: 0, + deleted_at: null, + user_deleted: false, + edit_reason: null, + can_view_edit_history: true, + wiki: false, + reviewable_id: null, + reviewable_score_count: 0, + reviewable_score_pending_count: 0, + calendar_details: [], + can_accept_answer: false, + can_unaccept_answer: false, + accepted_answer: false, + polls: [ + { + name: "poll", + type: "regular", + status: "open", + public: true, + results: "always", + options: [ + { + id: "db753fe0bc4e72869ac1ad8765341764", + html: + 'Option \u003cspan class="hashtag"\u003e#1\u003c/span\u003e', + votes: 1, + }, + { + id: "d8c22ff912e03740d9bc19e133e581e0", + html: + 'Option \u003cspan class="hashtag"\u003e#2\u003c/span\u003e', + votes: 0, + }, + ], + voters: 1, + preloaded_voters: { + db753fe0bc4e72869ac1ad8765341764: [ + { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + ], + }, + chart_type: "bar", + title: null, + }, + ], + polls_votes: { poll: ["db753fe0bc4e72869ac1ad8765341764"] }, + }); + }); + + server.get("/t/load-more-poll-voters.json", () => { + return helper.response({ + post_stream: { + posts: [ + { + 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, + read: true, + title_is_group: false, + bookmarked: false, + actions_summary: [ + { id: 3, can_act: true }, + { id: 4, can_act: true }, + { id: 8, can_act: true }, + { id: 7, can_act: true }, + ], + moderator: false, + admin: true, + staff: true, + user_id: 1, + hidden: false, + trust_level: 0, + deleted_at: null, + user_deleted: false, + edit_reason: null, + can_view_edit_history: true, + wiki: false, + reviewable_id: 0, + reviewable_score_count: 0, + reviewable_score_pending_count: 0, + calendar_details: [], + can_accept_answer: false, + can_unaccept_answer: false, + accepted_answer: false, + polls: [ + { + name: "poll", + type: "regular", + status: "open", + public: true, + results: "always", + options: [ + { + id: "db753fe0bc4e72869ac1ad8765341764", + html: + 'Option \u003cspan class="hashtag"\u003e#1\u003c/span\u003e', + votes: 1, + }, + { + id: "d8c22ff912e03740d9bc19e133e581e0", + html: + 'Option \u003cspan class="hashtag"\u003e#2\u003c/span\u003e', + votes: 0, + }, + ], + voters: 1, + preloaded_voters: { + db753fe0bc4e72869ac1ad8765341764: [ + { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + ], + }, + chart_type: "bar", + title: null, + }, + ], + polls_votes: { poll: ["db753fe0bc4e72869ac1ad8765341764"] }, + }, + ], + stream: [156], + }, + timeline_lookup: [[1, 0]], + suggested_topics: [ + { + id: 7, + title: "Welcome to Discourse", + fancy_title: "Welcome to Discourse", + slug: "welcome-to-discourse", + posts_count: 9, + reply_count: 0, + highest_post_number: 9, + image_url: + "//localhost:3000/uploads/default/original/1X/ba1a510603f5112dcaf06cf42c2eb671bff83681.png", + created_at: "2021-06-02T16:21:38.347Z", + last_posted_at: "2021-06-08T20:36:29.235Z", + bumped: true, + bumped_at: "2021-06-08T20:36:29.235Z", + archetype: "regular", + unseen: false, + last_read_post_number: 9, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: true, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + liked: false, + tags: [], + like_count: 0, + views: 2, + category_id: 1, + featured_link: null, + has_accepted_answer: false, + posters: [ + { + extras: null, + description: "Original Poster", + user: { + id: -1, + username: "system", + name: "system", + avatar_template: "/images/discourse-logo-sketch-small.png", + }, + }, + { + extras: "latest", + description: "Most Recent Poster", + user: { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + }, + ], + }, + { + id: 129, + title: "This is another test topic", + fancy_title: "This is another test topic", + slug: "this-is-another-test-topic", + posts_count: 1, + reply_count: 0, + highest_post_number: 1, + image_url: null, + created_at: "2021-06-03T15:48:27.262Z", + last_posted_at: "2021-06-03T15:48:27.537Z", + bumped: true, + bumped_at: "2021-06-08T12:52:36.650Z", + archetype: "regular", + unseen: false, + last_read_post_number: 1, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + liked: false, + tags: [], + like_count: 0, + views: 7, + category_id: 1, + featured_link: null, + has_accepted_answer: false, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user: { + id: 12, + username: "bar", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/b77776/{size}.png", + }, + }, + ], + }, + { + id: 131, + title: + "Welcome to Discourse — thanks for starting a new conversation!", + fancy_title: + "Welcome to Discourse — thanks for starting a new conversation!", + slug: "welcome-to-discourse-thanks-for-starting-a-new-conversation", + posts_count: 1, + reply_count: 0, + highest_post_number: 1, + image_url: null, + created_at: "2021-06-04T08:51:19.807Z", + last_posted_at: "2021-06-04T08:51:19.928Z", + bumped: true, + bumped_at: "2021-06-04T14:37:46.939Z", + archetype: "regular", + unseen: false, + last_read_post_number: 1, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + liked: false, + tags: ["abc", "e", "b"], + like_count: 0, + views: 3, + category_id: 1, + featured_link: null, + has_accepted_answer: false, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user: { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + }, + ], + }, + { + id: 133, + title: "This is a new tpoic", + fancy_title: "This is a new tpoic", + slug: "this-is-a-new-tpoic", + posts_count: 12, + reply_count: 0, + highest_post_number: 12, + image_url: null, + created_at: "2021-06-08T14:44:03.664Z", + last_posted_at: "2021-06-08T19:57:35.853Z", + bumped: true, + bumped_at: "2021-06-08T19:57:35.853Z", + archetype: "regular", + unseen: false, + last_read_post_number: 12, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + liked: false, + tags: [], + like_count: 0, + views: 1, + category_id: 1, + featured_link: null, + has_accepted_answer: false, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user: { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + }, + ], + }, + ], + tags: [], + id: 134, + title: "Load more poll voters", + fancy_title: "Load more poll voters", + posts_count: 1, + created_at: "2021-06-08T21:56:55.073Z", + views: 4, + reply_count: 0, + like_count: 0, + last_posted_at: "2021-06-08T21:56:55.166Z", + visible: true, + closed: false, + archived: false, + has_summary: false, + archetype: "regular", + slug: "load-more-poll-voters", + category_id: 1, + word_count: 14, + deleted_at: null, + user_id: 1, + featured_link: null, + pinned_globally: false, + pinned_at: null, + pinned_until: null, + image_url: null, + slow_mode_seconds: 0, + draft: null, + draft_key: "topic_134", + draft_sequence: 7, + posted: true, + unpinned: null, + pinned: false, + current_post_number: 1, + highest_post_number: 1, + last_read_post_number: 1, + last_read_post_id: 156, + deleted_by: null, + has_deleted: false, + actions_summary: [ + { id: 4, count: 0, hidden: false, can_act: true }, + { id: 8, count: 0, hidden: false, can_act: true }, + { id: 7, count: 0, hidden: false, can_act: true }, + ], + chunk_size: 20, + bookmarked: false, + topic_timer: null, + message_bus_last_id: 5, + participant_count: 1, + queued_posts_count: 0, + show_read_indicator: false, + thumbnails: null, + slow_mode_enabled_until: null, + details: { + can_edit: true, + notification_level: 3, + notifications_reason_id: 1, + can_move_posts: true, + can_delete: true, + can_remove_allowed_users: true, + can_invite_to: true, + can_invite_via_email: true, + can_create_post: true, + can_reply_as_new_topic: true, + can_flag_topic: true, + can_convert_topic: true, + can_review_topic: true, + can_close_topic: true, + can_archive_topic: true, + can_split_merge_topic: true, + can_edit_staff_notes: true, + can_toggle_topic_visibility: true, + can_pin_unpin_topic: true, + can_moderate_category: true, + can_remove_self_id: 1, + participants: [ + { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + post_count: 1, + primary_group_name: null, + primary_group_flair_url: null, + primary_group_flair_color: null, + primary_group_flair_bg_color: null, + admin: true, + trust_level: 0, + }, + ], + created_by: { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + last_poster: { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + }, + pending_posts: [], + }); + }); + + server.get("/polls/voters.json", (request) => { + if ( + request.queryParams.option_id === "d8c22ff912e03740d9bc19e133e581e0" + ) { + return helper.response({ + voters: { + d8c22ff912e03740d9bc19e133e581e0: [ + { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + title: null, + }, + ], + }, + }); + } else { + return helper.response({ + voters: { + [request.queryParams.option_id]: [], + }, + }); + } + }); + }); + + test("can load more voters", async function (assert) { + await visit("/t/-/load-more-poll-voters"); + + assert.equal( + find(".poll-container .results li:nth-child(1) .poll-voters li").length, + 1 + ); + + publishToMessageBus("/polls/134", { + post_id: "156", + polls: [ + { + name: "poll", + type: "regular", + status: "open", + public: true, + results: "always", + options: [ + { + id: "db753fe0bc4e72869ac1ad8765341764", + html: 'Option #1', + votes: 1, + }, + { + id: "d8c22ff912e03740d9bc19e133e581e0", + html: 'Option #2', + votes: 1, + }, + ], + voters: 2, + preloaded_voters: { + db753fe0bc4e72869ac1ad8765341764: [ + { + id: 1, + username: "bianca", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/b/3be4f8/{size}.png", + }, + ], + d8c22ff912e03740d9bc19e133e581e0: [ + { + id: 7, + username: "foo", + name: null, + avatar_template: + "/letter_avatar_proxy/v4/letter/f/b19c9b/{size}.png", + title: null, + }, + ], + }, + chart_type: "bar", + title: null, + }, + ], + }); + await visit("/t/-/load-more-poll-voters"); + + await click(".poll-voters-toggle-expand a"); + assert.equal( + find(".poll-container .results li:nth-child(1) .poll-voters li").length, + 0 + ); + assert.equal( + find(".poll-container .results li:nth-child(2) .poll-voters li").length, + 1 + ); + }); +});