From cb90a86eeac9c68d9052c9744f48a9642e8f9bdc Mon Sep 17 00:00:00 2001
From: Willie Ruemmele <willieruemmele@gmail.com>
Date: Wed, 27 Jan 2021 16:07:53 -0700
Subject: [PATCH] fix: code review 1

---
 src/commands/force/user/create.ts | 45 +++++++++++++++++++------------
 test/commands/user/create.test.ts |  7 +++--
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/commands/force/user/create.ts b/src/commands/force/user/create.ts
index bbf9d039..f7daf129 100644
--- a/src/commands/force/user/create.ts
+++ b/src/commands/force/user/create.ts
@@ -59,6 +59,22 @@ export class UserCreateCommand extends SfdxCommand {
   private failures: FailureMsg[] = [];
   private authInfo: AuthInfo;
 
+  /**
+   * removes fields that cause errors in sfdx-core's userCreate method
+   *
+   * @param fields a list of combined fields from varargs and the config file
+   * @private
+   */
+  private static stripInvalidAPIFields(fields: UserFields & Dictionary<string>): UserFields {
+    const copy = Object.assign({}, fields);
+    // remove invalid fields for userCreate()
+    delete copy.permsets;
+    delete copy.generatepassword;
+    delete copy.generatePassword;
+    delete copy.profileName;
+    return copy as UserFields;
+  }
+
   public async run(): Promise<UserFields> {
     this.logger = await Logger.child(this.constructor.name);
     const defaultUserFields: DefaultUserFields = await DefaultUserFields.create({
@@ -68,26 +84,14 @@ export class UserCreateCommand extends SfdxCommand {
 
     // merge defaults with provided values with cli > file > defaults
     const fields = await this.aggregateFields(defaultUserFields.getFields());
-    // because fields is type UserFields & Dictionary<string> we can access these
-    const permsets: string = fields.permsets;
-    const generatepassword: string = fields.generatepassword;
-    const profileName = fields.profileName;
-
-    // extract the fields and then delete, createUser doesn't expect a permsets or generatepassword
-    delete fields.permsets;
-    delete fields.generatepassword;
-    delete fields.generatePassword;
-    delete fields.profileName;
-
     try {
-      this.authInfo = await this.user.createUser(fields);
-      fields.permsets = permsets;
-      fields.profileName = profileName;
-      if (fields.profileName) await this.authInfo.save({ userProfileName: fields.profileName });
+      this.authInfo = await this.user.createUser(UserCreateCommand.stripInvalidAPIFields(fields));
     } catch (e) {
       await this.catchCreateUser(e, fields);
     }
 
+    if (fields.profileName) await this.authInfo.save({ userProfileName: fields.profileName });
+
     // Assign permission sets to the created user
     if (fields.permsets) {
       try {
@@ -111,7 +115,7 @@ export class UserCreateCommand extends SfdxCommand {
     }
 
     // Generate and set a password if specified
-    if (generatepassword === 'true' || generatepassword) {
+    if (fields.generatepassword === 'true') {
       try {
         const password = User.generatePasswordUtf8();
         await this.user.assignPassword(this.authInfo, password);
@@ -191,7 +195,14 @@ export class UserCreateCommand extends SfdxCommand {
     }
 
     // the file schema is camelCase while the cli arg is no capitialization
-    if (defaultFields['generatePassword'] || defaultFields['generatepassword']) {
+    // the flag and file can pass as a string or a proper boolean
+    // we can check only for existence because it could be set to false or 'false'
+    if (
+      defaultFields['generatePassword'] === 'true' ||
+      defaultFields['generatepassword'] === 'true' ||
+      defaultFields['generatePassword'] === true ||
+      defaultFields['generatepassword'] === true
+    ) {
       // standardize on 'generatepassword'
       defaultFields['generatepassword'] = 'true';
     }
diff --git a/test/commands/user/create.test.ts b/test/commands/user/create.test.ts
index 69a9b54a..74640865 100644
--- a/test/commands/user/create.test.ts
+++ b/test/commands/user/create.test.ts
@@ -118,7 +118,7 @@ describe('force:user:create', () => {
       'testUser1@test.com',
       '--targetdevhubusername',
       'devhub@test.com',
-      "permissionSets='permCLI, permCLI2'",
+      "permsets='permCLI, permCLI2'",
       'generatepassword=true',
       'profileName=profileFromArgs',
     ])
@@ -132,7 +132,8 @@ describe('force:user:create', () => {
         lastName: 'User',
         localeSidKey: 'en_US',
         orgId: 'abc123',
-        permissionSets: "'permCLI, permCLI2'",
+        generatepassword: 'true',
+        permsets: "'permCLI, permCLI2'",
         profileId: '12345678',
         profileName: 'profileFromArgs',
         timeZoneSidKey: 'America/Los_Angeles',
@@ -243,6 +244,7 @@ describe('force:user:create', () => {
         orgId: 'abc123',
         permsets: ['test1', 'test2'],
         profileId: '00e2D000000bNexWWR',
+        generatepassword: 'false',
         timeZoneSidKey: 'America/Los_Angeles',
         username: '1605130295132_test-j6asqt5qoprs@example.com',
       };
@@ -282,6 +284,7 @@ describe('force:user:create', () => {
           languageLocaleKey: 'en_US',
           lastName: 'User',
           localeSidKey: 'en_US',
+          generatepassword: 'false',
           profileName: "'Chatter Free User'",
           orgId: 'abc123',
           // note the new profileId 12345678 -> Chatter Free User from var args