-
Notifications
You must be signed in to change notification settings - Fork 112
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
feat: support dynamic DTOs: controller, service, repository and seed #550
feat: support dynamic DTOs: controller, service, repository and seed #550
Conversation
api/src/chat/dto/conversation.dto.ts
Outdated
@@ -45,7 +46,7 @@ export class ConversationCreateDto { | |||
@IsObjectId({ | |||
message: 'Current must be a valid objectId', | |||
}) | |||
current: string; | |||
current?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current?: string; | |
current: string | null; |
@@ -54,5 +55,9 @@ export class ConversationCreateDto { | |||
each: true, | |||
message: 'next must be a valid objectId', | |||
}) | |||
next: string[]; | |||
next?: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
next?: string[]; | |
next: string[]; |
|
||
@Transform(({ obj }) => obj.attachedBlock?.toString() || null) | ||
attachedBlock?: string; | ||
attachedBlock: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attachedBlock: string; | |
attachedBlock: string | null; |
@@ -81,10 +81,10 @@ export class Conversation extends ConversationStub { | |||
sender: string; | |||
|
|||
@Transform(({ obj }) => obj.current.toString()) | |||
current?: string; | |||
current: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current: string; | |
current: string | null; |
@Prop({ | ||
type: MongooseSchema.Types.ObjectId, | ||
ref: 'Attachment', | ||
default: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To check if the default reference to nul which is an object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't face an issue with the scenario tested from my side but, i think that it's better to investigate more about it. I have created a new issue with a short description for the mentioned bug #558
@@ -14,6 +14,7 @@ import { Document, Model, Query } from 'mongoose'; | |||
import { BaseRepository, DeleteResult } from '@/utils/generics/base-repository'; | |||
import { TFilterQuery } from '@/utils/types/filter.types'; | |||
|
|||
import { TNlpSampleDto } from '../dto/nlp-sample.dto'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it start with a 'T' or it is just a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, it's because we have already a DTO NlpSampleDto
@@ -27,7 +33,7 @@ export abstract class BaseSeeder< | |||
return count === 0; | |||
} | |||
|
|||
async seed(models: Omit<T, keyof BaseSchema>[]): Promise<boolean> { | |||
async seed(models: DtoInfer<DtoAction.Create, Dto, U>[]): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between DtoInfer and Omit parametrized classes ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DtoInfer is a dynamic type that can generate a type based on the DTO context
Omit type is a static type depending on the T type (schema type)
Motivation
The idea of this PR is to propose a solution for the payload type restriction inferred from the schema, gives the possibility to define custom DTO per action (CRUD) if needed, otherwise preserving the old way of handling data types (retro compatibility).
Fixes #548
Checklist: