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

ActiveStorage: DirectUploads - 'undefined' instead item.signed_id in params #32365

Closed
mesmerze opened this issue Mar 28, 2018 · 9 comments
Closed

Comments

@mesmerze
Copy link

Steps to reproduce

I want to add the incredible ActiveStorage feature to my project, but I've faced a problem with direct uploads.
I've used rails guides for setup.

Expected behavior

After the direct upload is done activestorage.js should put signed_id to the hidden input value and submit a form.

Actual behavior

File saved, everything is fine, but the value of the hidden input:
<input type="hidden" name="meeting[documents][]" value="undefined">
and form get submitted.

I've found what i.signed_id returns undefined here:

this.directUpload.create(function(n, i) {
        n ? (r.parentNode.removeChild(r), e.dispatchError(n)) : r.value = i.signed_id,e.dispatch("end"),t(n)}

And changed this to i.blob.signed_id, what (I'm not sure) fixes the problem.
Anyone else faced this problem? Or I broke something?

System configuration

Rails version: 5-2-stable

Ruby version: ruby 2.4.1

@georgeclaghorn georgeclaghorn added this to the 5.2.0 milestone Mar 28, 2018
@georgeclaghorn
Copy link
Contributor

/cc @javan

@javan
Copy link
Contributor

javan commented Apr 9, 2018

What does your response from /rails/active_storage/direct_uploads look like in the Network inspector? For example, here's what I see:

screen shot 2018-04-09 at 6 30 37 am

@mesmerze
Copy link
Author

mesmerze commented Apr 9, 2018

HI! Here's what I see:
screen shot 2018-04-09 at 13 46 29

@javan
Copy link
Contributor

javan commented Apr 9, 2018

Do you have ActiveRecord::Base.include_root_in_json = true set in your application?

@mesmerze
Copy link
Author

mesmerze commented Apr 9, 2018

Do you have ActiveRecord::Base.include_root_in_json = true set in your application?

Yes

@javan
Copy link
Contributor

javan commented Apr 9, 2018

Thanks, that's the issue. It should work if you set it to false, which is the Rails default:

# To enable root element in JSON for ActiveRecord objects.
# ActiveSupport.on_load(:active_record) do
# self.include_root_in_json = true
# end

We'll need to update the controller to ensure the response isn't wrapped. Any objections to this change, @georgeclaghorn?

--- a/activestorage/app/controllers/active_storage/direct_uploads_controller.rb
+++ b/activestorage/app/controllers/active_storage/direct_uploads_controller.rb
@@ -15,7 +15,7 @@ def blob_args
     end
 
     def direct_upload_json(blob)
-      blob.as_json(methods: :signed_id).merge(direct_upload: {
+      blob.as_json(root: false, methods: :signed_id).merge(direct_upload: {
         url: blob.service_url_for_direct_upload,
         headers: blob.service_headers_for_direct_upload
       })

@rafaelfranca rafaelfranca removed this from the 5.2.0 milestone Apr 9, 2018
@georgeclaghorn
Copy link
Contributor

No objection.

javan added a commit to javan/rails that referenced this issue Apr 10, 2018
The JavaScript component expects a bare response.

Fixes rails#32365
@memoht
Copy link

memoht commented Apr 26, 2018

Leaving a comment here just in case. I ran into this error

undefined method `signed_id' for #<ActiveStorage::Attached::One:0x0000000008e27ec8>

for a model with an attachment on the Show view. The User had edited the item in question to replace the attachment. In my app the above JSON setting is set to the default. Not sure what can lead to this scenario. The User didn't report any issues, so I only realized when another User tripped across the Show view.

Looking at the active_storage_blobs the blob_id for the original file is there, but in the active_storage_attachments that blob_id is no longer there.

@memoht
Copy link

memoht commented Apr 26, 2018

Upon further investigation, I believe the error I encountered matches the scenario in issue #31985 . The error message is what originally led me to this issue. Hopefully this will help someone else who stumbles across this page.

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

No branches or pull requests

5 participants