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 direct upload #1121

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add direct upload #1121

wants to merge 1 commit into from

Conversation

ehasrouni
Copy link

@ehasrouni ehasrouni commented Nov 19, 2024

Adds the ability to enable direct uploads, which can help circumvent upload limits on the server. This is helpful when attempting to upload larger CSV files without having to play with the max size of 20mb

re-attempts #1031

upload_url = page.server_url + "/rails/active_storage/direct_uploads"

assert_selector("input[type=file]") do |input|
assert_equal(input["data-direct-upload-url"], upload_url)
Copy link
Author

@ehasrouni ehasrouni Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input["data-direct-upload-url"] is returning nil for Rails <=7.0

More specifically, it is failing here from the looks of it: https://github.com/rails/rails/blob/c11312a918555aaad99b334d374a791cd6268152/actionview/lib/action_view/helpers/form_tag_helper.rb#L1078 and is setting "data-direct-upload-url" to null

@etiennebarrie
Copy link
Member

There was a previous attempt at this: #1031
The main issue you're going to get is the lack of asset pipeline here: #1029

We should definitely support direct uploads, but I'd like to do it without requiring the asset pipeline. I think we might be able to rely on config.assets being present, and support both propshaft and sprockets. In the worst case we could support only propshaft. We could just use direct uploads when the asset pipeline is configured, but not otherwise. We probably don't need a configuration option for this.

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 this pull request may close these issues.

2 participants