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(android, messaging): store notifications in prefs ring buffer for initial/open attribution #4317

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

vannt1991
Copy link
Contributor

Fix issue open old notification

Related issues

Fixes #4052

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Sep 28, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/cqsmbldz7
✅ Preview: https://react-native-firebase-git-fork-vannt1991-master.invertase.vercel.app

@vannt1991 vannt1991 changed the title Fix: [Firebase message][Android] - issue open old notify fix: [Firebase message][Android] - issue open old notify Sep 28, 2020
@mikehardy mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Sep 28, 2020
@mikehardy mikehardy changed the title fix: [Firebase message][Android] - issue open old notify fix(android, messaging): store notifications in prefs ring buffer for initial/open attribution Sep 28, 2020
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

I can't see anything wrong with this -> looks good to me
I'll merge and do a release and hopefully nothing blows up :-), please listen in case there are problems though - it is amazing how these things that look good fail in production in unexpected ways!

@mikehardy mikehardy merged commit a7cafc9 into invertase:master Sep 30, 2020
@hackrx
Copy link

hackrx commented Oct 1, 2020

Can you tell me how can I get these changes?
By using yarn upgrade @react-native-firebase/messaging?

@mikehardy
Copy link
Collaborator

@hackrx the publishing script here isn't behaving perfectly right now sadly so the changelog is not correct (yet), sorry about that.

You can check the commit reference on this PR though and look for the tags. They indicate the tags where the commit was included

You can see it will be present in messaging 7.8.10 or 7.8.11 as it's in the diff past 7.8.9

https://github.com/invertase/react-native-firebase/compare/@react-native-firebase/[email protected]...@react-native-firebase/[email protected]

@mikehardy mikehardy removed the Workflow: Needs Review Pending feedback or review from a maintainer. label Dec 2, 2020
hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this pull request Jul 13, 2021
…ution (invertase#4317)

Use a 100-notification limited ring buffer in prefs to store them
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.

Android: getInitialNotification/onNotificationOpenedApp not triggered for old notifications if app restarts
3 participants