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

Suggestion: Change UploadedFile name to use HTTP::FormData::Part filename #1147

Open
rnice01 opened this issue May 19, 2020 · 4 comments
Open

Comments

@rnice01
Copy link
Contributor

rnice01 commented May 19, 2020

I have an app where I upload an image and save it locally. I was saving the file using the Lucky::UploadedFile name attribute in my action and noticed the saved file was using the name of the form field and losing the extension name as well. I noticed tempfile was another attribute and attempted to use tempfile.name but got the same result as the temp file gets created using part.name as well.

I looked in the source code and figured out I could get the filename through UploadedFile.metadata.filename. Which is fine since now I know how to get the filename. However, I feel others could also run into this problem as well thinking UploadedFile.name will yield a filename.

So changing this line to use part.filename instead of part.name.

@UnsolvedCypher
Copy link
Contributor

The filename could be nil though, so what would you propose is used as a fallback? I think the inconsistent behavior (when the filename is nil or not) might be more confusing than just keeping it as the field name.

@rnice01
Copy link
Contributor Author

rnice01 commented May 20, 2020

@UnsolvedCypher if Part.filename is nil then Part.name could be the fallback. I could be totally alone here, but I feel that calling UploadedFile.name should try to provide the filename if possible, given the name of the class has File in it.

@UnsolvedCypher
Copy link
Contributor

No I agree that the current behavior is confusing, I ran into the same issue yesterday 😄 . I just think that having the .name behave differently depending on whether or not the file name is available would cause bugs for people who don't realize it behaves this way. Maybe the name attribute could just be renamed to field_name or param_name so it's more obvious what it does?

@rnice01
Copy link
Contributor Author

rnice01 commented May 20, 2020

Ah I see, you're right, changing it to either be the filename or part name would be just as confusing. I like the idea of changing the attribute name to either field_name or param_name.

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

No branches or pull requests

2 participants