fix(app-distribution, ios): avoid crash when releaseNotes is nil

[YukiKitagata]

Description

On iOS device crashes when called firebase.app().appDistribution().checkForUpdate().

This happens when release.releaseNotes is nil. Because the current implementation is assigned to an NSDictionary even if release.releaseNotes is nil.

This PR has been changed to return NSNull if it is nil.

react-native-firebase/packages/app-distribution/ios/RNFBAppDistribution/RNFBAppDistributionModule.m

Lines 96 to 102 in 600dee0

	resolve(@{
	@"displayVersion" : release.displayVersion,
	@"buildVersion" : release.buildVersion,
	@"releaseNotes" : release.releaseNotes,
	@"isExpired" : [NSNumber numberWithBool:release.isExpired],
	@"downloadURL" : release.downloadURL
	});

In addition, it fixes the issue of resolve being called twice.

Related issues

N/A

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:

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

[CLAassistant]

All committers have signed the CLA.

[vercel]

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

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/5EQwoKqfpGXSNmYhwwBURKe5ANVx
✅ Preview: https://react-native-firebase-git-fork-n1nc-fix-crash-8779c8-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/9rwfZEqWEBL19TFTckmKsoP1zztn
✅ Preview: Canceled

[mikehardy]

Thank you for this! I coded it to docs (it's supposed to always be there?) but didn't have a specific project to test on and you know docs and reality are always a little different. I'll do a release as soon as this is merged etc

[mikehardy]

Definitely need the cla signed pre merge then should be quick to merge

[codecov]

Codecov Report

Merging #5667 (3a43d41) into master (600dee0) will increase coverage by 0.40%.
The diff coverage is n/a.

❗ Current head 3a43d41 differs from pull request most recent head 877468f. Consider uploading reports for the commit 877468f to get more accurate results

@@             Coverage Diff              @@
##             master    #5667      +/-   ##
============================================
+ Coverage     53.26%   53.65%   +0.40%     
  Complexity      632      632              
============================================
  Files           208      208              
  Lines         10084    10084              
  Branches       1543     1543              
============================================
+ Hits           5370     5410      +40     
+ Misses         4427     4391      -36     
+ Partials        287      283       -4     

[YukiKitagata]

@mikehardy
Thanks for the Approve!
I've signed the CLA.

[mikehardy]

Great - merged and I have a separate crash fix for dynamic links queued up as well, will release both shortly.

It's worth saying that you may be the first person using App Distribution via react-native-firebase, which explains why you saw something immediately (this crash) but also leads me to ask in advance to please post anything else up you see since there may be more issues lurking - I want it to be a good experience but it's impossible to test really in an E2E context. I couldn't think of a way anyway.

There aren't many APIs and they are all really really simple at least, so hopefully it all works great from this point 🤞

[mikehardy]

app distribution is actually beta upstream as well (it really is brand new...) and the related issue I filed upstream was marked for a documentation change on the nullability of the property, so this is helpful throughout the firebase ecosystem firebase/firebase-ios-sdk#8602

[mikehardy]

Android does not have a public implementation yet, so there won't be problems after you're done integrating iOS (android just resolves via a reject saying it doesn't work on the platform...) but they are implementing it internally pre-release and I verified that it is marked correctly as nullable there

https://github.com/firebase/firebase-android-sdk/blob/82b02af331d4674682bfb90c195da45543b808c3/firebase-app-distribution/src/main/java/com/google/firebase/appdistribution/AppDistributionRelease.java#L46-L48

I think we're all set.