Indicate that a file is uploading because user has chosen a folder

[chiefGui]

Hello, there!

The idea behind this PR is simple: when a file is being added through a folder selection from a provider using ProviderView, I need an indication of that. I am reconstructing folder structure from files uploaded via Dropbox/Google Drive, but unfortunately, if I don't know whether the file was picked because the user has selected a folder, I'll be always reconstructing the folder path exposed by the mentioned provider.

This line of code is all that's needed to achieve that.

Please, let me know if you need further clarification.

[chiefGui]

FWIW, the force-push was because I saved the documentation file (see at https://github.com/transloadit/uppy/pull/2605/commits/cab9f6ff5fc63127babcac4e231e556ba40c81d4) with custom formatting, and pushed with it. Also, the revert for the "Release" commit (74d1c6b) and the "Release" commit itself ( 5f41acb) were stuff I pushed after playing a bit with Uppy last night (mistakenly, lol).

[ifedapoolarewaju]

Hi @chiefGui, I'd like to get some more clarifications here, what exactly is happening when you say "reconstructing folder structure", and how are you able to achieve it when a file is selected directly vs when a file is selected through a parent folder?

[chiefGui]

Hello @ifedapoolarewaju, thank you for your answer.

Pretty much, Dropbox files are giving me remote.url, which displays the relativePath of the file. I'm grabbing that data, decoding it, and using it to recreate the folder structure of such file.

Turns out I can't differentiate a "standalone" file, and a file from a folder, meaning direct files are always recreating their parent folder structure and that's not desired—I only want to recreate the structure if the files uploaded were selected through folder picking.

What do you think? Am I making sense?

[lakesare]

@chiefGui, thanks, that makes sense! I think we should expose the file.relativePath for all remote providers in the future (you can't see .relativePath for google drive e.g., but we do have access to it internally). .isFromFolder is a neat idea!

One issue with this PR is in docs we mention that .isFromFolder is accessible via file.isFromFolder, but it's in fact in file.data.isFromFolder.

@arturi, @ifedapoolarewaju,

1. How about we name it .isSelectedFromFolder instead to indicate that it's not about whether the file was nested, - it's about the way we selected it.
2. And we should place .isFromFolder somewhere else 🤔.
   It should be somewhere close to the .relativePath (when we add it). Atm we have file.meta.relativePath for dropped files. Was it ever a semantic place to put relativePath into, mb we should be assigning .relativePath & .isFromFolder directly to the file object, what do you think?

[chiefGui]

Hey @lakesare, thank you for the quick reply.

Yes, hands down on exposing relativePath 💯. The other day I was reading Drive/Dropbox APIs and I saw they exposed a file's folder structure, but that wasn't bridged by Uppy so I was like, hmmm maybe they have it buried somewhere so I found it through .url lol.

Now, am not sure if those points you raised were internal-only but I'll participate anyways for what it's worth.

1. isSelectedFromFolder sounds better to me, for sure. +1 on that, if you ask me.
2. After thoroughly reading your APIs, docs, and working with Uppy for 2 years now, I'd say .meta(.relativePath|.isSelectedFromFolder) would be the cleverest. To me, both relativePath and isSelectedFromFolder are about the file, and not children [data] of the file. What do you think?

Asking these for me to update the PR accordingly.

[chiefGui]

Oh, before I forget. Do you want me to expose relativePath through this PR or would you want to separate the two functionalities? Thanks!

cc @lakesare

[lakesare]

@chiefGui, sorry for the delay, talked over it in Slack.

1. isSelectedFromFolder sounds better to me, for sure. +1 on that, if you ask me.

Deal then 👍

2. After thoroughly reading your APIs, docs, and working with Uppy for 2 years now, I'd say .meta(.relativePath|.isSelectedFromFolder) would be the cleverest. To me, both relativePath and isSelectedFromFolder are about the file, and not children [data] of the file. What do you think?

We discussed this, and agreed that file.meta is the best place for .relativePath and .isSelectedFromFolder, because file.meta is for data that will be sent to backend, and file itself is for temporary information (like upload status).
We don't strictly follow this logic at the moment, but we plan to move in that direction. If you have some time on your hands we'd appreciate you adding the .relativePath to meta for remote providers, - but .isSelectedFromFolder is appreciated on its own too (once you change the name and remove it from the file object in docs).

[lakesare]

@chiefGui, is the PR ready for review?

[chiefGui]

Hey @lakesare, sorry for the delay. Yes, I guess it's now ready for review, although there are a few considerations I want to bring to the table.

1. This PR is working and tested in Dropbox, Google Drive and OneDrive and so far I only had successful results.

2. After working a little bit more on this PR, I think we can safely get rid of isSelectedFromFolder. Pretty much, what I'm proposing here is that if a file has a relativePath, then it also means this file was uploaded via a folder. I left the variable there just in case you want to keep it for clarity or else, but we're good to remove it.

3. Another thing to take into account is that this is a 100% front-end solution, and I feel it might break with pagination, although as mentioned, I haven't seen any issues so far. To make this a 100% reliable implementation, I'd say we'd need to retrieve the currentFolderName with the response of /list from the server. That because the relativePath Uppy is giving me is the file ID, and only Dropbox is using the folder's name as ID. Another option would be handling relative path entirely on the server, especially because it knows more than the front-end does.

It's been a while since I last touched this code. I wrote the code you see here and published our own Uppy's version shortly after just to respect our deadlines, and just now I've got a chance to give you feedback—so pardon me for any confusing explanation or rationale.

In a nutshell, that's all about it. Let me know what you think and count on me to update whatever is needed to match your expectations to have this update out. 🚀

[chiefGui]

I am already considering this as a dead pull request that won't go anywhere but the graveyard. However, am curious - did you guys fix the issue and just forgot to flag here?

[Murderlon]

Hi, sorry your contribution went stale. Looking at this PR I'm not convinced about the current state to merge it. It seems like quite a lot of changes for something that in my mind only needs something similar to what you said:

2. I think we can safely get rid of isSelectedFromFolder. Pretty much, what I'm proposing here is that if a file has a relativePath, then it also means this file was uploaded via a folder. I left the variable there just in case you want to keep it for clarity or else, but we're good to remove it.

I'm not a fan of opening the box on adding things like isSelectedFromFolder and instead do something more agnostic like relativePath and do with that what you like. The pattern of previousFilePath also feels off to me, do we need that?

If we only add relativePath for remote files from folders, would that be enough? I can focus on making it land if we agree.

[chiefGui]

@Murderlon My solution is completely front-end oriented. Given the context I was in, that's the best I could achieve—and am telling you this to quite justify the decisions I took in this Pull Request.

The above said I agree with you: the less noise we have on the (front-end) API (by omitting things like previousFilePath and isSelectedFromFolder), the better.

Meaning...

If we only add relativePath for remote files from folders, would that be enough? I can focus on making it land if we agree.

may work if, with it, we can reconstruct the folder directory of a certain uploading/uploaded file.

[Murderlon]

may work if, with it, we can reconstruct the folder directory of a certain uploading/uploaded file.

I can take a look at this but I have some other things to do first. One could also say this is a server-side concern, for which we could add it to the response for every provider, such as here for Google Drive:

https://github.com/transloadit/uppy/blob/d42badf995d0a89cf8b61554e21be57185850a59/packages/%40uppy/companion/src/server/provider/drive/index.js#L31-L52

[chiefGui]

My vote would be let's go 100% server-side if you ask me.

Another thing to take into account is that this is a 100% front-end solution, and I feel it might break with pagination, although as mentioned, I haven't seen any issues so far. To make this a 100% reliable implementation, I'd say we'd need to retrieve the currentFolderName with the response of /list from the server. That because the relativePath Uppy is giving me is the file ID, and only Dropbox is using the folder's name as ID. Another option would be handling relative path entirely on the server, especially because it knows more than the front-end does.

I mean, it's yet unknown to me what the consequences are of relying on a front-end solution for this specific case (although my solution worked just fine for the last 2 years).

[Murderlon]

I created a new issue for this, closing this PR.