refactor: adjust API endpoints for module creation and updates

gettakaro · Dec 17, 2024 · eef63df · eef63df
1 parent 1c38430
commit eef63df
Show file tree

Hide file tree

Showing 10 changed files with 1,065 additions and 1,030 deletions.
diff --git a/packages/app-api/src/controllers/Module/modules.ts b/packages/app-api/src/controllers/Module/modules.ts
@@ -90,7 +90,7 @@ export class ModuleController {
   @UseBefore(AuthService.getAuthMiddleware([PERMISSIONS.MANAGE_MODULES]))
   @ResponseSchema(ModuleOutputDTOAPI)
   @OpenAPI({
-    summary: 'Create a new module',
+    summary: 'Create module',
     description: 'Create a new module',
   })
   @Post('')
@@ -101,6 +101,10 @@ export class ModuleController {
 
   @UseBefore(AuthService.getAuthMiddleware([PERMISSIONS.MANAGE_MODULES]), builtinModuleModificationMiddleware)
   @ResponseSchema(ModuleOutputDTOAPI)
+  @OpenAPI({
+    summary: 'Update a module',
+    description: 'Update a module',
+  })
   @Put('/:id')
   async update(@Req() req: AuthenticatedRequest, @Params() params: ParamId, @Body() data: ModuleUpdateDTO) {
     const service = new ModuleService(req.domainId);

diff --git a/packages/app-api/src/controllers/Module/versions.ts b/packages/app-api/src/controllers/Module/versions.ts
@@ -3,22 +3,16 @@ import { ITakaroQuery } from '@takaro/db';
 import { APIOutput, apiResponse } from '@takaro/http';
 import { ModuleService } from '../../service/Module/index.js';
 import { AuthenticatedRequest, AuthService } from '../../service/AuthService.js';
-import { Body, Get, Post, Delete, JsonController, UseBefore, Req, Put, Params, Res } from 'routing-controllers';
+import { Body, Get, Post, JsonController, UseBefore, Req, Params, Res } from 'routing-controllers';
 import { OpenAPI, ResponseSchema } from 'routing-controllers-openapi';
 import { Type } from 'class-transformer';
 import { ParamId } from '../../lib/validators.js';
 import { PERMISSIONS } from '@takaro/auth';
 import { Response } from 'express';
 import { errors } from '@takaro/util';
-import { builtinModuleModificationMiddleware } from '../../middlewares/builtinModuleModification.js';
 import { BuiltinModule, ICommand, ICommandArgument, ICronJob, IFunction, IHook } from '@takaro/modules';
 import { AllowedFilters, RangeFilterCreatedAndUpdatedAt } from '../shared.js';
-import {
-  ModuleExportInputDTO,
-  ModuleVersionCreateAPIDTO,
-  ModuleVersionOutputDTO,
-  ModuleVersionUpdateDTO,
-} from '../../service/Module/dto.js';
+import { ModuleExportInputDTO, ModuleVersionCreateAPIDTO, ModuleVersionOutputDTO } from '../../service/Module/dto.js';
 import { PermissionCreateDTO } from '../../service/RoleService.js';
 
 export class ModuleVersionOutputDTOAPI extends APIOutput<ModuleVersionOutputDTO> {
@@ -105,37 +99,6 @@ export class ModuleVersionController {
     return apiResponse(res);
   }
 
-  @UseBefore(AuthService.getAuthMiddleware([PERMISSIONS.MANAGE_MODULES]), builtinModuleModificationMiddleware)
-  @ResponseSchema(ModuleVersionOutputDTOAPI)
-  @OpenAPI({
-    summary: 'Update a version',
-    description:
-      'Update a version of a module, note that you can only update the "latest" version. Tagged versions are immutable',
-  })
-  @Put('/:id')
-  async updateVersion(
-    @Req() req: AuthenticatedRequest,
-    @Params() params: ParamId,
-    @Body() data: ModuleVersionUpdateDTO,
-  ) {
-    const service = new ModuleService(req.domainId);
-    const result = await service.updateVersion(params.id, data);
-    return apiResponse(result);
-  }
-
-  @UseBefore(AuthService.getAuthMiddleware([PERMISSIONS.MANAGE_MODULES]), builtinModuleModificationMiddleware)
-  @ResponseSchema(APIOutput)
-  @OpenAPI({
-    summary: 'Remove a version',
-    description: 'Removes a version of a module, including all config that is linked to this version',
-  })
-  @Delete('/:id')
-  async removeVersion(@Req() req: AuthenticatedRequest, @Params() params: ParamId) {
-    const service = new ModuleService(req.domainId);
-    await service.deleteVersion(params.id);
-    return apiResponse();
-  }
-
   @UseBefore(AuthService.getAuthMiddleware([PERMISSIONS.MANAGE_MODULES]))
   @ResponseSchema(ModuleVersionOutputDTOAPI)
   @OpenAPI({

diff --git a/packages/app-api/src/controllers/__tests__/ModuleConfig.integration.test.ts b/packages/app-api/src/controllers/__tests__/ModuleConfig.integration.test.ts
@@ -20,32 +20,36 @@ const setup = async function (this: IntegrationTest<ISetupData>): Promise<ISetup
 
   const moduleRes = await this.client.module.moduleControllerCreate({
     name: 'Test module',
-    description: 'Test description',
-    configSchema: JSON.stringify({
-      $schema: 'http://json-schema.org/draft-07/schema#',
-      type: 'object',
-      properties: {
-        foo: {
-          type: 'string',
-          minLength: 3,
-          maxLength: 10,
+    latestVersion: {
+      description: 'Test description',
+      configSchema: JSON.stringify({
+        $schema: 'http://json-schema.org/draft-07/schema#',
+        type: 'object',
+        properties: {
+          foo: {
+            type: 'string',
+            minLength: 3,
+            maxLength: 10,
+          },
         },
-      },
-      required: ['foo'],
-      additionalProperties: false,
-    }),
+        required: ['foo'],
+        additionalProperties: false,
+      }),
+    },
   });
 
   const cronjobModuleCreateRes = await this.client.module.moduleControllerCreate({
     name: 'Test module cronjobs',
-    description: 'Test description',
-    configSchema: JSON.stringify({
-      $schema: 'http://json-schema.org/draft-07/schema#',
-      type: 'object',
-      properties: {},
-      required: [],
-      additionalProperties: false,
-    }),
+    latestVersion: {
+      description: 'Test description',
+      configSchema: JSON.stringify({
+        $schema: 'http://json-schema.org/draft-07/schema#',
+        type: 'object',
+        properties: {},
+        required: [],
+        additionalProperties: false,
+      }),
+    },
   });
 
   await this.client.cronjob.cronJobControllerCreate({

diff --git a/packages/app-api/src/controllers/__tests__/ModuleController.integration.test.ts b/packages/app-api/src/controllers/__tests__/ModuleController.integration.test.ts
@@ -42,7 +42,9 @@ const tests = [
       return (
         await this.client.module.moduleControllerCreate({
           name: 'Test module',
-          description: 'bla bla',
+          latestVersion: {
+            description: 'bla bla',
+          },
         })
       ).data.data;
     },
@@ -182,7 +184,9 @@ const tests = [
     test: async function () {
       return this.client.module.moduleControllerCreate({
         name: 'Test module',
-        permissions: [testPermission],
+        latestVersion: {
+          permissions: [testPermission],
+        },
       });
     },
     filteredFields: ['moduleId', 'moduleVersionId'],
@@ -199,9 +203,13 @@ const tests = [
       ).data.data;
     },
     test: async function () {
-      return this.client.module.moduleVersionControllerUpdateVersion(this.setupData.latestVersion.id, {
-        permissions: [testPermission],
+      await this.client.module.moduleControllerUpdate(this.setupData.id, {
+        latestVersion: {
+          permissions: [testPermission],
+        },
       });
+
+      return this.client.module.moduleVersionControllerGetModuleVersion(this.setupData.latestVersion.id);
     },
     filteredFields: ['moduleId', 'moduleVersionId'],
   }),
@@ -213,17 +221,25 @@ const tests = [
       return (
         await this.client.module.moduleControllerCreate({
           name: 'Test module',
-          permissions: [testPermission],
+          latestVersion: {
+            permissions: [testPermission],
+          },
         })
       ).data.data;
     },
     test: async function () {
       const secondPermission = { permission: 'test2', description: 'test2', friendlyName: 'test2' };
-      const updateRes = await this.client.module.moduleVersionControllerUpdateVersion(this.setupData.latestVersion.id, {
-        permissions: [testPermission, secondPermission],
+      await this.client.module.moduleControllerUpdate(this.setupData.id, {
+        latestVersion: {
+          permissions: [testPermission, secondPermission],
+        },
       });
 
-      const newPermission = updateRes.data.data.permissions.find((p) => p.permission === 'test');
+      const versionRes = await this.client.module.moduleVersionControllerGetModuleVersion(
+        this.setupData.latestVersion.id,
+      );
+
+      const newPermission = versionRes.data.data.permissions.find((p) => p.permission === 'test');
       const existingPermission = this.setupData.latestVersion.permissions.find((p) => p.permission === 'test');
 
       if (!existingPermission || !newPermission) {
@@ -232,7 +248,7 @@ const tests = [
 
       expect(existingPermission.id).to.equal(newPermission.id);
 
-      return updateRes;
+      return versionRes;
     },
     filteredFields: ['moduleId', 'moduleVersionId'],
   }),
@@ -244,13 +260,15 @@ const tests = [
       return (
         await this.client.module.moduleControllerCreate({
           name: 'Test module',
-          permissions: [testPermission],
+          latestVersion: {
+            permissions: [testPermission],
+          },
         })
       ).data.data;
     },
     test: async function () {
-      await this.client.module.moduleVersionControllerUpdateVersion(this.setupData.latestVersion.id, {
-        permissions: [],
+      await this.client.module.moduleControllerUpdate(this.setupData.id, {
+        latestVersion: { permissions: [] },
       });
 
       const getRes = await this.client.module.moduleControllerGetOne(this.setupData.id);
@@ -269,20 +287,24 @@ const tests = [
       return (
         await this.client.module.moduleControllerCreate({
           name: 'Test module',
-          permissions: [testPermission],
+          latestVersion: {
+            permissions: [testPermission],
+          },
         })
       ).data.data;
     },
     test: async function () {
-      await this.client.module.moduleVersionControllerUpdateVersion(this.setupData.latestVersion.id, {
-        permissions: [
-          {
-            permission: testPermission.permission,
-            description: 'new description',
-            friendlyName: 'new friendly name',
-            canHaveCount: false,
-          },
-        ],
+      await this.client.module.moduleControllerUpdate(this.setupData.id, {
+        latestVersion: {
+          permissions: [
+            {
+              permission: testPermission.permission,
+              description: 'new description',
+              friendlyName: 'new friendly name',
+              canHaveCount: false,
+            },
+          ],
+        },
       });
 
       const getRes = await this.client.module.moduleControllerGetOne(this.setupData.id);

diff --git a/packages/app-api/src/controllers/__tests__/UserController.integration.test.ts b/packages/app-api/src/controllers/__tests__/UserController.integration.test.ts
@@ -264,7 +264,10 @@ const tests = [
       // This is a bug repro, when you delete a user that has events, a FK constraint error is thrown
       const role = (await this.client.role.roleControllerSearch({ filters: { name: ['root'] } })).data.data[0];
       await this.client.user.userControllerAssignRole(this.setupData.user.id, role.id);
-      await this.setupData.userClient.module.moduleControllerCreate({ name: 'blabla', description: 'blabla' });
+      await this.setupData.userClient.module.moduleControllerCreate({
+        name: 'blabla',
+        latestVersion: { description: 'blabla' },
+      });
       // So, let's ensure there's an event for this user
       const events = await this.client.event.eventControllerSearch({
         filters: { actingUserId: [this.setupData.user.id] },

diff --git a/packages/app-api/src/db/module.ts b/packages/app-api/src/db/module.ts
@@ -268,12 +268,6 @@ export class ModuleRepo extends ITakaroRepo<ModuleModel, ModuleOutputDTO, Module
     return !!data;
   }
 
-  async deleteVersion(id: string): Promise<boolean> {
-    const { queryVersion } = await this.getModel();
-    const data = await queryVersion.deleteById(id);
-    return !!data;
-  }
-
   async update(id: string, data: ModuleUpdateDTO): Promise<ModuleOutputDTO> {
     const { query } = await this.getModel();
     const item = await query.updateAndFetchById(id, data.toJSON());

diff --git a/packages/app-api/src/service/Module/dto.ts b/packages/app-api/src/service/Module/dto.ts
@@ -91,10 +91,7 @@ export class ModuleCreateDTO extends TakaroDTO<ModuleCreateDTO> {
   builtin: string;
 }
 
-export class ModuleCreateAPIDTO extends TakaroDTO<ModuleCreateAPIDTO> {
-  @IsString()
-  @Length(3, 50)
-  name!: string;
+export class ModuleCreateVersionInputDTO extends TakaroDTO<ModuleCreateVersionInputDTO> {
   @IsString()
   @IsOptional()
   @Length(1, 5000)
@@ -127,6 +124,16 @@ export class ModuleCreateAPIDTO extends TakaroDTO<ModuleCreateAPIDTO> {
   cronJobs: CronJobCreateDTO[];
 }
 
+export class ModuleCreateAPIDTO extends TakaroDTO<ModuleCreateAPIDTO> {
+  @IsString()
+  @Length(3, 50)
+  name!: string;
+  @ValidateNested()
+  @Type(() => ModuleCreateVersionInputDTO)
+  @IsOptional()
+  latestVersion?: ModuleCreateVersionInputDTO;
+}
+
 export class ModuleCreateInternalDTO extends TakaroDTO<ModuleCreateInternalDTO> {
   @IsString()
   @IsOptional()
@@ -135,6 +142,12 @@ export class ModuleCreateInternalDTO extends TakaroDTO<ModuleCreateInternalDTO>
   @IsString()
   @Length(3, 50)
   name!: string;
+  @ValidateNested()
+  @Type(() => ModuleCreateVersionInputDTO)
+  latestVersion: ModuleCreateVersionInputDTO;
+}
+
+export class ModuleVersionUpdateDTO extends TakaroDTO<ModuleVersionUpdateDTO> {
   @IsString()
   @IsOptional()
   @Length(1, 5000)
@@ -149,29 +162,17 @@ export class ModuleCreateInternalDTO extends TakaroDTO<ModuleCreateInternalDTO>
   @ValidateNested({ each: true })
   @Type(() => PermissionCreateDTO)
   permissions: PermissionCreateDTO[];
-  @IsOptional()
-  @ValidateNested({ each: true })
-  @Type(() => FunctionCreateDTO)
-  functions: FunctionCreateDTO[];
-  @IsOptional()
-  @ValidateNested({ each: true })
-  @Type(() => CommandCreateDTO)
-  commands: CommandCreateDTO[];
-  @IsOptional()
-  @ValidateNested({ each: true })
-  @Type(() => HookCreateDTO)
-  hooks: HookCreateDTO[];
-  @IsOptional()
-  @ValidateNested({ each: true })
-  @Type(() => CronJobCreateDTO)
-  cronJobs: CronJobCreateDTO[];
 }
-
 export class ModuleUpdateDTO extends TakaroDTO<ModuleUpdateDTO> {
   @Length(3, 50)
   @IsOptional()
   @IsString()
   name?: string;
+
+  @ValidateNested()
+  @Type(() => ModuleVersionUpdateDTO)
+  @IsOptional()
+  latestVersion?: ModuleVersionUpdateDTO;
 }
 
 export class ModuleVersionCreateAPIDTO extends TakaroDTO<ModuleVersionCreateAPIDTO> {
@@ -182,23 +183,6 @@ export class ModuleVersionCreateAPIDTO extends TakaroDTO<ModuleVersionCreateAPID
   moduleId: string;
 }
 
-export class ModuleVersionUpdateDTO extends TakaroDTO<ModuleVersionUpdateDTO> {
-  @IsString()
-  @IsOptional()
-  @Length(1, 5000)
-  description?: string;
-  @IsJSON()
-  @IsOptional()
-  configSchema: string;
-  @IsJSON()
-  @IsOptional()
-  uiSchema: string;
-  @IsOptional()
-  @ValidateNested({ each: true })
-  @Type(() => PermissionCreateDTO)
-  permissions: PermissionCreateDTO[];
-}
-
 export class ModuleExportInputDTO extends TakaroDTO<ModuleExportInputDTO> {
   @IsUUID()
   versionId: string;