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

fix(storage): Stop transfer service if all transfers complete #2950

Merged
merged 1 commit into from Jul 22, 2022

Conversation

tylerjroach
Copy link
Contributor

Issue #, if available:
Amplify 1803

Description of changes:
When a transfer completes, check to see if there are any other transfers in progress. If not, stop the ongoing TransferService

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@tylerjroach tylerjroach changed the title Stop transfer service if all transfers complete fix(storage): Stop transfer service if all transfers complete Jul 20, 2022
@tylerjroach tylerjroach marked this pull request as ready for review July 22, 2022 16:14
@tylerjroach tylerjroach requested a review from a team as a code owner July 22, 2022 16:14
Copy link
Contributor

@sdhuka sdhuka left a comment

Choose a reason for hiding this comment

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

one minor comment, rest looks good!

Please co-ordinate merging PR's with the oncall, so that aws-sdk PR is merged, released first and then amplify PR is merged

@@ -137,7 +142,8 @@ public int onStartCommand(Intent intent, int flags, int startId) {
* b) An identifier for the ongoing notification c) Flag that determines if the notification
* needs to be removed when the service is moved out of the foreground state.
*/
if (Build.VERSION.SDK_INT >= ANDROID_OREO) {
if (Build.VERSION.SDK_INT >= ANDROID_OREO && !hasNotificationShown) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need hasNotificationShown check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to re-trigger showing the same notification again, which could look glitchy if you were observing the drawer and the notification kept getting recreated

@tylerjroach tylerjroach merged commit 99f42a7 into main Jul 22, 2022
@tylerjroach tylerjroach deleted the tjroach/fix-stop-transfer-service branch July 22, 2022 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants