Skip to content

fix(FilesPicker): Prevent selection of nodes without create permission #55796

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 22, 2025
Merged

fix(FilesPicker): Prevent selection of nodes without create permission #55796

merged 5 commits into from
Oct 22, 2025
+252 −248

Conversation

@artonge
Copy link
Contributor

@artonge artonge commented Oct 15, 2025 •
edited
Loading

It is misleading to show those nodes in the file picker, as picking them will always result in an error.

Todo:

The new version will also prevent navigation to non-readable folders. This will prevent navigating to E2EE folders.

Before After
image Screenshot From 2025-10-17 11-17-19

@artonge artonge requested review from a team and skjnldsv as code owners October 15, 2025 14:50
@artonge artonge requested review from sorbaugh and susnux and removed request for a team October 15, 2025 14:50
@artonge artonge self-assigned this Oct 15, 2025
@artonge artonge removed the request for review from sorbaugh October 15, 2025 14:53
@artonge artonge added this to the Nextcloud 33 milestone Oct 15, 2025
@susnux
Copy link
Contributor

susnux commented Oct 15, 2025

I think we discussed this somewhere with Jan, the outcome was that it is even more confusing to not see all your files.
The solution that was agreed is to grey out non-selectable files.

We probably need a second filter way for this on the library though...

@artonge
Copy link
Contributor Author

artonge commented Oct 15, 2025

We probably need a second filter way for this on the library though...

Or we instead gray out the files based on that already existing function? Is there a specific use case where we actually want to filter out some nodes?

@artonge artonge mentioned this pull request Oct 16, 2025
Merged
@artonge artonge force-pushed the artonge/fix/hide_non_writable_nodes branch from 9331209 to 37e2ebd Compare October 16, 2025 10:39
@artonge artonge requested a review from jancborchardt October 16, 2025 11:13
@artonge artonge mentioned this pull request Oct 16, 2025
Open
susnux
susnux reviewed Oct 16, 2025
View reviewed changes
@artonge
Copy link
Contributor Author

artonge commented Oct 16, 2025

I don't think this will work.

We might not have create permission on a specific folder, but still have them on one of its subfolders. Groupfolders with ACLs for example.

So we still need to be able to navigate into those folders.
But for E2EE folders, it makes sense to not be able to navigate to them.

Maybe we also need a canNavigate function?

@susnux
Copy link
Contributor

susnux commented Oct 16, 2025

But for E2EE folders, it makes sense to not be able to navigate to them.

Then you should be able to navigate into those greyed folder but not be able to pick them.
Thats all that this prop says: not pickable

@artonge
Copy link
Contributor Author

artonge commented Oct 17, 2025 •
edited
Loading

Then you should be able to navigate into those greyed folder but not be able to pick them.

No, for E2EE it does not make sense as we cannot even load their content. But luckily, they are also not readable, I'll push a better approach in a few minutes.

Edit: done

@artonge artonge force-pushed the artonge/fix/hide_non_writable_nodes branch from 37e2ebd to f2ab1c3 Compare October 17, 2025 09:20
@artonge artonge changed the title fix(FilesPicker): Filter out non writable nodes fix(FilesPicker): Prevent selection of nodes without create permission Oct 17, 2025
@artonge artonge force-pushed the artonge/fix/hide_non_writable_nodes branch from f2ab1c3 to 9216214 Compare October 17, 2025 10:36
@artonge artonge requested a review from susnux October 17, 2025 10:40
@artonge artonge force-pushed the artonge/fix/hide_non_writable_nodes branch 2 times, most recently from 3ffa65c to 35ec297 Compare October 17, 2025 13:08
jancborchardt
jancborchardt approved these changes Oct 20, 2025
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good!

Might additionally be good to have some textual support there, like a title on hover & focus which says "No permissions to upload to this folder", for example.

@jancborchardt jancborchardt requested a review from kra-mo October 20, 2025 09:14
@artonge artonge force-pushed the artonge/fix/hide_non_writable_nodes branch from 35ec297 to 2f97714 Compare October 21, 2025 15:14
@artonge artonge requested a review from a team as a code owner October 21, 2025 15:14
@artonge
Copy link
Contributor Author

artonge commented Oct 21, 2025

/compile /

@artonge
Copy link
Contributor Author

artonge commented Oct 21, 2025

/compile rebase /

artonge and others added 5 commits October 21, 2025 20:27
@artonge @nextcloud-command
fix(FilesPicker): Prevent selection of nodes without create permission
cb062d2 
And remove the disable callback for the button as it is not needed anymore.

This gives a better experience as the nodes do not have a pointer for cursor, and the submit button does not react when clicking on them.

Signed-off-by: Louis Chmn <louis@chmn.me>
@artonge @nextcloud-command
fix(FilePicker): Modernise dav imports
7b8463e 
Signed-off-by: Louis Chmn <louis@chmn.me>
@artonge @nextcloud-command
chore(deps): Bump @nextcloud/dialogs to v7.1.0
58a9fda 
Signed-off-by: Louis Chmn <louis@chmn.me>
@nextcloud-command
chore(assets): Recompile assets
c627561 
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command
chore(assets): Recompile assets
58e9be7 
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command nextcloud-command force-pushed the artonge/fix/hide_non_writable_nodes branch from f45ef38 to 58e9be7 Compare October 21, 2025 20:30
@artonge artonge merged commit f2eb772 into master Oct 22, 2025
124 checks passed
@artonge artonge deleted the artonge/fix/hide_non_writable_nodes branch October 22, 2025 05:18
@artonge
Copy link
Contributor Author

artonge commented Oct 22, 2025

/backport cb062d2 to stable32

@artonge
Copy link
Contributor Author

artonge commented Oct 22, 2025

/backport cb062d2 to stable31

@artonge
Copy link
Contributor Author

artonge commented Oct 22, 2025

/backport cb062d2 to stable30

@backportbot backportbot bot mentioned this pull request Oct 22, 2025
Merged
This was referenced Oct 22, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@susnux susnux susnux approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv skjnldsv is a code owner

@kra-mo kra-mo Awaiting requested review from kra-mo

Assignees

@artonge artonge

Labels

3. to review Waiting for reviews bug feature: files javascript

Projects

None yet

Milestone

Nextcloud 33

Development

Successfully merging this pull request may close these issues.

5 participants

@artonge @susnux @jancborchardt @nextcloud-command