Skip to content
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

audit all DTOs to use appropriate helpers #73

Closed
jd2rogers2 opened this issue Nov 21, 2021 · 0 comments · Fixed by #137
Closed

audit all DTOs to use appropriate helpers #73

jd2rogers2 opened this issue Nov 21, 2021 · 0 comments · Fixed by #137
Assignees

Comments

@jd2rogers2
Copy link
Contributor

jd2rogers2 commented Nov 21, 2021

here's some good documentation on this one https://docs.nestjs.com/openapi/mapped-types#partial
i don't know if we fully understand DTOs, they look like they're just being used as typescript types but according to the quote below, found in docs here, it sounds like they are used at run time too
"In this fashion, any route that uses the CreateUserDto will automatically enforce these validation rules."
would also be nice to add some stronger validation on user email like in the example also found at the link above

so basically we need to update all of our DTOs to do one of the following:

  1. use PartialType only when ALL properties on this entity can be updated
    would look like this export class UpdateAssetDto extends PartialType(Asset) {}
    (but don't actually use it for that one as I think with assets we would never change the poster_id)
  2. use pick or omit mapped types to extend the entity except only enforce a specific set of properties
    i.e. export class UpdateAssetDto extends OmitType(Asset, ['poster_id'] as const) {}
    (this actually might be what we want for this one)
  3. use none of the mapped types (i think this will be the case for most create DTOs)
    these will look like:
export class CreateAssetDto {
  // all of the properties from the entity
}

actually, i think we don't want to extend the entities, but rather the createDto (which should require all properties)

@dbrandt1990 dbrandt1990 self-assigned this Jan 10, 2022
@jd2rogers2 jd2rogers2 changed the title remove PartialType extension from all createBlahDTOs audit create and update DTOs to use appropriate helpers Jan 12, 2022
@jd2rogers2 jd2rogers2 changed the title audit create and update DTOs to use appropriate helpers audit all DTOs to use appropriate helpers Jan 12, 2022
@jd2rogers2 jd2rogers2 linked a pull request Feb 2, 2022 that will close this issue
@jd2rogers2 jd2rogers2 self-assigned this Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants