review feedback

rsgowman · rsgowman · commit caac5bc58d74 · 2020-01-28T10:41:27.000-05:00
diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts
@@ -980,17 +980,17 @@ export abstract class AbstractAuthRequestHandler {
           'providerToLink.uid of properties argument must be a non-empty string.');
       }
     } else if (typeof properties.providersToDelete !== 'undefined') {
-      if (!validator.isNonEmptyArray(properties.providersToDelete)) {
+      if (!validator.isArray(properties.providersToDelete)) {
         throw new FirebaseAuthError(
           AuthClientErrorCode.INVALID_ARGUMENT,
-          'providersToDelete of properties argument must be a non-empty array of strings.');
+          'providersToDelete of properties argument must be an array of strings.');
       }
 
       properties.providersToDelete.forEach((providerId) => {
         if (!validator.isNonEmptyString(providerId)) {
           throw new FirebaseAuthError(
             AuthClientErrorCode.INVALID_ARGUMENT,
-            'providersToDelete of properties argument must be a non-empty array of strings.');
+            'providersToDelete of properties argument must be an array of strings.');
         }
       });
     }
diff --git a/src/index.d.ts b/src/index.d.ts
@@ -724,12 +724,12 @@ declare namespace admin.auth {
     photoURL?: string | null;
 
     /**
-     * Links this user to the specified federated provider.
+     * Links this user to the specified provider.
      */
     providerToLink?: UserProvider;
 
     /**
-     * Unlinks this user from the specified federated providers.
+     * Unlinks this user from the specified providers.
      */
     providersToDelete?: string[];
   }
diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts
@@ -366,17 +366,17 @@ describe('admin.auth', () => {
     });
 
     it('can link/unlink with a federated provider', async () => {
-      const federatedUid = 'google_uid_' + generateRandomString(10);
+      const googleFederatedUid = 'google_uid_' + generateRandomString(10);
       let userRecord = await admin.auth().updateUser(updateUser.uid, {
         providerToLink: {
           providerId: 'google.com',
-          uid: federatedUid,
+          uid: googleFederatedUid,
         },
       });
 
       let providerUids = userRecord.providerData.map((userInfo) => userInfo.uid);
       let providerIds = userRecord.providerData.map((userInfo) => userInfo.providerId);
-      expect(providerUids).to.deep.include(federatedUid);
+      expect(providerUids).to.deep.include(googleFederatedUid);
       expect(providerIds).to.deep.include('google.com');
 
       userRecord = await admin.auth().updateUser(updateUser.uid, {
@@ -385,7 +385,7 @@ describe('admin.auth', () => {
 
       providerUids = userRecord.providerData.map((userInfo) => userInfo.uid);
       providerIds = userRecord.providerData.map((userInfo) => userInfo.providerId);
-      expect(providerUids).to.not.deep.include(federatedUid);
+      expect(providerUids).to.not.deep.include(googleFederatedUid);
       expect(providerIds).to.not.deep.include('google.com');
     });
 
@@ -423,6 +423,19 @@ describe('admin.auth', () => {
       expect(providerUids).to.not.deep.include.members([googleFederatedUid, facebookFederatedUid, '+15555550001']);
       expect(providerIds).to.not.deep.include.members(['google.com', 'facebook.com', 'phone']);
     });
+
+    it('noops successfully when given an empty providersToDelete list', async () => {
+      const userRecord = await createTestUser('NoopWithEmptyProvidersToDeleteUser');
+      try {
+        const updatedUserRecord = await admin.auth().updateUser(userRecord.uid, {
+          providersToDelete: [],
+        });
+
+        expect(updatedUserRecord).to.deep.equal(userRecord);
+      } finally {
+        safeDelete(userRecord.uid);
+      }
+    });
   });
 
   it('getUser() fails when called with a non-existing UID', () => {
diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts
@@ -1456,14 +1456,6 @@ AUTH_CONFIGS.forEach((testConfig) => {
           });
       });
 
-      it('should be rejected given an empty providersToDelete list', () => {
-        expect(() => {
-          auth.updateUser(uid, {
-            providersToDelete: [],
-          });
-        }).to.throw(FirebaseAuthError).with.property('code', 'auth/argument-error');
-      });
-
       INVALID_PROVIDER_IDS.forEach((invalidProviderId) => {
         it('should be rejected given a deleteProvider list with an invalid provider ID '
             + JSON.stringify(invalidProviderId), () => {

Original file line number	Diff line number	Diff line change
`@@ -980,17 +980,17 @@ export abstract class AbstractAuthRequestHandler {`
`980`	`980`	`'providerToLink.uid of properties argument must be a non-empty string.');`
`981`	`981`	`}`
`982`	`982`	`} else if (typeof properties.providersToDelete !== 'undefined') {`
`983`		`- if (!validator.isNonEmptyArray(properties.providersToDelete)) {`
	`983`	`+ if (!validator.isArray(properties.providersToDelete)) {`
`984`	`984`	`throw new FirebaseAuthError(`
`985`	`985`	`AuthClientErrorCode.INVALID_ARGUMENT,`
`986`		`- 'providersToDelete of properties argument must be a non-empty array of strings.');`
	`986`	`+ 'providersToDelete of properties argument must be an array of strings.');`
`987`	`987`	`}`
`988`	`988`
`989`	`989`	`properties.providersToDelete.forEach((providerId) => {`
`990`	`990`	`if (!validator.isNonEmptyString(providerId)) {`
`991`	`991`	`throw new FirebaseAuthError(`
`992`	`992`	`AuthClientErrorCode.INVALID_ARGUMENT,`
`993`		`- 'providersToDelete of properties argument must be a non-empty array of strings.');`
	`993`	`+ 'providersToDelete of properties argument must be an array of strings.');`
`994`	`994`	`}`
`995`	`995`	`});`
`996`	`996`	`}`