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 1/2] 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(); + }); }); }); From 353c70ad75ff79616c8cb9426b89885e7aad5470 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 27 Jul 2021 14:45:36 +0100 Subject: [PATCH 2/2] i18n --- src/i18n/strings/en_EN.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 102a481f52..de432d6177 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -798,9 +798,6 @@ "Beta available for web, desktop and Android. Some features may be unavailable on your homeserver.": "Beta available for web, desktop and Android. Some features may be unavailable on your homeserver.", "Your feedback will help make spaces better. The more detail you can go into, the better.": "Your feedback will help make spaces better. The more detail you can go into, the better.", "Show all rooms in Home": "Show all rooms in Home", - "Show people in spaces": "Show people in spaces", - "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.": "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.", - "Show notification badges for People in Spaces": "Show notification badges for People in Spaces", "Show options to enable 'Do not disturb' mode": "Show options to enable 'Do not disturb' mode", "Send and receive voice messages": "Send and receive voice messages", "Render LaTeX maths in messages": "Render LaTeX maths in messages",