Fix message edition and reply when multiple rooms at displayed the same moment (#31280)

* feat: implement `ExtrasApi#setRoomIdsForSpace`

* fix: message reply with multiple room views

* fix: message edition when multiple rooms are displayed

* test: check that the view room action is not dispatch when replying

* test: check that the view room action is not dispatch when editing

* refactor: use `ExtraApis#getVisibleRoomBySpaceKey` instead of  `ExtraApis#setRoomIdsForSpace`

* test: update tests to use `getVisibleRoomBySpaceKey`
This commit is contained in:
Florian Duros 2025-11-21 15:51:23 +01:00 committed by GitHub
parent a79f6e7aa5
commit fbb43d5e61
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 120 additions and 2 deletions

View File

@ -1207,7 +1207,13 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
case Action.EditEvent: {
// Quit early if we're trying to edit events in wrong rendering context
if (payload.timelineRenderingType !== this.state.timelineRenderingType) return;
if (payload.event && payload.event.getRoomId() !== this.state.roomId) {
const roomId: string | undefined = payload.event?.getRoomId();
if (payload.event && roomId !== this.state.roomId) {
// if the room is displayed in a module, we don't want to change the room view
if (roomId && this.roomViewStore.isRoomDisplayedInModule(roomId)) return;
// If the event is in a different room (e.g. because the event to be edited is being displayed
// in the results of an all-rooms search), we need to view that room first.
defaultDispatcher.dispatch<ViewRoomPayload>({

View File

@ -25,11 +25,16 @@ interface EmittedEvents {
export class ElementWebExtrasApi extends TypedEventEmitter<keyof EmittedEvents, EmittedEvents> implements ExtrasApi {
public spacePanelItems = new Map<string, SpacePanelItemProps>();
public visibleRoomBySpaceKey = new Map<string, () => string[]>();
public setSpacePanelItem(spacekey: string, item: SpacePanelItemProps): void {
this.spacePanelItems.set(spacekey, item);
this.emit(ExtrasApiEvent.SpacePanelItemsChanged);
}
public getVisibleRoomBySpaceKey(spaceKey: string, cb: () => string[]): void {
this.visibleRoomBySpaceKey.set(spaceKey, cb);
}
}
export function useModuleSpacePanelItems(api: ElementWebExtrasApi): ModuleSpacePanelItem[] {

View File

@ -52,6 +52,7 @@ import { ModuleRunner } from "../modules/ModuleRunner";
import { setMarkedUnreadState } from "../utils/notifications";
import { ConnectionState, ElementCall } from "../models/Call";
import { isVideoRoom } from "../utils/video-rooms";
import { ModuleApi } from "../modules/Api";
const NUM_JOIN_RETRY = 5;
@ -292,10 +293,15 @@ export class RoomViewStore extends EventEmitter {
case "reply_to_event":
// Thread timeline view handles its own reply-to-state
if (TimelineRenderingType.Thread !== payload.context) {
const roomId: string | undefined = payload.event?.getRoomId();
// If currently viewed room does not match the room in which we wish to reply then change rooms this
// can happen when performing a search across all rooms. Persist the data from this event for both
// room and search timeline rendering types, search will get auto-closed by RoomView at this time.
if (payload.event && payload.event.getRoomId() !== this.state.roomId) {
if (payload.event && roomId !== this.state.roomId) {
// if the room is displayed in a module, we don't want to change the room view
if (roomId && this.isRoomDisplayedInModule(roomId)) return;
this.dis?.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
room_id: payload.event.getRoomId(),
@ -802,4 +808,16 @@ export class RoomViewStore extends EventEmitter {
ModuleRunner.instance.invoke(RoomViewLifecycle.ViewRoom, viewRoomOpts, this.getRoomId());
this.setState({ viewRoomOpts });
}
/**
* Checks if a room is already displayed in the current active space module.
* @param roomId
*/
public isRoomDisplayedInModule(roomId: string): boolean {
const currentSpace = this.stores.spaceStore.activeSpace;
const cb = ModuleApi.instance.extras.visibleRoomBySpaceKey.get(currentSpace);
if (!cb) return false;
return cb().includes(roomId);
}
}

View File

@ -78,6 +78,8 @@ import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../../src/MediaDe
import Modal, { type ComponentProps } from "../../../../src/Modal.tsx";
import ErrorDialog from "../../../../src/components/views/dialogs/ErrorDialog.tsx";
import * as pinnedEventHooks from "../../../../src/hooks/usePinnedEvents";
import { TimelineRenderingType } from "../../../../src/contexts/RoomContext";
import { ModuleApi } from "../../../../src/modules/Api";
// Used by group calls
jest.spyOn(MediaDeviceHandler, "getDevices").mockResolvedValue({
@ -1006,4 +1008,52 @@ describe("RoomView", () => {
});
});
});
it("should not change room when editing event in a room displayed in module", async () => {
const room2 = new Room("!room2:example.org", cli, "@alice:example.org");
rooms.set(room2.roomId, room2);
room.getMyMembership = jest.fn().mockReturnValue(KnownMembership.Join);
room2.getMyMembership = jest.fn().mockReturnValue(KnownMembership.Join);
await mountRoomView();
// Mock the spaceStore activeSpace and ModuleApi setup
jest.spyOn(stores.spaceStore, "activeSpace", "get").mockReturnValue("space1");
// Mock that room2 is displayed in a module
ModuleApi.instance.extras.getVisibleRoomBySpaceKey("space1", () => [room2.roomId]);
// Mock the roomViewStore method
jest.spyOn(stores.roomViewStore, "isRoomDisplayedInModule").mockReturnValue(true);
// Create an event in room2 to edit
const eventInRoom2 = new MatrixEvent({
type: "m.room.message",
event_id: "$edit-event:example.org",
room_id: room2.roomId,
sender: "@alice:example.org",
content: {
body: "Original message",
msgtype: "m.text",
},
});
const dispatchSpy = jest.spyOn(defaultDispatcher, "dispatch");
// Dispatch EditEvent for event in room2 (which is displayed in module)
defaultDispatcher.dispatch({
action: Action.EditEvent,
event: eventInRoom2,
timelineRenderingType: TimelineRenderingType.Room,
});
await flushPromises();
// Should not dispatch ViewRoom action since room2 is displayed in module
expect(dispatchSpy).not.toHaveBeenCalledWith(
expect.objectContaining({
action: Action.ViewRoom,
room_id: room2.roomId,
}),
);
});
});

View File

@ -18,6 +18,7 @@ import EventEmitter from "events";
import { RoomViewStore } from "../../../src/stores/RoomViewStore";
import { Action } from "../../../src/dispatcher/actions";
import {
flushPromises,
getMockClientWithEventEmitter,
setupAsyncStoreWithClient,
untilDispatch,
@ -45,6 +46,7 @@ import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../src/MediaDeviceHandler";
import { storeRoomAliasInCache } from "../../../src/RoomAliasCache.ts";
import { type Call } from "../../../src/models/Call.ts";
import { ModuleApi } from "../../../src/modules/Api";
jest.mock("../../../src/Modal");
@ -201,6 +203,12 @@ describe("RoomViewStore", function () {
// @ts-expect-error
MockPosthogAnalytics.instance = stores._PosthogAnalytics;
stores._SpaceStore = new MockSpaceStore();
// Add activeSpace property to the mock
Object.defineProperty(stores._SpaceStore, "activeSpace", {
value: null,
writable: true,
configurable: true,
});
roomViewStore = new RoomViewStore(dis, stores);
stores._RoomViewStore = roomViewStore;
});
@ -351,6 +359,37 @@ describe("RoomViewStore", function () {
},
);
it("does not change room when replying to event in a room displayed in module", async () => {
// Spy on dispatch to check later if ViewRoom was dispatched
jest.spyOn(dis, "dispatch");
// Set up current room
dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
await untilDispatch(Action.ActiveRoomChanged, dis);
expect(roomViewStore.getRoomId()).toEqual(roomId);
ModuleApi.instance.extras.getVisibleRoomBySpaceKey("space1", () => [roomId, roomId2]);
// @ts-ignore
stores.spaceStore.activeSpace = "space1";
// Create reply event for roomId2 (which is displayed in module)
const replyToEvent = {
getRoomId: () => roomId2,
};
// Dispatch reply_to_event - should not change room since roomId2 is in module
dis.dispatch({ action: "reply_to_event", event: replyToEvent, context: TimelineRenderingType.Room });
await flushPromises();
// Room should remain the same (roomId), not change to roomId2
expect(dis.dispatch).not.toHaveBeenCalledWith({
action: Action.ViewRoom,
room_id: roomId2,
replyingToEvent: replyToEvent,
metricsTrigger: undefined,
});
});
it("removes the roomId on ViewHomePage", async () => {
dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
await untilDispatch(Action.ActiveRoomChanged, dis);