Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Regression: broken message exhibition when edited #28180

Merged
merged 9 commits into from Mar 2, 2023
Merged

Regression: broken message exhibition when edited #28180

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eda4718
Review `IEditedMessage` type safety
tassoevan Feb 27, 2023
dd3eb6f
Merge remote-tracking branch 'origin/develop' into regression/edited-…
ggazzo Mar 1, 2023
4f7e5e8
Define a stricter type guard
tassoevan Mar 1, 2023
5851c0a
Fix Federation unit tests
tassoevan Mar 1, 2023
980dcf6
Use Sinon assertions
tassoevan Mar 1, 2023
23f636e
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into regr…
tassoevan Mar 1, 2023
90efc31
Merge branch 'develop' into regression/edited-message-bug
tassoevan Mar 1, 2023
4b9a4e9
Merge branch 'develop' into regression/edited-message-bug
kodiakhq[bot] Mar 1, 2023
476ed72
Merge branch 'develop' into regression/edited-message-bug
kodiakhq[bot] Mar 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/meteor/.eslintcache
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions apps/meteor/app/federation-v2/server/infrastructure/rocket-chat/hooks/index.ts
View file
Open in desktop
Expand Up @@ -45,7 +45,7 @@ export class FederationHooks {
callbacks.add(
'federation.beforeAddUserToARoom',
(params: { user: IUser | string; inviter?: IUser }, room: IRoom): void => {
if (!params || !params.user || !room) {
if (!params?.user || !room) {
return;
}
Promise.await(callback(params.user, room));
Expand All @@ -59,7 +59,7 @@ export class FederationHooks {
callbacks.add(
'federation.beforeAddUserToARoom',
(params: { user: IUser | string; inviter: IUser }, room: IRoom): void => {
if (!params || !params.user || !params.inviter || !room || !settings.get('Federation_Matrix_enabled')) {
if (!params?.user || !params.inviter || !room || !settings.get('Federation_Matrix_enabled')) {
return;
}

Expand Down Expand Up @@ -160,7 +160,7 @@ export class FederationHooks {
) {
return message;
}
if (!isEditedMessage(message)) {
if (!isEditedMessage(message) || !message.editedBy) {
ggazzo marked this conversation as resolved.
Show resolved Hide resolved
return message;
}
Promise.await(callback(message, room._id, message.editedBy._id));
Expand Down
14 changes: 8 additions & 6 deletions apps/meteor/app/lib/server/functions/updateMessage.ts
View file
Open in desktop
@@ -1,4 +1,4 @@
import type { IMessage, IMessageEdited, IUser } from '@rocket.chat/core-typings';
import type { IEditedMessage, IMessage, IUser } from '@rocket.chat/core-typings';
import { Meteor } from 'meteor/meteor';

import { Messages, Rooms } from '../../../models/server';
Expand Down Expand Up @@ -35,11 +35,13 @@ export const updateMessage = function (message: IMessage, user: IUser, originalM
Messages.cloneAndSaveAsHistoryById(message._id, user);
}

(message as IMessageEdited).editedAt = new Date();
(message as IMessageEdited).editedBy = {
_id: user._id,
username: user.username,
};
Object.assign<IMessage, Omit<IEditedMessage, keyof IMessage>>(message, {
editedAt: new Date(),
editedBy: {
_id: user._id,
username: user.username,
},
});

parseUrlsInMessage(message);

Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/components/message/StatusIndicators.tsx
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@ const StatusIndicators = ({ message }: StatusIndicatorsProps): ReactElement => {
{following && <MessageStatusIndicatorItem name='bell' title={t('Following')} />}

{message.sentByEmail && <MessageStatusIndicatorItem name='mail' title={t('Message_sent_by_email')} />}
{isEditedMessage(message) && (
{isEditedMessage(message) && message.editedBy !== null && message.editedAt && (
<MessageStatusIndicatorItem
name='edit'
color={message.u._id !== message.editedBy._id ? 'danger' : undefined}
Expand Down
16 changes: 3 additions & 13 deletions packages/core-typings/src/IMessage/IMessage.ts
View file
Open in desktop
Expand Up @@ -208,13 +208,12 @@ export type MessageSystem = {
};

export interface IEditedMessage extends IMessage {
editedAt: Date;
editedBy: Pick<IUser, '_id' | 'username'>;
editedAt: Date | null;
editedBy: Pick<IUser, '_id' | 'username'> | null;
}

export const isEditedMessage = (message: IMessage): message is IEditedMessage => 'editedAt' in message && 'editedBy' in message;
export const isDeletedMessage = (message: IMessage): message is IEditedMessage =>
'editedAt' in message && 'editedBy' in message && message.t === 'rm';
export const isDeletedMessage = (message: IMessage): message is IEditedMessage => isEditedMessage(message) && message.t === 'rm';
export const isMessageFromMatrixFederation = (message: IMessage): boolean =>
'federation' in message && Boolean(message.federation?.eventId);

Expand Down Expand Up @@ -316,15 +315,6 @@ export const isMessageDiscussion = (message: IMessage): message is IMessageDiscu
return 'drid' in message;
};

export type IMessageEdited = IMessage & {
editedAt: Date;
editedBy: Pick<IUser, '_id' | 'username'>;
};

export const isMessageEdited = (message: IMessage): message is IMessageEdited => {
return 'editedAt' in message && 'editedBy' in message;
};

export type IMessageInbox = IMessage & {
// email inbox fields
email?: {
Expand Down