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

Add file uploading and file sharing functionality #734

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

alborzjafari
Copy link

@alborzjafari alborzjafari commented Jul 29, 2023

  • I need to upload media files such as images and use them in markdown format. I have added this feature in this pull request.
  • It is configurable using config.ini file to activate or deactivate this feature in the instance. It is disabled by default please enable it for testing in config.ini (allow_upload_media = true)
  • Size of the files can be limited using media_max_size, please set media_max_size in config.ini before running. Its unit is megabyte, for example 10 megabytes:
media_max_size = 10 
  • The uploaded media can be shared using markdown for example:
![picture](/media/<author>/<post_name>/<uuid>.png
or
![ducument](/media/<author>/<post_name>/<uuid>.pdf
  • Uploading media files is possible in Edit Post Metadata section. (edit the post-> edit post metadata or here: http://<example.com>/<author_name>/<post_name>/edit/meta), also the list of uploaded files is there.
  • The front-end part needs more improvements.
  • Total space can be limited using total_media_space, its unit is megabyte(s).

  • I have signed the CLA

@sssilver
Copy link

This is such a huge feature. Can we expedite reviewing this and getting this merged?

@alborzjafari
Copy link
Author

Could you please review this pull request?

@thebaer
Copy link
Member

thebaer commented Sep 8, 2023

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:

  • Media metadata should probably be stored in the database, to help with management and permissions. Note that we've done a ton of work on this with our Write.as instance and Snap.as (using this table schema), and we could build on this if you want to collaborate on this instead of writing from scratch.
  • We'll need accompanying documentation in https://github.com/writefreely/documentation to ensure people configure any reverse proxy to also allow large file uploads (e.g. most servers are configured for <10MB by default)
  • Please run go fmt on all files, per the Code Guidelines

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 {
Copy link
Member

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.

Copy link
Author

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)
Copy link
Member

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")
Copy link
Member

@thebaer thebaer Sep 8, 2023

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.

@alborzjafari
Copy link
Author

  1. "Media metadata should probably be stored in the database, to help with management and permissions." : I tried to to do something like inheriting permissions of a post for its files.
  2. "...another way forward would be for you to work only on the UI for this -- uploading images and managing them...": Would you address me in source codes, I didn't see about uploading images. My purpose is uploading all kind of files not only the images.

@antranigv
Copy link

Any news on this? My users keep bugging me that they want this :)

Cheers, y'all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants