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

Don't send old FCM tokens when new token is generated during startup #1440

Merged
merged 5 commits into from
Jul 23, 2024

Conversation

tsightler
Copy link
Collaborator

This fix stops the code from pushing the old FCM token to Ring when a new token is generated during registration, for example, due to migration to FCMv1 APIs for registration. The bug happens because the very last command in the push notification initialization sequence currently sends the FCM token to Ring any time credentials is defined:

if (credentials) {
  onPushNotificationToken.next(credentials.fcm.token)
}

The problem with this is credentials is assigned prior to registration so it would still hold the old FCM token even if a new token was generated during the registration process. Because of this, the sequence of events was something like this:

  1. credentials = config.pnc (assuming credentials already existed)
  2. Push notification would register using existing credentials
  3. If registration updated the FCM token, onCredentialsChanged() was called which subsequently pushed the new generated FCM token to Ring
  4. However, because crendentials was previously defined and still contained the old FCM token, the command noted above would also run immediately pushing the old FCM token back to Ring thus making it impossible to receive notifications with the new push registration

The new code only pushes the stored FCM token if it was unchanged during registration.

Tom Sightler added 5 commits June 24, 2024 23:04
This fix stops the code from push the old FCM token to Ring when a new token is required due to FCMv1 registration.
Copy link
Owner

@dgreif dgreif left a comment

Choose a reason for hiding this comment

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

Merging as-is and will fix up in the beta branch

@dgreif dgreif merged commit 39536a7 into beta Jul 23, 2024
0 of 4 checks passed
@dgreif dgreif deleted the fcm-upgrade-fix branch July 23, 2024 01:36
dgreif pushed a commit that referenced this pull request Jul 23, 2024
…1440)

Co-authored-by: Tom Sightler <tom.sightler@veeam.com>
dgreif pushed a commit that referenced this pull request Aug 4, 2024
…1440)

Co-authored-by: Tom Sightler <tom.sightler@veeam.com>
koush pushed a commit to koush/ring that referenced this pull request Sep 26, 2024
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.

2 participants