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

Selecting files inside search results produces wrong download link #11949

Closed
wants to merge 1 commit into from
Closed

Selecting files inside search results produces wrong download link #11949

wants to merge 1 commit into from

Conversation

js94x
Copy link

@js94x js94x commented Oct 20, 2018

regarding Issues #1274 and #2075

The method isAllSelected() in filelist.js returns true and in consequence a download link for the whole folder is built:
https://test-nc.example.org/index.php/apps/files/ajax/download.php?dir=/Pics/Wallpaper&files=Parrot&downloadStartSecret=xxxxxxxxx

Selecting files outside search results produces a correct download link:
https://test-nc.example.org/index.php/apps/files/ajax/download.php?dir=/Pics/Wallpaper/Parrot&files=["blade.jpg","blocks.jpg"]&downloadStartSecret=xxxxxxxxx
(both link are not URL-encoded due to better representing)

Removing the if-condition enables download from the search results via "select all" checkbox.
As a side effect, the list of files now is also transferred via GET parameter when explicitly downloading a entire directory. Mayby someone should look at it again.

Signed-off-by: js94x jan@js94x.de

regarding Issues #1274 and #2075

Signed-off-by: js94x <jan@js94x.de>
@skjnldsv skjnldsv added bug 3. to review Waiting for reviews labels Oct 25, 2018
@skjnldsv skjnldsv added this to the Nextcloud 15 milestone Oct 25, 2018
@blizzz
Copy link
Member

blizzz commented Oct 30, 2018

Thanks for you contribution @js94x! It seems this changes makes jsunit test fail. Mind having another look?

@js94x
Copy link
Author

js94x commented Oct 30, 2018

@blizzz Yes, I noticed but I wasn´t sure how to proceed. I'm not the jsunit specialist either. 😁

The failing test is there:

it('Downloads parent folder when all selected in subfolder', function() {

Maybe someone can give a hint?

@blizzz
Copy link
Member

blizzz commented Oct 30, 2018

@js94x does this help? https://docs.nextcloud.com/server/14/developer_manual/core/unit-testing.html#javascript-unit-testing-for-core

@MorrisJobke MorrisJobke mentioned this pull request Nov 6, 2018
29 tasks
@blizzz blizzz added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 7, 2018
@MorrisJobke MorrisJobke mentioned this pull request Nov 8, 2018
24 tasks
@ChristophWurst
Copy link
Member

@js94x any updates?

I see there are failures with jsunit and a new todo in the source code. We'll close this should it not receive any more updates.

@js94x
Copy link
Author

js94x commented Jan 30, 2019

@ChristophWurst
Sorry I didn't have had time yet. At first I have to deal a little bit with the subject of unit testing.
You can close the PR first.

I currently roll out the fix at the affected nextcloud instance after every update.

@ChristophWurst
Copy link
Member

Okay, thanks for the update.

I'll close this now. Please send us the patch when it's ready to be integrated :)

@ChristophWurst ChristophWurst removed the 2. developing Work in progress label Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants