Skip to content

Commit

Permalink
msglist: Get dm-recipient-header names from user, not message, when able
Browse files Browse the repository at this point in the history
But if it's not in user data, just fall back to the user's name on
the message.

Soon we'll use the user data to show a "(guest)" marker if it
applies; this is zulip#5804.

The header doesn't actually live-update when a recipient user's name
changes (zulip#5208), because we don't yet use InboundEventEditSequence.
But I've added logic in the code that builds that inbound event, so
that the live-updating will happen if we ever do start using it.
Like we did for the followed-topic marker on stream recipient
headers, in 4ef0a0a.
  • Loading branch information
chrisbobbe committed Jan 4, 2024
1 parent 0dbcf8f commit 3cae733
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`getEditSequence correct for interesting changes DM recipient headers recipient user's name changed 1`] = `1`;

exports[`getEditSequence correct for interesting changes from empty to empty 1`] = `0`;

exports[`getEditSequence correct for interesting changes from empty to many messages 1`] = `40`;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`getEditSequence correct for interesting changes DM recipient headers recipient user's name changed 1`] = `1`;

exports[`getEditSequence correct for interesting changes from empty to empty 1`] = `0`;

exports[`getEditSequence correct for interesting changes from empty to many messages 1`] = `40`;
Expand Down
19 changes: 19 additions & 0 deletions src/webview/__tests__/generateInboundEventEditSequence-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,25 @@ describe('getEditSequence correct for interesting changes', () => {
});
});

describe('DM recipient headers', () => {
test("recipient user's name changed", () => {
const sender = eg.selfUser;
const message = eg.pmMessage({ sender, recipients: [eg.selfUser, eg.otherUser] });
check(
{
messages: [message],
state: eg.reduxStatePlus({ users: [eg.selfUser, eg.otherUser] }),
},
{
messages: [message],
state: eg.reduxStatePlus({
users: [eg.selfUser, { ...eg.otherUser, full_name: `${sender.full_name}, Jr.` }],
}),
},
);
});
});

describe('within a given message', () => {
test('add reactions to a message', () => {
const message = eg.streamMessage();
Expand Down
36 changes: 31 additions & 5 deletions src/webview/generateInboundEventEditSequence.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type { BackgroundData } from './backgroundData';
import messageListElementHtml from './html/messageListElementHtml';
import { getUserStatusFromModel } from '../user-statuses/userStatusesCore';
import { isTopicFollowed } from '../mute/muteModel';
import { pmUiRecipientsFromMessage } from '../utils/recipient';

const NODE_ENV = process.env.NODE_ENV;

Expand Down Expand Up @@ -71,21 +72,46 @@ function doElementsDifferInterestingly(
case 'time':
// TODO(?): False positives on `.subsequentMessage.content` changes
return !isEqual(oldElement, newElement);
case 'header':
case 'header': {
invariant(newElement.type === 'header', 'oldElement.type equals newElement.type');

// TODO(?): False positives on `.subsequentMessage.content` changes
if (!isEqual(oldElement, newElement)) {
return true;
}
if (newElement.subsequentMessage?.type === 'stream') {
const message = newElement.subsequentMessage;
const subsequentMessage = oldElement.subsequentMessage;
if (subsequentMessage.type === 'stream') {
if (
isTopicFollowed(message.stream_id, message.subject, oldBackgroundData.mute)
!== isTopicFollowed(message.stream_id, message.subject, newBackgroundData.mute)
isTopicFollowed(
subsequentMessage.stream_id,
subsequentMessage.subject,
oldBackgroundData.mute,
)
!== isTopicFollowed(
subsequentMessage.stream_id,
subsequentMessage.subject,
newBackgroundData.mute,
)
) {
return true;
}
} else {
invariant(
oldBackgroundData.ownUser.user_id === newBackgroundData.ownUser.user_id,
'self-user ID not supposed to change',
);
const ownUserId = oldBackgroundData.ownUser.user_id;
const messageRecipients = pmUiRecipientsFromMessage(subsequentMessage, ownUserId);
for (const recipient of messageRecipients) {
const oldRecipientUser = oldBackgroundData.allUsersById.get(recipient.id);
const newRecipientUser = newBackgroundData.allUsersById.get(recipient.id);
if (oldRecipientUser?.full_name !== newRecipientUser?.full_name) {
return true;
}
}
}
return false;
}
case 'message': {
invariant(newElement.type === 'message', 'oldElement.type equals newElement.type');

Expand Down
4 changes: 2 additions & 2 deletions src/webview/html/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const renderTopic = message =>
* This is a private helper of messageListElementHtml.
*/
export default (
{ mute, ownUser, subscriptions }: BackgroundData,
{ mute, ownUser, subscriptions, allUsersById }: BackgroundData,
element: HeaderMessageListElement,
): string => {
const { subsequentMessage: message, style: headerStyle } = element;
Expand Down Expand Up @@ -97,7 +97,7 @@ export default (
data-msg-id="${message.id}"
>
${uiRecipients
.map(r => r.full_name)
.map(r => allUsersById.get(r.id)?.full_name ?? r.full_name)
.sort()
.join(', ')}
</div>`;
Expand Down

0 comments on commit 3cae733

Please sign in to comment.