Skip to content

Commit 330b374

Browse files
committed
resolved code review comments and updated tests
1 parent 5f0b3d6 commit 330b374

File tree

3 files changed

+5
-19
lines changed

3 files changed

+5
-19
lines changed

packages/auth/demo/src/index.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ function activeUser() {
130130
* Returns the active user after sign in (i.e. currentUser or lastUser).
131131
* @return {!firebase.User}
132132
*/
133-
async function getActiveUserAfterSignIn() {
133+
async function getActiveUserBlocking() {
134134
const type = $('input[name=toggle-user-selection]:checked').val();
135135
if (type === 'lastUser') {
136136
return lastUser;
@@ -150,7 +150,7 @@ async function getActiveUserAfterSignIn() {
150150
*/
151151
async function refreshUserData() {
152152
try {
153-
let user = await getActiveUserAfterSignIn();
153+
let user = await getActiveUserBlocking();
154154
if (user) {
155155
$('.profile').show();
156156
$('body').addClass('user-info-displayed');
@@ -456,7 +456,6 @@ function onReauthenticateWithEmailLink() {
456456
const link = $('#link-with-email-link-link').val() || undefined;
457457
const credential = EmailAuthProvider.credentialWithLink(email, link);
458458
// This will not set auth.currentUser to lastUser if the lastUser is reauthenticated.
459-
460459
reauthenticateWithCredential(activeUser(), credential).then(result => {
461460
logAdditionalUserInfo(result);
462461
refreshUserData();

packages/auth/src/core/auth/auth_impl.test.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -789,24 +789,18 @@ describe('core/auth/auth_impl', () => {
789789

790790
describe('AuthStateReady', () => {
791791
let user: UserInternal;
792-
let onAuthStateChangedCallback: sinon.SinonSpy;
793792
let authStateChangedSpy: sinon.SinonSpy;
794793

795794
beforeEach(async () => {
796795
user = testUser(auth, 'uid');
797796

798-
onAuthStateChangedCallback = sinon.spy();
799-
auth.onAuthStateChanged(onAuthStateChangedCallback);
800797
authStateChangedSpy = sinon.spy(auth, 'onAuthStateChanged');
801798

802799
await auth._updateCurrentUser(null);
803800
});
804801

805-
//case one: if(this.currentUser) is true --> resolves immediately
806802
it('immediately returns resolved promise if the user is previously logged in', async () => {
807-
expect(onAuthStateChangedCallback).to.be.calledOnce;
808803
await auth._updateCurrentUser(user);
809-
expect(onAuthStateChangedCallback).to.be.calledTwice;
810804

811805
await auth
812806
.authStateReady()
@@ -817,11 +811,8 @@ describe('core/auth/auth_impl', () => {
817811
.catch(error => {
818812
throw new Error(error);
819813
});
820-
821-
expect(onAuthStateChangedCallback).to.be.calledTwice;
822814
});
823815

824-
//case two: if(this.currentUser) is false --> calls onAuthStateChanged
825816
it('returns resolved promise once the user is initialized to object of type UserInternal', async () => {
826817
expect(authStateChangedSpy).to.not.have.been.called;
827818
const promiseVar = auth.authStateReady();
@@ -831,19 +822,16 @@ describe('core/auth/auth_impl', () => {
831822

832823
await promiseVar
833824
.then(() => {
834-
// onAuthStateChangedCallback();
835825
expect(auth.currentUser).to.eq(user);
836826
})
837827
.catch(error => {
838828
throw new Error(error);
839829
});
840830

841831
expect(authStateChangedSpy).to.be.calledOnce;
842-
// expect(onAuthStateChangedCallback).to.be.calledThrice;
843832
});
844833

845-
//case three: if user logged out more than once, promise should still be resolved with currentUser remained as null
846-
it('resolves promise with currentUser remains as null when user logs out more than once', async () => {
834+
it('resolves the promise during repeated logout', async () => {
847835
expect(authStateChangedSpy).to.not.have.been.called;
848836
const promiseVar = auth.authStateReady();
849837
expect(authStateChangedSpy).to.be.calledOnce;
@@ -861,7 +849,6 @@ describe('core/auth/auth_impl', () => {
861849
expect(authStateChangedSpy).to.be.calledOnce;
862850
});
863851

864-
//case four: user sign in failed, expect promise to resolve and allow currentUser to be null.
865852
it('resolves the promise with currentUser remain null during log in failure', async () => {
866853
expect(authStateChangedSpy).to.not.have.been.called;
867854
const promiseVar = auth.authStateReady();
@@ -886,7 +873,6 @@ describe('core/auth/auth_impl', () => {
886873
expect(authStateChangedSpy).to.be.calledOnce;
887874
});
888875

889-
//case five: user sign in delay, promise should be resolved after delay.
890876
it('resolves the promise in a delayed user log in process', async () => {
891877
setTimeout(async () => {
892878
await auth._updateCurrentUser(user);

packages/auth/src/model/public_types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ export interface Auth {
293293
): Unsubscribe;
294294
/**
295295
* return a promise that resolves immediately when the initial
296-
* auth state is settled and currentUser is available
296+
* auth state is settled. The current user might be a valid user,
297+
* or null if there is no user signed in currently.
297298
*/
298299
authStateReady(): Promise<void>;
299300
/** The currently signed-in user (or null). */

0 commit comments

Comments
 (0)