-
Notifications
You must be signed in to change notification settings - Fork 0
Fixed swagger mismatches #27
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
base: main
Are you sure you want to change the base?
Conversation
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.
It looks like a lot of different changes are being made in this PR, maybe we want to discuss a way of breaking them up
@@ -16,7 +16,7 @@ export class Message { | |||
example: 'Hello, how can you help me today?' | |||
}) | |||
@IsString() | |||
text: string; | |||
content: 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.
The field should be text
not content
@@ -24,27 +24,27 @@ import { CreateProxyMappingDto } from './dtos/create.dto'; | |||
export class DeepchatProxyController { | |||
constructor(private readonly deepchatProxyService: DeepchatProxyService) {} | |||
|
|||
@Get('/:id') | |||
@Get('/:model') |
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.
I'm not sure we want to move away from using the ID for handling the proxying
return response; | ||
} | ||
|
||
async proxyRequestByModel(model: string, body: any): Promise<any> { |
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.
I think for now we want to keep the proxy request by ID
toJSON: { | ||
virtuals: true, | ||
transform: (_, ret) => { | ||
ret.id = ret._id; |
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.
We currently use _id
through the code, so for now I wouldn't apply the transformation
createdAt: Date; | ||
|
||
@ApiProperty({ description: 'Last update timestamp', format: 'date-time' }) | ||
@Prop({ default: Date.now }) |
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.
I think these defaults are ok
Retested and made changes |
0c3565a
to
96377d3
Compare
Test plan: Ran though swaggerdocs and UI to fix name mismatches cause 500 errors