fix: API improvements aligning the types to our schemas (#4650)

Some of our types in OSS have drifted apart from our OpenAPI schemas. This will help them be aligned again
Unleash · Sep 12, 2023 · c39d815 · c39d815
1 parent 2b2f5e2
commit c39d815
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 86 deletions.
diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts
@@ -20,6 +20,7 @@ import {
     ROOT_PERMISSION_TYPE,
 } from '../util/constants';
 import { Db } from './db';
+import { IdPermissionRef } from 'lib/services/access-service';
 
 const T = {
     ROLE_USER: 'role_user',
@@ -221,7 +222,7 @@ export class AccessStore implements IAccessStore {
 
     async addEnvironmentPermissionsToRole(
         role_id: number,
-        permissions: IPermission[],
+        permissions: IdPermissionRef[],
     ): Promise<void> {
         const rows = permissions.map((permission) => {
             return {

diff --git a/src/lib/openapi/spec/create-strategy-schema.ts b/src/lib/openapi/spec/create-strategy-schema.ts
@@ -12,12 +12,29 @@ export const createStrategySchema = {
             description: 'The name of the strategy type. Must be unique.',
             example: 'my-custom-strategy',
         },
+        title: {
+            type: 'string',
+            description: 'The title of the strategy',
+            example: 'My awesome strategy',
+        },
         description: {
             type: 'string',
             description: 'A description of the strategy type.',
             example:
                 'Enable the feature for users who have not logged in before.',
         },
+        editable: {
+            type: 'boolean',
+            description:
+                'Whether the strategy type is editable or not. Defaults to `true`.',
+            example: false,
+        },
+        deprecated: {
+            type: 'boolean',
+            description:
+                'Whether the strategy type is deprecated or not. Defaults to `false`.',
+            example: true,
+        },
         parameters: {
             type: 'array',
             description:

diff --git a/src/lib/routes/admin-api/strategy.ts b/src/lib/routes/admin-api/strategy.ts
@@ -238,7 +238,7 @@ class StrategyController extends Controller {
     }
 
     async createStrategy(
-        req: IAuthRequest<unknown, CreateStrategySchema>,
+        req: IAuthRequest<unknown, unknown, CreateStrategySchema>,
         res: Response<StrategySchema>,
     ): Promise<void> {
         const userName = extractUsername(req);

diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts
@@ -52,25 +52,30 @@ const PROJECT_ADMIN = [
     permissions.DELETE_FEATURE,
 ];
 
+/** @deprecated prefer to use NamePermissionRef */
+export type IdPermissionRef = Pick<IPermission, 'id' | 'environment'>;
+export type NamePermissionRef = Pick<IPermission, 'name' | 'environment'>;
+export type PermissionRef = IdPermissionRef | NamePermissionRef;
+
 interface IRoleCreation {
     name: string;
     description: string;
     type?: 'root-custom' | 'custom';
-    permissions?: IPermission[];
+    permissions?: PermissionRef[];
 }
 
 export interface IRoleValidation {
     name: string;
     description?: string;
-    permissions?: Pick<IPermission, 'id' | 'environment'>[];
+    permissions?: PermissionRef[];
 }
 
 interface IRoleUpdate {
     id: number;
     name: string;
     description: string;
     type?: 'root-custom' | 'custom';
-    permissions?: IPermission[];
+    permissions?: PermissionRef[];
 }
 
 export interface AccessWithRoles {
@@ -627,7 +632,7 @@ export class AccessService {
             if (roleType === CUSTOM_ROOT_ROLE_TYPE) {
                 await this.store.addPermissionsToRole(
                     newRole.id,
-                    rolePermissions.map(({ name }) => name),
+                    rolePermissions.map((p: NamePermissionRef) => p.name),
                 );
             } else {
                 await this.store.addEnvironmentPermissionsToRole(
@@ -668,7 +673,7 @@ export class AccessService {
             if (roleType === CUSTOM_ROOT_ROLE_TYPE) {
                 await this.store.addPermissionsToRole(
                     newRole.id,
-                    rolePermissions.map(({ name }) => name),
+                    rolePermissions.map((p: NamePermissionRef) => p.name),
                 );
             } else {
                 await this.store.addEnvironmentPermissionsToRole(

diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts
@@ -39,6 +39,7 @@ import {
     IProjectRoleUsage,
     ProjectAccessUserRolesDeleted,
     IFeatureNaming,
+    CreateProject,
 } from '../types';
 import { IProjectQuery, IProjectStore } from '../types/stores/project-store';
 import {
@@ -190,10 +191,7 @@ export default class ProjectService {
     };
 
     async createProject(
-        newProject: Pick<
-            IProject,
-            'id' | 'name' | 'mode' | 'defaultStickiness'
-        >,
+        newProject: CreateProject,
         user: IUser,
     ): Promise<IProject> {
         const data = await projectSchema.validateAsync(newProject);

diff --git a/src/lib/services/state-service.test.ts b/src/lib/services/state-service.test.ts
@@ -320,6 +320,7 @@ test('should export strategies', async () => {
     await stores.strategyStore.createStrategy({
         name: 'a-strategy',
         editable: true,
+        parameters: [],
     });
 
     const data = await stateService.export({ includeStrategies: true });
@@ -595,7 +596,10 @@ test('exporting to new format works', async () => {
     await stores.featureToggleStore.create('fancy', {
         name: 'Some-feature',
     });
-    await stores.strategyStore.createStrategy({ name: 'format' });
+    await stores.strategyStore.createStrategy({
+        name: 'format',
+        parameters: [],
+    });
     await stores.featureEnvironmentStore.addEnvironmentToFeature(
         'Some-feature',
         'dev',
@@ -650,7 +654,10 @@ test('featureStrategies can keep existing', async () => {
     await stores.featureToggleStore.create('fancy', {
         name: 'Some-feature',
     });
-    await stores.strategyStore.createStrategy({ name: 'format' });
+    await stores.strategyStore.createStrategy({
+        name: 'format',
+        parameters: [],
+    });
     await stores.featureEnvironmentStore.addEnvironmentToFeature(
         'Some-feature',
         'dev',
@@ -697,7 +704,10 @@ test('featureStrategies should not keep existing if dropBeforeImport', async ()
     await stores.featureToggleStore.create('fancy', {
         name: 'Some-feature',
     });
-    await stores.strategyStore.createStrategy({ name: 'format' });
+    await stores.strategyStore.createStrategy({
+        name: 'format',
+        parameters: [],
+    });
     await stores.featureEnvironmentStore.addEnvironmentToFeature(
         'Some-feature',
         'dev',

diff --git a/src/lib/services/version-service.test.ts b/src/lib/services/version-service.test.ts
@@ -163,6 +163,7 @@ test('counts toggles', async () => {
     await stores.strategyStore.createStrategy({
         name: uuidv4(),
         editable: true,
+        parameters: [],
     });
     const latest = {
         oss: '4.0.0',
@@ -213,10 +214,12 @@ test('counts custom strategies', async () => {
     await stores.strategyStore.createStrategy({
         name: strategyName,
         editable: true,
+        parameters: [],
     });
     await stores.strategyStore.createStrategy({
         name: uuidv4(),
         editable: true,
+        parameters: [],
     });
     await stores.featureStrategiesStore.createStrategyFeatureEnv({
         featureName: toggleName,

diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts
@@ -402,6 +402,14 @@ export interface IImportData extends ImportCommon {
     data: any;
 }
 
+// Create project aligns with #/components/schemas/createProjectSchema
+// joi is providing default values when the optional inputs are not provided
+// const data = await projectSchema.validateAsync(newProject);
+export type CreateProject = Pick<IProject, 'id' | 'name'> & {
+    mode?: ProjectMode;
+    defaultStickiness?: string;
+};
+
 export interface IProject {
     id: string;
     name: string;

diff --git a/src/lib/types/stores/access-store.ts b/src/lib/types/stores/access-store.ts
@@ -1,3 +1,4 @@
+import { PermissionRef } from 'lib/services/access-service';
 import { IGroupModelWithProjectRole } from '../group';
 import { IPermission, IUserWithRole } from '../model';
 import { Store } from './store';
@@ -100,7 +101,7 @@ export interface IAccessStore extends Store<IRole, number> {
 
     addEnvironmentPermissionsToRole(
         role_id: number,
-        permissions: IPermission[],
+        permissions: PermissionRef[],
     ): Promise<void>;
 
     addUserToRole(

diff --git a/src/lib/types/stores/strategy-store.ts b/src/lib/types/stores/strategy-store.ts
@@ -1,3 +1,4 @@
+import { CreateStrategySchema } from 'lib/openapi';
 import { Store } from './store';
 
 export interface IStrategy {
@@ -18,24 +19,22 @@ export interface IEditableStrategy {
     title?: string;
 }
 
-export interface IMinimalStrategy {
-    name: string;
-    description?: string;
-    editable?: boolean;
-    parameters?: any[];
-    title?: string;
-}
+export type IMinimalStrategy = Pick<
+    CreateStrategySchema,
+    'name' | 'description' | 'editable' | 'parameters' | 'title'
+>;
 
-export interface IStrategyImport {
-    name: string;
-    description?: string;
-    deprecated?: boolean;
-    parameters?: object[];
-    builtIn?: boolean;
-    sortOrder?: number;
-    displayName?: string;
-    title?: string;
-}
+export type IStrategyImport = Pick<
+    CreateStrategySchema,
+    | 'name'
+    | 'description'
+    | 'deprecated'
+    | 'parameters'
+    | 'builtIn'
+    | 'sortOrder'
+    | 'displayName'
+    | 'title'
+>;
 
 export interface IMinimalStrategyRow {
     name: string;

diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts
@@ -801,18 +801,10 @@ test('should add a user to the project with a custom role', async () => {
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                environment: undefined,
-                displayName: 'Create Feature Toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
             {
-                id: 8,
-                name: 'DELETE_FEATURE',
-                environment: undefined,
-                displayName: 'Delete Feature Toggles',
-                type: 'project',
+                id: 8, // DELETE_FEATURE
             },
         ],
     });
@@ -859,18 +851,10 @@ test('should delete role entries when deleting project', async () => {
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                environment: undefined,
-                displayName: 'Create Feature Toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
             {
-                id: 8,
-                name: 'DELETE_FEATURE',
-                environment: undefined,
-                displayName: 'Delete Feature Toggles',
-                type: 'project',
+                id: 8, // DELETE_FEATURE
             },
         ],
     });
@@ -907,18 +891,10 @@ test('should change a users role in the project', async () => {
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                environment: undefined,
-                displayName: 'Create Feature Toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
             {
-                id: 8,
-                name: 'DELETE_FEATURE',
-                environment: undefined,
-                displayName: 'Delete Feature Toggles',
-                type: 'project',
+                id: 8, // DELETE_FEATURE
             },
         ],
     });
@@ -1098,11 +1074,7 @@ test('Should allow bulk update of group permissions', async () => {
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                environment: undefined,
-                displayName: 'Create Feature Toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
         ],
     });
@@ -1129,11 +1101,7 @@ test('Should bulk update of only users', async () => {
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                environment: undefined,
-                displayName: 'Create Feature Toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
         ],
     });
@@ -1168,11 +1136,7 @@ test('Should allow bulk update of only groups', async () => {
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                environment: undefined,
-                displayName: 'Create Feature Toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
         ],
     });
@@ -1221,10 +1185,7 @@ test('Should allow permutations of roles, groups and users when adding a new acc
         description: '',
         permissions: [
             {
-                id: 2,
-                name: 'CREATE_FEATURE',
-                displayName: 'Create feature toggles',
-                type: 'project',
+                id: 2, // CREATE_FEATURE
             },
         ],
     });
@@ -1234,10 +1195,7 @@ test('Should allow permutations of roles, groups and users when adding a new acc
         description: '',
         permissions: [
             {
-                id: 7,
-                name: 'UPDATE_FEATURE',
-                displayName: 'Update feature toggles',
-                type: 'project',
+                id: 7, // UPDATE_FEATURE
             },
         ],
     });