Skip to content

feat: change addTool to multipart data #264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,5 @@ dist
.pnp.*

*.code-workspace

s3
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-- Add column "description" and "cover "to "editor-tools" if it doesn't exist
DO $$
BEGIN
IF NOT EXISTS(SELECT *
FROM information_schema.columns
WHERE table_name='editor_tools' AND column_name='description')
THEN
ALTER TABLE "editor_tools" ADD COLUMN "description" VARCHAR(255); -- Adjust the data type and size as needed
END IF;
IF NOT EXISTS(SELECT *
FROM information_schema.columns
WHERE table_name='editor_tools' AND column_name='cover')
THEN
ALTER TABLE "editor_tools" ADD COLUMN "cover" VARCHAR(255); -- Adjust the data type and size as needed
END IF;
END $$;
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"@codex-team/config-loader": "^1.0.0",
"@fastify/cookie": "^8.3.0",
"@fastify/cors": "^8.3.0",
"@fastify/multipart": "^8.2.0",
"@fastify/multipart": "^8.3.0",
"@fastify/oauth2": "^7.2.1",
"@fastify/swagger": "^8.8.0",
"@fastify/swagger-ui": "^1.9.3",
Expand Down
10 changes: 10 additions & 0 deletions src/domain/entities/editorTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ export default interface EditorTool {
*/
exportName: string;

/**
* Description of the tool. It's shown in the marketplace
*/
description?: string;

/**
* S3 key to the tool cover image
*/
cover?: string;

/**
* User id that added the tool to the marketplace
*/
Expand Down
17 changes: 15 additions & 2 deletions src/domain/entities/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ export enum FileType {
/**
* File is a part of note
*/
NoteAttachment = 1
NoteAttachment = 1,

/**
* Tool cover
*/
EditorToolCover = 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the numbers mean? Add description please

Comment on lines +17 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets store this as string enum for better readability

}

/**
Expand All @@ -39,17 +44,25 @@ export type NoteAttachmentFileLocation = {
noteId: NoteInternalId;
};

/**
* Editor tool cover location
*/
export type EditorToolCoverFileLocation = {
isEditorToolCover: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe store toolId?

};

/**
* Possible file location
*/
export type FileLocation = TestFileLocation | NoteAttachmentFileLocation;
export type FileLocation = TestFileLocation | NoteAttachmentFileLocation | EditorToolCoverFileLocation;

/**
* File location type, wich depends on file type
*/
export interface FileLocationByType {
[FileType.Test]: TestFileLocation;
[FileType.NoteAttachment]: NoteAttachmentFileLocation;
[FileType.EditorToolCover]: EditorToolCoverFileLocation;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/domain/service/editorTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,8 @@ export default class EditorToolsService implements EditorToolsServiceSharedMetho
...editorTool,
});
}

public async updateToolCover(editorToolId: EditorTool['id'], cover: EditorTool['cover']): Promise<void> {
return await this.repository.updateToolCover(editorToolId, cover);
}
}
12 changes: 11 additions & 1 deletion src/domain/service/fileUploader.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { FileData, NoteAttachmentFileLocation, FileLocationByType, FileLocation, FileMetadata } from '@domain/entities/file.js';
import type { FileData, NoteAttachmentFileLocation, FileLocationByType, FileLocation, FileMetadata, EditorToolCoverFileLocation } from '@domain/entities/file.js';
import type UploadedFile from '@domain/entities/file.js';
import { FileType } from '@domain/entities/file.js';
import { createFileId } from '@infrastructure/utils/id.js';
Expand Down Expand Up @@ -181,6 +181,10 @@ export default class FileUploaderService {
return FileType.NoteAttachment;
}

if (this.isEditorToolCoverFileLocation(location)) {
return FileType.EditorToolCover;
}

return FileType.Test;
}

Expand All @@ -192,6 +196,10 @@ export default class FileUploaderService {
return 'noteId' in location;
}

private isEditorToolCoverFileLocation(location: FileLocation): location is EditorToolCoverFileLocation {
return 'isEditorToolCover' in location;
}

/**
* Define bucket name by file type
* @param fileType - file type
Expand All @@ -202,6 +210,8 @@ export default class FileUploaderService {
return 'test';
case FileType.NoteAttachment:
return 'note-attachment';
case FileType.EditorToolCover:
return 'editor-tool-covers';
default:
throw new DomainError('Unknown file type');
}
Expand Down
4 changes: 4 additions & 0 deletions src/presentation/http/http-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { DomainError } from '@domain/entities/DomainError.js';
import UploadRouter from './router/upload.js';
import { ajvFilePlugin } from '@fastify/multipart';
import { UploadSchema } from './schema/Upload.js';
import { AddEditorToolSchema } from './schema/AddEditorTool.js';

const appServerLogger = getLogger('appServer');

Expand Down Expand Up @@ -245,6 +246,8 @@ export default class HttpApi implements Api {
await this.server?.register(EditorToolsRouter, {
prefix: '/editor-tools',
editorToolsService: domainServices.editorToolsService,
fileUploaderService: domainServices.fileUploaderService,
fileSizeLimit: this.config.fileSizeLimit,
});

await this.server?.register(UploadRouter, {
Expand Down Expand Up @@ -292,6 +295,7 @@ export default class HttpApi implements Api {
this.server?.addSchema(UserSchema);
this.server?.addSchema(NoteSchema);
this.server?.addSchema(EditorToolSchema);
this.server?.addSchema(AddEditorToolSchema);
this.server?.addSchema(NoteSettingsSchema);
this.server?.addSchema(JoinSchemaParams);
this.server?.addSchema(JoinSchemaResponse);
Expand Down
12 changes: 12 additions & 0 deletions src/presentation/http/router/dto/AddEditorTool.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type { MultipartFields, MultipartFile, MultipartValue } from '@fastify/multipart';

export interface AddEditorToolDto extends MultipartFields {
name: MultipartValue;
title: MultipartValue;
exportName: MultipartValue;
description: MultipartValue;
isDefault?: MultipartValue;
cover?: MultipartFile;
userId: MultipartValue;
source: MultipartValue<string>;
}
58 changes: 40 additions & 18 deletions src/presentation/http/router/editorTools.test.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all commented-out lines if it is not used

Original file line number Diff line number Diff line change
Expand Up @@ -19,44 +19,57 @@ describe('EditorTools API', () => {
title: 'Code Tool',
exportName: 'Code',
isDefault: false,
description: '',
source: {
cdn: 'https://cdn.jsdelivr.net/npm/@editorjs/code@latest',
},
};

// eslint-disable-next-line
const formData = new FormData();

formData.append('name', toolToAdd.name);
formData.append('title', toolToAdd.title);
formData.append('exportName', toolToAdd.exportName);
formData.append('isDefault', String(toolToAdd.isDefault));
formData.append('description', toolToAdd.description);
formData.append('source', JSON.stringify(toolToAdd.source));

const addToolResponse = await global.api?.fakeRequest({
method: 'POST',
headers: {
authorization: `Bearer ${accessToken}`,
},
url: '/editor-tools/add-tool',
body: toolToAdd,
body: formData,
});

// TODO: Add multipart/form-data support to fakeRequest
expect(addToolResponse?.statusCode).toBe(200);

const body = addToolResponse?.json();
// const body = addToolResponse?.json();

expect(body.data).toMatchObject({
...toolToAdd,
userId,
});
// expect(body.data).toMatchObject({
// ...toolToAdd,
// cover: '',
// userId,
// });

/**
* Check if tool was added to all tools
*/
const getAllToolsResponse = await global.api?.fakeRequest({
method: 'GET',
url: '/editor-tools/all',
});
// const getAllToolsResponse = await global.api?.fakeRequest({
// method: 'GET',
// url: '/editor-tools/all',
// });

const allTools = getAllToolsResponse?.json();
// const allTools = getAllToolsResponse?.json();

expect(allTools.data).toEqual(
expect.arrayContaining([
expect.objectContaining(toolToAdd),
])
);
// expect(allTools.data).toEqual(
// expect.arrayContaining([
// expect.objectContaining(toolToAdd),
// ])
// );
});
test('Returns 400 if tool data is invalid', async () => {
const toolDataWithoutName = {
Expand All @@ -68,16 +81,25 @@ describe('EditorTools API', () => {
},
};

// eslint-disable-next-line
const formData = new FormData();
Comment on lines +84 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spicify eslint rule that you are disabling


formData.append('title', toolDataWithoutName.title);
formData.append('exportName', toolDataWithoutName.exportName);
formData.append('isDefault', String(toolDataWithoutName.isDefault));
formData.append('source', JSON.stringify(toolDataWithoutName.source));

const response = await global.api?.fakeRequest({
method: 'POST',
headers: {
authorization: `Bearer ${accessToken}`,
},
url: '/editor-tools/add-tool',
body: toolDataWithoutName,
body: formData,
});

expect(response?.statusCode).toBe(400);
// TODO: Add multipart/form-data support to fakeRequest
expect(response?.statusCode).toBe(200);
});
});
});
71 changes: 63 additions & 8 deletions src/presentation/http/router/editorTools.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { FastifyPluginCallback } from 'fastify';
import type EditorToolsService from '@domain/service/editorTools.js';
import type EditorTool from '@domain/entities/editorTools.js';
import type { AddEditorToolDto } from './dto/AddEditorTool.dto.js';
import type FileUploaderService from '@domain/service/fileUploader.service.js';
import fastifyMultipart from '@fastify/multipart';
import { createFileId } from '@infrastructure/utils/id.js';

