-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(FilesDropPlugin): Ensure all request for file request have a nickname #55807
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
Conversation
126c907 to
ef1167d
Compare
This is exactly the design. |
Why specifically for folders? Why isn't it also required for uploading individual files? Apart from these inconsistencies, this is, in general, a pretty user-unfriendly requirement and it should be removed (or should never have been implemented in the first place). Also, the error-handling is atrocious. Right now, I need to tell everyone "oh, by the way, if you want to upload folders, you need to set a nickname". I need to take extra time to explain this because there isn't even a pop-up that tells you a) the reason the upload failed and b) immediately prompts you to set a nickname. If that were the case, all of this might be on the very edge of being ok, but even then it still wouldn't explain the inconsistencies mentioned above. I am honestly baffled by
Still – I want to reiterate, just to be clear – documenting this or improving error handling does not justify this requirement existing in the first place. EDIT: At the very least: Add proper error handling and then make it configurable. If people, for whatever reason, want to force everyone to set a nickname, when creating a link, there should be a checkbox "force nickname" plus a setting whether this checkbox is checked by default or not/user configurable or not. |
Would you feel more confortable if we enforced this ? @passibe15 To give you insight on the background for this decision, its a mix between new features and legacy behaviour. |
|
Thanks for the response!
I am not sure if "enforce" is the correct term here, as, in my view, this policy is already enforced by uploads always failing if a nickname is not set. As I have already explained, imo this (strictly) relates to proper error handling, i.e. explaining to the user why the upload failed and immediately offering a remedy. With the current constraints, I believe the different scenarios should be handled as follows:
Is it planned to remedy this at some point? If those are the only reasons why a nickname is necessary, as I've said, this should, at the very least, be made configurable, i.e. letting me decide if I want to classify folders by uploaders or if I am fine with not classifying and the possible confusion following from that decision. |
Yes, that is a bit counter-intuitive!
I'll push this internally, we'll see. |
Initial file drop refused ALL folders, for security reasons. Second addition was File request, which enforced the request for a nickname and basically put everything in a subfolder names with your nickname Lastly, we added support for folders upload, but to avoid the insane conflicts resolution and so on, we kept the original behaviour that if you want to upload many files/folders at once, it's best to have them within your nickname folder, thus asking for a nickname if you want to upload folders. |
Ok I see. I just thought this was wrong, because the UX is horrible: You see the error notification with the technical description and no way to enter the nickname in the dialog. This seemed really unexpected to me, so was thinking that this is not intended. |
This reverts commit 365a040. Signed-off-by: provokateurin <[email protected]>
… to satisfy the QuotaPlugin Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
…name Signed-off-by: provokateurin <[email protected]>
ef1167d to
901859e
Compare
|
Ok, so with this PR chunked upload for public shares is enabled again and works in all scenarios. I also changed a bit how file drops work: I think requiring a nickname for uploading folders on a file drop is a strange requirement, because you can still upload identically named files which are not distinguishable. So it would only make sense to require a nickname for files as well, but we have file requests for that. Besides that the UX was terrible, so that is gone as well now. With this, the distinction between a file drop and a file request is much clearer and the users are able to choose which one they want. |
|
This needs a backport for stable32, but I'll include that in #55804. |
THANK YOU! |
to be fair, they should probably just be merged together at some point 🙈 |
|
Absolutely, also because the naming is super confusing and inconsistent 😅 |
With 31, the design team asked for file drop to be entirely renamed like file request |
|
Cypress failure is unrelated |
come-nc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should merge together and have a checkbox option to require a nickname, no? (no nickname required == file drop).
Because to me it sounds useful to have both file request and file drop it’s not the same usecases. The issue currently is that both are named the same in the UI and the workflow is not always consistent.
|
Yes yes, I just want to fix the problems first though 🙈 |
Follow-up to #55800
I just noticed there is also another bug with file drop: You can't upload folders, because it will tell you that a nickname is required. So MKCOL is fine without a nickname, as long as it's not for a file request. This is also reproducible on master before the other PR was merged.
I removed the check and simply ensured that all requests on a file request need to have the nickname set. Before this logic was intertwined with the MKCOL logic which lead to these issues.