FineUploader / server-examples

Server-side examples for the Fine Uploader library
https://fineuploader.com
MIT License
279 stars 204 forks source link

Make uploadName consistent regardless of chunking #35

Closed TedAvery closed 9 years ago

TedAvery commented 9 years ago

Currently if a file is not chunked, the uploadName is set to only the filename. If it is chunked, it is set to uuid/filename. This fix makes both cases use just the filename.

rnicholus commented 9 years ago

@feltnerm You speak more fluent PHP than I. What do you think?

feltnerm commented 9 years ago

Oh PHP, it has been a while ...

my question is, why is $uploadName being set there in the first place? I don't see it being used anywhere. I'll try and manually test this sometime today.

TedAvery commented 9 years ago

@feltnerm It's used in the getUploadName() function, which is in turn used by the accompanying endpoint.php file to return the filename back in the JSON response to the uploader. So the issue I encountered was that in a non-chunking case, I would get just the filename here, so I built according to that. After I tried a larger file which got chunked, I instead got the full uuid/filename path. This should be consistent regardless of whether chunking occurred.

rnicholus commented 9 years ago

I’m wondering why this information is even needed by the client.

On Thu, Nov 20, 2014 at 10:01 AM, Ted Avery notifications@github.com wrote:

@feltnerm It's used in the getUploadName() function, which is in turn used by the accompanying endpoint.php file to return the filename back in the JSON response to the uploader. So the issue I encountered was that in a non-chunking case, I would get just the filename here, so I built according to that. After I tried a larger file which got chunked, I instead got the full uuid/filename path. This should be consistent regardless of whether chunking occurred.

Reply to this email directly or view it on GitHub: https://github.com/FineUploader/server-examples/pull/35#issuecomment-63830708

TedAvery commented 9 years ago

@rnicholus Well you guys put it in your server example in the first place, did you not? :) I think there's plenty of use cases. In mine, I update some UI in my complete callback so the user can see a list of files they've uploaded, along with their filenames. I need to know the filename to do that.

rnicholus commented 9 years ago

I don't see the benefit of reporting the file name as the server sees it to the user.  This may be confusing even.  The name as the user knows it is always available via fine uploaders api.  The php example has been adjusted over the course of several years.  The portion that returns the name in the response is likely just code that never died.  I'd just assume remove it.

On Thu, Nov 20, 2014 at 10:09 AM, Ted Avery notifications@github.com wrote:

@rnicholus Well you guys put it in your server example in the first place, did you not? :) I think there's plenty of use cases. In mine, I update some UI in my complete callback so the user can see a list of files they've uploaded, along with their filenames. I need to know the filename to do that.

Reply to this email directly or view it on GitHub: https://github.com/FineUploader/server-examples/pull/35#issuecomment-63832103

TedAvery commented 9 years ago

@rnicholus Unless the server is overriding the filename for some reason. I think surely there are use cases to get this from handler.php, like if you want to do additional server-side processing based on the file path within your endpoint.php. I'm sure there's other ways to do all of this, but I was able to buy FineUploader yesterday and be up and running in an hour because of ready-to-use code like this, and I found it useful, so just my 2 cents :) I'll be leaving my code with this modification in.

rnicholus commented 9 years ago

Fair enough. @feltnerm Were you able to run a sanity check? If so, we can merge this though.

feltnerm commented 9 years ago

This looks good to me. The original server was responding with { uploadName: null } so I think this is a good improvement.