Handle empty m.room.topic (#4673)

* Define topic as optional.

* Change isProvided so that types work better.

* allow makeTopicContent and parseTopicContent to handle optional values for plain text

* linting

* Remove usage of optional

* Topic key may only contain legacy key.

* Add tests for other branches.

spec/unit/content-helpers.spec.ts

@@ -207,6 +207,17 @@ describe("Topic content helpers", () => {
                 ],
             });
         });
+
+        it("creates an empty event when the topic is falsey", () => {
+            expect(makeTopicContent(undefined)).toEqual({
+                topic: undefined,
+                [M_TOPIC.name]: [],
+            });
+            expect(makeTopicContent(null)).toEqual({
+                topic: null,
+                [M_TOPIC.name]: [],
+            });
+        });
     });
 
     describe("parseTopicContent()", () => {
@@ -257,5 +268,26 @@ describe("Topic content helpers", () => {
                 html: "<b>pizza</b>",
             });
         });
+
+        it("parses legacy event content", () => {
+            expect(
+                parseTopicContent({
+                    topic: "pizza",
+                }),
+            ).toEqual({
+                text: "pizza",
+            });
+        });
+
+        it("uses legacy event content when new topic key is invalid", () => {
+            expect(
+                parseTopicContent({
+                    "topic": "pizza",
+                    "m.topic": {} as any,
+                }),
+            ).toEqual({
+                text: "pizza",
+            });
+        });
     });
 });

src/@types/state_events.ts

@@ -90,7 +90,7 @@ export interface RoomNameEventContent {
 }
 
 export interface RoomTopicEventContent {
-    topic: string;
+    topic: string | undefined | null;
 }
 
 export interface RoomAvatarEventContent {

src/@types/topic.ts

@@ -60,4 +60,4 @@ export type MTopicEvent = EitherAnd<{ [M_TOPIC.name]: MTopicContent }, { [M_TOPI
 /**
  * The event content for an m.room.topic event
  */
-export type MRoomTopicEventContent = { topic: string } & MTopicEvent;
+export type MRoomTopicEventContent = { topic: string | null | undefined } & (MTopicEvent | {});

src/client.ts

@@ -4490,11 +4490,13 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     }
 
     /**
+     * @param roomId - The room to update the topic in.
+     * @param topic - The plaintext topic. May be empty to remove the topic.
      * @param htmlTopic - Optional.
      * @returns Promise which resolves: TODO
      * @returns Rejects: with an error response.
      */
-    public setRoomTopic(roomId: string, topic: string, htmlTopic?: string): Promise<ISendEventResponse> {
+    public setRoomTopic(roomId: string, topic?: string, htmlTopic?: string): Promise<ISendEventResponse> {
         const content = ContentHelpers.makeTopicContent(topic, htmlTopic);
         return this.sendStateEvent(roomId, EventType.RoomTopic, content);
     }

src/content-helpers.ts

@@ -185,27 +185,31 @@ export const parseLocationEvent = (wireEventContent: LocationEventWireContent):
 /**
  * Topic event helpers
  */
-export type MakeTopicContent = (topic: string, htmlTopic?: string) => MRoomTopicEventContent;
+export type MakeTopicContent = (topic: string | null | undefined, htmlTopic?: string) => MRoomTopicEventContent;
 
 export const makeTopicContent: MakeTopicContent = (topic, htmlTopic) => {
-    const renderings = [{ body: topic, mimetype: "text/plain" }];
+    const renderings = [];
+    if (isProvided(topic)) {
+        renderings.push({ body: topic, mimetype: "text/plain" });
+    }
     if (isProvided(htmlTopic)) {
-        renderings.push({ body: htmlTopic!, mimetype: "text/html" });
+        renderings.push({ body: htmlTopic, mimetype: "text/html" });
     }
     return { topic, [M_TOPIC.name]: renderings };
 };
 
 export type TopicState = {
-    text: string;
+    text?: string;
     html?: string;
 };
 
 export const parseTopicContent = (content: MRoomTopicEventContent): TopicState => {
     const mtopic = M_TOPIC.findIn<MTopicContent>(content);
     if (!Array.isArray(mtopic)) {
-        return { text: content.topic };
+        return { text: content.topic ?? undefined };
     }
-    const text = mtopic?.find((r) => !isProvided(r.mimetype) || r.mimetype === "text/plain")?.body ?? content.topic;
+    const text =
+        mtopic?.find((r) => !isProvided(r.mimetype) || r.mimetype === "text/plain")?.body ?? content.topic ?? undefined;
     const html = mtopic?.find((r) => r.mimetype === "text/html")?.body;
     return { text, html };
 };

src/extensible_events_v1/utilities.ts

@@ -21,7 +21,7 @@ import { Optional } from "matrix-events-sdk";
  * @param s - The optional to test.
  * @returns True if the value is defined.
  */
-export function isProvided<T>(s: Optional<T>): boolean {
+export function isProvided<T>(s: Optional<T>): s is T {
     return s !== null && s !== undefined;
 }