Skip to content

Commit 99a0ee7

Browse files
megothssmegothss
committed
Revert "DEV: Improve quoted post handling and enhance decorator stability (#33111)"
This reverts commit cf9e4de.
1 parent cf9e4de commit 99a0ee7

File tree

5 files changed

+48
-182
lines changed

5 files changed

+48
-182
lines changed

‎app/assets/javascripts/discourse/app/components/post/cooked-html.gjs

Lines changed: 40 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { htmlSafe } from "@ember/template";
66
import curryComponent from "ember-curry-component";
77
import DecoratedHtml from "discourse/components/decorated-html";
88
import { bind } from "discourse/lib/decorators";
9-
import { isRailsTesting, isTesting } from "discourse/lib/environment";
109
import { makeArray } from "discourse/lib/helpers";
1110
import decorateLinkCounts from "discourse/lib/post-cooked-html-decorators/link-counts";
1211
import decorateMentions from "discourse/lib/post-cooked-html-decorators/mentions";
@@ -48,62 +47,48 @@ export default class PostCookedHtml extends Component {
4847

4948
[...POST_COOKED_DECORATORS, ...this.extraDecorators].forEach(
5049
(decorator) => {
51-
try {
52-
if (!this.#decoratorState.has(decorator)) {
53-
this.#decoratorState.set(decorator, {});
54-
}
55-
56-
const owner = getOwner(this);
57-
const decorationCleanup = decorator(element, {
58-
data: {
59-
post: this.args.post,
60-
cooked: this.cooked,
50+
if (!this.#decoratorState.has(decorator)) {
51+
this.#decoratorState.set(decorator, {});
52+
}
53+
54+
const owner = getOwner(this);
55+
const decorationCleanup = decorator(element, {
56+
data: {
57+
post: this.args.post,
58+
cooked: this.cooked,
59+
highlightTerm: this.highlightTerm,
60+
isIgnored: this.isIgnored,
61+
ignoredUsers: this.ignoredUsers,
62+
},
63+
createDetachedElement: this.#createDetachedElement,
64+
currentUser: this.currentUser,
65+
helper,
66+
renderNestedPostCookedHtml: (
67+
nestedElement,
68+
nestedPost,
69+
extraDecorators
70+
) => {
71+
const nestedArguments = {
72+
post: nestedPost,
73+
streamElement: false,
6174
highlightTerm: this.highlightTerm,
62-
isIgnored: this.isIgnored,
63-
ignoredUsers: this.ignoredUsers,
64-
},
65-
createDetachedElement: this.#createDetachedElement,
66-
currentUser: this.currentUser,
67-
helper,
68-
renderNestedPostCookedHtml: (
75+
extraDecorators: [
76+
...this.extraDecorators,
77+
...makeArray(extraDecorators),
78+
],
79+
};
80+
81+
helper.renderGlimmer(
6982
nestedElement,
70-
nestedPost,
71-
extraDecorators,
72-
extraArguments
73-
) => {
74-
const nestedArguments = {
75-
...extraArguments,
76-
post: nestedPost,
77-
streamElement: false,
78-
highlightTerm: this.highlightTerm,
79-
extraDecorators: [
80-
...this.extraDecorators,
81-
...makeArray(extraDecorators),
82-
],
83-
};
84-
85-
helper.renderGlimmer(
86-
nestedElement,
87-
curryComponent(PostCookedHtml, nestedArguments, owner)
88-
);
89-
},
90-
owner,
91-
state: this.#decoratorState.get(decorator),
92-
});
93-
94-
if (typeof decorationCleanup === "function") {
95-
this.#pendingDecoratorCleanup.push(decorationCleanup);
96-
}
97-
} catch (e) {
98-
if (isRailsTesting() || isTesting()) {
99-
throw e;
100-
} else {
101-
// in case one of the decorators throws an error we want to surface it to the console but prevent
102-
// the application from crashing
103-
104-
// eslint-disable-next-line no-console
105-
console.error(e);
106-
}
83+
curryComponent(PostCookedHtml, nestedArguments, owner)
84+
);
85+
},
86+
owner,
87+
state: this.#decoratorState.get(decorator),
88+
});
89+
90+
if (typeof decorationCleanup === "function") {
91+
this.#pendingDecoratorCleanup.push(decorationCleanup);
10792
}
10893
}
10994
);

‎app/assets/javascripts/discourse/app/lib/post-cooked-html-decorators/quote-controls.js

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { spinnerHTML } from "discourse/helpers/loading-spinner";
22
import { ajax } from "discourse/lib/ajax";
3-
import { popupAjaxError } from "discourse/lib/ajax-error";
43
import escape from "discourse/lib/escape";
54
import highlightHTML from "discourse/lib/highlight-html";
65
import { iconHTML } from "discourse/lib/icon-library";
@@ -73,8 +72,7 @@ function _updateQuoteElements(aside, desc, context) {
7372
if (
7473
topicNumber &&
7574
postNumber &&
76-
topicNumber === data.post.topic_id?.toString() &&
77-
data.post.topic
75+
topicNumber === data.post.topic_id?.toString()
7876
) {
7977
const topicId = data.post.topic_id;
8078
const slug = data.post.topic.slug;
@@ -119,13 +117,8 @@ function _updateQuoteElements(aside, desc, context) {
119117
}
120118

121119
async function _toggleQuote(aside, context) {
122-
const {
123-
createDetachedElement,
124-
data,
125-
renderNestedPostCookedHtml,
126-
state,
127-
owner,
128-
} = context;
120+
const { createDetachedElement, data, renderNestedPostCookedHtml, state } =
121+
context;
129122

130123
if (state.expanding) {
131124
return;
@@ -162,21 +155,12 @@ async function _toggleQuote(aside, context) {
162155
const postId = parseInt(aside.dataset.post, 10);
163156

164157
try {
165-
const post = data.post;
166-
const quotedPost = owner
167-
.lookup("service:store")
168-
.createRecord(
169-
"post",
170-
await ajax(`/posts/by_number/${topicId}/${postId}`)
171-
);
172-
173-
if (quotedPost.topic_id === post?.topic_id) {
174-
quotedPost.topic = post.topic;
175-
}
158+
const quotedPost = await ajax(`/posts/by_number/${topicId}/${postId}`);
176159

160+
const post = data.post;
177161
const quotedPosts = post.quoted || {};
178162
quotedPosts[quotedPost.id] = quotedPost;
179-
post.quoted = quotedPosts;
163+
post.set("quoted", quotedPosts);
180164

181165
const div = createDetachedElement("div");
182166
div.classList.add("expanded-quote");
@@ -193,11 +177,9 @@ async function _toggleQuote(aside, context) {
193177
blockQuote.innerHTML = "";
194178
blockQuote.appendChild(div);
195179
} catch (e) {
196-
if (e.jqXHR && [403, 404].includes(e.jqXHR.status)) {
180+
if ([403, 404].includes(e.jqXHR.status)) {
197181
const icon = iconHTML(e.jqXHR.status === 403 ? "lock" : "trash-can");
198182
blockQuote.innerHTML = `<div class='expanded-quote icon-only'>${icon}</div>`;
199-
} else {
200-
popupAjaxError(e);
201183
}
202184
}
203185
} else {

‎app/assets/javascripts/discourse/app/models/post.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,12 @@ export default class Post extends RestModel {
192192
@trackedPostProperty post_number;
193193
@trackedPostProperty post_type;
194194
@trackedPostProperty primary_group_name;
195-
@trackedPostProperty quoted;
196195
@trackedPostProperty read;
197196
@trackedPostProperty reply_count;
198197
@trackedPostProperty reply_to_user;
199198
@trackedPostProperty staff;
200199
@trackedPostProperty staged;
201200
@trackedPostProperty title_is_group;
202-
@trackedPostProperty topic_id;
203-
@trackedPostProperty topic;
204201
@trackedPostProperty trust_level;
205202
@trackedPostProperty updated_at;
206203
@trackedPostProperty user;

‎app/assets/javascripts/discourse/tests/acceptance/topic-test.js

Lines changed: 0 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -666,12 +666,6 @@ import { i18n } from "discourse-i18n";
666666
topicFixtures["/t/280/1.json"].post_stream.posts[2]
667667
)
668668
);
669-
server.get("/posts/by_number/280/5", () =>
670-
helper.response(
671-
200,
672-
topicFixtures["/t/280/1.json"].post_stream.posts[4]
673-
)
674-
);
675669
});
676670

677671
test("The quoted content is toggled correclty", async function (assert) {
@@ -717,98 +711,6 @@ import { i18n } from "discourse-i18n";
717711
)
718712
.hasAttribute("aria-expanded", "false");
719713
});
720-
721-
// TODO (glimmer-post-stream): this test only works with the Glimmer Post Stream.
722-
// When the removing the legacy code we should remove the `if` and run it unconditionally.
723-
if (postStreamMode === "enabled") {
724-
test("Nesting quoted content works", async function (assert) {
725-
await visit("/t/internationalization-localization/280");
726-
727-
// outer quote
728-
assert
729-
.dom(
730-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] > .title .quote-controls .quote-toggle"
731-
)
732-
.hasAttribute("aria-expanded", "false");
733-
734-
assert
735-
.dom("#post_7 .quote[data-topic='280'][data-post='5']")
736-
.includesText("The problem I see here")
737-
.doesNotIncludeText("So you could replace that lookup table");
738-
739-
await click(
740-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] > .title .quote-controls .quote-toggle"
741-
);
742-
743-
assert
744-
.dom(
745-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] > .title .quote-controls .quote-toggle"
746-
)
747-
.hasAttribute("aria-expanded", "true");
748-
assert
749-
.dom("#post_7 .quote[data-topic='280'][data-post='5']")
750-
.includesText("The problem I see here")
751-
.includesText("So you could replace that lookup table");
752-
753-
// nested quote
754-
assert
755-
.dom(
756-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3'] .quote-controls .quote-toggle"
757-
)
758-
.hasAttribute("aria-expanded", "false");
759-
assert
760-
.dom(
761-
"#post_7 .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3']"
762-
)
763-
.includesText(
764-
'So you could replace that lookup table with the "de" one to get German.'
765-
)
766-
.doesNotIncludeText(
767-
"Yep, all strings are going through a lookup table.*"
768-
);
769-
770-
await click(
771-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3'] .quote-controls .quote-toggle"
772-
);
773-
774-
assert
775-
.dom(
776-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3'] .quote-controls .quote-toggle"
777-
)
778-
.hasAttribute("aria-expanded", "true");
779-
assert
780-
.dom(
781-
"#post_7 .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3']"
782-
)
783-
.includesText(
784-
'So you could replace that lookup table with the "de" one to get German.'
785-
)
786-
.includesText(
787-
"Yep, all strings are going through a lookup table.*"
788-
);
789-
790-
await click(
791-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3'] .quote-controls .quote-toggle"
792-
);
793-
794-
assert
795-
.dom(
796-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote[data-topic='280'][data-post='3'] .quote-controls .quote-toggle"
797-
)
798-
.hasAttribute("aria-expanded", "false");
799-
800-
// outer quote
801-
await click(
802-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote-controls .quote-toggle"
803-
);
804-
805-
assert
806-
.dom(
807-
"#post_7 .cooked .quote[data-topic='280'][data-post='5'] .quote-controls .quote-toggle"
808-
)
809-
.hasAttribute("aria-expanded", "false");
810-
});
811-
}
812714
}
813715
);
814716

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,7 @@ export default {
874874
uploaded_avatar_id: 5253,
875875
created_at: "2013-02-07T14:08:17.493Z",
876876
cooked:
877-
'<aside class="quote" data-post="5" data-topic="280"><div class="title">\n<div class="quote-controls"></div>\n<img width="20" height="20" src="/user_avatar/meta.discourse.org/sam/40/5243.png" class="avatar">pekka said:</div>\n<blockquote><p>The problem I see here</p></blockquote></aside><p>Looks interesting, I\'ll take a peek.</p>\n\n<p>As said on dev, the best tool I can see in terms of giving translators a proper interface <em>and</em> quality control would be something like <a href="http://translate.wordpress.org/projects/bbpress/dev" rel="nofollow">GlotPress</a>. It\'s based on the PO messages format (is that somehow related to ICU?) but looks pretty great.</p>\n\n<p><aside class="quote" data-post="6" data-topic="280"><div class="title">\n<div class="quote-controls"></div>\n<img width="20" height="20" src="/user_avatar/meta.discourse.org/sam/40/5243.png" class="avatar">sam said:</div>\n<blockquote><p>fuzzy matching for localization</p></blockquote></aside></p>\n\n<p>I\'m not familiar with the term in this context, you mean keeping the English version in the code base (instead of a generic code like <code>message_error_nametooshort</code> ?)</p>',
877+
'<p>Looks interesting, I\'ll take a peek.</p>\n\n<p>As said on dev, the best tool I can see in terms of giving translators a proper interface <em>and</em> quality control would be something like <a href="http://translate.wordpress.org/projects/bbpress/dev" rel="nofollow">GlotPress</a>. It\'s based on the PO messages format (is that somehow related to ICU?) but looks pretty great.</p>\n\n<p><aside class="quote" data-post="6" data-topic="280"><div class="title">\n<div class="quote-controls"></div>\n<img width="20" height="20" src="/user_avatar/meta.discourse.org/sam/40/5243.png" class="avatar">sam said:</div>\n<blockquote><p>fuzzy matching for localization</p></blockquote></aside></p>\n\n<p>I\'m not familiar with the term in this context, you mean keeping the English version in the code base (instead of a generic code like <code>message_error_nametooshort</code> ?)</p>',
878878
post_number: 7,
879879
post_type: 1,
880880
updated_at: "2013-02-07T14:12:02.965Z",

0 commit comments

Comments
 (0)