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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[馃悰] 馃敟 enrolledFactors not accessible on MultiFactorUser #7189

Closed
2 of 10 tasks
huwmartin opened this issue Jun 19, 2023 · 6 comments
Closed
2 of 10 tasks

[馃悰] 馃敟 enrolledFactors not accessible on MultiFactorUser #7189

huwmartin opened this issue Jun 19, 2023 · 6 comments
Labels
Help: Needs Triage Issue needs additional investigation/triaging. Impact: Bug New bug report Type: Stale Issue has become stale - automatically added by Stale bot

Comments

@huwmartin
Copy link

Issue

Documentation states that MultiFactorUser has a property enrolledFactors and so do the TypeScript types, but in code this is named enrolledFactor https://github.com/invertase/react-native-firebase/blob/main/packages/auth/lib/multiFactor.js#L17, so this isn't working as expected. Happy to open a PR for this if that would be helpful - should MultiFactorUser in multiFactor.js be updated with property naming as-per the docs?


Project Files

Javascript

Click To Expand

package.json:

# N/A

firebase.json for react-native-firebase v6:

# N/A

iOS

Click To Expand

ios/Podfile:

  • I'm not using Pods
  • I'm using Pods and my Podfile looks like:
# N/A

AppDelegate.m:

// N/A


Android

Click To Expand

Have you converted to AndroidX?

  • my application is an AndroidX application?
  • I am using android/gradle.settings jetifier=true for Android compatibility?
  • I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->


Environment

Click To Expand

react-native info output:

 OUTPUT GOES HERE
  • Platform that you're experiencing the issue on:
    • iOS
    • Android
    • iOS but have not tested behavior on Android
    • Android but have not tested behavior on iOS
    • Both
  • react-native-firebase version you're using that has this issue:
    • e.g. 5.4.3
  • Firebase module(s) you're using that has the issue:
    • e.g. Instance ID
  • Are you using TypeScript?
    • Y/N & VERSION


@huwmartin huwmartin added Help: Needs Triage Issue needs additional investigation/triaging. Impact: Bug New bug report labels Jun 19, 2023
@github-actions
Copy link

Hello 馃憢, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

@github-actions github-actions bot added the Type: Stale Issue has become stale - automatically added by Stale bot label Jul 17, 2023
@mikehardy
Copy link
Collaborator

Thanks for your patience, sorry for the delay, updating docs to match code is not breaking, updating code to match docs is breaking, so prefer to update docs in general and not break code

Only trouble is if firebase-js-sdk also has these typings we must match them, at which point updating code is best, but adding ability to access with new property while deprecating but not removing old property helps existing users adapt

A PR that did this would be truly appreciated

@github-actions github-actions bot removed the Type: Stale Issue has become stale - automatically added by Stale bot label Jul 18, 2023
@github-actions
Copy link

Hello 馃憢, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

@github-actions github-actions bot added the Type: Stale Issue has become stale - automatically added by Stale bot label Aug 15, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 30, 2023
@yuvalhermelin-fijoya
Copy link

@mikehardy Did @huwmartin open a PR on this? If not I will open and send to you for approval. I would love access to the last four of the phone number in my app before the user completes signin with MFA

@mikehardy
Copy link
Collaborator

@yuvalhermelin-fijoya no but it looks like @mnahkies has something good cooking in #7565 that is 馃槵 waiting on me.

Suggest you follow that one and I'll work to get it through now

@mnahkies
Copy link
Contributor

Thanks for your patience, sorry for the delay, updating docs to match code is not breaking, updating code to match docs is breaking, so prefer to update docs in general and not break code

Only trouble is if firebase-js-sdk also has these typings we must match them, at which point updating code is best, but adding ability to access with new property while deprecating but not removing old property helps existing users adapt

A PR that did this would be truly appreciated

Raised #7652 to cover the enrolledFactor vs enrolledFactors discrepancy (although I've made it a breaking change so far, as I figured it might make sense per #7567 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help: Needs Triage Issue needs additional investigation/triaging. Impact: Bug New bug report Type: Stale Issue has become stale - automatically added by Stale bot
Projects
None yet
Development

No branches or pull requests

4 participants