-
-
Notifications
You must be signed in to change notification settings - Fork 296
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 file uploading and file sharing functionality #734
base: develop
Are you sure you want to change the base?
Add file uploading and file sharing functionality #734
Conversation
This is such a huge feature. Can we expedite reviewing this and getting this merged? |
Could you please review this pull request? |
Thanks for submitting this. This is a pretty huge feature with many repercussions for the product, so this will need to be pretty solid before we can merge. I'll leave comments in the code as well, but here are some important things that need to be addressed:
Again if you'd like to work on this together, another way forward would be for you to work only on the UI for this -- uploading images and managing them -- and then I could contribute the backend work that we've already put in. Either way, let me know what you'd like to do. |
|
||
fileSize := r.ContentLength | ||
|
||
if err := okToEdit(app, w, r); err != nil { |
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.
What's the purpose of this? It doesn't seem to do much.
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.
In some cases, it is necessary to check the situation before uploading a file, especially for silenced users. While the UI may not display any issues, using Postman or similar tools can allow file uploads for any post by a silenced user. The purpose of using this function is to prevent such problems.
defer file.Close() | ||
user := getUserSession(app, r) | ||
mediaDirectoryPath := filepath.Join(app.cfg.Server.MediaParentDir, mediaDir, | ||
user.Username, slug) |
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.
Storing media in a directory with the user's username will break when they change their username. This should use something stable, like their user ID.
return nil | ||
} | ||
w.Header().Set("Content-Disposition", "attachment; filename="+fileInfo.Name()) | ||
w.Header().Set("Content-Type", "application/octet-stream") |
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.
Files should be delivered as their correct MIME type, instead of application/octet-stream
. There are default Go packages that help with this.
|
Any news on this? My users keep bugging me that they want this :) Cheers, y'all! |
allow_upload_media = true
)media_max_size
, please setmedia_max_size
in config.ini before running. Its unit is megabyte, for example 10 megabytes:total_media_space
, its unit is megabyte(s).