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

Fix missing events during account connection #9825

Merged
merged 3 commits into from
Jan 24, 2025
Merged

Fix missing events during account connection #9825

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07ae88c
Fix missing events during account connection
FelixMalfait Jan 23, 2025
959bbbd
Missing dependency
FelixMalfait Jan 23, 2025
14b330f
Fixing missing workspaceId in various places
FelixMalfait Jan 24, 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
2 changes: 2 additions & 0 deletions packages/twenty-server/src/engine/core-modules/auth/auth.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { WorkspaceInvitationModule } from 'src/engine/core-modules/workspace-inv
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { WorkspaceModule } from 'src/engine/core-modules/workspace/workspace.module';
import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/workspace-datasource.module';
import { WorkspaceManagerModule } from 'src/engine/workspace-manager/workspace-manager.module';
import { ConnectedAccountModule } from 'src/modules/connected-account/connected-account.module';
Expand Down Expand Up @@ -71,6 +72,7 @@ import { JwtAuthStrategy } from './strategies/jwt.auth.strategy';
],
'core',
),
TypeOrmModule.forFeature([ObjectMetadataEntity], 'metadata'),
HttpModule,
UserWorkspaceModule,
WorkspaceModule,
Expand Down
126 changes: 120 additions & 6 deletions packages/twenty-server/src/engine/core-modules/auth/services/google-apis.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';

import { EntityManager } from 'typeorm';
import { EntityManager, Repository } from 'typeorm';
import { v4 } from 'uuid';

