Skip to content

Update what is being audit logged #11833

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 0 additions & 3 deletions packages/twenty-front/src/generated-metadata/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -625,11 +625,8 @@ export type FeatureFlagDto = {

export enum FeatureFlagKey {
IsAirtableIntegrationEnabled = 'IsAirtableIntegrationEnabled',
IsAnalyticsV2Enabled = 'IsAnalyticsV2Enabled',
IsApprovedAccessDomainsEnabled = 'IsApprovedAccessDomainsEnabled',
IsCopilotEnabled = 'IsCopilotEnabled',
IsCustomDomainEnabled = 'IsCustomDomainEnabled',
IsEventObjectEnabled = 'IsEventObjectEnabled',
IsJsonFilterEnabled = 'IsJsonFilterEnabled',
IsNewRelationEnabled = 'IsNewRelationEnabled',
IsPermissionsV2Enabled = 'IsPermissionsV2Enabled',
Expand Down
5 changes: 1 addition & 4 deletions packages/twenty-front/src/generated/graphql.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { gql } from '@apollo/client';
import * as Apollo from '@apollo/client';
import { gql } from '@apollo/client';
export type Maybe<T> = T | null;
export type InputMaybe<T> = Maybe<T>;
export type Exact<T extends { [key: string]: unknown }> = { [K in keyof T]: T[K] };
Expand Down Expand Up @@ -575,11 +575,8 @@ export type FeatureFlagDto = {

export enum FeatureFlagKey {
IsAirtableIntegrationEnabled = 'IsAirtableIntegrationEnabled',
IsAnalyticsV2Enabled = 'IsAnalyticsV2Enabled',
IsApprovedAccessDomainsEnabled = 'IsApprovedAccessDomainsEnabled',
IsCopilotEnabled = 'IsCopilotEnabled',
IsCustomDomainEnabled = 'IsCustomDomainEnabled',
IsEventObjectEnabled = 'IsEventObjectEnabled',
IsJsonFilterEnabled = 'IsJsonFilterEnabled',
IsNewRelationEnabled = 'IsNewRelationEnabled',
IsPermissionsV2Enabled = 'IsPermissionsV2Enabled',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ import { SettingsApprovedAccessDomainsListCard } from '@/settings/security/compo
import { ToggleImpersonate } from '@/settings/workspace/components/ToggleImpersonate';
import { SettingsPath } from '@/types/SettingsPath';
import { SubMenuTopBarContainer } from '@/ui/layout/page/components/SubMenuTopBarContainer';
import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled';
import { useRecoilValue } from 'recoil';
import { FeatureFlagKey } from '~/generated/graphql';
import { getSettingsPath } from '~/utils/navigation/getSettingsPath';
import { Tag } from 'twenty-ui/components';
import { H2Title, IconLock } from 'twenty-ui/display';
import { Section } from 'twenty-ui/layout';
import { Tag } from 'twenty-ui/components';
import { getSettingsPath } from '~/utils/navigation/getSettingsPath';

const StyledContainer = styled.div`
width: 100%;
Expand All @@ -36,9 +34,6 @@ export const SettingsSecurity = () => {
const { t } = useLingui();

const isMultiWorkspaceEnabled = useRecoilValue(isMultiWorkspaceEnabledState);
const IsApprovedAccessDomainsEnabled = useIsFeatureEnabled(
FeatureFlagKey.IsApprovedAccessDomainsEnabled,
);

return (
<SubMenuTopBarContainer
Expand Down Expand Up @@ -68,15 +63,13 @@ export const SettingsSecurity = () => {
/>
<SettingsSSOIdentitiesProvidersListCard />
</StyledSection>
{IsApprovedAccessDomainsEnabled && (
<StyledSection>
<H2Title
title={t`Approved Domains`}
description={t`Anyone with an email address at these domains is allowed to sign up for this workspace.`}
/>
<SettingsApprovedAccessDomainsListCard />
</StyledSection>
)}
<StyledSection>
<H2Title
title={t`Approved Domains`}
description={t`Anyone with an email address at these domains is allowed to sign up for this workspace.`}
/>
<SettingsApprovedAccessDomainsListCard />
</StyledSection>
<Section>
<StyledContainer>
<H2Title
Expand Down
4 changes: 2 additions & 2 deletions packages/twenty-server/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,14 @@
"executor": "nx:run-commands",
"options": {
"cwd": "packages/twenty-server",
"command": "nx ts-node-no-deps -- src/database/clickhouse/migrations/run-migrations.ts"
"command": "nx ts-node-no-deps -- src/database/clickHouse/migrations/run-migrations.ts"
}
},
"clickhouse:seed": {
"executor": "nx:run-commands",
"options": {
"cwd": "packages/twenty-server",
"command": "nx ts-node-no-deps -- src/database/clickhouse/seeds/run-seeds.ts"
"command": "nx ts-node-no-deps -- src/database/clickHouse/seeds/run-seeds.ts"
}
},
"lingui:extract": {
Expand Down
2 changes: 2 additions & 0 deletions packages/twenty-server/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module';
import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module';
import { ModulesModule } from 'src/modules/modules.module';

import { ClickHouseModule } from './database/clickHouse/clickHouse.module';
import { CoreEngineModule } from './engine/core-modules/core-engine.module';
import { I18nModule } from './engine/core-modules/i18n/i18n.module';

Expand All @@ -52,6 +53,7 @@ const MIGRATED_REST_METHODS = [
useClass: GraphQLConfigService,
}),
TwentyORMModule,
ClickHouseModule,
// Core engine module, contains all the core modules
CoreEngineModule,
// Modules module, contains all business logic modules
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Module } from '@nestjs/common';

import { TwentyConfigModule } from 'src/engine/core-modules/twenty-config/twenty-config.module';

import { ClickHouseService } from './clickHouse.service';

@Module({
imports: [TwentyConfigModule],
providers: [ClickHouseService],
exports: [ClickHouseService],
})
export class ClickHouseModule {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
import { Test, TestingModule } from '@nestjs/testing';

import { ClickHouseClient } from '@clickhouse/client';

import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';

import { ClickHouseService } from './clickHouse.service';

// Mock the createClient function from @clickhouse/client
jest.mock('@clickhouse/client', () => ({
createClient: jest.fn().mockReturnValue({
insert: jest.fn().mockResolvedValue({}),
query: jest.fn().mockResolvedValue({
json: jest.fn().mockResolvedValue([{ test: 'data' }]),
}),
ping: jest.fn().mockResolvedValue({ success: true }),
close: jest.fn().mockResolvedValue({}),
exec: jest.fn().mockResolvedValue({}),
}),
}));

describe('ClickHouseService', () => {
let service: ClickHouseService;
let twentyConfigService: TwentyConfigService;
let mockClickHouseClient: jest.Mocked<ClickHouseClient>;

beforeEach(async () => {
jest.clearAllMocks();

mockClickHouseClient = {
insert: jest.fn().mockResolvedValue({}),
query: jest.fn().mockResolvedValue({
json: jest.fn().mockResolvedValue([{ test: 'data' }]),
}),
ping: jest.fn().mockResolvedValue({ success: true }),
close: jest.fn().mockResolvedValue({}),
exec: jest.fn().mockResolvedValue({}),
} as unknown as jest.Mocked<ClickHouseClient>;

const module: TestingModule = await Test.createTestingModule({
providers: [
ClickHouseService,
{
provide: TwentyConfigService,
useValue: {
get: jest.fn((key) => {
if (key === 'CLICKHOUSE_URL') return 'http://localhost:8123';

return undefined;
}),
},
},
],
}).compile();

service = module.get<ClickHouseService>(ClickHouseService);
twentyConfigService = module.get<TwentyConfigService>(TwentyConfigService);

// Set the mock client
(service as any).mainClient = mockClickHouseClient;
});

it('should be defined', () => {
expect(service).toBeDefined();
});

describe('constructor', () => {
it('should not initialize clickhouse client when clickhouse is disabled', async () => {
jest.spyOn(twentyConfigService, 'get').mockImplementation((key) => {
if (key === 'CLICKHOUSE_URL') return '';

return undefined;
});

const newModule: TestingModule = await Test.createTestingModule({
providers: [
ClickHouseService,
{
provide: TwentyConfigService,
useValue: twentyConfigService,
},
],
}).compile();

const newService = newModule.get<ClickHouseService>(ClickHouseService);

expect((newService as any).mainClient).toBeUndefined();
});
});

describe('insert', () => {
it('should insert data into clickhouse and return success', async () => {
const testData = [{ id: 1, name: 'test' }];
const result = await service.insert('test_table', testData);

expect(result).toEqual({ success: true });
expect(mockClickHouseClient.insert).toHaveBeenCalledWith({
table: 'test_table',
values: testData,
format: 'JSONEachRow',
});
});

it('should return failure when clickhouse client is not defined', async () => {
(service as any).mainClient = undefined;

const testData = [{ id: 1, name: 'test' }];
const result = await service.insert('test_table', testData);

expect(result).toEqual({ success: false });
});

it('should handle errors and return failure', async () => {
const testError = new Error('Test error');

mockClickHouseClient.insert.mockRejectedValueOnce(testError);

const testData = [{ id: 1, name: 'test' }];
const result = await service.insert('test_table', testData);

expect(result).toEqual({ success: false });
// Since the service uses logger.error instead of exceptionHandlerService.captureExceptions,
// we don't need to assert on exceptionHandlerService
});
});

describe('select', () => {
it('should execute a query and return results', async () => {
const query = 'SELECT * FROM test_table WHERE id = {id:Int32}';
const params = { id: 1 };

mockClickHouseClient.query.mockResolvedValueOnce({
json: jest.fn().mockResolvedValueOnce([{ id: 1, name: 'test' }]),
} as any);
Comment on lines +132 to +134
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Type assertion to 'any' could hide type mismatches. Consider creating a proper mock type that matches ClickHouseClient's QueryResult interface.


const result = await service.select(query, params);

expect(result).toEqual([{ id: 1, name: 'test' }]);
expect(mockClickHouseClient.query).toHaveBeenCalledWith({
query,
format: 'JSONEachRow',
query_params: params,
});
});

it('should return empty array when clickhouse client is not defined', async () => {
(service as any).mainClient = undefined;

const query = 'SELECT * FROM test_table';
const result = await service.select(query);

expect(result).toEqual([]);
});

it('should handle errors and return empty array', async () => {
const testError = new Error('Test error');

mockClickHouseClient.query.mockRejectedValueOnce(testError);

const query = 'SELECT * FROM test_table';
const result = await service.select(query);

expect(result).toEqual([]);
// Since the service uses logger.error instead of exceptionHandlerService.captureExceptions,
// we don't need to assert on exceptionHandlerService
});
});

describe('createDatabase', () => {
it('should create a database and return true', async () => {
const result = await service.createDatabase('test_db');

expect(result).toBe(true);
expect(mockClickHouseClient.exec).toHaveBeenCalledWith({
query: 'CREATE DATABASE IF NOT EXISTS test_db',
});
});

it('should return false when clickhouse client is not defined', async () => {
(service as any).mainClient = undefined;

const result = await service.createDatabase('test_db');

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

describe('dropDatabase', () => {
it('should drop a database and return true', async () => {
const result = await service.dropDatabase('test_db');

expect(result).toBe(true);
expect(mockClickHouseClient.exec).toHaveBeenCalledWith({
query: 'DROP DATABASE IF EXISTS test_db',
});
});

it('should return false when clickhouse client is not defined', async () => {
(service as any).mainClient = undefined;

const result = await service.dropDatabase('test_db');

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

describe('connectToClient', () => {
it('should connect to a client and return it', async () => {
jest
.spyOn(service, 'connectToClient')
.mockResolvedValueOnce(mockClickHouseClient);
Comment on lines +209 to +211
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Mocking connectToClient while testing connectToClient creates a circular dependency and may mask real issues. Consider testing the actual implementation instead.


const result = await service.connectToClient('test-client');

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

it('should reuse an existing client if available', async () => {
// Set up a client in the map
(service as any).clients.set('test-client', mockClickHouseClient);

const result = await service.connectToClient('test-client');

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

describe('disconnectFromClient', () => {
it('should disconnect from a client', async () => {
// Set up a client in the map
(service as any).clients.set('test-client', mockClickHouseClient);

await service.disconnectFromClient('test-client');

expect(mockClickHouseClient.close).toHaveBeenCalled();
expect((service as any).clients.has('test-client')).toBe(false);
});

it('should do nothing if client does not exist', async () => {
await service.disconnectFromClient('non-existent-client');

expect(mockClickHouseClient.close).not.toHaveBeenCalled();
});
});

describe('lifecycle hooks', () => {
it('should ping server on module init', async () => {
await service.onModuleInit();

expect(mockClickHouseClient.ping).toHaveBeenCalled();
});

it('should close all clients on module destroy', async () => {
// Set up a couple of clients
(service as any).clients.set('client1', mockClickHouseClient);
(service as any).clients.set('client2', mockClickHouseClient);

await service.onModuleDestroy();

// One for mainClient, and two for the clients in the map
expect(mockClickHouseClient.close).toHaveBeenCalledTimes(3);
});
});
});
Loading
Loading