From efa7eefe8ab2b7b5dfdf2f11b381d95bf92908a0 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Wed, 28 May 2025 12:05:09 +0700 Subject: [PATCH] UBERF-11347: Fix gmail recipients (#9115) * UBERF-11347: Fix gmail recipients Signed-off-by: Artem Savchenko * UBERF-11347: Fix gmail recipients Signed-off-by: Artem Savchenko * UBERF-11347: Formatting Signed-off-by: Artem Savchenko * UBERF-11347: Fix js doc Signed-off-by: Artem Savchenko --------- Signed-off-by: Artem Savchenko --- services/gmail/pod-gmail/src/gmail.ts | 8 +- .../gmail/pod-gmail/src/message/adapter.ts | 9 +- .../gmail/pod-gmail/src/message/v2/message.ts | 9 +- .../mail-common/src/__tests__/message.test.ts | 153 +++++++++++++----- services/mail/mail-common/src/message.ts | 59 ++----- services/mail/mail-common/src/types.ts | 9 ++ 6 files changed, 150 insertions(+), 97 deletions(-) diff --git a/services/gmail/pod-gmail/src/gmail.ts b/services/gmail/pod-gmail/src/gmail.ts index f4b82f7d64..628bcdda07 100644 --- a/services/gmail/pod-gmail/src/gmail.ts +++ b/services/gmail/pod-gmail/src/gmail.ts @@ -26,6 +26,7 @@ import { isWorkspaceLoginInfo, AccountClient } from '@hcengineering/account-client' +import { MailRecipient } from '@hcengineering/mail-common' import { encode64 } from './base64' import config from './config' @@ -114,6 +115,11 @@ export class GmailClient { this.account = this.user.userId this.attachmentHandler = new AttachmentHandler(ctx, wsInfo, storageAdapter, this.gmail, this.client) const keyValueClient = getKvsClient(this.integrationToken) + const recipient: MailRecipient = { + email: this.email, + socialId: this.socialId._id, + uuid: this.user.userId + } this.messageManager = createMessageManager( ctx, this.client, @@ -122,7 +128,7 @@ export class GmailClient { this.attachmentHandler, this.workspace, this.integrationToken, - socialId + recipient ) this.syncManager = new SyncManager( ctx, diff --git a/services/gmail/pod-gmail/src/message/adapter.ts b/services/gmail/pod-gmail/src/message/adapter.ts index 8b8c96f8f7..a346bea0fa 100644 --- a/services/gmail/pod-gmail/src/message/adapter.ts +++ b/services/gmail/pod-gmail/src/message/adapter.ts @@ -13,9 +13,10 @@ // limitations under the License. // -import { MeasureContext, TxOperations, SocialId } from '@hcengineering/core' +import { MeasureContext, TxOperations } from '@hcengineering/core' import { type KeyValueClient } from '@hcengineering/kvs-client' import { AccountClient } from '@hcengineering/account-client' +import { type MailRecipient } from '@hcengineering/mail-common' import config from '../config' import { AttachmentHandler } from './attachments' @@ -32,11 +33,11 @@ export function createMessageManager ( attachmentHandler: AttachmentHandler, workspace: { getChannel: (email: string) => Channel | undefined }, token: string, - socialId: SocialId + recipient: MailRecipient ): IMessageManager { if (config.Version === 'v2') { - return new MessageManagerV2(ctx, attachmentHandler, client, keyValueClient, accountClient, token, socialId._id) + return new MessageManagerV2(ctx, attachmentHandler, client, keyValueClient, accountClient, token, recipient) } else { - return new MessageManagerV1(ctx, client, attachmentHandler, socialId._id, workspace) + return new MessageManagerV1(ctx, client, attachmentHandler, recipient.socialId, workspace) } } diff --git a/services/gmail/pod-gmail/src/message/v2/message.ts b/services/gmail/pod-gmail/src/message/v2/message.ts index 16c76dc062..a44a013ffb 100644 --- a/services/gmail/pod-gmail/src/message/v2/message.ts +++ b/services/gmail/pod-gmail/src/message/v2/message.ts @@ -16,13 +16,14 @@ import { type GaxiosResponse } from 'gaxios' import { gmail_v1 } from 'googleapis' import sanitizeHtml from 'sanitize-html' -import { type MeasureContext, PersonId, TxOperations } from '@hcengineering/core' +import { type MeasureContext, TxOperations } from '@hcengineering/core' import { createMessages, parseEmailHeader, parseNameFromEmailHeader, EmailMessage, - getProducer + getProducer, + MailRecipient } from '@hcengineering/mail-common' import { type KeyValueClient } from '@hcengineering/kvs-client' import { AccountClient, isWorkspaceLoginInfo, WorkspaceLoginInfo } from '@hcengineering/account-client' @@ -41,7 +42,7 @@ export class MessageManagerV2 implements IMessageManager { private readonly keyValueClient: KeyValueClient, private readonly accountClient: AccountClient, private readonly token: string, - private readonly personId: PersonId + private readonly recipient: MailRecipient ) {} async saveMessage (message: GaxiosResponse, me: string): Promise { @@ -67,7 +68,7 @@ export class MessageManagerV2 implements IMessageManager { this.wsInfo, res, attachments, - [this.personId] + [this.recipient] ) } } diff --git a/services/mail/mail-common/src/__tests__/message.test.ts b/services/mail/mail-common/src/__tests__/message.test.ts index da35eb416a..dec52f8679 100644 --- a/services/mail/mail-common/src/__tests__/message.test.ts +++ b/services/mail/mail-common/src/__tests__/message.test.ts @@ -20,7 +20,7 @@ import { PersonSpace } from '@hcengineering/contact' import { MeasureContext, PersonId, PersonUuid, Ref, TxOperations } from '@hcengineering/core' import { KeyValueClient } from '@hcengineering/kvs-client' import { Producer } from 'kafkajs' -import { Attachment, BaseConfig, EmailContact, EmailMessage } from '../types' +import { Attachment, BaseConfig, EmailContact, EmailMessage, MailRecipient } from '../types' import { PersonCacheFactory } from '../person' import { PersonSpacesCacheFactory } from '../personSpaces' import { ChannelCacheFactory } from '../channel' @@ -221,49 +221,9 @@ describe('createMessages', () => { ) }) - it('should use provided targetPersons when specified', async () => { - // Arrange - const customTargetPersons = ['custom-person-1', 'custom-person-2'] as PersonId[] - - // Act - await createMessages( - mockConfig, - mockCtx, - mockTxClient, - mockKvsClient, - mockProducer, - mockToken, - mockWsInfo, - mockMessage, - mockAttachments, - customTargetPersons - ) - - // Assert - // Check that createDoc was called with the custom participants list - expect(mockTxClient.createDoc).toHaveBeenCalledWith( - expect.any(String), - expect.any(String), - expect.objectContaining({ - members: customTargetPersons - }), - expect.any(String), - expect.any(Number), - expect.any(String) - ) - - // Also check that the channel was created with the custom participants - expect(mockChannelCache.getOrCreateChannel).toHaveBeenCalledWith( - mockSpace._id, - customTargetPersons, - mockToContact1.email, - expect.any(String) - ) - }) - it('should handle empty targetPersons array', async () => { // Arrange - const emptyTargetPersons: PersonId[] = [] + const emptyTargetPersons: MailRecipient[] = [] // Act await createMessages( @@ -285,7 +245,7 @@ describe('createMessages', () => { expect.any(String), expect.any(String), expect.objectContaining({ - members: [] // Empty array + members: ['from-person-id', 'to-person-id-1', 'to-person-id-2'] }), expect.any(String), expect.any(Number), @@ -398,4 +358,111 @@ describe('createMessages', () => { expect.any(String) ) }) + + it('should call getPersonSpaces for both sender and recipients if target persons are not provided', async () => { + // Act + await createMessages( + mockConfig, + mockCtx, + mockTxClient, + mockKvsClient, + mockProducer, + mockToken, + mockWsInfo, + mockMessage, + mockAttachments + ) + + // Assert + // Based on the error message, the actual calls are with different parameters + // Update the assertions to match the actual implementation + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledWith('test-mail-id', 'from-uuid', 'from@example.com') + + // Check that getPersonSpaces was called for each recipient + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledWith('test-mail-id', 'to-uuid-1', 'to1@example.com') + + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledWith('test-mail-id', 'to-uuid-2', 'to2@example.com') + + // Verify the total number of calls matches our expectations + // Called once for sender and once for each recipient + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledTimes(3) + }) + + it('should call getPersonSpaces only for specified targetPerson when provided', async () => { + const customTargetPersons: MailRecipient[] = [ + { + socialId: 'to-person-id-1' as PersonId, + uuid: 'to-uuid-1' as PersonUuid, + email: 'test-mail@example.com' + } + ] + + // Act + await createMessages( + mockConfig, + mockCtx, + mockTxClient, + mockKvsClient, + mockProducer, + mockToken, + mockWsInfo, + mockMessage, + mockAttachments, + customTargetPersons + ) + + // Assert + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledWith( + 'test-mail-id', + 'to-uuid-1', + 'test-mail@example.com' + ) + + // Verify the total number of calls matches the number of targetPersons + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledTimes(1) + }) + + it('should call getPersonSpaces only for specified targetPersons when provided', async () => { + const customTargetPersons: MailRecipient[] = [ + { + socialId: 'to-person-id-1' as PersonId, + uuid: 'to-uuid-1' as PersonUuid, + email: 'to-person-email-1@example.com' + }, + { + socialId: 'to-person-id-2' as PersonId, + uuid: 'to-uuid-2' as PersonUuid, + email: 'to-person-email-2@example.com' + } + ] + + // Act + await createMessages( + mockConfig, + mockCtx, + mockTxClient, + mockKvsClient, + mockProducer, + mockToken, + mockWsInfo, + mockMessage, + mockAttachments, + customTargetPersons + ) + + // Assert + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledWith( + 'test-mail-id', + 'to-uuid-1', + 'to-person-email-1@example.com' + ) + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledWith( + 'test-mail-id', + 'to-uuid-2', + 'to-person-email-2@example.com' + ) + + // Verify the total number of calls matches the number of targetPersons + expect(mockPersonSpacesCache.getPersonSpaces).toHaveBeenCalledTimes(2) + }) }) diff --git a/services/mail/mail-common/src/message.ts b/services/mail/mail-common/src/message.ts index bbb726e75a..51fd8693d6 100644 --- a/services/mail/mail-common/src/message.ts +++ b/services/mail/mail-common/src/message.ts @@ -27,7 +27,6 @@ import { type TxOperations, Doc, generateId, - PersonUuid, RateLimiter, Space } from '@hcengineering/core' @@ -39,7 +38,7 @@ import { MessageRequestEventType } from '@hcengineering/communication-sdk-types' import { generateMessageId } from '@hcengineering/communication-shared' import { BaseConfig, type Attachment } from './types' -import { EmailMessage } from './types' +import { EmailMessage, MailRecipient } from './types' import { getMdContent } from './utils' import { PersonCacheFactory } from './person' import { PersonSpacesCacheFactory } from './personSpaces' @@ -65,8 +64,8 @@ import { ThreadLookupService } from './thread' * @param {WorkspaceLoginInfo} wsInfo - Workspace information including ID and URLs * @param {EmailMessage} message - The email message to process * @param {Attachment[]} attachments - Array of attachments for the message - * @param {PersonId[]} [targetPersons] - Optional list of specific persons who should receive the message. - * If not provided, all existing accounts from email addresses will be used. + * @param {MailRecipient[]} [recipients] - Optional list of specific persons who should receive the message. + * If not provided, all existing accounts from email addresses will be used. * @returns {Promise} A promise that resolves when all messages have been created * @throws Will log errors but not throw exceptions for partial failures * @@ -82,7 +81,7 @@ export async function createMessages ( wsInfo: WorkspaceLoginInfo, message: EmailMessage, attachments: Attachment[], - targetPersons?: PersonId[] + recipients?: MailRecipient[] ): Promise { const { mailId, from, subject, replyTo } = message const tos = [...(message.to ?? []), ...(message.copy ?? [])] @@ -95,13 +94,13 @@ export async function createMessages ( const fromPerson = await personCache.ensurePerson(from) - const toPersons: { address: string, uuid: PersonUuid, socialId: PersonId }[] = [] + const toPersons: MailRecipient[] = [] for (const to of tos) { const toPerson = await personCache.ensurePerson(to) if (toPerson === undefined) { continue } - toPersons.push({ address: to.email, ...toPerson }) + toPersons.push({ email: to.email, ...toPerson }) } if (toPersons.length === 0) { ctx.error('Unable to create message without a proper TO', { mailId, from }) @@ -109,7 +108,7 @@ export async function createMessages ( } const modifiedBy = fromPerson.socialId - const participants = targetPersons ?? [fromPerson.socialId, ...toPersons.map((p) => p.socialId)] + const participants = [fromPerson.socialId, ...toPersons.map((p) => p.socialId)] const content = getMdContent(ctx, message) const attachedBlobs: Attachment[] = [] @@ -141,42 +140,12 @@ export async function createMessages ( } } - try { - const spaces = await personSpacesCache.getPersonSpaces(mailId, fromPerson.uuid, from.email) - if (spaces.length > 0) { - await saveMessageToSpaces( - config, - ctx, - txClient, - producer, - threadLookup, - wsInfo, - mailId, - spaces, - participants, - modifiedBy, - subject, - content, - attachedBlobs, - from.email, - fromPerson.socialId, - message.sendOn, - channelCache, - replyTo - ) - } - } catch (error) { - ctx.error('Failed to save message to personal spaces', { - error, - mailId, - personUuid: fromPerson.uuid, - email: from - }) - } + const allPersons = [{ ...fromPerson, email: from.email }, ...toPersons] + const messageRecipients = recipients != null && recipients.length > 0 ? recipients : allPersons - for (const to of toPersons) { + for (const person of messageRecipients) { try { - const spaces = await personSpacesCache.getPersonSpaces(mailId, to.uuid, to.address) + const spaces = await personSpacesCache.getPersonSpaces(mailId, person.uuid, person.email) if (spaces.length > 0) { await saveMessageToSpaces( config, @@ -192,15 +161,15 @@ export async function createMessages ( subject, content, attachedBlobs, - to.address, - to.socialId, + person.email, + person.socialId, message.sendOn, channelCache, replyTo ) } } catch (error) { - ctx.error('Failed to save message spaces', { error, mailId, personUuid: to.uuid, email: to.address }) + ctx.error('Failed to save message spaces', { error, mailId, personUuid: person.uuid, email: person.email }) } } } diff --git a/services/mail/mail-common/src/types.ts b/services/mail/mail-common/src/types.ts index 11ac14e731..f1e860d63c 100644 --- a/services/mail/mail-common/src/types.ts +++ b/services/mail/mail-common/src/types.ts @@ -11,6 +11,9 @@ // // See the License for the specific language governing permissions and // limitations under the License. + +import { PersonId, PersonUuid } from '@hcengineering/core' + // export interface Attachment { id: string @@ -41,6 +44,12 @@ export interface EmailMessage { sendOn: number } +export interface MailRecipient { + email: string + uuid: PersonUuid + socialId: PersonId +} + export interface BaseConfig { AccountsURL: string KvsUrl: string