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

Separate activation broadcast receiver from channel handler class #307

Merged
merged 7 commits into from
Feb 15, 2022

Conversation

ikbalkaya
Copy link
Contributor

@ikbalkaya ikbalkaya commented Jan 24, 2022

This PR fixes activation handling code by

  • Fixes some issues with PushActivationHandlers singleton, by providing an instance method and converting some static methods into instance methods
  • Moves activation broadcast receiver to a separate file, creating methods for registering with callback and unregistering.
  • Respects the FlutterEngine lifecycle by registering the receiver when it's attached to engine and deregisters it when it's detached.

Closes #304

Please note that changes here won't cause a manual application launch, meaning that any message from broadcast receiver will be received only when the Flutter engine is attached. I did this to prevent further bugs raising from inconsistencies related to Flutter engine lifecycle.
However this is still a temporary solution. A full solution would be to separate third party push plugin so that we can reuse code directly from Firebase library (#292) or completely refactor push notification code on our plugin

Note: Java library will send activation results using broadcasts. That method might have been chosen for native Android apps, but it could be nice if we could have a async alternative that would give results through callback. If so we wouldn't need to deal with broadcast receivers on our plugin.

@github-actions github-actions bot temporarily deployed to staging/pull/307/dartdoc January 24, 2022 17:45 Inactive
…AttachedToEngine and onDetachedFromEngine callbacks
@github-actions github-actions bot temporarily deployed to staging/pull/307/dartdoc January 24, 2022 17:57 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/307/dartdoc January 24, 2022 18:08 Inactive
@ikbalkaya ikbalkaya marked this pull request as ready for review January 25, 2022 09:43
@ikbalkaya ikbalkaya self-assigned this Jan 25, 2022
@ikbalkaya ikbalkaya added bug Something isn't working. It's clear that this does need to be fixed. code-quality Affects the developer experience when working in our codebase. labels Jan 25, 2022
Copy link

@KacperKluka KacperKluka left a comment

Choose a reason for hiding this comment

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

Overall looks good 👍 One small problem but it was present before so I'll let you decide what to do with it 😉

@github-actions github-actions bot temporarily deployed to staging/pull/307/dartdoc January 25, 2022 15:25 Inactive
Copy link
Contributor

@ikurek ikurek left a comment

Choose a reason for hiding this comment

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

LGTM 🙂

@ikurek ikurek merged commit 63b4d75 into main Feb 15, 2022
@ikurek ikurek deleted the bug/activation_results_are_null branch February 15, 2022 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed. code-quality Affects the developer experience when working in our codebase.
Development

Successfully merging this pull request may close these issues.

resultForDeactivate and resultForActivate are set to null in some situations in Android plugin
3 participants