Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@Nash0x7E2
Copy link

@Nash0x7E2 Nash0x7E2 commented May 18, 2019

Description

This PR exposes the user's additional information. Useful for retrieving whether the user is new to your app, their IDP-specific user data and username (if the provider is Github or Twitter) .

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@CosmicPangolin
Copy link

@Nash0x7E2 @collinjackson

Hey guys - I see some checks failed here that look pretty easy to solve. Really hoping this makes it in, exposing the new user flag is quite helpful for me.

@collinjackson
Copy link
Contributor

Thanks for this PR. The integration test I added is currently failing on iOS because there's no iOS implementation. Would you be open to adding one?

@Nash0x7E2
Copy link
Author

Hi,
Unfortunately I do not have a Mac so I am unable to implement this on iOS

@Nash0x7E2
Copy link
Author

Is it possible for you to add this implementation? @collinjackson

@collinjackson
Copy link
Contributor

I'm looking at this change more carefully and I think a different approach is needed. The additionalUserInfo object is a return value for an authentication attempt rather than a signed-in user. The authentication methods (signInAnonymously etc) and onAuthStateChanged should return an instance of a new class (perhaps we can call this AuthResult like Android, AuthDataResult like iOS, or UserCredential like React Native Firebase) rather than a FirebaseUser. The new return value class should include properties user and optionally additionalUserInfo to mirror the native SDKs.

Unfortunately changing the return value of the sign-in methods is a breaking change that will affect every developer using this plugin. I'll try to get some feedback on naming internally so we don't have to do this more than once.

FYI @Ehesp

@jeroen-meijer
Copy link

I'd be willing to work on the iOS implementation once the naming issue has been resolved and breaking changes are made clear. 👋🏻

@Ehesp
Copy link
Member

Ehesp commented Jul 18, 2019

Hey - We took the UserCredential naming from the Web SDK. This is do-able for Flutter but as Collin mentioned, the credential class needs to be returned from the authentication methods.

@collinjackson
Copy link
Contributor

I got some feedback internally and we're going to call the result class AuthResult.

I went ahead and implemented the changes we've discussed in #1911. @jeroen-meijer I appreciate your generous offer to help; if you could take a quick look at #1911 and let me know if you have any feedback I'd appreciate it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants