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 (messaging) and signInWithPhoneNumber #3425

Closed

Conversation

mateusmirandaalmeida
Copy link

Summary

Currently, when trying to use the phone login. conflicts with notifications

Error:
NativeFirebaseError: [auth/notification-not-forwarded] If app delegate swizzling is disabled, remote notifications received by UIApplicationDelegate need to be forwarded to FIRAuth's canHandleNotificaton: method.
at FirebaseAuthModule.signInWithPhoneNumber (http://192.168.0.167:8081/index.bundle?platform=ios&dev=true&minify=false:107323:28)

NativeFirebaseError: [auth/notification-not-forwarded] If app delegate swizzling is disabled, remote notifications received by UIApplicationDelegate need to be forwarded to FIRAuth's canHandleNotificaton: method.
    at FirebaseAuthModule.signInWithPhoneNumber (http://192.168.0.167:8081/index.bundle?platform=ios&dev=true&minify=false:107323:28)
@CLAassistant
Copy link

CLAassistant commented Apr 9, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@4f61188). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #3425   +/-   ##
=========================================
  Coverage          ?   85.25%           
=========================================
  Files             ?      108           
  Lines             ?     3429           
  Branches          ?        0           
=========================================
  Hits              ?     2923           
  Misses            ?      506           
  Partials          ?        0           

@mateusmirandaalmeida
Copy link
Author

This works for me and is already in my production application.
The problem is that the whole "npm install" loses my change.
Do you have a forecast to review this?

@mikehardy
Copy link
Collaborator

Patch Package to the rescue: https://github.com/ds300/patch-package

Do not develop in react-native without it

@Salakar Salakar self-assigned this Apr 17, 2020
@Salakar Salakar added the blocked: do-not-merge Do not merge this issue without approval by the person who labelled this issue as Do Not Merge label Apr 17, 2020
@Salakar
Copy link
Contributor

Salakar commented Apr 17, 2020

This change needs updating to handle if the Auth package is not in use by the users application - FIRAuth class won't be found in that case and would cause a build failure

I can look at this though if needed

@mateusmirandaalmeida
Copy link
Author

what's the best way to do this?

@tautvilas
Copy link

This fix works for me.

Salakar added a commit that referenced this pull request Apr 22, 2020
@Salakar
Copy link
Contributor

Salakar commented Apr 22, 2020

Have fixed this in fb70797 with the FIRAuth class existence check

Thanks for sending this PR - it was helpful

@Salakar Salakar closed this Apr 22, 2020
@Salakar
Copy link
Contributor

Salakar commented Apr 22, 2020

(a release will be out later today)

Salakar added a commit that referenced this pull request Apr 22, 2020
* fix(messaging,ios): keep original UNUserNotificationCenter delegate

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call. This should keep compatibility with other RN modules that set the delegate.

* v6.4.1-alpha.0

* Revert "v6.4.1-alpha.0"

This reverts commit b355a86

* feat: automatically register with APNs

* docs: typos

* fix: forward delegate call to FIRAuth

Fixes / supersedes #3425

* fix(messaging): add activity check to getInitialNotification (#3495)

* fix(messaging): add activity check to getInitialNotification

* fix(messaging): add activity check to getInitialNotification

* Update .spellcheck.dict.txt

Co-authored-by: Mike Diarmid <[email protected]>

Co-authored-by: Elliot Hesp <[email protected]>
andersondanilo pushed a commit to vixtech/react-native-firebase that referenced this pull request Nov 9, 2020
…ertase#3427)

* fix(messaging,ios): keep original UNUserNotificationCenter delegate

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call. This should keep compatibility with other RN modules that set the delegate.

* v6.4.1-alpha.0

* Revert "v6.4.1-alpha.0"

This reverts commit b355a86

* feat: automatically register with APNs

* docs: typos

* fix: forward delegate call to FIRAuth

Fixes / supersedes invertase#3425

* fix(messaging): add activity check to getInitialNotification (invertase#3495)

* fix(messaging): add activity check to getInitialNotification

* fix(messaging): add activity check to getInitialNotification

* Update .spellcheck.dict.txt

Co-authored-by: Mike Diarmid <[email protected]>

Co-authored-by: Elliot Hesp <[email protected]>
hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this pull request Jul 13, 2021
…ertase#3427)

* fix(messaging,ios): keep original UNUserNotificationCenter delegate

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call. This should keep compatibility with other RN modules that set the delegate.

* v6.4.1-alpha.0

* Revert "v6.4.1-alpha.0"

This reverts commit b355a86

* feat: automatically register with APNs

* docs: typos

* fix: forward delegate call to FIRAuth

Fixes / supersedes invertase#3425

* fix(messaging): add activity check to getInitialNotification (invertase#3495)

* fix(messaging): add activity check to getInitialNotification

* fix(messaging): add activity check to getInitialNotification

* Update .spellcheck.dict.txt

Co-authored-by: Mike Diarmid <[email protected]>

Co-authored-by: Elliot Hesp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked: do-not-merge Do not merge this issue without approval by the person who labelled this issue as Do Not Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants