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

Rename serviceName to directUploadToken in AS' DirectUpload #44249

Conversation

julianrubisch
Copy link
Contributor

Summary

in ActiveStorage's DirectUpload class, one parameter was (presumably) incorrectly named serviceName, while the BlobRecord class, which it is passed into, refers to it as directUploadToken. I think the latter is correct and the first is a bit misleading, since the Rails Guides refer to it as token, too.

@p8
Copy link
Member

p8 commented Jan 25, 2022

Hi @julianrubisch Thanks for contributing to Rails!

This got added in #38957.
I think you are correct as it seems that everywhere a DirectUpload is instantiated
an uploadToken is passed instead of a serviceName:

this.directUpload = new DirectUpload(this.file, this.url, this.directUploadToken, this.attachmentName, this);

this.directUpload = new DirectUpload(this.file, this.url, this.directUploadToken, this.attachmentName, this);

this.directUpload = new DirectUpload(attachment.file, this.directUploadUrl, this.directUploadToken, this.attachmentName, this)

cc-ing @DmitryTsepelev as the original author of that PR.

Copy link
Contributor

@DmitryTsepelev DmitryTsepelev left a comment

Choose a reason for hiding this comment

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

Good catch!

@p8
Copy link
Member

p8 commented Jan 29, 2022

@julianrubisch Sorry, closing this as this feature is getting reverted and rewritten. See #44287

@p8 p8 closed this Jan 29, 2022
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