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.

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

5629 update blocklist for messaging v2 #5756

Merged
merged 16 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,68 @@ import { isEmailBlocklisted } from 'src/modules/calendar-messaging-participant/u

describe('isEmailBlocklisted', () => {
it('should return true if email is blocklisted', () => {
const email = 'hello@example.com';
const blocklist = ['hello@example.com', 'hey@example.com'];
const result = isEmailBlocklisted(email, blocklist);
const channelHandle = 'abc@example.com';
const email = 'hello@twenty.com';
const blocklist = ['hello@twenty.com', 'hey@twenty.com'];
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(true);
});
it('should return false if email is not blocklisted', () => {
const channelHandle = 'abc@example.com';
const email = 'hello@twenty.com';
const blocklist = ['hey@example.com'];
const result = isEmailBlocklisted(email, blocklist);
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(false);
});
it('should return false if email is null', () => {
const channelHandle = 'abc@twenty.com';
const email = null;
const blocklist = ['@example.com'];
const result = isEmailBlocklisted(email, blocklist);
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(false);
});
it('should return true for subdomains', () => {
const channelHandle = 'abc@example.com';
const email = 'hello@twenty.twenty.com';
const blocklist = ['@twenty.com'];
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(true);
});
it('should return false for domains which end with blocklisted domain but are not subdomains', () => {
const channelHandle = 'abc@example.com';
const email = 'hello@twentytwenty.com';
const blocklist = ['@twenty.com'];
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(false);
});
it('should return false if email is undefined', () => {
const channelHandle = 'abc@example.com';
const email = undefined;
const blocklist = ['@example.com'];
const result = isEmailBlocklisted(email, blocklist);
const blocklist = ['@twenty.com'];
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(false);
});
it('should return true if email ends with blocklisted domain', () => {
const email = 'hello@example.com';
const blocklist = ['@example.com'];
const result = isEmailBlocklisted(email, blocklist);
const channelHandle = 'abc@example.com';
const email = 'hello@twenty.com';
const blocklist = ['@twenty.com'];
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(true);
});

it('should return false if email is same as channel handle', () => {
const channelHandle = 'hello@twenty.com';
const email = 'hello@twenty.com';
const blocklist = ['@twenty.com'];
const result = isEmailBlocklisted(channelHandle, email, blocklist);

expect(result).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
export const isEmailBlocklisted = (
channelHandle: string,
email: string | null | undefined,
blocklist: string[],
): boolean => {
if (!email) {
if (!email || email === channelHandle) {
return false;
}

return blocklist.some((item) => {
if (item.startsWith('@')) {
return email.endsWith(item);
const domain = email.split('@')[1];

return domain === item.slice(1) || domain.endsWith(`.${item.slice(1)}`);
}

return email === item;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,13 @@ export class GoogleCalendarSyncService {

const blocklist = await this.getBlocklist(workspaceMemberId, workspaceId);

let filteredEvents = filterOutBlocklistedEvents(events, blocklist).filter(
(event) => event.status !== 'cancelled',
);
let filteredEvents = filterOutBlocklistedEvents(
calendarChannel.handle,
events,
blocklist,
).filter((event) => event.status !== 'cancelled');

if (emailOrDomainToReimport) {
// We still need to filter the events to only keep the ones that have the email or domain we want to reimport
// because the q parameter in the list method also filters the events that have the email or domain in their summary, description ...
// The q parameter allows us to narrow down the events
filteredEvents = filteredEvents.filter(
(event) =>
event.attendees?.some(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { calendar_v3 as calendarV3 } from 'googleapis';
import { isEmailBlocklisted } from 'src/modules/calendar-messaging-participant/utils/is-email-blocklisted.util';

export const filterOutBlocklistedEvents = (
calendarChannelHandle: string,
events: calendarV3.Schema$Event[],
blocklist: string[],
) => {
Expand All @@ -12,7 +13,8 @@ export const filterOutBlocklistedEvents = (
}

return event.attendees.every(
(attendee) => !isEmailBlocklisted(attendee.email, blocklist),
(attendee) =>
!isEmailBlocklisted(calendarChannelHandle, attendee.email, blocklist),
);
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,14 @@ export class BlocklistItemDeleteMessagesJob

const rolesToDelete: ('from' | 'to')[] = ['from', 'to'];

await this.messageChannelMessageAssociationRepository.deleteByMessageParticipantHandleAndMessageChannelIdsAndRoles(
handle,
messageChannelIds,
rolesToDelete,
workspaceId,
);
for (const messageChannelId of messageChannelIds) {
await this.messageChannelMessageAssociationRepository.deleteByMessageParticipantHandleAndMessageChannelIdAndRoles(
handle,
messageChannelId,
rolesToDelete,
workspaceId,
);
}

await this.threadCleanerService.cleanWorkspaceThreads(workspaceId);

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,28 @@ import { ObjectRecordDeleteEvent } from 'src/engine/integrations/event-emitter/t
import { MessageQueue } from 'src/engine/integrations/message-queue/message-queue.constants';
import { MessageQueueService } from 'src/engine/integrations/message-queue/services/message-queue.service';
import { BlocklistWorkspaceEntity } from 'src/modules/connected-account/standard-objects/blocklist.workspace-entity';
import {
BlocklistReimportMessagesJob,
BlocklistReimportMessagesJobData,
} from 'src/modules/messaging/blocklist-manager/jobs/messaging-blocklist-reimport-messages.job';
import {
BlocklistItemDeleteMessagesJobData,
BlocklistItemDeleteMessagesJob,
} from 'src/modules/messaging/blocklist-manager/jobs/messaging-blocklist-item-delete-messages.job';
import { ObjectRecordUpdateEvent } from 'src/engine/integrations/event-emitter/types/object-record-update.event';
import { InjectObjectMetadataRepository } from 'src/engine/object-metadata-repository/object-metadata-repository.decorator';
import { ConnectedAccountRepository } from 'src/modules/connected-account/repositories/connected-account.repository';
import { ConnectedAccountWorkspaceEntity } from 'src/modules/connected-account/standard-objects/connected-account.workspace-entity';
import { MessageChannelRepository } from 'src/modules/messaging/common/repositories/message-channel.repository';
import { MessagingChannelSyncStatusService } from 'src/modules/messaging/common/services/messaging-channel-sync-status.service';
import { MessageChannelWorkspaceEntity } from 'src/modules/messaging/common/standard-objects/message-channel.workspace-entity';

@Injectable()
export class MessagingBlocklistListener {
constructor(
@Inject(MessageQueue.messagingQueue)
private readonly messageQueueService: MessageQueueService,
@InjectObjectMetadataRepository(ConnectedAccountWorkspaceEntity)
private readonly connectedAccountRepository: ConnectedAccountRepository,
private readonly messagingChannelSyncStatusService: MessagingChannelSyncStatusService,
@InjectObjectMetadataRepository(MessageChannelWorkspaceEntity)
private readonly messageChannelRepository: MessageChannelRepository,
) {}

@OnEvent('blocklist.created')
Expand All @@ -40,35 +47,65 @@ export class MessagingBlocklistListener {
async handleDeletedEvent(
payload: ObjectRecordDeleteEvent<BlocklistWorkspaceEntity>,
) {
await this.messageQueueService.add<BlocklistReimportMessagesJobData>(
BlocklistReimportMessagesJob.name,
{
workspaceId: payload.workspaceId,
workspaceMemberId: payload.properties.before.workspaceMember.id,
handle: payload.properties.before.handle,
},
const workspaceMemberId = payload.properties.before.workspaceMember.id;
const workspaceId = payload.workspaceId;

const connectedAccount =
await this.connectedAccountRepository.getAllByWorkspaceMemberId(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it feels wrong that getAll is returning only one connectedAccount but we will change this once we have the ORM

workspaceMemberId,
workspaceId,
);

if (!connectedAccount || connectedAccount.length === 0) {
return;
}

const messageChannel =
await this.messageChannelRepository.getByConnectedAccountId(
connectedAccount[0].id,
workspaceId,
);

await this.messagingChannelSyncStatusService.resetAndScheduleFullMessageListFetch(
messageChannel[0].id,
workspaceId,
);
}

@OnEvent('blocklist.updated')
async handleUpdatedEvent(
payload: ObjectRecordUpdateEvent<BlocklistWorkspaceEntity>,
) {
const workspaceMemberId = payload.properties.before.workspaceMember.id;
const workspaceId = payload.workspaceId;

await this.messageQueueService.add<BlocklistItemDeleteMessagesJobData>(
BlocklistItemDeleteMessagesJob.name,
{
workspaceId: payload.workspaceId,
workspaceId,
blocklistItemId: payload.recordId,
},
);

await this.messageQueueService.add<BlocklistReimportMessagesJobData>(
BlocklistReimportMessagesJob.name,
{
workspaceId: payload.workspaceId,
workspaceMemberId: payload.properties.after.workspaceMember.id,
handle: payload.properties.before.handle,
},
const connectedAccount =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like duplicated code here

await this.connectedAccountRepository.getAllByWorkspaceMemberId(
workspaceMemberId,
workspaceId,
);

if (!connectedAccount || connectedAccount.length === 0) {
return;
}

const messageChannel =
await this.messageChannelRepository.getByConnectedAccountId(
connectedAccount[0].id,
workspaceId,
);

await this.messagingChannelSyncStatusService.resetAndScheduleFullMessageListFetch(
messageChannel[0].id,
workspaceId,
);
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import { Module } from '@nestjs/common';

import { BlocklistItemDeleteMessagesJob } from 'src/modules/messaging/blocklist-manager/jobs/messaging-blocklist-item-delete-messages.job';
import { MessagingBlocklistListener } from 'src/modules/messaging/blocklist-manager/listeners/messaging-blocklist.listener';
import { MessagingCommonModule } from 'src/modules/messaging/common/messaging-common.module';
import { MessagingMessageCleanerModule } from 'src/modules/messaging/message-cleaner/messaging-message-cleaner.module';

@Module({
imports: [],
providers: [],
imports: [MessagingCommonModule, MessagingMessageCleanerModule],
providers: [
MessagingBlocklistListener,
{
provide: BlocklistItemDeleteMessagesJob.name,
useClass: BlocklistItemDeleteMessagesJob,
},
],
exports: [],
})
export class MessagingBlocklistManagerModule {}
Loading
Loading