From 2aebc8c58e946e1830fc15624624b5ce9ca244ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Mon, 23 Oct 2023 10:15:25 +0200 Subject: [PATCH] fix: don't clean up settings when optional data is not present (#5118) ## About the changes This fixes a bug updating a project, when optional data (defaultStickiness and featureLimit are not part of the payload). The problem happens due to: 1. ProjectController does not use the type: UpdateProjectSchema for the request body (will be addressed in another PR in unleash-enterprise) 2. Project Store interface does not match UpdateProjectSchema (but it relies on accepting `additional properties: true`, which is what we agreed on for input) 3. Feature limit is not defined in UpdateProjectSchema (also addressed in the other PR) --- src/lib/db/project-store.ts | 30 +++++++++++++++++------------ src/lib/services/project-service.ts | 11 +++++++++-- src/lib/types/model.ts | 10 ++++++++++ 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/lib/db/project-store.ts b/src/lib/db/project-store.ts index 85d2b7886b..7f04bff875 100644 --- a/src/lib/db/project-store.ts +++ b/src/lib/db/project-store.ts @@ -6,6 +6,7 @@ import { IEnvironment, IFlagResolver, IProject, + IProjectUpdate, IProjectWithCount, ProjectMode, } from '../types'; @@ -259,25 +260,30 @@ class ProjectStore implements IProjectStore { return present; } - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - async update(data): Promise { + async update(data: IProjectUpdate): Promise { try { await this.db(TABLE) .where({ id: data.id }) .update(this.fieldToRow(data)); - if (await this.hasProjectSettings(data.id)) { - await this.db(SETTINGS_TABLE) - .where({ project: data.id }) - .update({ + if ( + data.defaultStickiness !== undefined || + data.featureLimit !== undefined + ) { + if (await this.hasProjectSettings(data.id)) { + await this.db(SETTINGS_TABLE) + .where({ project: data.id }) + .update({ + default_stickiness: data.defaultStickiness, + feature_limit: data.featureLimit, + }); + } else { + // What happens with project mode in this case? + await this.db(SETTINGS_TABLE).insert({ + project: data.id, default_stickiness: data.defaultStickiness, feature_limit: data.featureLimit, }); - } else { - await this.db(SETTINGS_TABLE).insert({ - project: data.id, - default_stickiness: data.defaultStickiness, - feature_limit: data.featureLimit, - }); + } } } catch (err) { this.logger.error('Could not update project, error: ', err); diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 961b20c71f..0683c4883c 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -39,6 +39,7 @@ import { ProjectAccessUserRolesDeleted, IFeatureNaming, CreateProject, + IProjectUpdate, } from '../types'; import { IProjectQuery, @@ -259,16 +260,22 @@ export default class ProjectService { return data; } - async updateProject(updatedProject: IProject, user: IUser): Promise { + async updateProject( + updatedProject: IProjectUpdate, + user: IUser, + ): Promise { const preData = await this.projectStore.get(updatedProject.id); await this.projectStore.update(updatedProject); + // updated project contains instructions to update the project but it may not represent a whole project + const afterData = await this.projectStore.get(updatedProject.id); + await this.eventStore.store({ type: PROJECT_UPDATED, project: updatedProject.id, createdBy: getCreatedBy(user), - data: updatedProject, + data: afterData, preData, }); } diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 060ce32bd8..63f9718bbd 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -449,6 +449,16 @@ export interface IProject { featureNaming?: IFeatureNaming; } +// mimics UpdateProjectSchema +export interface IProjectUpdate { + id: string; + name: string; + description?: string; + mode?: ProjectMode; + defaultStickiness?: string; + featureLimit?: number; +} + /** * Extends IRole making description mandatory */