-
-
Notifications
You must be signed in to change notification settings - Fork 362
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
Use WorkManager for Upload/Download #5492
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FloEdelmann
reviewed
Feb 21, 2024
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 looks good but I haven't run it. I just have two small comment suggestions.
app/src/main/java/de/westnordost/streetcomplete/data/download/DownloadWorker.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/upload/UploadWorker.kt
Outdated
Show resolved
Hide resolved
…DownloadWorker.kt Co-authored-by: Flo Edelmann <[email protected]>
…loadWorker.kt Co-authored-by: Flo Edelmann <[email protected]>
FloEdelmann
reviewed
Feb 21, 2024
app/src/main/java/de/westnordost/streetcomplete/data/download/DownloadWorker.kt
Outdated
Show resolved
Hide resolved
…DownloadWorker.kt Co-authored-by: Flo Edelmann <[email protected]>
…ete/StreetComplete into workmanager-for-sync
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
StreetComplete update got rejected from the Google Play Store because all apps targeting API 34 must declare for what they use the foreground service and policy-wise, it is quite limited what Google will allow as foreground services.
StreetComplete uses a foreground service (that notification that tells you "Data syncing...") for the upload and download, so I declared it being used for "data sync". Now, however, as far as I understand the rejection notice, for the "data sync" use case some very strict rules apply. One of these rules is that the action that triggers the data sync must either be user-initiated or clearly perceptible to the user. With the latter, they mean something like e.g. music playing, i.e. a notification is not enough.
Expand to read full rejection notice
Issue found: Permission use is either not declared or incorrectly declared
We found that one or more of the use cases utilizing the foreground service permission in your app has either been incorrectly submitted or is missing from your Console declaration.
We found that one or more of the declared use cases is not compliant with how foreground service permission is allowed to be used. Specifically, the user is not made aware of functionality requiring permission when active.
Issue details
We found an issue in the following area(s):
To bring your app into compliance, follow these steps:
To make APK/App bundle-level updates, follow these steps:
If all declared use cases are impacted, the FOREGROUND_SERVICE permission must also be removed.
About the Permissions for Foreground Services
The Foreground Service permission ensures the appropriate use of user-facing foreground services. For apps targeting Android 14 and above, you must specify a valid foreground service type for each foreground service used in your app, and declare the foreground service permission that is appropriate for that type. For example, if your app’s use case requires map geolocation, you must declare the FOREGROUND_SERVICE_LOCATION permission in your app’s manifest.
With the exceptions of systemExempted and shortService foreground service types, apps are only allowed to declare a foreground service permission if the use:
The use of foreground service is further explained on this Play Console Help Center page.
So, if the download (and upload) was always triggered by the user, I understand that using a foreground service would continue to be okay. But since the app downloads map data around the user's location automatically, this use case is not okay.
The PR now migrates away from foreground services and instead uses the WorkManager for upload and download. (Luckily) it looks like that WorkManager work is started immediately when the app is in foreground.
On Android versions below 12, the WorkManager actually uses a foreground service to execute the work, so on these versions, you'll continue to see a "syncing data" notification. Bonus: You'll actually cancel it now. Not on Android 13 and above, though, because there is no notification anymore ... Google 🙄