[google_sign_in] Fix "pick account" on iOS

[JanKn]

Description

iOS Google Sign-in flow was always forcing user to fill email and password, the null check fixes the account picker, so the flow suggest Google Accounts already signed-in on iOS device.

Related Issues

Fixes: flutter/flutter#48602

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• 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 pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• 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.

[JanKn]

Cc @cyanglaz

[cyanglaz]

LGTM modulo the version updates. Thank you so much for fixing this! Do you think you can add tests for this?

[cyanglaz]

You might need to rebase and fix the version.

[JanKn]

Sure I think it might be possible to mock GoogleSignIn iOS library and do at least a unit test for the plugin that it sends "nil" to GoogleSignIn iOS library if the hostedDomain is not set (and correct hosted domain if it is set).
Or may be even E2E plugin test to the same point. However I am not an iOS developer and iOS side of testing is missing in the guidance. Checking the iOS WebView if it displays the account selection will be quite an effort :-)
Do you have some sample where I can get some inspiration on how to do this in iOS @cyanglaz ?

[cyanglaz]

#2599 is setting up unit test in iOS and android as well as adding some tests. You can follow that pattern when it is landed.

[enzobonggio]

this is must have for an app that have social login. What is needed in order to merge this one? Could I help with something?

[Zfinix]

Hi can this be merged already its a real issue that affects my apps UX

[cyanglaz]

Hi @JanKn, it's been a while since the unit tests are set up for the plugin. Do you plan to add tests for this PR? I can land it after you adding the tests.

[atrope]

@cyanglaz && @JanKn do you intend to finish this PR? It's a terrible UX for iOS Users..

[orestesgaolin]

@cyanglaz && @JanKn do you intend to finish this PR? It's a terrible UX for iOS Users..

@atrope what I can recommend for now is to fork the plugin; I'm doing this right now and it works pretty well

Sample fork:

dependency_overrides:
  google_sign_in:
    git:
      url: https://github.com/orestesgaolin/plugins.git
      path: packages/google_sign_in/google_sign_in
      ref: fix/ios-sign-in-with-google

[scaraux]

Can this be merged please this is a huge UX issue and blocker for our app. 🙏

[atrope]

@cyanglaz Which test should be done on this PR so it get merged? Just like it does not have tests for "clientId" i don't see which tests we have to create for this change..
This is long Stalled and i could code the test but i can't see any

[JanKn]

@atrope I was looking at it myself couple of weeks ago. My main issue for me was to actually run the tests in the latest XCode (I am mostly Android dev and I did not find any documentation on how to run tests on this project).
The change in this PR is making sure that hostedDomain is set to "nil" on iOS side of the Platform Plugin in case hostedDomain is not set from the Flutter platform channel call.
I wanted to create a test for this behavior. (The test should fail on master, but pass on this PR)

To check:
[GIDSignIn sharedInstance].hostedDomain = nil;

I did not have much time to investigate it more, thanks for help.

[atrope]

@cyanglaz this is a one liner PR affecting a major UX..
Almoçar 1 year and still not merged..
What should we do to get this merged?

I Can't believe 1 liner code should take this long to reach master..

[cyanglaz]

@atrope We should add a test to check if [GIDSignIn sharedInstance].hostedDomain is nil when [NSNull null] is passed from method channel. There are some similar tests here: https://github.com/flutter/plugins/blob/master/packages/google_sign_in/google_sign_in/ios/Tests/GoogleSignInPluginTest.m

[vmeretin]

Any news?

[vmeretin]

@cyanglaz Tried to write required test here #3805

[cyanglaz]

@JanKn Thank you again for providing the PR, unfortunately I'm going to close this in favor of #3805
The other PR is closer to land as it has tests.

Please feel free to keep contributing to our repo :)