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

Improve Storage Permission Handling #14229

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

Conversation

alperozturk96
Copy link
Collaborator

@alperozturk96 alperozturk96 commented Dec 23, 2024

  • Tests written, or not not needed

  • Remove the internal boolean flag used to determine whether the app needs to request storage permission. The app should always request the necessary permission if it has not been granted.

  • Simplify the StoragePermissionDialog logic by removing the external listener. The dialog’s sole responsibility should be to either open the app settings to request “Manage All Files” permission or request read-only media access. Decoupling this functionality from external listeners improves code readability and reduces complexity

  • Use precise and descriptive function names to improve code clarity and maintainability

  • Remove redundant checks in the requestStoragePermissionIfNeeded() function. This function already internally verifies whether storage permission is required, making additional checks unnecessary

  • Remove the readOnly parameter as its logic is implicitly tied to Android 11 and above (API level 30+). The current implementation redundantly checks the same condition (readOnly && Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) multiple times, which is unnecessary and can lead to confusion

  • Add READ_MEDIA_VISUAL_USER_SELECTED permission (Documentation)

Android 7 GPlayImage 1 Android 11 GenericImage 2 Android 11 GPlayImage 3 Android 15 GenericImage 4 Android 15 GPlayImage 5

@alperozturk96 alperozturk96 force-pushed the improve-storage-permission-handling branch 2 times, most recently from 8cf4d1d to b5e9287 Compare January 14, 2025 12:15
@alperozturk96 alperozturk96 force-pushed the improve-storage-permission-handling branch from bbfe854 to bf0c209 Compare January 21, 2025 08:30
@alperozturk96 alperozturk96 added this to the Nextcloud App 3.31.0 milestone Jan 21, 2025
@alperozturk96 alperozturk96 force-pushed the improve-storage-permission-handling branch 2 times, most recently from 879b0fd to 9ed1092 Compare January 22, 2025 11:10
@alperozturk96 alperozturk96 force-pushed the improve-storage-permission-handling branch from 9ed1092 to e6656ee Compare February 10, 2025 08:35
@alperozturk96
Copy link
Collaborator Author

/backport to stable-3.31

@alperozturk96 alperozturk96 force-pushed the improve-storage-permission-handling branch 3 times, most recently from 4c54653 to 7e9ff37 Compare February 11, 2025 08:08
@tobiasKaminsky tobiasKaminsky removed this from the Nextcloud App 3.31.0 milestone Feb 11, 2025
Signed-off-by: alperozturk <[email protected]>
Signed-off-by: alperozturk <[email protected]>
Signed-off-by: alperozturk <[email protected]>
Signed-off-by: alperozturk <[email protected]>
Signed-off-by: alperozturk <[email protected]>
@alperozturk96 alperozturk96 force-pushed the improve-storage-permission-handling branch from 7e9ff37 to cab2ba1 Compare February 12, 2025 08:11
Copy link

Codacy

Lint

TypemasterPR
Warnings5454
Errors33

SpotBugs

CategoryBaseNew
Bad practice6565
Correctness5858
Dodgy code294294
Experimental11
Internationalization77
Malicious code vulnerability11
Multithreaded correctness77
Performance5151
Security1818
Total502502

Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14229.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

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.

2 participants