Skip to content

fix: inner attachment empty when quoting multiple times #35369

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

Merged
kodiakhq merged 2 commits into from
Mar 10, 2025
Merged

fix: inner attachment empty when quoting multiple times #35369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13776e9
fix: inner attachment empty when quoting multiple times
abhinavkrin Mar 1, 2025
57af783
Merge branch 'develop' into fix/quoting-attachment-inner-empty
kodiakhq[bot] Mar 9, 2025
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
6 changes: 6 additions & 0 deletions .changeset/good-singers-allow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/core-typings': patch
'@rocket.chat/meteor': patch
---

Fixes an issue where recursively quoting messages multiple times (up to the configured chained quote limit) caused the inner attachment to appear empty.
12 changes: 9 additions & 3 deletions apps/meteor/server/services/messages/hooks/BeforeSaveJumpToMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ const recursiveRemoveAttachments = (attachments: MessageAttachment, deep = 1, qu
if (attachments && isQuoteAttachment(attachments)) {
if (deep < quoteChainLimit - 1) {
attachments.attachments?.map((msg) => recursiveRemoveAttachments(msg, deep + 1, quoteChainLimit));
} else {
delete attachments.attachments;
} else if (attachments.attachments) {
attachments.attachments = attachments.attachments.filter((attachment) => !isQuoteAttachment(attachment));
if (attachments.attachments.length === 0) {
delete attachments.attachments;
}
}
}

Expand All @@ -24,7 +27,10 @@ const validateAttachmentDeepness = (message: IMessage, quoteChainLimit: number):
}

if ((message.attachments && quoteChainLimit < 2) || isNaN(quoteChainLimit)) {
delete message.attachments;
message.attachments = message.attachments.filter((attachment) => !isQuoteAttachment(attachment));
if (message.attachments.length === 0) {
delete message.attachments;
}
}

message.attachments = message.attachments?.map((attachment) => recursiveRemoveAttachments(attachment, 1, quoteChainLimit));
Expand Down
196 changes: 182 additions & 14 deletions apps/meteor/tests/unit/server/services/messages/hooks/BeforeSaveJumpToMessage.tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { IMessage } from '@rocket.chat/core-typings';
import { expect } from 'chai';

import { BeforeSaveJumpToMessage } from '../../../../../../server/services/messages/hooks/BeforeSaveJumpToMessage';

const createMessage = (msg?: string, extra: any = {}) => ({
const createMessage = (msg?: string, extra: Partial<IMessage> = {}) => ({
_id: 'random',
rid: 'GENERAL',
ts: new Date(),
Expand Down Expand Up @@ -79,7 +80,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://google.com' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://google.com',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand All @@ -101,7 +109,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://open.rocket.chat' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand All @@ -123,7 +138,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://open.rocket.chat/?token=value' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/?token=value',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand All @@ -145,7 +167,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://open.rocket.chat/?msg=value' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/?msg=value',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand All @@ -167,7 +196,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://open.rocket.chat/?msg=value' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/?msg=value',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand All @@ -189,7 +225,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://open.rocket.chat/?msg=value' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/?msg=value',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand All @@ -212,7 +255,12 @@ describe('Create attachments for message URLs', () => {

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [{ url: 'https://open.rocket.chat/linked?msg=linked' }],
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linked',
meta: {},
},
],
attachments: [
{
text: 'old attachment',
Expand Down Expand Up @@ -322,7 +370,14 @@ describe('Create attachments for message URLs', () => {
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', { urls: [{ url: 'https://open.rocket.chat/linked?msg=linked' }] }),
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linked',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 10,
Expand Down Expand Up @@ -388,7 +443,12 @@ describe('Create attachments for message URLs', () => {

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [{ url: 'https://open.rocket.chat/linked?msg=linked' }],
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linked',
meta: {},
},
],
}),
user: createUser(),
config: {
Expand All @@ -405,6 +465,89 @@ describe('Create attachments for message URLs', () => {
expect(deep).to.be.eq(3);
});

it('should respect chain limit config but keep the file attachments of last chained message', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [
createMessage('linked message', {
_id: 'linked',
attachments: [
{
text: 'chained 1',
author_name: 'username',
author_icon: 'url',
ts: new Date(),
message_link: 'https://open.rocket.chat/linked?msg=linkedMsgId',
attachments: [
{
text: 'chained 2',
author_name: 'username',
author_icon: 'url',
message_link: 'https://open.rocket.chat/linked?msg=linkedMsgId',
ts: new Date(),
attachments: [
{
ts: new Date(),
title: 'file.png',
title_link: 'title-link',
title_link_download: true,
image_dimensions: {
width: 480,
height: 269,
},
image_preview: 'image-preview',
image_url: 'image-url',
image_type: 'image/png',
image_size: 68016,
type: 'file',
description: 'chained 3 - file',
descriptionMd: [
{
type: 'PARAGRAPH',
value: [
{
type: 'PLAIN_TEXT',
value: 'chained 3 - file',
},
],
},
],
},
],
},
],
},
],
}),
],
getRooms: async () => [createRoom()],
canAccessRoom: async () => true,
getUserAvatarURL: () => 'url',
});

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linked',
meta: {},
},
],
}),
user: createUser(),
config: {
chainLimit: 3,
siteUrl: 'https://open.rocket.chat',
useRealName: true,
},
});

expect(message).to.have.property('urls').and.to.have.lengthOf(1);
expect(message).to.have.property('attachments').and.to.have.lengthOf(1);

const deep = countDeep(message);
expect(deep).to.be.eq(4, 'The chain limit should be respected, but the file attachments of the last chained message should be kept.');
});

it('should create the attachment if cannot access room but message has a livechat token', async () => {
const jumpToMessage = new BeforeSaveJumpToMessage({
getMessages: async () => [createMessage('linked message', { _id: 'linked' })],
Expand All @@ -415,7 +558,12 @@ describe('Create attachments for message URLs', () => {

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [{ url: 'https://open.rocket.chat/linked?msg=linked' }],
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linked',
meta: {},
},
],
token: 'livechatToken',
}),
user: createUser(),
Expand Down Expand Up @@ -452,7 +600,12 @@ describe('Create attachments for message URLs', () => {

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [{ url: 'https://open.rocket.chat/linked?msg=linked' }],
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linked',
meta: {},
},
],
token: 'another-token',
}),
user: createUser(),
Expand All @@ -477,6 +630,7 @@ describe('Create attachments for message URLs', () => {
text: 'chained 1',
author_name: 'username',
author_icon: 'url',
message_link: 'https://open.rocket.chat/linked?msg=linkedMsgId1',
ts: new Date(),
},
],
Expand All @@ -489,7 +643,12 @@ describe('Create attachments for message URLs', () => {

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [{ url: 'https://open.rocket.chat/linked?msg=linkedMsgId' }],
urls: [
{
url: 'https://open.rocket.chat/linked?msg=linkedMsgId',
meta: {},
},
],
}),
user: createUser(),
config: {
Expand Down Expand Up @@ -535,7 +694,16 @@ describe('Create attachments for message URLs', () => {

const message = await jumpToMessage.createAttachmentForMessageURLs({
message: createMessage('hey', {
urls: [{ url: 'https://open.rocket.chat/linked?msg=msg1' }, { url: 'https://open.rocket.chat/linked?msg=msg2' }],
urls: [
{
url: 'https://open.rocket.chat/linked?msg=msg1',
meta: {},
},
{
url: 'https://open.rocket.chat/linked?msg=msg2',
meta: {},
},
],
}),
user: createUser(),
config: {
Expand Down
Loading