-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
fix(server): stacked assets for full sync, userIds as array for delta sync #9100
Conversation
… sync
server/src/dtos/sync.dto.ts
Outdated
@ValidateUUID({ each: true }) | ||
@ApiProperty({ name: 'userIds[]' }) |
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.
You'd want @IsArray()
instead. That also gives you proper validation
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.
Are you sure? I think, I tried that first. IsArray is already set in the open API json description.
This change makes the client send a single value so that express sees it as an array in any case.
I'll try it again tomorrow just to be sure.
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.
Apparently you are right, sorry. I think you want both annotations then though, right?
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.
You should be able to delete the API property without impacting the spec file
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.
After you delete this line, feel free to merge it.
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.
This was specifically for a query param, which indeed was needed. I just changed it to post to avoid these types of serialization issues entirely.
server changes required for #8842