Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
140 changes: 140 additions & 0 deletions spec/unit/room-state.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1308,4 +1308,144 @@ describe("RoomState", function () {
).toBeFalsy();
});
});

describe("reactive display name disambiguation", function () {
it("should disambiguate existing member when another member changes to the same name", function () {
// Create a fresh state
const testState = new RoomState(roomId);

// Alice joins with display name "Alice"
const aliceJoinEvent = utils.mkMembership({
user: userA,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Alice",
});

// Bob joins with display name "Bob"
const bobJoinEvent = utils.mkMembership({
user: userB,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Bob",
});

testState.setStateEvents([aliceJoinEvent, bobJoinEvent]);

// Verify no disambiguation needed initially
const aliceBefore = testState.getMember(userA);
const bobBefore = testState.getMember(userB);
expect(aliceBefore?.disambiguate).toBe(false);
expect(bobBefore?.disambiguate).toBe(false);
expect(aliceBefore?.name).toBe("Alice");
expect(bobBefore?.name).toBe("Bob");

// Bob changes display name to "Alice"
const bobRenameEvent = utils.mkMembership({
user: userB,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Alice",
});

testState.setStateEvents([bobRenameEvent]);

// Now both should be disambiguated
const aliceAfter = testState.getMember(userA);
const bobAfter = testState.getMember(userB);
expect(aliceAfter?.disambiguate).toBe(true);
expect(bobAfter?.disambiguate).toBe(true);
expect(aliceAfter?.name).toContain(userA);
expect(bobAfter?.name).toContain(userB);
});

it("should un-disambiguate member when conflicting member changes to different name", function () {
// Create a fresh state
const testState = new RoomState(roomId);

// Both Alice and Bob join with display name "Alice"
const aliceJoinEvent = utils.mkMembership({
user: userA,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Alice",
});

const bobJoinEvent = utils.mkMembership({
user: userB,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Alice",
});

testState.setStateEvents([aliceJoinEvent, bobJoinEvent]);

// Verify both are disambiguated
const aliceBefore = testState.getMember(userA);
const bobBefore = testState.getMember(userB);
expect(aliceBefore?.disambiguate).toBe(true);
expect(bobBefore?.disambiguate).toBe(true);

// Bob changes display name to "Bob"
const bobRenameEvent = utils.mkMembership({
user: userB,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Bob",
});

testState.setStateEvents([bobRenameEvent]);

// Alice should no longer be disambiguated, Bob should not be either
const aliceAfter = testState.getMember(userA);
const bobAfter = testState.getMember(userB);
expect(aliceAfter?.disambiguate).toBe(false);
expect(bobAfter?.disambiguate).toBe(false);
expect(aliceAfter?.name).toBe("Alice");
expect(bobAfter?.name).toBe("Bob");
});

it("should emit RoomState.members for affected members when disambiguation changes", function () {
// Create a fresh state
const testState = new RoomState(roomId);

// Alice joins with display name "Alice"
const aliceJoinEvent = utils.mkMembership({
user: userA,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Alice",
});

testState.setStateEvents([aliceJoinEvent]);

// Set up listener for Members event
const membersEmitted: string[] = [];
testState.on(RoomStateEvent.Members, (_ev, _state, member) => {
membersEmitted.push(member.userId);
});

// Bob joins with display name "Alice" - should trigger disambiguation for Alice
const bobJoinEvent = utils.mkMembership({
user: userB,
mship: KnownMembership.Join,
room: roomId,
event: true,
name: "Alice",
});

testState.setStateEvents([bobJoinEvent]);

// Both Alice and Bob should have emitted Members events
expect(membersEmitted).toContain(userA);
expect(membersEmitted).toContain(userB);
});
});
});
36 changes: 36 additions & 0 deletions src/models/room-member.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,42 @@ export class RoomMember extends TypedEventEmitter<RoomMemberEvent, RoomMemberEve
}
}

/**
* Recalculate the disambiguation flag for this member based on current room state.
* This should be called when another member's display name changes and may affect
* whether this member needs disambiguation.
*
* @param roomState - The current room state to use for disambiguation check
* @returns true if the member's name changed as a result of the disambiguation update
*
* @remarks
* Fires {@link RoomMemberEvent.Name}
*/
public recalculateDisambiguatedName(roomState: RoomState): boolean {
if (!this.events.member) {
return false;
}

const displayName = this.events.member.getDirectionalContent().displayname ?? "";
const newDisambiguate = shouldDisambiguate(this.userId, displayName, roomState);

if (newDisambiguate === this.disambiguate) {
return false;
}

this.disambiguate = newDisambiguate;
const oldName = this.name;
this.name = calculateDisplayName(this.userId, displayName, this.disambiguate);

if (oldName !== this.name) {
this.updateModifiedTime();
this.emit(RoomMemberEvent.Name, this.events.member, this, oldName);
return true;
}

return false;
}

/**
* Update this room member's power level event. Will fire
* "RoomMember.powerLevel" if the new power level is different
Expand Down
43 changes: 42 additions & 1 deletion src/models/room-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,9 @@ export class RoomState extends TypedEventEmitter<EmittedEvents, EventHandlerMap>
this.updateModifiedTime();

// update the core event dict
// Track display names that change so we can recalculate disambiguation
const affectedDisplayNames = new Set<string>();

stateEvents.forEach((event) => {
if (event.getRoomId() !== this.roomId || !event.isState()) return;

Expand All @@ -448,7 +451,21 @@ export class RoomState extends TypedEventEmitter<EmittedEvents, EventHandlerMap>
const lastStateEvent = this.getStateEventMatching(event);
this.setStateEvent(event);
if (event.getType() === EventType.RoomMember) {
this.updateDisplayNameCache(event.getStateKey()!, event.getContent().displayname ?? "");
const userId = event.getStateKey()!;
const newDisplayName = event.getContent().displayname ?? "";
const oldDisplayName = this.userIdsToDisplayNames[userId];

// Track both old and new display names for disambiguation recalculation
if (oldDisplayName) {
const strippedOld = removeHiddenChars(oldDisplayName);
if (strippedOld) affectedDisplayNames.add(strippedOld);
}
if (newDisplayName) {
const strippedNew = removeHiddenChars(newDisplayName);
if (strippedNew) affectedDisplayNames.add(strippedNew);
}

this.updateDisplayNameCache(userId, newDisplayName);
this.updateThirdPartyTokenCache(event);
}
this.emit(RoomStateEvent.Events, event, this, lastStateEvent);
Expand Down Expand Up @@ -514,6 +531,29 @@ export class RoomState extends TypedEventEmitter<EmittedEvents, EventHandlerMap>
}
});

// Recalculate disambiguation for all members whose display names were affected.
// This ensures that when a user changes their name to match (or stop matching)
// another user, all affected users' disambiguation flags are updated correctly.
if (affectedDisplayNames.size > 0) {
// Collect all affected user IDs first to avoid duplicate processing
const affectedUserIds = new Set<string>();
for (const displayName of affectedDisplayNames) {
const userIds = this.displayNameToUserIds.get(displayName) ?? [];
userIds.forEach((id) => affectedUserIds.add(id));
}

// Process each affected member once
for (const userId of affectedUserIds) {
const member = this.members[userId];
if (member?.events.member) {
const nameChanged = member.recalculateDisambiguatedName(this);
if (nameChanged) {
this.emit(RoomStateEvent.Members, member.events.member, this, member);
}
}
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking much nicer, thanks. One thing though: your previous logic excluded the user whose name was being changed. Do we need to do that here too to avoid duplicate events?


this.emit(RoomStateEvent.Update, this);
}

Expand Down Expand Up @@ -1110,6 +1150,7 @@ export class RoomState extends TypedEventEmitter<EmittedEvents, EventHandlerMap>

private updateDisplayNameCache(userId: string, displayName: string): void {
const oldName = this.userIdsToDisplayNames[userId];

delete this.userIdsToDisplayNames[userId];
if (oldName) {
// Remove the old name from the cache.
Expand Down