Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>({
Expand Down
5 changes: 5 additions & 0 deletions src/modules/ExtrasApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] {
Expand Down
20 changes: 19 additions & 1 deletion src/stores/RoomViewStore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
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;

Expand Down Expand Up @@ -204,7 +205,7 @@
this.emit(UPDATE_EVENT);
}

private onDispatch(payload: ActionPayload): void {

Check failure on line 208 in src/stores/RoomViewStore.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=element-web&issues=AZqlrJLo5jEYlD_Wa1ob&open=AZqlrJLo5jEYlD_Wa1ob&pullRequest=31280
if (this.lockedToRoomId && payload.room_id && this.lockedToRoomId !== payload.room_id) {
return;
}
Expand Down Expand Up @@ -292,10 +293,15 @@
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(),
Expand Down Expand Up @@ -802,4 +808,16 @@
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);
}
}
50 changes: 50 additions & 0 deletions test/unit-tests/components/structures/RoomView-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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,
}),
);
});
});
39 changes: 39 additions & 0 deletions test/unit-tests/stores/RoomViewStore-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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);
Expand Down
Loading