-
Notifications
You must be signed in to change notification settings - Fork 141
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
Add a route for uploading avatars #4499
base: master
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.
No risks of name collision in the afero
version?
return &avatar{fs} | ||
} | ||
|
||
type avatar struct { |
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.
avatarFs
seems more appropriate since this struct does not old data about avatars themselves but rather implements the interface allowing to manage them.
_ = u.fs.Remove(u.tmpname) | ||
return err | ||
} | ||
return u.fs.Rename(u.tmpname, "avatar") |
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.
nit: a constant storing the avatar filename would be nice
header := c.Request().Header | ||
size := c.Request().ContentLength | ||
if size > 20_000_000 { | ||
return jsonapi.BadRequest(errors.New("Avatar is too big")) |
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 we should return a 413 Entity Too Large
error here.
} | ||
header := c.Request().Header | ||
size := c.Request().ContentLength | ||
if size > 20_000_000 { |
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 could have a MaxFileSize()
method in the AvatarFS
struct like the one in the Fs
interface.
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.
Maybe this should be an error returned by CreateAvatar()
?!
No description provided.