Skip to content

[BUGFIX] Prevents the selection of encrypted files at the bottom of the list(before scrolling)#35521

Merged
szaimen merged 1 commit intonextcloud:masterfrom
nextmcloud:nmcpr/1416-prevents-selection-of-encrypted-folders
Dec 6, 2022
Merged

[BUGFIX] Prevents the selection of encrypted files at the bottom of the list(before scrolling)#35521
szaimen merged 1 commit intonextcloud:masterfrom
nextmcloud:nmcpr/1416-prevents-selection-of-encrypted-folders

Conversation

@TSI-kavitasonawane
Copy link
Contributor

@TSI-kavitasonawane TSI-kavitasonawane commented Nov 30, 2022

#35299
this PR is get merged but now we found one bug.
bug description - encrypted files which are not in viewport are getting selected when we click on select all options, whereas they should not be get selected.

@szaimen szaimen added the 3. to review Waiting for reviews label Nov 30, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Nov 30, 2022
@szaimen szaimen requested review from a team, CarlSchwan, PVince81, Pytal, artonge and szaimen and removed request for a team November 30, 2022 15:01
@PVince81
Copy link
Member

PVince81 commented Nov 30, 2022

this is the issue I pointed out in the original code and I expected https://github.com/nextcloud/server/pull/35299/files#diff-dedceb236d4709ee562b67512f8ad10bce9dc7a494e9b1cfdabe8ea8ad33ca25R1424 to fix it

the code that you proposed here will cause performance issues with longer file lists as it will re-process the previous pages

maybe another place in _nextPage needs to have similar processing
can you check with a debugger to see why https://github.com/nextcloud/server/pull/35299/files#diff-dedceb236d4709ee562b67512f8ad10bce9dc7a494e9b1cfdabe8ea8ad33ca25R1424 is not reached in the tested case ?

@TSI-kavitasonawane
Prevents the selection of encrypted files when scroll the page
4d0b80d 
Signed-off-by: kavita.sonawane@t-systems.com <kavita.sonawane@t-systems.com>
@TSI-kavitasonawane TSI-kavitasonawane force-pushed the nmcpr/1416-prevents-selection-of-encrypted-folders branch from d02cb11 to 4d0b80d Compare December 1, 2022 06:14
@TSI-kavitasonawane
Copy link
Contributor Author

TSI-kavitasonawane commented Dec 1, 2022

this is the issue I pointed out in the original code and I expected https://github.com/nextcloud/server/pull/35299/files#diff-dedceb236d4709ee562b67512f8ad10bce9dc7a494e9b1cfdabe8ea8ad33ca25R1424 to fix it

the code that you proposed here will cause performance issues with longer file lists as it will re-process the previous pages

maybe another place in _nextPage needs to have similar processing can you check with a debugger to see why https://github.com/nextcloud/server/pull/35299/files#diff-dedceb236d4709ee562b67512f8ad10bce9dc7a494e9b1cfdabe8ea8ad33ca25R1424 is not reached in the tested case ?

@PVince81
I have moved https://github.com/nextcloud/server/pull/35299/files#diff-dedceb236d4709ee562b67512f8ad10bce9dc7a494e9b1cfdabe8ea8ad33ca25R1424 this logic to another place in _nextPage.
Now issue is fixed. Please check it.

PVince81
PVince81 approved these changes Dec 1, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice one

@szaimen szaimen merged commit decf2b4 into nextcloud:master Dec 6, 2022
@tsdicloud tsdicloud deleted the nmcpr/1416-prevents-selection-of-encrypted-folders branch July 14, 2023 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PVince81 PVince81 PVince81 approved these changes

@artonge artonge artonge approved these changes

@szaimen szaimen Awaiting requested review from szaimen

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@Pytal Pytal Awaiting requested review from Pytal

Assignees

No one assigned

Labels

3. to review Waiting for reviews

Projects

None yet

Milestone

Nextcloud 26

Development

Successfully merging this pull request may close these issues.

4 participants

@TSI-kavitasonawane @PVince81 @artonge @szaimen