Skip to content

Commit 32b6b7e

Browse files
megothssmartin-brennan
megothss
authored and
martin-brennan
committed
DEV: Ensure poll with @mentions and user statuses renders without errors (#34014)
This commit ensures that polls including @mentions of users with active user statuses are rendered correctly without errors. It introduces a document validation check before rendering components, adds fixtures for polls with mentions, and provides acceptance tests to verify proper rendering of polls containing @mentions.
1 parent e20ab4d commit 32b6b7e

File tree

3 files changed

+267
-10
lines changed

3 files changed

+267
-10
lines changed

‎app/assets/javascripts/discourse/app/components/decorated-html.gjs

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { htmlSafe, isHTMLSafe } from "@ember/template";
44
import { TrackedArray } from "@ember-compat/tracked-built-ins";
55
import helperFn from "discourse/helpers/helper-fn";
66
import deprecated from "discourse/lib/deprecated";
7-
import { isRailsTesting, isTesting } from "discourse/lib/environment";
7+
import {
8+
isProduction,
9+
isRailsTesting,
10+
isTesting,
11+
} from "discourse/lib/environment";
812
import { POST_STREAM_DEPRECATION_OPTIONS } from "discourse/widgets/widget";
913

1014
const detachedDocument = document.implementation.createHTMLDocument("detached");
@@ -83,19 +87,47 @@ export default class DecoratedHtml extends Component {
8387
return cookedDiv;
8488
}
8589

90+
/**
91+
* Checks if a given HTML element belongs to the current document.
92+
* In development mode, it warns if the element is not in the document.
93+
*
94+
* This is used to ensure components added using `renderGlimmer` are only rendered in the same document, preventing
95+
* rendering errors that otherwise would crash the application.
96+
*
97+
* @param {Object} info - Object containing element information
98+
* @param {Element} info.element - The DOM element to check
99+
* @returns {boolean} True if element belongs to current document, false otherwise
100+
*/
101+
isElementInDocument(info) {
102+
const result = info.element.ownerDocument === document;
103+
104+
if (!isProduction() && !result) {
105+
// eslint-disable-next-line no-console
106+
console.warn(
107+
"The `renderGlimmer` definition below was unable to render the decorated HTML because the target element is not in the " +
108+
"current document. This likely occurred because the element was removed by another decorator.\n",
109+
info
110+
);
111+
}
112+
113+
return result;
114+
}
115+
86116
<template>
87117
{{~this.decoratedContent decorateArgs=@decorateArgs~}}
88118

89119
{{~#each this.renderGlimmerInfos as |info|~}}
90-
{{~#if info.append}}
91-
{{~#in-element info.element insertBefore=null~}}
92-
<info.component @data={{info.data}} />
93-
{{~/in-element~}}
94-
{{~else}}
95-
{{~#in-element info.element~}}
96-
<info.component @data={{info.data}} />
97-
{{~/in-element~}}
98-
{{~/if}}
120+
{{~#if (this.isElementInDocument info)~}}
121+
{{~#if info.append}}
122+
{{~#in-element info.element insertBefore=null~}}
123+
<info.component @data={{info.data}} />
124+
{{~/in-element~}}
125+
{{~else}}
126+
{{~#in-element info.element~}}
127+
<info.component @data={{info.data}} />
128+
{{~/in-element~}}
129+
{{~/if}}
130+
{{~/if~}}
99131
{{~/each~}}
100132
</template>
101133
}

‎app/assets/javascripts/discourse/tests/fixtures/poll.js

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,4 +736,206 @@ export default {
736736
},
737737
},
738738
},
739+
"/t/pie_chart_poll_with_mention.json": {
740+
post_stream: {
741+
posts: [
742+
{
743+
id: 294,
744+
name: "",
745+
username: "markvanlan",
746+
avatar_template: "/user_avatar/localhost/markvanlan/{size}/11_2.png",
747+
created_at: "2019-11-22T18:55:41.439Z",
748+
cooked:
749+
'\u003cdiv class="poll" data-poll-status="open" data-poll-max="3" data-poll-min="1" data-poll-results="always" data-poll-charttype="pie" data-poll-type="multiple" data-poll-name="poll"\u003e\n\u003cdiv\u003e\n\u003cdiv class="poll-container"\u003e\n\u003cul\u003e\n\u003cli data-poll-option-id="687a1ccf3c6a260f9aeeb7f68a1d463c"\u003e<a class="mention" href="/u/user1">@user1</a>\u003c/li\u003e\n\u003cli data-poll-option-id="9377906763a1221d31d656ea0c4a4495"\u003eA test for sure\u003c/li\u003e\n\u003cli data-poll-option-id="ecf47c65a85a0bb20029072b1b721977"\u003eWhy not give it some more\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',
750+
post_number: 1,
751+
post_type: 1,
752+
updated_at: "2019-11-22T18:55:41.439Z",
753+
reply_count: 0,
754+
reply_to_post_number: null,
755+
quote_count: 0,
756+
incoming_link_count: 0,
757+
reads: 2,
758+
readers_count: 1,
759+
score: 0.2,
760+
yours: false,
761+
topic_id: 256,
762+
topic_slug: "14-the-title-must-be-longer-i-guess",
763+
display_username: "",
764+
primary_group_name: "Team",
765+
flair_name: null,
766+
flair_url: null,
767+
flair_bg_color: "",
768+
flair_color: "",
769+
version: 1,
770+
can_edit: false,
771+
can_delete: false,
772+
can_recover: false,
773+
can_wiki: false,
774+
read: true,
775+
user_title: "You are a member of the team",
776+
actions_summary: [
777+
{ id: 2, can_act: true },
778+
{ id: 3, can_act: true },
779+
{ id: 4, can_act: true },
780+
{ id: 8, can_act: true },
781+
{ id: 6, can_act: true },
782+
{ id: 7, can_act: true },
783+
],
784+
moderator: true,
785+
admin: true,
786+
staff: true,
787+
user_id: 1,
788+
hidden: false,
789+
trust_level: 4,
790+
deleted_at: null,
791+
user_deleted: false,
792+
edit_reason: null,
793+
can_view_edit_history: true,
794+
wiki: false,
795+
user_custom_fields: { team: "Engineering", votes: [247, 251, 248] },
796+
can_accept_answer: false,
797+
can_unaccept_answer: false,
798+
accepted_answer: false,
799+
can_translate: false,
800+
can_vote: true,
801+
mentioned_users: [
802+
{
803+
status: {
804+
"description": "Does it still works?",
805+
"emoji": "thinking",
806+
"ends_at": null,
807+
"message_bus_last_id": 0
808+
},
809+
id: 196,
810+
username: "user1",
811+
name: "User1",
812+
avatar_template: "/letter_avatar_proxy/v4/letter/m/34f0e0/{size}.png"
813+
}
814+
],
815+
polls: [
816+
{
817+
name: "poll",
818+
type: "multiple",
819+
status: "open",
820+
results: "always",
821+
min: 1,
822+
max: 3,
823+
options: [
824+
{
825+
id: "687a1ccf3c6a260f9aeeb7f68a1d463c",
826+
html: '<a class="mention" href="/u/user1">@user1</a>',
827+
votes: 2,
828+
},
829+
{
830+
id: "9377906763a1221d31d656ea0c4a4495",
831+
html: "A test for sure",
832+
votes: 2,
833+
},
834+
{
835+
id: "ecf47c65a85a0bb20029072b1b721977",
836+
html: "Why not give it some more",
837+
votes: 1,
838+
},
839+
],
840+
voters: 2,
841+
chart_type: "pie",
842+
},
843+
],
844+
polls_votes: {
845+
poll: [
846+
"687a1ccf3c6a260f9aeeb7f68a1d463c",
847+
"9377906763a1221d31d656ea0c4a4495",
848+
],
849+
},
850+
},
851+
],
852+
stream: [294],
853+
},
854+
timeline_lookup: [[1, 2]],
855+
suggested_topics: [],
856+
tags: [],
857+
id: 256,
858+
title: "14 the title must be longer i guess",
859+
fancy_title: "14 the title must be longer i guess",
860+
posts_count: 1,
861+
created_at: "2019-11-22T18:55:41.259Z",
862+
views: 3,
863+
reply_count: 0,
864+
like_count: 0,
865+
last_posted_at: "2019-11-22T18:55:41.439Z",
866+
visible: true,
867+
closed: false,
868+
archived: false,
869+
has_summary: false,
870+
archetype: "regular",
871+
slug: "14-the-title-must-be-longer-i-guess",
872+
category_id: 1,
873+
word_count: 24,
874+
deleted_at: null,
875+
user_id: 1,
876+
featured_link: null,
877+
pinned_globally: false,
878+
pinned_at: null,
879+
pinned_until: null,
880+
image_url: null,
881+
draft: null,
882+
draft_key: "topic_256",
883+
draft_sequence: 0,
884+
posted: false,
885+
unpinned: null,
886+
pinned: false,
887+
current_post_number: 1,
888+
highest_post_number: 1,
889+
last_read_post_number: 1,
890+
last_read_post_id: 294,
891+
deleted_by: null,
892+
actions_summary: [
893+
{ id: 4, count: 0, hidden: false, can_act: true },
894+
{ id: 8, count: 0, hidden: false, can_act: true },
895+
{ id: 7, count: 0, hidden: false, can_act: true },
896+
],
897+
chunk_size: 20,
898+
bookmarked: false,
899+
bookmarks: [],
900+
topic_timer: null,
901+
message_bus_last_id: 1,
902+
participant_count: 1,
903+
show_read_indicator: false,
904+
can_vote: true,
905+
vote_count: 0,
906+
user_voted: false,
907+
details: {
908+
notification_level: 1,
909+
notifications_reason_id: null,
910+
can_create_post: true,
911+
can_reply_as_new_topic: true,
912+
can_flag_topic: true,
913+
participants: [
914+
{
915+
id: 1,
916+
username: "markvanlan",
917+
name: "",
918+
avatar_template: "/user_avatar/localhost/markvanlan/{size}/11_2.png",
919+
post_count: 1,
920+
primary_group_name: "Team",
921+
flair_name: null,
922+
flair_url: null,
923+
flair_color: "",
924+
flair_bg_color: "",
925+
},
926+
],
927+
created_by: {
928+
id: 1,
929+
username: "markvanlan",
930+
name: "",
931+
avatar_template: "/user_avatar/localhost/markvanlan/{size}/11_2.png",
932+
},
933+
last_poster: {
934+
id: 1,
935+
username: "markvanlan",
936+
name: "",
937+
avatar_template: "/user_avatar/localhost/markvanlan/{size}/11_2.png",
938+
},
939+
},
940+
},
739941
};

‎plugins/poll/test/javascripts/acceptance/poll-pie-chart-test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,27 @@ acceptance("Rendering polls with pie charts", function (needs) {
2828
.dom(".poll .poll-results-chart")
2929
.exists({ count: 1 }, "Renders the chart div instead of bar container");
3030
});
31+
32+
test("Renders results containing @mention correctly", async function (assert) {
33+
// enable user status for the test to ensure the application renders without errors
34+
this.siteSettings.enable_user_status = true;
35+
36+
await visit("/t/-/pie_chart_poll_with_mention");
37+
38+
assert
39+
.dom(".poll .poll-info_counts-count:first-child .info-number")
40+
.hasText("2", "it should display the right number of voters");
41+
42+
assert
43+
.dom(".poll .poll-info_counts-count:last-child .info-number")
44+
.hasText("5", "it should display the right number of votes");
45+
46+
assert
47+
.dom(".poll-outer")
48+
.hasClass("pie", "pie class is present on poll div");
49+
50+
assert
51+
.dom(".poll .poll-results-chart")
52+
.exists({ count: 1 }, "Renders the chart div instead of bar container");
53+
});
3154
});

0 commit comments

Comments
 (0)