fix(admob): improve defense logic to prevent multiple calls

[ifsnow]

Description

loaded: Whether the advert is loaded and can be shown.

If _loaded is changed to true when load() is called, the meaning of loaded getter becomes ambiguous.

If the goal is to prevent multiple calls, it would be better to use other variables for this.

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

[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/hq6jrn4kz
✅ Preview: https://react-native-firebase-git-fork-ifsnow-fix-admob-loaded.invertase.vercel.app

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #4849 (d8b2e37) into master (51edf96) will increase coverage by 0.41%.
The diff coverage is 86.67%.

@@            Coverage Diff             @@
##           master    #4849      +/-   ##
==========================================
+ Coverage   88.66%   89.06%   +0.41%     
==========================================
  Files         109      109              
  Lines        3712     3720       +8     
  Branches      347      347              
==========================================
+ Hits         3291     3313      +22     
+ Misses        379      365      -14     
  Partials       42       42              

[mikehardy]

Should it set loadCalled to false here as the event to set loaded == false is handled?

https://github.com/invertase/react-native-firebase/pull/4849/files#diff-82dfca6ec9ac0ffb90446c057bb3e58807ae9249382e2e85e3ea953f8122b6fbR48

[mikehardy]

...I should add - thanks for the PR :-)

[mikehardy]

Hey @ifsnow - I had a comment on this PR, can you take a look?

[ifsnow]

@mikehardy Sorry for being late. If load() can be called again after receiving CLOSED event, it will need to be initialized here.

_handleAdEvent(event) {
    const { type, error, data } = event.body;

    ...

    if (type === AdEventType.CLOSED || type === RewardedAdEventType.CLOSED) {
      this._loaded = false;
      this._isLoadCalled = false; // <= here
    }

    ....
}

Let me know if this is what you said. I'll fix it right away.

[mikehardy]

Yes! I think that's the spot. I'm not sure if it could happen but it seems better to match the states precisely?

[ifsnow]

@mikehardy That's a good point. My commit was modified. Thanks!

[mikehardy]

Great! @ifsnow last thing left is to sign the CLA so we can merge the change? You can follow the details link on the github actions check to do it - I think it's this https://cla-assistant.io/invertase/react-native-firebase?pullRequest=4849

[mikehardy]

looks good to me!

[ifsnow]

@mikehardy Did it. Thanks

[mikehardy]

v10.6.3 is newly published with this change 🚀