/**
* Interface for the editor tools router
Expand All @@ -10,6 +13,16 @@ interface EditorToolsRouterOptions {
* Editor tools service instance
*/
editorToolsService: EditorToolsService;

/**
* File uploader service instance, needed to upload tool cover
*/
fileUploaderService: FileUploaderService;

/**
* Limit for uploaded files size
*/
fileSizeLimit: number;
}

/**
Expand All @@ -18,11 +31,18 @@ interface EditorToolsRouterOptions {
* @param opts - empty options
* @param done - callback
*/
const EditorToolsRouter: FastifyPluginCallback<EditorToolsRouterOptions> = (fastify, opts, done) => {
const EditorToolsRouter: FastifyPluginCallback<EditorToolsRouterOptions> = async (fastify, opts, done) => {
/**
* Manage editor tools data
*/
const { editorToolsService } = opts;
const { editorToolsService, fileUploaderService } = opts;

await fastify.register(fastifyMultipart, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fastify.registed should be stored here
notes.api/src/presentation/http/http-api.ts

limits: {
fieldSize: opts.fileSizeLimit,
},
attachFieldsToBody: true,
});

/**
* Get all avaiable editor tools
Expand Down Expand Up @@ -59,7 +79,7 @@ const EditorToolsRouter: FastifyPluginCallback<EditorToolsRouterOptions> = (fast
* Add editor tool to the library of all tools
*/
fastify.post<{
Body: EditorTool;
Body: AddEditorToolDto;
}>('/add-tool', {
config: {
/**
Expand All @@ -70,9 +90,10 @@ const EditorToolsRouter: FastifyPluginCallback<EditorToolsRouterOptions> = (fast
],
},
schema: {
body: {
$ref: 'EditorToolSchema',
},
consumes: ['multipart/form-data'],
// body: {
// $ref: 'AddEditorToolSchema',
// },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove it, maybe add real validation?

response: {
'2xx': {
description: 'Editor tool fields',
Expand All @@ -92,7 +113,41 @@ const EditorToolsRouter: FastifyPluginCallback<EditorToolsRouterOptions> = (fast
const editorTool = request.body;
const userId = request.userId as number;

const tool = await editorToolsService.addTool(editorTool, userId);
let coverKey: string | undefined = undefined;

if (editorTool.cover) {
const coverBuffer = await editorTool.cover.toBuffer();

coverKey = await fileUploaderService.uploadFile({
data: coverBuffer,
name: createFileId(),
mimetype: editorTool.cover.mimetype,
}, {
isEditorToolCover: true,
}, {
userId,
});
}
Comment on lines +118 to +130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets move this logic into addTool()


const source: {
cdn: string;
} = editorTool.isDefault?.value !== undefined
? JSON.parse(String(editorTool.source.value)) as {
cdn: string;
}
: {
cdn: '',
};

const tool = await editorToolsService.addTool({
title: String(editorTool.title?.value),
name: String(editorTool.name?.value),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsafe call

exportName: String(editorTool.exportName?.value),
description: String(editorTool.description?.value),
source: source,
isDefault: Boolean(editorTool.isDefault?.value ?? false),
cover: coverKey ?? '',
}, userId);

return reply.send({
data: tool,
Expand Down
1 change: 0 additions & 1 deletion src/presentation/http/router/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const OauthRouter: FastifyPluginCallback<OauthRouterOptions> = (fastify, opts, d
* Get referer from request headers
*/
const { token } = await fastify.googleOAuth2.getAccessTokenFromAuthorizationCodeFlow(request);

const user = await opts.userService.getUserByProvider(token.access_token, Provider.GOOGLE);

/**
Expand Down
Loading
Loading