Skip to content

Commit

Permalink
fix: don't clean up settings when optional data is not present (#5118)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
gastonfournier committed Oct 23, 2023
1 parent 34395d3 commit 2aebc8c
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 14 deletions.
30 changes: 18 additions & 12 deletions src/lib/db/project-store.ts
Expand Up @@ -6,6 +6,7 @@ import {
IEnvironment,
IFlagResolver,
IProject,
IProjectUpdate,
IProjectWithCount,
ProjectMode,
} from '../types';
Expand Down Expand Up @@ -259,25 +260,30 @@ class ProjectStore implements IProjectStore {
return present;
}

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
async update(data): Promise<void> {
async update(data: IProjectUpdate): Promise<void> {
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);
Expand Down
11 changes: 9 additions & 2 deletions src/lib/services/project-service.ts
Expand Up @@ -39,6 +39,7 @@ import {
ProjectAccessUserRolesDeleted,
IFeatureNaming,
CreateProject,
IProjectUpdate,
} from '../types';
import {
IProjectQuery,
Expand Down Expand Up @@ -259,16 +260,22 @@ export default class ProjectService {
return data;
}

async updateProject(updatedProject: IProject, user: IUser): Promise<void> {
async updateProject(
updatedProject: IProjectUpdate,
user: IUser,
): Promise<void> {
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,
});
}
Expand Down
10 changes: 10 additions & 0 deletions src/lib/types/model.ts
Expand Up @@ -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
*/
Expand Down

0 comments on commit 2aebc8c

Please sign in to comment.