Skip to content

Commit 3d3c3ba

Browse files
tulirtulir
authored
Fix m.topic format (#4984)
* Fix m.topic format Fixes #4902 Signed-off-by: Tulir Asokan <[email protected]> * Update tests Signed-off-by: Tulir Asokan <[email protected]> * Fix formatting * Re-add temporary support for invalid form Signed-off-by: Tulir Asokan <[email protected]> --------- Signed-off-by: Tulir Asokan <[email protected]>
1 parent d62c658 commit 3d3c3ba

File tree

3 files changed

+85
-41
lines changed

3 files changed

+85
-41
lines changed

‎spec/unit/content-helpers.spec.ts‎

Lines changed: 70 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -183,39 +183,43 @@ describe("Topic content helpers", () => {
183183
it("creates fully defined event content without html", () => {
184184
expect(makeTopicContent("pizza")).toEqual({
185185
topic: "pizza",
186-
[M_TOPIC.name]: [
187-
{
188-
body: "pizza",
189-
mimetype: "text/plain",
190-
},
191-
],
186+
[M_TOPIC.name]: {
187+
"m.text": [
188+
{
189+
body: "pizza",
190+
mimetype: "text/plain",
191+
},
192+
],
193+
},
192194
});
193195
});
194196

195197
it("creates fully defined event content with html", () => {
196198
expect(makeTopicContent("pizza", "<b>pizza</b>")).toEqual({
197199
topic: "pizza",
198-
[M_TOPIC.name]: [
199-
{
200-
body: "<b>pizza</b>",
201-
mimetype: "text/html",
202-
},
203-
{
204-
body: "pizza",
205-
mimetype: "text/plain",
206-
},
207-
],
200+
[M_TOPIC.name]: {
201+
"m.text": [
202+
{
203+
body: "<b>pizza</b>",
204+
mimetype: "text/html",
205+
},
206+
{
207+
body: "pizza",
208+
mimetype: "text/plain",
209+
},
210+
],
211+
},
208212
});
209213
});
210214

211215
it("creates an empty event when the topic is falsey", () => {
212216
expect(makeTopicContent(undefined)).toEqual({
213217
topic: undefined,
214-
[M_TOPIC.name]: [],
218+
[M_TOPIC.name]: { "m.text": [] },
215219
});
216220
expect(makeTopicContent(null)).toEqual({
217221
topic: null,
218-
[M_TOPIC.name]: [],
222+
[M_TOPIC.name]: { "m.text": [] },
219223
});
220224
});
221225
});
@@ -225,11 +229,13 @@ describe("Topic content helpers", () => {
225229
expect(
226230
parseTopicContent({
227231
topic: "pizza",
228-
[M_TOPIC.name]: [
229-
{
230-
body: "pizza",
231-
},
232-
],
232+
[M_TOPIC.name]: {
233+
"m.text": [
234+
{
235+
body: "pizza",
236+
},
237+
],
238+
},
233239
}),
234240
).toEqual({
235241
text: "pizza",
@@ -240,12 +246,14 @@ describe("Topic content helpers", () => {
240246
expect(
241247
parseTopicContent({
242248
topic: "pizza",
243-
[M_TOPIC.name]: [
244-
{
245-
body: "pizza",
246-
mimetype: "text/plain",
247-
},
248-
],
249+
[M_TOPIC.name]: {
250+
"m.text": [
251+
{
252+
body: "pizza",
253+
mimetype: "text/plain",
254+
},
255+
],
256+
},
249257
}),
250258
).toEqual({
251259
text: "pizza",
@@ -256,12 +264,14 @@ describe("Topic content helpers", () => {
256264
expect(
257265
parseTopicContent({
258266
topic: "pizza",
259-
[M_TOPIC.name]: [
260-
{
261-
body: "<b>pizza</b>",
262-
mimetype: "text/html",
263-
},
264-
],
267+
[M_TOPIC.name]: {
268+
"m.text": [
269+
{
270+
body: "<b>pizza</b>",
271+
mimetype: "text/html",
272+
},
273+
],
274+
},
265275
}),
266276
).toEqual({
267277
text: "pizza",
@@ -279,11 +289,34 @@ describe("Topic content helpers", () => {
279289
});
280290
});
281291

282-
it("uses legacy event content when new topic key is invalid", () => {
292+
// TODO delete this test and re-enable the next one after support for the invalid form is removed
293+
// https://github.com/matrix-org/matrix-js-sdk/pull/4984#pullrequestreview-3174251065
294+
it("parses malformed event content with html topic", () => {
295+
expect(
296+
parseTopicContent({
297+
"topic": "pizza",
298+
"m.topic": [
299+
{
300+
body: "<b>pizza</b>",
301+
mimetype: "text/html",
302+
},
303+
] as any,
304+
}),
305+
).toEqual({
306+
text: "pizza",
307+
html: "<b>pizza</b>",
308+
});
309+
});
310+
it.skip("uses legacy event content when new topic key is invalid", () => {
283311
expect(
284312
parseTopicContent({
285313
"topic": "pizza",
286-
"m.topic": {} as any,
314+
"m.topic": [
315+
{
316+
body: "<b>pizza</b>",
317+
mimetype: "text/html",
318+
},
319+
] as any,
287320
}),
288321
).toEqual({
289322
text: "pizza",

‎src/@types/topic.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,19 @@ export const M_TOPIC = new NamespacedValue("m.topic");
4747
/**
4848
* The event content for an m.topic event (in content)
4949
*/
50-
export type MTopicContent = IMessageRendering[];
50+
export type MTopicContent = { "m.text": IMessageRendering[] };
51+
52+
/**
53+
* A previous incorrect form of m.topic used by matrix-js-sdk
54+
* TODO remove this after a few releases
55+
* https://github.com/matrix-org/matrix-js-sdk/pull/4984#pullrequestreview-3174251065
56+
*/
57+
export type MalformedMTopicEvent = { "m.topic": IMessageRendering[] };
5158

5259
/**
5360
* The event definition for an m.topic event (in content)
5461
*/
55-
export type MTopicEvent = { "m.topic": MTopicContent };
62+
export type MTopicEvent = { "m.topic": MTopicContent } | MalformedMTopicEvent;
5663

5764
/**
5865
* The event content for an m.room.topic event

‎src/content-helpers.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ export const makeTopicContent: MakeTopicContent = (topic, htmlTopic) => {
197197
if (isProvided(topic)) {
198198
renderings.push({ body: topic, mimetype: "text/plain" });
199199
}
200-
return { topic, [M_TOPIC.name]: renderings };
200+
return { topic, [M_TOPIC.name]: { "m.text": renderings } };
201201
};
202202

203203
export type TopicState = {
@@ -206,7 +206,11 @@ export type TopicState = {
206206
};
207207

208208
export const parseTopicContent = (content: MRoomTopicEventContent): TopicState => {
209-
const mtopic = M_TOPIC.findIn<MTopicContent>(content);
209+
const mtopicParent = M_TOPIC.findIn<MTopicContent>(content);
210+
const mtopic = Array.isArray(mtopicParent) ? mtopicParent : mtopicParent?.["m.text"];
211+
// TODO remove support for the old malformed m.topic arrays after a few releases (only allow array in m.text)
212+
// https://github.com/matrix-org/matrix-js-sdk/pull/4984#pullrequestreview-3174251065
213+
//const mtopic = M_TOPIC.findIn<MTopicContent>(content)?.["m.text"];
210214
if (!Array.isArray(mtopic)) {
211215
return { text: content.topic ?? undefined };
212216
}

0 commit comments

Comments
 (0)