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

Fix Rails 7 Regression - ActiveStorage Content Disposition #43637

Merged
merged 2 commits into from Dec 6, 2021

Conversation

alxjrvs
Copy link
Contributor

@alxjrvs alxjrvs commented Nov 12, 2021

Summary

When upgrading our app from 6.1.4.1 to 7, we noticed that the following code no longer functioned:

class Settings::My::StaffUploadsController < Settings::MyController

  # GET /1/settings/my/downloads/staff_uploads/1
  def show
    @staff_upload = Current.user.staff_uploads.active.find(params[:id])

    redirect_to rails_blob_path(@staff_upload.file, disposition: :attachment)
  end
end

Originally, this worked as expected, redirecting the file and triggering the browser to download it as an attachment. After the upgrade, it redirected to the attachment, but the disposition was always inline.

Root Cause

After a brief investigation, I determined that this regression was introduced in ab8e3d22 where @dhh refactored/simplifed the ActiveStorage ProxyController code, but forgot to pass the disposition parameter into the callers of his newly created method ( send_blob_stream).

Resolution

This PR passes the missing option to the Blob:ProxyController and the Representations:ProxyController's send_blob_stream call, and added tests to ensure these do not regress in the future.

We are using this solution locally in our Rails 7 Upgrade to great effect.

@alxjrvs alxjrvs changed the title Blob representation disposition Fix Rails 7 Regression - ActiveStorage Content Disposition Nov 12, 2021
@ghiculescu ghiculescu added this to the 7.0 milestone Nov 12, 2021
@rafaelfranca rafaelfranca merged commit 47fc79c into rails:main Dec 6, 2021
rafaelfranca added a commit that referenced this pull request Dec 6, 2021
Fix Rails 7 Regression - ActiveStorage Content Disposition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants