You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
alextran1502
merged 8 commits into
immich-app:main
from
michelheusschen:fix/asset-upload-validation-docsFeb 12, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I've made some changes to the /asset/upload endpoint in order to add proper validation. Before these changes, the assetType enum could be set to an arbitrary value and the assetData was allowed to be empty. Now, assetType will only accept values defined in the enum and assetData will be required. I've also added missing API documentation to the endpoint.
This was my first time working with NestJS, so feedback is welcome.
Regarding the issue with the enum. This looks to be specifically related to our enum/object type being used in a form. I'd really rather not do another custom patch, if it can be avoided. I'm happy to have this enum be { enum } to avoid the weirdness with the form data and keep the implementation simple/clean.
Looking at the patch some more, I think it's an actual bug in the generator that should be fixed in the upstream repo. I'm fine either way (patch or reverting to inline-enum), but it would be nice to get this fixed upstream if possible.
An upstream fix would be the ideal solution, but the amount of open issues and PRs are scary. The bug seems to be introduced in OpenAPITools/openapi-generator#7816 as suggested. I tried using the template file before that PR got merged, but several other improvements and fixes have been made since so it's not worth it IMO.
I'm good with leaving it like this, unless you want the custom template removed.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've made some changes to the
/asset/uploadendpoint in order to add proper validation. Before these changes, theassetTypeenum could be set to an arbitrary value and theassetDatawas allowed to be empty. Now,assetTypewill only accept values defined in the enum andassetDatawill be required. I've also added missing API documentation to the endpoint.This was my first time working with NestJS, so feedback is welcome.