Room list: add robustness to custom section loading (#33475)

* refactor: move all access to custom sections settings into `section.ts`

* fix: add robustness when getting the order list of custom sections

* fix: add robustness when getting the custom section data

* fix: ignore malformed section but don't erase them

* fix: remove useless await operator

* test: add more tests

apps/web/src/stores/room-list-v3/RoomListStoreV3.ts

@@ -40,7 +40,7 @@ import { DefaultTagID } from "./skip-list/tag";
 import { ExcludeTagsFilter } from "./skip-list/filters/ExcludeTagsFilter";
 import { TagFilter } from "./skip-list/filters/TagFilter";
 import { filterBoolean } from "../../utils/arrays";
-import { CHATS_TAG, createSection, deleteSection, editSection } from "./section";
+import { CHATS_TAG, createSection, deleteSection, editSection, getOrderedCustomSections } from "./section";
 
 /**
  * These are the filters passed to the room skip list.
@@ -522,7 +522,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient<EmptyObject> {
      * Load the custom sections from the settings store and update the sorted tags.
      */
     private loadCustomSections(): void {
-        const orderedCustomSections = SettingsStore.getValue("RoomList.OrderedCustomSections");
+        const orderedCustomSections = getOrderedCustomSections();
         this.sortedTags = [DefaultTagID.Favourite, ...orderedCustomSections, CHATS_TAG, DefaultTagID.LowPriority];
     }
 }

apps/web/src/stores/room-list-v3/section.ts

@@ -14,8 +14,6 @@ import { CreateSectionDialog } from "../../components/views/dialogs/CreateSectio
 import { RemoveSectionDialog } from "../../components/views/dialogs/RemoveSectionDialog";
 import { DefaultTagID, type TagID } from "./skip-list/tag";
 
-type Tag = string;
-
 /**
  * A synthetic tag used to represent the "Chats" section, which contains
  * every room that does not belong to any other explicit tag section.
@@ -27,12 +25,14 @@ export const CHATS_TAG = "chats";
  */
 export const CUSTOM_SECTION_TAG_PREFIX = "element.io.section.";
 
+type CustomTag = `${typeof CUSTOM_SECTION_TAG_PREFIX}${string}`;
+
 /**
  * Checks if a given tag is a custom section tag.
  * @param tag - The tag to check.
  * @returns True if the tag is a custom section tag, false otherwise.
  */
