Skip to content

BugFix - Do not Filter Share Type #1741

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alperozturk96
Copy link
Contributor

@alperozturk96 alperozturk96 commented Jun 11, 2025

Signed-off-by: alperozturk <[email protected]>
@alperozturk96 alperozturk96 force-pushed the bugfix/create-sharee-user branch from 11b3961 to 7c591c3 Compare June 11, 2025 14:03
Signed-off-by: tobiasKaminsky <[email protected]>
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3534
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness311
Performance88
Total161168

SpotBugs increased!

} else {
null
}
return ShareeUser(userId, displayName, shareType)
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes what this function returns in some cases. For example, if a public link share was parsed, null was previously returned, now ShareeUser("", "", PUBLIC_LINK) is returned. I'm not sure if or what implications this change has.

Copy link
Contributor Author

@alperozturk96 alperozturk96 Jun 13, 2025

Choose a reason for hiding this comment

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

This change is intentional. Previously, the corresponding remote file lacked a shareType due to a missing username. Now, even if the username is absent, we can still determine the shareType, allowing it to be displayed in the UI.

A missing username should not cause issues (crash etc), as external share links do not include one, and the username should not be mandatory. We might consider renaming the data class and adding documentation in a separate PR.

See, there is no username for this share.

Screenshot 2025-06-13 at 09 03 57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants