feat(files_sharing): Improve user experience when user attempts to download folder with non-downloadable files

[Koc]

This PR is continuation of the recently merged #57573.

Current master branch requires to perform recursive iteration twice:

• 1st time when we check can node to be downloaded
• 2nd time when we actually populate zip archive

With this PR we recursively iterating only once as checks performed right before node added to archive.

But that's not all. Current master branch not allows download anything if downloaded folder contains at least 1 non-downloadable file. With this PR non-downloadable files are just excluded from the archive. At the same downloadable files added to it.

TODO

• Check more edge-cases (e.g. shared and own files located in nested folder)
• Actualize tests

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

[come-nc]

I do not understand the PR, it checks the same files twice, once as paths, once as nodes?
Why is that needed? Why was it not working before?

[Koc]

@come-nc

Why was it not working before?

Good question, but I don't know. For some reason we're still able to download non-downloadable files using workarounds that I explained in the PR description. I've double-tested it on master branch before PR opening.

Regarding duplicated checks - I've added one more commit that simplifies this. So, now we recursively collecting files to download only once and perform check only once. Also in previous implementation non-downloaded files break whole download (check not works, but idea was the same). Now we're just skipping such files and not break download process.

[Koc]

@susnux @AndyScherzinger guys, can you take a look on it, please?

[AndyScherzinger]

@Koc I pinge @sorbaugh to have engineering reviewing the PR. I am not a php/js dev unfortunately.

[juliusknorr]

If we create such an array and pass it along I could imagine we run into memory limits here with very large directory structures. Would it make sense to rather pass the generator itself?

[Koc]

yeah, I thought about that too. Refactored, I hope laziness will work, please check

[juliusknorr]

Code wise this makes sense and solves the issue 👍

Just one comment left

[Koc]

@juliusknorr I've updated PR description. Actually fix itself done in mentioned PR. But this PR improves behavior