-
-
Notifications
You must be signed in to change notification settings - Fork 45
Remove media permissions #2963
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
Remove media permissions #2963
Conversation
📝 WalkthroughWalkthroughThe pull request removes media permissions for reading images and videos from the Android manifest, permissions utility functions, and instrumentation tests, narrowing permissions to only audio and notifications. A new service declaration for Sequence Diagram(s)sequenceDiagram
participant User
participant Widget
participant WidgetUtils
participant PackageManager
participant System
User->>Widget: Initiate media selection (image/video)
Widget->>WidgetUtils: createPickMediaIntent(context, mimeType)
WidgetUtils->>PackageManager: Check photo picker support
alt Photo Picker Supported
WidgetUtils-->>Widget: Return Intent with MediaStore.ACTION_PICK_IMAGES
else
WidgetUtils-->>Widget: Return Intent with Intent.ACTION_GET_CONTENT
end
Widget->>System: Launch media selection
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🔇 Additional comments (11)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@damagatchi retest this please |
| try { | ||
| ((AppCompatActivity)getContext()).startActivityForResult(i, | ||
| ((AppCompatActivity)getContext()).startActivityForResult( | ||
| WidgetUtils.createPickMediaIntent (getContext(), "image/*"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code formatting seems off
| try { | ||
| ((AppCompatActivity)getContext()).startActivityForResult(i, | ||
| ((AppCompatActivity)getContext()).startActivityForResult( | ||
| WidgetUtils.createPickMediaIntent (getContext(), "video/*"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code formatting.
|
@damagatchi retest this please |
In AppCompat 1.4+, ActivityController.visible() triggers a layout pass which is triggering OnGlobalLayoutListener().onGlobalLayout() and causing the DemoUserRestoreTest.demoUserRestoreAndUpdateTest() unit test to fail
|
@damagatchi retest this please |
|
@damagatchi retest this please |
1 similar comment
|
@damagatchi retest this please |
|
@damagatchi retest this please |
1 similar comment
|
@damagatchi retest this please |
|
@damagatchi retest this please |
Product Description
Google issued a new policy that states that READ_MEDIA_IMAGES and READ_MEDIA_VIDEO permissions can only be used by apps whose core functionality requires broad access to images and videos stored in the device. These permissions were introduced in Android 13 and as such, this policy mostly affects that version and later. For apps that don't require this level of access, the recommended approach is to use a system picker, like the Android photo picker, which is available in Android 13 onward. Additionally, Google is committed to making the photo picker the default behaviour and is working to make it available to older versions too:
This PR implements the necessary changes to comply with this policy considering that CommCare doesn't need broad access to media files.
Note to reviewers: this work also involved assessing whether to adopt the ActivityResult API, since startActivityForResult is now deprecated. However, these two approaches can't be mixed, meaning that migrating would require the definition of ActivitResult contracts for every option in the
FormEntryActivity.onAtivityResultmethod.Safety Assurance
Safety story
This changes the current approach when selecting media files to upload in CommCare, it sets preference to system photo and video pickers that can handle the ACTION_PICK_IMAGE action, falling back to
ACTION_GET_CONTENTwhen an appropriate component is not available, so is not supposed to break anything. Moreover, these change were tested on Android 5, 6, 8, 9, 10, 11, 12 and 13QA Plan
A QA label has been added to ensure that this gets tested on both the minimum (Android 5) and maximum (Android 14) supported versions.
Labels and Review
cross-request: dimagi/commcare-core#1462