-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Query MediaStore for media file URI for setting ringtone #4389
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
base: release/4.0
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds logic to query MediaStore for the correct content URI when setting a custom ringtone and updates media-scanning utilities to handle multiple files more idiomatically.
- Adds a new string resource for MediaStore URI query errors
- Implements
MediaStoreHack.getUriForMusicMediaFrom
and integrates it inMainFragment
to build the ringtone URI - Renames and refactors
MediaConnectionUtils.scanFile
toscanFiles
and updates all call sites accordingly
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
app/src/main/res/values/strings.xml | Added error.mediastore.query.uri message |
app/src/main/java/com/amaze/filemanager/ui/fragments/MainFragment.java | Queries MediaStore for ringtone URI; handles null fallback |
app/src/main/java/com/amaze/filemanager/filesystem/files/MediaConnectionUtils.kt | Renamed scanFile → scanFiles ; updated implementation |
app/src/main/java/com/amaze/filemanager/filesystem/MediaStoreHack.java | Added getUriForMusicMediaFrom helper |
Multiple *.java / *.kt files |
Replaced all scanFile calls with scanFiles |
Comments suppressed due to low confidence (3)
app/src/main/res/values/strings.xml:851
- Resource names cannot contain dots; this will cause a build error. Rename to
error_mediastore_query_uri
to match Android resource naming conventions and the usage in code (R.string.error_mediastore_query_uri).
<string name="error.mediastore.query.uri">Error querying MediaStore for content URI of select media file. Refresh the MediaStore and try again.</string>
app/src/main/java/com/amaze/filemanager/filesystem/MediaStoreHack.java:168
- [nitpick] The method name
getUriForMusicMediaFrom
is redundant. Consider renaming togetUriForMusicMedia
orgetMusicMediaUri
for conciseness and consistency.
public static Uri getUriForMusicMediaFrom(@NonNull String path, @NonNull Context context) {
app/src/main/java/com/amaze/filemanager/filesystem/MediaStoreHack.java:168
- You’ve added new query logic in
MediaStoreHack
but no unit or integration tests cover successful and failed query paths. Consider adding tests to verify both returned URIs and null behavior.
public static Uri getUriForMusicMediaFrom(@NonNull String path, @NonNull Context context) {
intent.putExtra(RingtoneManager.EXTRA_RINGTONE_PICKED_URI, properUri); | ||
} else { | ||
Toast.makeText(requireContext(), R.string.error_mediastore_query_uri, Toast.LENGTH_LONG) | ||
.show(); |
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.
[nitpick] Calling finish()
here abruptly ends the host activity. Consider setting a canceled result (e.g., setResult(RESULT_CANCELED)
) so the ringtone picker can handle the fallback gracefully instead of closing the entire app.
.show(); | |
.show(); | |
requireActivity().setResult(FragmentActivity.RESULT_CANCELED); |
Copilot uses AI. Check for mistakes.
@@ -36,13 +36,14 @@ object MediaConnectionUtils { | |||
* @param hybridFiles files to be scanned | |||
*/ | |||
@JvmStatic | |||
fun scanFile( | |||
fun scanFiles( |
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.
The KDoc comment above still refers to scanFile(...)
. Update it to reflect the new scanFiles(...)
method name and its parameters for clarity.
Copilot uses AI. Check for mistakes.
.appendQueryParameter("title", StringsKt.substringBeforeLast(filename, ".", "")) | ||
.build(); | ||
// Set the proper content URI as result | ||
intent.setDataAndType(properUri, "audio/*"); |
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.
Using a generic "audio/*"
MIME type may not convey the exact format. Consider retrieving the specific MIME type (e.g., via MimeTypes.getMimeType
) to improve compatibility with clients expecting precise types.
intent.setDataAndType(properUri, "audio/*"); | |
String mimeType = MimeTypes.getMimeType(resultBaseFile.getPath()); | |
intent.setDataAndType(properUri, mimeType != null ? mimeType : "audio/*"); |
Copilot uses AI. Check for mistakes.
Description
With reference to #4367, by inspecting the ringtone settings of a device
It becomes known that ringtone requires a content URI to the selected audio file on the device.
This PR tries to create that URI by querying the MediaStore. Therefore there is a prerequisite that even the audio file is visible in Amaze, it must also be visible to the MediaStore too, that refreshing the MediaStore is required.
Had attempted to do this for end user in code, but only worked on Android 14 or above, so gave up and not reinvent the wheel instead.
Issue tracker
Fixes #4367
Automatic tests
Manual tests
Device:
Build tasks success
Successfully running following tasks on local:
./gradlew assembledebug
./gradlew spotlessCheck