From 9883472d55c8d0e7e8e47168b7973b603835b7a2 Mon Sep 17 00:00:00 2001 From: Weiko Date: Fri, 14 Mar 2025 15:01:06 +0100 Subject: [PATCH] fix public feature flag update (#10887) ## Context upsert from typeorm does not seem to return keys that are not updated, I'm reverting back to find/save since upsert is not consistent --- .../__tests__/feature-flag.service.spec.ts | 22 ++++----- .../services/feature-flag.service.ts | 48 +++++++++++-------- .../workspace.integration-spec.ts | 2 +- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts b/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts index fdefa21d9..21475475e 100644 --- a/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts @@ -25,6 +25,8 @@ describe('FeatureFlagService', () => { findOneBy: jest.fn(), find: jest.fn(), upsert: jest.fn(), + findOne: jest.fn(), + save: jest.fn(), }; const workspaceId = 'workspace-id'; @@ -179,9 +181,7 @@ describe('FeatureFlagService', () => { workspaceId, }; - mockFeatureFlagRepository.upsert.mockResolvedValue({ - generatedMaps: [mockFeatureFlag], - }); + mockFeatureFlagRepository.save.mockResolvedValue(mockFeatureFlag); ( featureFlagValidator.assertIsFeatureFlagKey as jest.Mock @@ -196,17 +196,11 @@ describe('FeatureFlagService', () => { // Assert expect(result).toEqual(mockFeatureFlag); - expect(mockFeatureFlagRepository.upsert).toHaveBeenCalledWith( - { - key: FeatureFlagKey[featureFlag], - value, - workspaceId, - }, - { - conflictPaths: ['workspaceId', 'key'], - skipUpdateIfNoValuesChanged: true, - }, - ); + expect(mockFeatureFlagRepository.save).toHaveBeenCalledWith({ + key: FeatureFlagKey[featureFlag], + value, + workspaceId, + }); }); it('should throw an exception when feature flag key is invalid', async () => { diff --git a/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts b/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts index 8225b9e3d..dfdbd48f5 100644 --- a/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts +++ b/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts @@ -82,16 +82,6 @@ export class FeatureFlagService { value: boolean; shouldBePublic?: boolean; }): Promise { - if (shouldBePublic) { - publicFeatureFlagValidator.assertIsPublicFeatureFlag( - featureFlag, - new FeatureFlagException( - 'Invalid feature flag key, flag is not public', - FeatureFlagExceptionCode.INVALID_FEATURE_FLAG_KEY, - ), - ); - } - featureFlagValidator.assertIsFeatureFlagKey( featureFlag, new FeatureFlagException( @@ -100,18 +90,36 @@ export class FeatureFlagService { ), ); - const upsertResult = await this.featureFlagRepository.upsert( - { - key: FeatureFlagKey[featureFlag], - value, + const featureFlagKey = FeatureFlagKey[featureFlag]; + + if (shouldBePublic) { + publicFeatureFlagValidator.assertIsPublicFeatureFlag( + featureFlagKey, + new FeatureFlagException( + 'Invalid feature flag key, flag is not public', + FeatureFlagExceptionCode.INVALID_FEATURE_FLAG_KEY, + ), + ); + } + + const existingFeatureFlag = await this.featureFlagRepository.findOne({ + where: { + key: featureFlagKey, workspaceId: workspaceId, }, - { - conflictPaths: ['workspaceId', 'key'], - skipUpdateIfNoValuesChanged: true, - }, - ); + }); - return upsertResult.generatedMaps[0] as FeatureFlag; + const featureFlagToSave = existingFeatureFlag + ? { + ...existingFeatureFlag, + value, + } + : { + key: featureFlagKey, + value, + workspaceId: workspaceId, + }; + + return await this.featureFlagRepository.save(featureFlagToSave); } } diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts index a54df968d..addd0fcc1 100644 --- a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts @@ -472,7 +472,7 @@ describe('workspace permissions', () => { `, variables: { input: { - publicFeatureFlag: 'TestFeature', + publicFeatureFlag: 'IsStripeIntegrationEnabled', value: true, }, },