Allow setting mock user to null
#112
Conversation
|
I'll fix conflicts today |
|
@atn832 should be ready for review now |
test/firebase_auth_mocks_test.dart
Outdated
| final decodedToken = | ||
| JwtDecoder.decode((await auth.currentUser!.getIdToken())!); | ||
| expect(decodedToken['role'], 'admin'); | ||
| expect(decodedToken['bodyHeight'], 169); | ||
| await auth.signOut(); | ||
|
|
||
| auth.mockUser = user; |
There was a problem hiding this comment.
I think I have misnamed mockUser. It seems like mockUser is the user for the next signin, but, if signed in, also lets you set the current user (#80).
In the future, renaming mockUser to userOnNextSignIn, and exposing the currently signed in user to allow #80 's use case might be more appropriate.
I need to think about it more before changing the API and breaking people's projects.
There was a problem hiding this comment.
If I think of mockUser as the userOnNextSignIn, it feels strange that signing out should set it to null. It will also break people's projects if they sign out and sign in again, like in this test.
Since you've allowed setting mockUser to null, would that be enough? Then you can implement your use case as :
auth.signIn(...);
auth.signOut();
auth.mockUser = null;
auth.signInAnonymously();It's not perfect but it won't break any existing project until we figure out a better solution.
There was a problem hiding this comment.
No problem - I definitely think my understanding of how mockUser is intended to be used was lacking. I am happy to revert some changes here as you suggest.
I need to specifically look at how we handle the case in #110 as per your suggestion though, as I'm not sure we have that fine grained control. I will check back shortly.
There was a problem hiding this comment.
@atn832 Having revisited our problem with your explanation of mockUser, I think your suggestion makes sense and works for us. I've reverted the change to clear the mock user now, and left in everything else. Let me know what you think.
| return Future.value(userCredential); | ||
| } | ||
|
|
||
| void _notifyCredential(MockUserCredential? credential) { |
michaelowolf
changed the title
null
| expect(auth.currentUser, isNull); | ||
| expect(auth.authStateChanges(), emitsInOrder([user, null])); | ||
| expect(auth.userChanges(), emitsInOrder([user, null])); | ||
| auth.mockUser = tUser; |
There was a problem hiding this comment.
Since signing out does not set mockUser to null anymore, do you still need to set mockUser again if signing in with the same user? Maybe you can remove this line.
There was a problem hiding this comment.
Ah yes, you're right - this should have been removed too. I'll clean that up
There was a problem hiding this comment.
Ah, looks like you fixed it - I missed your other comment. Thanks!
|
I removed the line (b28ab70), then published both #111 and #112 to https://pub.dev/packages/firebase_auth_mocks/changelog#0141. Thanks again @michaelowolf ! |
Resolves #110.
I've also tidied up some repeated logic when adding new user credentials to the various streams.