Fix 2 notification bugs#3237
Open
julien4215 wants to merge 2 commits into
Open
Conversation
The wrong channel id caused the notification to be parsed incorrectly when the user taps on it. Because of that, it would bring the user to the home screen instead of the challenge screen.
app is in the foregroud because fcm already shows the notification when the app is in the background. Before this fix, when the app is in the background, it might show the notification from the socket (when the socket is still alive in the background) which is later overriden by the notification from fcm resulting in a confusing experience for the user.
julien4215
force-pushed
the
fix-notification
branch
from
ee8c142 to
fa82af9
Compare
veloce
reviewed
May 24, 2026
|
|
||
| // notifications from socket are only displayed if app is in foreground | ||
| final binding = TestWidgetsFlutterBinding.ensureInitialized(); | ||
| binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed); |
Contributor
There was a problem hiding this comment.
I don't think this is useful. More useful would be a test that assert that the notification is not shown in background.
Collaborator
Author
There was a problem hiding this comment.
The tests fail without it. I think it doesn't consider that the app is in foreground by default.
Contributor
There was a problem hiding this comment.
That would be very weird if the app wasn't in foreground by default during tests. The tests would not even work if that was the case.
I can't accept the PR until I have the real explanation for why the tests fail with the new code.
And also it needs a test that shows the notification is not shown in background.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fix wrong channel id for challenge created notifications.
The wrong channel id caused the notification to be parsed incorrectly when the user taps on it. Because of that, it would bring the user to the home screen instead of the challenge screen.
Only display the challenge received notifications from the socket if the app is in the foregroud because fcm already shows the notification when the app is in the background.
Before this fix, when the app is in the background, it might show the notification from the socket (when the socket is still alive in the background) which is later overriden by the notification from fcm resulting in a confusing experience for the user.