import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action';
import { getGoogleApisOauthScopes } from 'src/engine/core-modules/auth/utils/get-google-apis-oauth-scopes';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator';
import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants';
import { MessageQueueService } from 'src/engine/core-modules/message-queue/services/message-queue.service';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter';
import {
CalendarEventListFetchJob,
CalendarEventListFetchJobData,
Expand Down Expand Up @@ -45,6 +49,9 @@ export class GoogleAPIsService {
private readonly calendarQueueService: MessageQueueService,
private readonly environmentService: EnvironmentService,
private readonly accountsToReconnectService: AccountsToReconnectService,
private readonly workspaceEventEmitter: WorkspaceEventEmitter,
@InjectRepository(ObjectMetadataEntity, 'metadata')
private readonly objectMetadataRepository: Repository<ObjectMetadataEntity>,
) {}

async refreshGoogleRefreshToken(input: {
Expand Down Expand Up @@ -100,7 +107,7 @@ export class GoogleAPIsService {

await workspaceDataSource.transaction(async (manager: EntityManager) => {
if (!existingAccountId) {
await connectedAccountRepository.save(
const newConnectedAccount = await connectedAccountRepository.save(
{
id: newOrExistingConnectedAccountId,
handle,
Expand All @@ -114,7 +121,27 @@ export class GoogleAPIsService {
manager,
);

await messageChannelRepository.save(
const connectedAccountMetadata =
Copy link
Member

@charlesBochet charlesBochet Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

First read:

  • Shouldn't we also check the workspaceId?

Second read: (optional)

  • i think it's fine as we are in the write path but we could also leverage the cached metadata (through WorkspaceCacheStorageService) to avoid performing additional queries on the database.

Third read: (optional)
I would actually remove objectMetadata from the Event payload completly (so no need to fetch it anymore):

  • as we already have objectMetadataNameSingular as part of the BatchEvent.
  • Also looks a bit heavy to force the caller to provide the whole objectMetadata everytime. I think there is only a few places we will need to have more than the nameSingular (maybe timelineActivity need the id for instance), we could leave it to timelineActivity to fetch the objectMetadata based on the nameSingular

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree with the comments. I'm not sure how far I can go as part of this PR.
1 should be done for sure (I copied the query from somewhere else so I will check it's probably missing in other places). As this pattern is already used elsewhere, 2 and 3 might require large refactoring that shouldn't be part of this PR - in that case should I create a followup issue that details (3) and merge with (1) for now?

Copy link
Member

Choose a reason for hiding this comment

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

yes LGTM

await this.objectMetadataRepository.findOneOrFail({
where: { nameSingular: 'connectedAccount' },
});

this.workspaceEventEmitter.emitDatabaseBatchEvent({
objectMetadataNameSingular: 'connectedAccount',
action: DatabaseEventAction.CREATED,
events: [
{
recordId: newConnectedAccount.id,
objectMetadata: connectedAccountMetadata,
properties: {
after: newConnectedAccount,
},
},
],
workspaceId,
});

const newMessageChannel = await messageChannelRepository.save(
{
id: v4(),
connectedAccountId: newOrExistingConnectedAccountId,
Expand All @@ -128,8 +155,28 @@ export class GoogleAPIsService {
manager,
);

const messageChannelMetadata =
await this.objectMetadataRepository.findOneOrFail({
where: { nameSingular: 'messageChannel' },
});

this.workspaceEventEmitter.emitDatabaseBatchEvent({
objectMetadataNameSingular: 'messageChannel',
action: DatabaseEventAction.CREATED,
events: [
{
recordId: newMessageChannel.id,
objectMetadata: messageChannelMetadata,
properties: {
after: newMessageChannel,
},
},
],
workspaceId,
});

if (isCalendarEnabled) {
await calendarChannelRepository.save(
const newCalendarChannel = await calendarChannelRepository.save(
{
id: v4(),
connectedAccountId: newOrExistingConnectedAccountId,
Expand All @@ -141,9 +188,29 @@ export class GoogleAPIsService {
{},
manager,
);

const calendarChannelMetadata =
await this.objectMetadataRepository.findOneOrFail({
where: { nameSingular: 'calendarChannel' },
});

this.workspaceEventEmitter.emitDatabaseBatchEvent({
objectMetadataNameSingular: 'calendarChannel',
action: DatabaseEventAction.CREATED,
events: [
{
recordId: newCalendarChannel.id,
objectMetadata: calendarChannelMetadata,
properties: {
after: newCalendarChannel,
},
},
],
workspaceId,
});
}
} else {
await connectedAccountRepository.update(
const updatedConnectedAccount = await connectedAccountRepository.update(
{
id: newOrExistingConnectedAccountId,
},
Expand All @@ -155,6 +222,30 @@ export class GoogleAPIsService {
manager,
);

const connectedAccountMetadata =
await this.objectMetadataRepository.findOneOrFail({
where: { nameSingular: 'connectedAccount' },
});

this.workspaceEventEmitter.emitDatabaseBatchEvent({
objectMetadataNameSingular: 'connectedAccount',
action: DatabaseEventAction.UPDATED,
events: [
{
recordId: newOrExistingConnectedAccountId,
objectMetadata: connectedAccountMetadata,
properties: {
before: connectedAccount,
after: {
...connectedAccount,
...updatedConnectedAccount.raw[0],
},
},
},
],
workspaceId,
});

const workspaceMemberRepository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkspaceMemberWorkspaceEntity>(
workspaceId,
Expand All @@ -173,7 +264,11 @@ export class GoogleAPIsService {
newOrExistingConnectedAccountId,
);

await messageChannelRepository.update(
const messageChannels = await messageChannelRepository.find({
where: { connectedAccountId: newOrExistingConnectedAccountId },
});

const messageChannelUpdates = await messageChannelRepository.update(
{
connectedAccountId: newOrExistingConnectedAccountId,
},
Expand All @@ -185,6 +280,25 @@ export class GoogleAPIsService {
},
manager,
);

const messageChannelMetadata =
await this.objectMetadataRepository.findOneOrFail({
where: { nameSingular: 'messageChannel' },
});

this.workspaceEventEmitter.emitDatabaseBatchEvent({
objectMetadataNameSingular: 'messageChannel',
action: DatabaseEventAction.UPDATED,
events: messageChannels.map((messageChannel) => ({
recordId: messageChannel.id,
objectMetadata: messageChannelMetadata,
properties: {
before: messageChannel,
after: { ...messageChannel, ...messageChannelUpdates.raw[0] },
},
})),
workspaceId,
});
}
});

Expand Down
Loading
Loading