fix(messaging): Missing notification on restart

[mcorner]

Description

When deserializing saved notifications from the store on Android, this pull will include the notification object as well. Previously the code used the Firebase RemoteMessage class, but that does not allow setting the notification object. This uses writablemaps instead.

This is an Android only issue.

Related issues

Fixes #5167

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

After following the reproduction steps in the linked issue, I can now get the notification object on start:

LOG {"ttl":2419200,"from":"557990742577","to":"0:1618554006404925%26d15a5d26d15a5d","data":{},"sentTime":2147483647,"messageId":"0:1618554006404925%26d15a5d26d15a5d","collapseKey":"com.repromessagingbug","notification":{"android":{},"title":"test","body":"test"}}

---

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

• 👉 Star this repo on GitHub ⭐️
• 👉 Follow React Native Firebase and Invertase on Twitter

[vercel]

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/EoJLSKvZgM8nZoGaetT819EEZrGJ
✅ Preview: https://react-native-f-git-fork-mcorner-missing-notification-on-3a6d08.vercel.app

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #5181 (d853e5c) into master (90bceb2) will increase coverage by 2.59%.
The diff coverage is n/a.

❗ Current head d853e5c differs from pull request most recent head 1fa1d45. Consider uploading reports for the commit 1fa1d45 to get more accurate results

@@            Coverage Diff             @@
##           master    #5181      +/-   ##
==========================================
+ Coverage   85.26%   87.85%   +2.59%     
==========================================
  Files         109      105       -4     
  Lines        3744     6096    +2352     
  Branches      360      360              
==========================================
+ Hits         3192     5355    +2163     
- Misses        481      696     +215     
+ Partials       71       45      -26     

[mcorner]

@mikehardy One development question... What is the easiest way to link to this pull/commit in another project to use/test it? Normally in "simpler" packages I can just link to the branch in github directly in a package.json, but that doesn't work here (no version and seemingly other complexities). I couldn't get yarn link to work properly either with the packages structure.

I would like to use this in our product in advance of the pull getting approved and published.

Thanks!

[mikehardy]

Hi there! Thanks for the PR - I'll look through it as soon as I get a chance - but to your specific question, I noticed this was a problem for collaboration (both in development and testing) among the community here so I implemented an automated patch-package patch creator :-) #4182

• find the "Details" link for the "Create Test Patches" GitHub Action run
• look for the "Artifacts" dropdown near-ish to top, sort of on right side and grab the patches.zip file
• install patch-package if you have not already (it's fantastic)
• drop the patches in and yarn install
• test the feature, integrated in your own app(s), until it's released

[mikehardy]

Interesting! I like this in general and it appears at the JS level this is non-breaking, it is just adding something that should be there, so that's fantastic as it means I can merge it and release it quickly

As commented in the specific code spots I really really wish that the change had been minimal, it appears the diff is much larger than it needs to be as the conditionals were restructured

Also even though it's at the java level some people use this library in brownfield apps and since the Store was a public interface we need to preserve the meaning of the old symbols as this is not something worth doing a breaking change over (but if they're marked 'deprecated' at least we can purge them in the next breaking change round)

I'm about to do an 11.3 release with a large-ish batch of other items and I can release this after once it's shaped up, that will also have the side-effect - since you are interested in using it immediately - of making the patch-set vs "current release" much smaller since it will only have this change once "current release" is more current and you re-push this to generate a new patch set

[mikehardy]

Nice! That is a squeaky clean diff - I was pretty sure it was correct before hand just on visual inspection - notwithstanding the actual test results posted - but now it's obvious, and I really appreciate the backwards-compatibility on the Java symbols

I should be able to get this out as a 11.3.1 shortly but in the meantime since 11.3.0 did just go out, the patch set should be empty except this change, making the patch-package usage a lot less anxiety-inducing

Thanks!

[mikehardy]

I was having a really hard time getting CI to chew through this correctly but it was not the PR's fault - I was sure of that after the last run even though it still had a failure in the storage area. Merged and will release a 11.3.2 shortly

Thanks again for this!