5015 make gmail filters work for partial sync #5695
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Simplified regex pattern in
is-person-email.util.tsfor identifying non-personal emails. - Added attachment handling in
fetch-messages-by-batches.service.ts. - Introduced
gmailSearchFilteringmail-full-message-list-fetch-v2.service.tsfor refined message retrieval. - Updated Gmail filter usage in
gmail-full-message-list-fetch.service.tsfor partial sync. - Added blocklist filtering in
gmail-messages-import-v2.service.ts.
| export const isPersonEmail = (email: string | undefined): boolean => { | ||
| if (!email) return false; | ||
|
|
||
| export const isPersonEmail = (email: string): boolean => { |
There was a problem hiding this comment.
Removing the check for undefined email input may lead to runtime errors if the function is called with an undefined value.
| @@ -186,6 +186,8 @@ export class FetchMessagesByBatchesService { | |||
| const bodyData = this.getBodyData(message); | |||
| const text = bodyData ? Buffer.from(bodyData, 'base64').toString() : ''; | |||
|
|
|||
| const attachments = this.getAttachmentData(message); | |||
There was a problem hiding this comment.
Ensure that getAttachmentData handles cases where message.payload or message.payload.parts might be undefined.
| private getAttachmentData(message: gmail_v1.Schema$Message) { | ||
| return ( | ||
| message.payload?.parts | ||
| ?.filter((part) => part.filename && part.body?.attachmentId) | ||
| .map((part) => ({ | ||
| filename: part.filename || '', | ||
| id: part.body?.attachmentId || '', | ||
| mimeType: part.mimeType || '', | ||
| size: part.body?.size || 0, | ||
| })) || [] | ||
| ); |
There was a problem hiding this comment.
Consider adding a check to handle cases where message.payload or message.payload.parts might be undefined to avoid potential runtime errors.
| .map((history) => history.messagesAdded) | ||
| .flat() | ||
| .map((message) => message?.message?.id) | ||
| .filter((id) => id) as string[]; |
There was a problem hiding this comment.
Combine the map and filter operations to improve readability and performance.
| ); | ||
|
|
||
| this.logger.log( | ||
| `Added ${messagesAdded.length} messages to import for workspace ${workspaceId} and account ${connectedAccount.id}`, | ||
| `Added ${messagesAddedFiltered.length} messages to import for workspace ${workspaceId} and account ${connectedAccount.id}`, |
There was a problem hiding this comment.
Update log message to reflect filtering: 'Added ${messagesAddedFiltered.length} of ${messagesAdded.length} messages...'
| export const filterEmails = (messages: GmailMessage[], blocklist: string[]) => { | ||
| return filterOutBlocklistedMessages( | ||
| filterOutIcsAttachments(filterOutNonPersonalEmails(messages)), | ||
| blocklist, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Consider adding JSDoc comments to filterEmails to explain its purpose and parameters.
| return message.participants.every( | ||
| (participant) => !isEmailBlocklisted(participant.handle, blocklist), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Potential performance issue: every method will check all participants even if one is blocklisted. Consider short-circuiting.
| return message.attachments.every( | ||
| (attachment) => !attachment.filename.endsWith('.ics'), | ||
| ); |
There was a problem hiding this comment.
Potential performance issue: every method will check all attachments even if one ends with .ics. Consider short-circuiting.
|
|
||
| const isPersonEmail = (email: string): boolean => { | ||
| const nonPersonalPattern = | ||
| /noreply|no-reply|do_not_reply|no\.reply|^(info@|contact@|hello@|support@|feedback@|service@|help@)/; |
There was a problem hiding this comment.
Consider moving the regex pattern to a constant for better readability and reusability.
| return message.participants.every((participant) => | ||
| isPersonEmail(participant.handle), | ||
| ); |
There was a problem hiding this comment.
Potential performance issue: every method will check all participants even if one is non-personal. Consider short-circuiting.
Closes twentyhq#5015 --------- Co-authored-by: Charles Bochet <[email protected]>
Closes #5015