From 755007cbee4a91f59a025b59cd1c342443b01da5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 27 Jul 2021 14:39:14 +0100 Subject: [PATCH] Conclude labs flags and write more tests --- src/settings/Settings.tsx | 16 --- src/stores/SpaceStore.tsx | 27 ++--- .../notifications/SpaceNotificationState.ts | 2 +- test/stores/SpaceStore-setup.ts | 2 - test/stores/SpaceStore-test.ts | 108 ++++++++++++++++-- 5 files changed, 112 insertions(+), 43 deletions(-) diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index f0bdb2e0e5..5aa49df8a1 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -181,8 +181,6 @@ export const SETTINGS: {[setting: string]: ISetting} = { feedbackLabel: "spaces-feedback", extraSettings: [ "feature_spaces.all_rooms", - "feature_spaces.space_member_dms", - "feature_spaces.space_dm_badges", ], }, }, @@ -192,20 +190,6 @@ export const SETTINGS: {[setting: string]: ISetting} = { default: true, controller: new ReloadOnChangeController(), }, - "feature_spaces.space_member_dms": { - displayName: _td("Show people in spaces"), - description: _td("If disabled, you can still add Direct Messages to Personal Spaces. " + - "If enabled, you'll automatically see everyone who is a member of the Space."), - supportedLevels: LEVELS_FEATURE, - default: true, - controller: new ReloadOnChangeController(), - }, - "feature_spaces.space_dm_badges": { - displayName: _td("Show notification badges for People in Spaces"), - supportedLevels: LEVELS_FEATURE, - default: false, - controller: new ReloadOnChangeController(), - }, "feature_dnd": { isFeature: true, displayName: _td("Show options to enable 'Do not disturb' mode"), diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index a338e71838..d064b01257 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -72,8 +72,6 @@ const MAX_SUGGESTED_ROOMS = 20; // All of these settings cause the page to reload and can be costly if read frequently, so read them here only const spacesEnabled = SettingsStore.getValue("feature_spaces"); const spacesTweakAllRoomsEnabled = SettingsStore.getValue("feature_spaces.all_rooms"); -const spacesTweakSpaceMemberDMsEnabled = SettingsStore.getValue("feature_spaces.space_member_dms"); -const spacesTweakSpaceDMBadgesEnabled = SettingsStore.getValue("feature_spaces.space_dm_badges"); const homeSpaceKey = spacesTweakAllRoomsEnabled ? "ALL_ROOMS" : "HOME_SPACE"; const getSpaceContextKey = (space?: Room) => `mx_space_context_${space?.roomId || homeSpaceKey}`; @@ -535,15 +533,13 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const roomIds = new Set(childRooms.map(r => r.roomId)); const space = this.matrixClient?.getRoom(spaceId); - if (spacesTweakSpaceMemberDMsEnabled) { - // Add relevant DMs - space?.getMembers().forEach(member => { - if (member.membership !== "join" && member.membership !== "invite") return; - DMRoomMap.shared().getDMRoomsForUserId(member.userId).forEach(roomId => { - roomIds.add(roomId); - }); + // Add relevant DMs + space?.getMembers().forEach(member => { + if (member.membership !== "join" && member.membership !== "invite") return; + DMRoomMap.shared().getDMRoomsForUserId(member.userId).forEach(roomId => { + roomIds.add(roomId); }); - } + }); const newPath = new Set(parentPath).add(spaceId); childSpaces.forEach(childSpace => { @@ -568,14 +564,13 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.spaceFilteredRooms.forEach((roomIds, s) => { // Update NotificationStates this.getNotificationState(s)?.setRooms(visibleRooms.filter(room => { - if (roomIds.has(room.roomId)) { - if (s !== HOME_SPACE && spacesTweakSpaceDMBadgesEnabled) return true; + if (!roomIds.has(room.roomId)) return false; - return !DMRoomMap.shared().getUserIdForRoomId(room.roomId) - || RoomListStore.instance.getTagsForRoom(room).includes(DefaultTagID.Favourite); + if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { + return s === HOME_SPACE; } - return false; + return true; })); }); }, 100, { trailing: true, leading: true }); @@ -878,8 +873,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { export default class SpaceStore { public static spacesEnabled = spacesEnabled; public static spacesTweakAllRoomsEnabled = spacesTweakAllRoomsEnabled; - public static spacesTweakSpaceMemberDMsEnabled = spacesTweakSpaceMemberDMsEnabled; - public static spacesTweakSpaceDMBadgesEnabled = spacesTweakSpaceDMBadgesEnabled; private static internalInstance = new SpaceStoreClass(); diff --git a/src/stores/notifications/SpaceNotificationState.ts b/src/stores/notifications/SpaceNotificationState.ts index 4c0a582f3f..f8eb07251b 100644 --- a/src/stores/notifications/SpaceNotificationState.ts +++ b/src/stores/notifications/SpaceNotificationState.ts @@ -23,7 +23,7 @@ import { NOTIFICATION_STATE_UPDATE, NotificationState } from "./NotificationStat import { FetchRoomFn } from "./ListNotificationState"; export class SpaceNotificationState extends NotificationState { - private rooms: Room[] = []; + public rooms: Room[] = []; // exposed only for tests private states: { [spaceId: string]: RoomNotificationState } = {}; constructor(private spaceId: string | symbol, private getRoomFn: FetchRoomFn) { diff --git a/test/stores/SpaceStore-setup.ts b/test/stores/SpaceStore-setup.ts index 67d492255f..b9b865e89a 100644 --- a/test/stores/SpaceStore-setup.ts +++ b/test/stores/SpaceStore-setup.ts @@ -19,5 +19,3 @@ limitations under the License. localStorage.setItem("mx_labs_feature_feature_spaces", "true"); localStorage.setItem("mx_labs_feature_feature_spaces.all_rooms", "true"); -localStorage.setItem("mx_labs_feature_feature_spaces.space_member_dms", "true"); -localStorage.setItem("mx_labs_feature_feature_spaces.space_dm_badges", "false"); diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 8855f4e470..d772a7a658 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -17,6 +17,7 @@ limitations under the License. import { EventEmitter } from "events"; import { EventType } from "matrix-js-sdk/src/@types/event"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; +import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import "./SpaceStore-setup"; // enable space lab import "../skinned-sdk"; // Must be first for skinning to work @@ -53,18 +54,22 @@ const emitPromise = (e: EventEmitter, k: string | symbol) => new Promise(r => e. const testUserId = "@test:user"; const getUserIdForRoomId = jest.fn(); +const getDMRoomsForUserId = jest.fn(); // @ts-ignore -DMRoomMap.sharedInstance = { getUserIdForRoomId }; +DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; const fav1 = "!fav1:server"; const fav2 = "!fav2:server"; const fav3 = "!fav3:server"; const dm1 = "!dm1:server"; -const dm1Partner = "@dm1Partner:server"; +const dm1Partner = new RoomMember(dm1, "@dm1Partner:server"); +dm1Partner.membership = "join"; const dm2 = "!dm2:server"; -const dm2Partner = "@dm2Partner:server"; +const dm2Partner = new RoomMember(dm2, "@dm2Partner:server"); +dm2Partner.membership = "join"; const dm3 = "!dm3:server"; -const dm3Partner = "@dm3Partner:server"; +const dm3Partner = new RoomMember(dm3, "@dm3Partner:server"); +dm3Partner.membership = "join"; const orphan1 = "!orphan1:server"; const orphan2 = "!orphan2:server"; const invite1 = "!invite1:server"; @@ -320,11 +325,40 @@ describe("SpaceStore", () => { getUserIdForRoomId.mockImplementation(roomId => { return { - [dm1]: dm1Partner, - [dm2]: dm2Partner, - [dm3]: dm3Partner, + [dm1]: dm1Partner.userId, + [dm2]: dm2Partner.userId, + [dm3]: dm3Partner.userId, }[roomId]; }); + getDMRoomsForUserId.mockImplementation(userId => { + switch (userId) { + case dm1Partner.userId: + return [dm1]; + case dm2Partner.userId: + return [dm2]; + case dm3Partner.userId: + return [dm3]; + default: + return []; + } + }); + + // have dmPartner1 be in space1 with you + const mySpace1Member = new RoomMember(space1, testUserId); + mySpace1Member.membership = "join"; + (rooms.find(r => r.roomId === space1).getMembers as jest.Mock).mockReturnValue([ + mySpace1Member, + dm1Partner, + ]); + // have dmPartner2 be in space2 with you + const mySpace2Member = new RoomMember(space2, testUserId); + mySpace2Member.membership = "join"; + (rooms.find(r => r.roomId === space2).getMembers as jest.Mock).mockReturnValue([ + mySpace2Member, + dm2Partner, + ]); + // dmPartner3 is not in any common spaces with you + await run(); }); @@ -375,6 +409,66 @@ describe("SpaceStore", () => { const space = client.getRoom(space3); expect(store.getSpaceFilteredRoomIds(space).has(invite2)).toBeTruthy(); }); + + it("spaces contain dms which you have with members of that space", () => { + expect(store.getSpaceFilteredRoomIds(client.getRoom(space1)).has(dm1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space2)).has(dm1)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space3)).has(dm1)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space1)).has(dm2)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space2)).has(dm2)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space3)).has(dm2)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space1)).has(dm3)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space2)).has(dm3)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space3)).has(dm3)).toBeFalsy(); + }); + + it("dms are only added to Notification States for only the Home Space", () => { + // XXX: All rooms space is forcibly enabled, as part of a future PR test Home space better + // [dm1, dm2, dm3].forEach(d => { + // expect(store.getNotificationState(HOME_SPACE).rooms.map(r => r.roomId).includes(d)).toBeTruthy(); + // }); + [space1, space2, space3].forEach(s => { + [dm1, dm2, dm3].forEach(d => { + expect(store.getNotificationState(s).rooms.map(r => r.roomId).includes(d)).toBeFalsy(); + }); + }); + }); + + it("orphan rooms are added to Notification States for only the Home Space", () => { + // XXX: All rooms space is forcibly enabled, as part of a future PR test Home space better + // [orphan1, orphan2].forEach(d => { + // expect(store.getNotificationState(HOME_SPACE).rooms.map(r => r.roomId).includes(d)).toBeTruthy(); + // }); + [space1, space2, space3].forEach(s => { + [orphan1, orphan2].forEach(d => { + expect(store.getNotificationState(s).rooms.map(r => r.roomId).includes(d)).toBeFalsy(); + }); + }); + }); + + it("favourites are added to Notification States for all spaces containing the room inc Home", () => { + // XXX: All rooms space is forcibly enabled, as part of a future PR test Home space better + // [fav1, fav2, fav3].forEach(d => { + // expect(store.getNotificationState(HOME_SPACE).rooms.map(r => r.roomId).includes(d)).toBeTruthy(); + // }); + expect(store.getNotificationState(space1).rooms.map(r => r.roomId).includes(fav1)).toBeTruthy(); + expect(store.getNotificationState(space1).rooms.map(r => r.roomId).includes(fav2)).toBeFalsy(); + expect(store.getNotificationState(space1).rooms.map(r => r.roomId).includes(fav3)).toBeFalsy(); + expect(store.getNotificationState(space2).rooms.map(r => r.roomId).includes(fav1)).toBeTruthy(); + expect(store.getNotificationState(space2).rooms.map(r => r.roomId).includes(fav2)).toBeTruthy(); + expect(store.getNotificationState(space2).rooms.map(r => r.roomId).includes(fav3)).toBeTruthy(); + expect(store.getNotificationState(space3).rooms.map(r => r.roomId).includes(fav1)).toBeFalsy(); + expect(store.getNotificationState(space3).rooms.map(r => r.roomId).includes(fav2)).toBeFalsy(); + expect(store.getNotificationState(space3).rooms.map(r => r.roomId).includes(fav3)).toBeFalsy(); + }); + + it("other rooms are added to Notification States for all spaces containing the room exc Home", () => { + // XXX: All rooms space is forcibly enabled, as part of a future PR test Home space better + // expect(store.getNotificationState(HOME_SPACE).rooms.map(r => r.roomId).includes(room1)).toBeFalsy(); + expect(store.getNotificationState(space1).rooms.map(r => r.roomId).includes(room1)).toBeTruthy(); + expect(store.getNotificationState(space2).rooms.map(r => r.roomId).includes(room1)).toBeTruthy(); + expect(store.getNotificationState(space3).rooms.map(r => r.roomId).includes(room1)).toBeFalsy(); + }); }); });