-export function isCustomSectionTag(tag: string): boolean {
+export function isCustomSectionTag(tag: string): tag is CustomTag {
     return tag.startsWith(CUSTOM_SECTION_TAG_PREFIX);
 }
 
@@ -58,18 +58,56 @@ export function isSectionTag(tagId: TagID): boolean {
  * Structure of the custom section stored in the settings. The tag is used as a unique identifier for the section, and the name is given by the user.
  */
 type CustomSection = {
-    tag: Tag;
+    tag: CustomTag;
     name: string;
 };
 
+/**
+ * Type guard to check if a value is a valid CustomSection object.
+ */
+function isValidCustomSection(value: unknown): value is CustomSection {
+    return (
+        typeof value === "object" &&
+        value !== null &&
+        isCustomSectionTag((value as Record<string, unknown>).tag as string) &&
+        typeof (value as Record<string, unknown>).name === "string"
+    );
+}
+
 /**
  * The custom sections data is stored as a record in the settings, where the key is the section tag and the value is the section data (name and tag).
  */
-export type CustomSectionsData = Record<Tag, CustomSection>;
+export type CustomSectionsData = Record<CustomTag, CustomSection>;
 /**
  * Ordered list of custom section tags.
  */
-export type OrderedCustomSections = Tag[];
+export type OrderedCustomSections = CustomTag[];
+
+/**
+ * Retrieves the custom sections data from the settings.
+ * Invalid or malformed entries are dropped and the cleaned data is persisted back to settings.
+ */
+export function getCustomSectionData(): CustomSectionsData {
+    const raw = SettingsStore.getValue("RoomList.CustomSectionData");
+    // Data are malformed
+    if (typeof raw !== "object" || raw === null || Array.isArray(raw)) return {};
+
+    // Filter out invalid entries
+    return Object.fromEntries(
+        Object.entries(raw).filter(([key, value]) => isValidCustomSection(value) && value.tag === key),
+    ) as CustomSectionsData;
+}
+
+/**
+ * Retrieves the ordered list of custom section tags from the settings.
+ * If the settings contain tags that are not present in the custom section data, they will be filtered out and the settings will be updated to remove the unknown tags.
+ */
+export function getOrderedCustomSections(): OrderedCustomSections {
+    const sectionData = getCustomSectionData();
+    const rawValue = SettingsStore.getValue("RoomList.OrderedCustomSections");
+    const orderedSections: OrderedCustomSections = Array.isArray(rawValue) ? rawValue : [];
+    return orderedSections.filter((tag) => tag in sectionData);
+}
 
 /**
  * Creates a new custom section by showing a dialog to the user to enter the section name.
@@ -83,16 +121,16 @@ export async function createSection(): Promise<string | undefined> {
     const [shouldCreateSection, sectionName] = await modal.finished;
     if (!shouldCreateSection || !sectionName) return undefined;
 
-    const tag = `${CUSTOM_SECTION_TAG_PREFIX}${window.crypto.randomUUID()}`;
+    const tag: CustomTag = `${CUSTOM_SECTION_TAG_PREFIX}${window.crypto.randomUUID()}`;
     const newSection: CustomSection = { tag, name: sectionName };
 
     // Save the new section data
-    const sectionData = SettingsStore.getValue("RoomList.CustomSectionData") || {};
+    const sectionData = getCustomSectionData();
     sectionData[tag] = newSection;
     await SettingsStore.setValue("RoomList.CustomSectionData", null, SettingLevel.ACCOUNT, sectionData);
 
     // Add the new section to the ordered list of sections
-    const orderedSections = SettingsStore.getValue("RoomList.OrderedCustomSections") || [];
+    const orderedSections = getOrderedCustomSections();
     orderedSections.push(tag);
     await SettingsStore.setValue("RoomList.OrderedCustomSections", null, SettingLevel.ACCOUNT, orderedSections);
     return tag;
@@ -103,7 +141,11 @@ export async function createSection(): Promise<string | undefined> {
  * @param tag - The tag of the section to edit.
  */
 export async function editSection(tag: string): Promise<void> {
-    const sectionData = SettingsStore.getValue("RoomList.CustomSectionData") || {};
+    if (!isCustomSectionTag(tag)) {
+        logger.info("Unknown section tag, cannot edit section", tag);
+        return;
+    }
+    const sectionData = getCustomSectionData();
     const section = sectionData[tag];
     if (!section) {
         logger.info("Unknown section tag, cannot edit section", tag);
@@ -127,7 +169,11 @@ export async function editSection(tag: string): Promise<void> {
  * @param isEmpty - Whether the section is empty (has no rooms). If the section is not empty, the confirmation dialog will show a warning message.
  */
 export async function deleteSection(tag: string, isEmpty: boolean): Promise<void> {
-    const sectionData = SettingsStore.getValue("RoomList.CustomSectionData");
+    if (!isCustomSectionTag(tag)) {
+        logger.info("Unknown section tag, cannot delete section", tag);
+        return;
+    }
+    const sectionData = getCustomSectionData();
     if (!sectionData[tag]) {
         logger.info("Unknown section tag, cannot delete section", tag);
         return;
@@ -138,8 +184,7 @@ export async function deleteSection(tag: string, isEmpty: boolean): Promise<void
     if (!shouldRemoveSection) return;
 
     // Remove the section from the ordered list of sections
-    const orderedSections = SettingsStore.getValue("RoomList.OrderedCustomSections");
+    const newOrderedSections = getOrderedCustomSections().filter((sectionTag) => sectionTag !== tag);
-    const newOrderedSections = orderedSections.filter((sectionTag) => sectionTag !== tag);
     await SettingsStore.setValue("RoomList.OrderedCustomSections", null, SettingLevel.ACCOUNT, newOrderedSections);
 
     // Remove the section data

apps/web/src/viewmodels/room-list/RoomListItemViewModel.ts

@@ -39,8 +39,8 @@ import type { ViewRoomPayload } from "../../dispatcher/payloads/ViewRoomPayload"
 import PosthogTrackers from "../../PosthogTrackers";
 import { type Call, CallEvent } from "../../models/Call";
 import RoomListStoreV3 from "../../stores/room-list-v3/RoomListStoreV3";
+import { getCustomSectionData, isDefaultSectionTag } from "../../stores/room-list-v3/section";
 import { _t } from "../../languageHandler";
-import { isDefaultSectionTag } from "../../stores/room-list-v3/section";
 
 interface RoomItemProps {
     room: Room;
@@ -424,7 +424,7 @@ export class RoomListItemViewModel
      * Order follows the canonical section order from RoomListStoreV3.
      */
     private static buildSections(roomTags: Room["tags"]): Section[] {
-        const customSectionData = SettingsStore.getValue("RoomList.CustomSectionData") || {};
+        const customSectionData = getCustomSectionData();
 
         return (
             RoomListStoreV3.instance.orderedSectionTags

apps/web/src/viewmodels/room-list/RoomListSectionHeaderViewModel.ts

@@ -17,7 +17,7 @@ import { NotificationStateEvents } from "../../stores/notifications/Notification
 import { type RoomNotificationState } from "../../stores/notifications/RoomNotificationState";
 import SettingsStore from "../../settings/SettingsStore";
 import RoomListStoreV3 from "../../stores/room-list-v3/RoomListStoreV3";
-import { isDefaultSectionTag } from "../../stores/room-list-v3/section";
+import { getCustomSectionData, isCustomSectionTag, isDefaultSectionTag } from "../../stores/room-list-v3/section";
 
 interface RoomListSectionHeaderViewModelProps {
     tag: string;
@@ -139,8 +139,7 @@ export class RoomListSectionHeaderViewModel
      * Handle changes to custom section data.
      */
     private onCustomSectionDataChange(): void {
-        const customSectionData = SettingsStore.getValue("RoomList.CustomSectionData") || {};
+        const sectionData = isCustomSectionTag(this.props.tag) ? getCustomSectionData()[this.props.tag] : undefined;
-        const sectionData = customSectionData[this.props.tag];
         if (sectionData) {
             this.snapshot.merge({ title: sectionData.name });
         }

apps/web/src/viewmodels/room-list/RoomListViewModel.ts

@@ -36,10 +36,10 @@ import { hasCreateRoomRights } from "./utils";
 import { keepIfSame } from "../../utils/keepIfSame";
 import { DefaultTagID } from "../../stores/room-list-v3/skip-list/tag";
 import { RoomListSectionHeaderViewModel } from "./RoomListSectionHeaderViewModel";
+import { getCustomSectionData, isCustomSectionTag, CHATS_TAG } from "../../stores/room-list-v3/section";
 import SettingsStore from "../../settings/SettingsStore";
 import { tagRoom } from "../../utils/room/tagRoom";
 import { getSectionTagForRoom } from "../../utils/room/getSectionTagForRoom";
-import { CHATS_TAG } from "../../stores/room-list-v3/section";
 
 /**
  * Tracks the position of the active room within a specific section.
@@ -287,8 +287,7 @@ export class RoomListViewModel
     public getSectionHeaderViewModel(tag: string): RoomListSectionHeaderViewModel {
         if (this.roomSectionHeaderViewModels.has(tag)) return this.roomSectionHeaderViewModels.get(tag)!;
 
-        const customSections = SettingsStore.getValue("RoomList.CustomSectionData");
+        const title = TAG_TO_TITLE_MAP[tag] || (isCustomSectionTag(tag) && getCustomSectionData()[tag]?.name) || tag;
-        const title = TAG_TO_TITLE_MAP[tag] || customSections[tag]?.name || tag;
         const viewModel = new RoomListSectionHeaderViewModel({
             tag,
             title,
@@ -692,11 +691,13 @@ function computeSections(
     roomsResult: RoomsResult,
     isSectionExpanded: (tag: string) => boolean,
 ): { sections: Section[]; isFlatList: boolean } {
-    const customSections = SettingsStore.getValue("RoomList.CustomSectionData");
+    const customSections = getCustomSectionData();
 
     const sections = roomsResult.sections
         // Only include sections that have rooms or are custom sections (which may be empty but should still be shown)
-        .filter((section) => section.rooms.length > 0 || customSections[section.tag])
+        .filter(
+            (section) => section.rooms.length > 0 || (isCustomSectionTag(section.tag) && customSections[section.tag]),
+        )
         // Remove roomIds for sections that are currently collapsed according to their section header view model
         .map((section) => ({
             ...section,

apps/web/test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts

@@ -833,6 +833,7 @@ describe("RoomListStoreV3", () => {
             jest.spyOn(SettingsStore, "getValue").mockImplementation((setting: string) => {
                 if (setting === "feature_room_list_sections") return true;
                 if (setting === "RoomList.OrderedCustomSections") return [];
+                if (setting === "RoomList.CustomSectionData") return {};
                 return false;
             });
         }
@@ -1093,6 +1094,7 @@ describe("RoomListStoreV3", () => {
             jest.spyOn(SettingsStore, "getValue").mockImplementation((setting: string) => {
                 if (setting === "feature_room_list_sections") return true;
                 if (setting === "RoomList.OrderedCustomSections") return [];
+                if (setting === "RoomList.CustomSectionData") return {};
                 return false;
             });
 
@@ -1107,11 +1109,13 @@ describe("RoomListStoreV3", () => {
             jest.spyOn(SettingsStore, "getValue").mockImplementation((setting: string) => {
                 if (setting === "feature_room_list_sections") return true;
                 if (setting === "RoomList.OrderedCustomSections") return [customTag];
+                if (setting === "RoomList.CustomSectionData")
+                    return { [customTag]: { tag: customTag, name: "Custom" } };
                 return false;
             });
 
             // Trigger the settings watcher
-            settingsWatcher("RoomList.OrderedCustomSections");
+            await Promise.resolve(settingsWatcher("RoomList.OrderedCustomSections"));
 
             // Now there should be 4 sections (Favourite, custom, Chats, LowPriority)
             expect(store.getSortedRoomsInActiveSpace().sections).toHaveLength(4);

apps/web/test/unit-tests/stores/room-list-v3/section-test.ts

@@ -8,12 +8,14 @@
 import Modal from "../../../../src/Modal";
 import SettingsStore from "../../../../src/settings/SettingsStore";
 import {
-    CHATS_TAG,
-    CUSTOM_SECTION_TAG_PREFIX,
     createSection,
     editSection,
     deleteSection,
+    getCustomSectionData,
+    getOrderedCustomSections,
     isDefaultSectionTag,
+    CHATS_TAG,
+    CUSTOM_SECTION_TAG_PREFIX,
     isSectionTag,
 } from "../../../../src/stores/room-list-v3/section";
 import { CreateSectionDialog } from "../../../../src/components/views/dialogs/CreateSectionDialog";
@@ -25,6 +27,68 @@ describe("section", () => {
         jest.restoreAllMocks();
     });
 
+    describe("getCustomSectionData", () => {
+        const validTag = "element.io.section.valid";
+        const invalidTag = "element.io.section.invalid";
+        const validEntry = { tag: validTag, name: "Valid" };
+
+        beforeEach(() => {
+            jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
+        });
+
+        it.each([null, false, 42, "string", []] as const)("returns an empty object when the raw value is %p", (raw) => {
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue(raw as any);
+            expect(getCustomSectionData()).toEqual({});
+        });
+
+        it("returns valid entries and drops invalid ones", () => {
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue({
+                [validTag]: validEntry,
+                [invalidTag]: { tag: "element.io.section.mismatch", name: "Bad" },
+            });
+            expect(getCustomSectionData()).toEqual({ [validTag]: validEntry });
+        });
+
+        it("drops entries that fail the isValidCustomSection check", () => {
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue({
+                "element.io.section.null-val": null,
+                "element.io.section.str-val": "not-an-object",
+                "element.io.section.bad-tag": { tag: "not-a-custom-tag", name: "Bad" },
+                "element.io.section.bad-name": { tag: "element.io.section.bad-name", name: 42 },
+            });
+            expect(getCustomSectionData()).toEqual({});
+        });
+    });
+
+    describe("getOrderedCustomSections", () => {
+        const tag = "element.io.section.abc";
+
+        beforeEach(() => {
+            jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
+        });
+
+        it("returns an empty array when the raw value is not an array", () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
+                if (setting === "RoomList.OrderedCustomSections") return "not-an-array";
+                return null;
+            });
+
+            const result = getOrderedCustomSections();
+            expect(result).toEqual([]);
+        });
+
+        it("removes unknown sections and saves the cleaned list", () => {
+            const knownTag = "element.io.section.known";
+            jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
+                if (setting === "RoomList.CustomSectionData") return { [knownTag]: { tag: knownTag, name: "Known" } };
+                if (setting === "RoomList.OrderedCustomSections") return [knownTag, tag];
+                return null;
+            });
+
+            expect(getOrderedCustomSections()).toEqual([knownTag]);
+        });
+    });
+
     describe("createSection", () => {
         beforeEach(() => {
             jest.spyOn(SettingsStore, "getValue").mockReturnValue(null);
@@ -69,6 +133,8 @@ describe("section", () => {
             const existingTag = "element.io.section.existing";
             jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
                 if (setting === "RoomList.OrderedCustomSections") return [existingTag];
+                if (setting === "RoomList.CustomSectionData")
+                    return { [existingTag]: { tag: existingTag, name: "Existing" } };
                 return null;
             });
             jest.spyOn(Modal, "createDialog").mockReturnValue({
@@ -79,15 +145,16 @@ describe("section", () => {
 
             await createSection();
 
-            const customDataCall = setValueSpy.mock.calls.find(([name]) => name === "RoomList.CustomSectionData");
-            const savedSection = Object.values(customDataCall![3] as Record<string, { tag: string; name: string }>)[0];
-            expect(savedSection.name).toBe("My Section");
-            expect(savedSection.tag).toMatch(/^element\.io\.section\./);
-
             const orderedCall = setValueSpy.mock.calls.find(([name]) => name === "RoomList.OrderedCustomSections");
             const savedOrder = orderedCall![3] as string[];
             expect(savedOrder[0]).toBe(existingTag);
             expect(savedOrder[1]).toMatch(/^element\.io\.section\./);
+
+            const newTag = savedOrder[1];
+            const customDataCall = setValueSpy.mock.calls.find(([name]) => name === "RoomList.CustomSectionData");
+            const savedSection = (customDataCall![3] as Record<string, { tag: string; name: string }>)[newTag];
+            expect(savedSection.name).toBe("My Section");
+            expect(savedSection.tag).toBe(newTag);
         });
     });
 
@@ -100,6 +167,12 @@ describe("section", () => {
             jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
         });
 
+        it("does nothing if the tag is not a custom section tag", async () => {
+            const createDialogSpy = jest.spyOn(Modal, "createDialog");
+            await editSection("m.favourite");
+            expect(createDialogSpy).not.toHaveBeenCalled();
+        });
+
         it("does nothing if the section does not exist", async () => {
             jest.spyOn(SettingsStore, "getValue").mockReturnValue({});
             const createDialogSpy = jest.spyOn(Modal, "createDialog");
@@ -157,13 +230,20 @@ describe("section", () => {
 
         beforeEach(() => {
             jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
-                if (setting === "RoomList.CustomSectionData") return { [tag]: { tag, name: "My Section" } };
+                if (setting === "RoomList.CustomSectionData")
+                    return { [tag]: { tag, name: "My Section" }, [otherTag]: { tag: otherTag, name: "Other Section" } };
                 if (setting === "RoomList.OrderedCustomSections") return [otherTag, tag];
                 return null;
             });
             jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
         });
 
+        it("does nothing if the tag is not a custom section tag", async () => {
+            const createDialogSpy = jest.spyOn(Modal, "createDialog");
+            await deleteSection("m.favourite", false);
+            expect(createDialogSpy).not.toHaveBeenCalled();
+        });
+
         it("does nothing if the section does not exist", async () => {
             jest.spyOn(SettingsStore, "getValue").mockReturnValue({});
             const createDialogSpy = jest.spyOn(Modal, "createDialog");

apps/web/test/viewmodels/room-list/RoomListItemViewModel-test.tsx

@@ -80,8 +80,10 @@ describe("RoomListItemViewModel", () => {
         jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
             if (setting === "RoomList.showMessagePreview") return false;
             if (setting === "RoomList.OrderedCustomSections") return [];
+            if (setting === "RoomList.CustomSectionData") return {};
             return false;
         });
+        jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
         jest.spyOn(SettingsStore, "watchSetting").mockImplementation(() => "watcher-id");
 
         jest.spyOn(MessagePreviewStore.instance, "getPreviewForRoom").mockResolvedValue(null);

apps/web/test/viewmodels/room-list/RoomListSectionHeaderViewModel-test.ts

@@ -154,6 +154,19 @@ describe("RoomListSectionHeaderViewModel", () => {
 
             expect(vm.getSnapshot().title).toBe("My Section");
         });
+
+        it("should not update title when tag is not a custom section tag", () => {
+            const vm = new RoomListSectionHeaderViewModel({
+                tag: "m.favourite",
+                title: "Favourites",
+                spaceId: "!space:server",
+                onToggleExpanded,
+            });
+
+            watchCallback();
+
+            expect(vm.getSnapshot().title).toBe("Favourites");
+        });
     });
 
     describe("editSection", () => {