Repository: Accenture/sfmc-devtools (js)

Issue:
add fixKeys command / CSCLSROZ-29
add fixKeys command that re-uses the --changeKeyField logic together with this.definition.nameField

this is a basic wrapper around --changeKeyField that searches for metadata of the given type with name/key mismatches.

**Acceptance Criteria:**
- types that cannot be used with this method are identified by:
  - use id as key (this.definition.keyField === this.definition.idField)
  - use the same field for name and key (this.definition.keyField === this.definition.nameField)
- this command should search offline --> do a full retrieve of the BU first before executing the actual find logic.
- after the key-change we need to then iterate over the definition/*.definition.js files to check for types that are dependent on the type you just fixed the keys for

**out of scope:** 
- rewriting assets / scripts based on new key values (handled in #1001)

**basic steps:**
1. retrieve selected type
2. find items with key != name (`metadata[(this.definition.keyField] !== metadata[(this.definition.nameField]`)
3. run `deploy cred/bu type --fromRetrieve --changeKeyField=${this.definition.nameField}` on found keys for selected type
4. iterate over elements in `lib\MetadataTypeDefinitions.js` to check array in `x.definition.dependencies` for matches with the selected type
    - do notice that there are sometimes subtypes defined here as well, eg. "asset-message" --> if you applied fixKeys to "asset" then this would be a match
   - run full retrieve on types that have our selected type as a dependency to ensure that we have keys properly updated in our local folder

... the user then has to git add / commit & push on their own. 

Pull request:
Feature/38 fix keys should ask to retrieve other types
# PR details

## What changes did you make? (Give an overview)

- closes #38

## Further details (optional)

...

## Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] test scripts updated
- [x] Wiki updated **(via #1007)**


Patch:
diff --git a/docs/dist/documentation.md b/docs/dist/documentation.md
index 8a803091..00debb98 100644
--- a/docs/dist/documentation.md
+++ b/docs/dist/documentation.md
@@ -6150,7 +6150,8 @@ used to ensure the program tells surrounding software that an unrecoverable erro
 <a name="Util.isTrue"></a>
 
 ### Util.isTrue(attrValue) ⇒ <code>boolean</code>
-SFMC accepts multiple true values for Boolean attributes for which we are checking here
+SFMC accepts multiple true values for Boolean attributes for which we are checking here.
+The same problem occurs when evaluating boolean CLI flags
 
 **Kind**: static method of [<code>Util</code>](#Util)  
 **Returns**: <code>boolean</code> - attribute value == true ? true : false  
@@ -6162,7 +6163,8 @@ SFMC accepts multiple true values for Boolean attributes for which we are checki
 <a name="Util.isFalse"></a>
 
 ### Util.isFalse(attrValue) ⇒ <code>boolean</code>
-SFMC accepts multiple false values for Boolean attributes for which we are checking here
+SFMC accepts multiple false values for Boolean attributes for which we are checking here.
+The same problem occurs when evaluating boolean CLI flags
 
 **Kind**: static method of [<code>Util</code>](#Util)  
 **Returns**: <code>boolean</code> - attribute value == false ? true : false  
@@ -6535,6 +6537,7 @@ CLI helper class
 * [Cli](#Cli)
     * [.initMcdevConfig()](#Cli.initMcdevConfig) ⇒ <code>Promise.&lt;boolean&gt;</code>
     * [.addExtraCredential(properties)](#Cli.addExtraCredential) ⇒ <code>Promise.&lt;(boolean\|string)&gt;</code>
+    * [.postFixKeysReretrieve(type, dependentTypes)](#Cli.postFixKeysReretrieve) ⇒ <code>Promise.&lt;boolean&gt;</code>
     * [.logExistingCredentials(properties)](#Cli.logExistingCredentials) ⇒ <code>void</code>
     * [.updateCredential(properties, credName)](#Cli.updateCredential) ⇒ <code>Promise.&lt;boolean&gt;</code>
     * [.getCredentialObject(properties, target, [isCredentialOnly], [allowAll])](#Cli.getCredentialObject) ⇒ <code>Promise.&lt;TYPE.BuObject&gt;</code>
@@ -6565,6 +6568,17 @@ Extends template file for properties.json
 | --- | --- | --- |
 | properties | <code>TYPE.Mcdevrc</code> | config file's json |
 
+<a name="Cli.postFixKeysReretrieve"></a>
+
+### Cli.postFixKeysReretrieve(type, dependentTypes) ⇒ <code>Promise.&lt;boolean&gt;</code>
+**Kind**: static method of [<code>Cli</code>](#Cli)  
+**Returns**: <code>Promise.&lt;boolean&gt;</code> - true if user wants to continue with retrieve  
+
+| Param | Type | Description |
+| --- | --- | --- |
+| type | <code>TYPE.SupportedMetadataTypes</code> | limit execution to given metadata type |
+| dependentTypes | <code>Array.&lt;TYPE.SupportedMetadataTypes&gt;</code> | types that depent on type |
+
 <a name="Cli.logExistingCredentials"></a>
 
 ### Cli.logExistingCredentials(properties) ⇒ <code>void</code>
@@ -8042,7 +8056,8 @@ used to ensure the program tells surrounding software that an unrecoverable erro
 <a name="Util.isTrue"></a>
 
 ### Util.isTrue(attrValue) ⇒ <code>boolean</code>
-SFMC accepts multiple true values for Boolean attributes for which we are checking here
+SFMC accepts multiple true values for Boolean attributes for which we are checking here.
+The same problem occurs when evaluating boolean CLI flags
 
 **Kind**: static method of [<code>Util</code>](#Util)  
 **Returns**: <code>boolean</code> - attribute value == true ? true : false  
@@ -8054,7 +8069,8 @@ SFMC accepts multiple true values for Boolean attributes for which we are checki
 <a name="Util.isFalse"></a>
 
 ### Util.isFalse(attrValue) ⇒ <code>boolean</code>
-SFMC accepts multiple false values for Boolean attributes for which we are checking here
+SFMC accepts multiple false values for Boolean attributes for which we are checking here.
+The same problem occurs when evaluating boolean CLI flags
 
 **Kind**: static method of [<code>Util</code>](#Util)  
 **Returns**: <code>boolean</code> - attribute value == false ? true : false  
diff --git a/lib/index.js b/lib/index.js
index 80d9b80f..cd927a09 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -865,6 +865,7 @@ class Mcdev {
         }
 
         if (businessUnit === '*') {
+            Util.OPTIONS._multiBuExecution = true;
             Util.logger.info(
                 `:: ${lang_present} the ${selectedType} on all BUs for all credentials`
             );
@@ -916,6 +917,7 @@ class Mcdev {
                 }
             }
             if (bu === '*' && properties.credentials && properties.credentials[cred]) {
+                Util.OPTIONS._multiBuExecution = true;
                 Util.logger.info(`:: ${lang_present} ${selectedType} on all BUs for ${cred}`);
                 for (const bu in properties.credentials[cred].businessUnits) {
                     resultObj[cred + '/' + bu] = await this.#runOnBU(
@@ -1123,11 +1125,25 @@ class Mcdev {
                         actuallyFixedKeys.length === 1 ? '' : 's'
                     } of type ${type}`
                 );
-                Util.logger.warn(
-                    ` Please manually re-retrieve the following types as your local copies might now be outdated: ${Util.getGrayMsg(
-                        dependentTypes.join(', ')
-                    )}`
-                );
+                if (dependentTypes.length) {
+                    Util.logger.warn(
+                        `Please re-retrieve the following types as your local copies might now be outdated: ${Util.getGrayMsg(
+                            dependentTypes.join(', ')
+                        )}`
+                    );
+                    const reRetrieve = await Cli.postFixKeysReretrieve(type, dependentTypes);
+                    if (reRetrieve) {
+                        Util.logger.info(
+                            `Retrieving latest versions of ${dependentTypes.join(', ')} from server`
+                        );
+                        const retriever = new Retriever(properties, buObject);
+                        await retriever.retrieve(dependentTypes, null, null, false);
+                    }
+                } else {
+                    Util.logger.info(
+                        `No dependent types found that need to be re-retrieved after fixing keys of type ${type}.`
+                    );
+                }
             } else {
                 Util.logger.warn(`No keys of type ${type} updated.`);
             }
diff --git a/lib/metadataTypes/MetadataType.js b/lib/metadataTypes/MetadataType.js
index 646a3035..36f5f52b 100644
--- a/lib/metadataTypes/MetadataType.js
+++ b/lib/metadataTypes/MetadataType.js
@@ -2054,41 +2054,46 @@ class MetadataType {
      */
     static getKeysForFixing(metadataMap) {
         const keysForDeploy = [];
-
-        for (const item of Object.values(metadataMap)) {
-            if (item[this.definition.nameField].length > this.definition.maxKeyLength) {
-                Util.logger.warn(
-                    `Name of the item ${
-                        item[this.definition.keyField]
-                    } is too long for a key. Consider renaming your item. Key will be equal first ${
+        if (Object.keys(metadataMap).length) {
+            Util.logger.info(
+                `Searching for ${this.definition.type} keys among downloaded items that need fixing:`
+            );
+            for (const item of Object.values(metadataMap)) {
+                if (item[this.definition.nameField].length > this.definition.maxKeyLength) {
+                    Util.logger.warn(
+                        `Name of the item ${
+                            item[this.definition.keyField]
+                        } is too long for a key. Consider renaming your item. Key will be equal first ${
+                            this.definition.maxKeyLength
+                        } characters of the name`
+                    );
+                    item[this.definition.nameField] = item[this.definition.nameField].slice(
+                        0,
                         this.definition.maxKeyLength
-                    } characters of the name`
-                );
-                item[this.definition.nameField] = item[this.definition.nameField].slice(
-                    0,
-                    this.definition.maxKeyLength
-                );
-            }
+                    );
+                }
 
-            if (
-                item[this.definition.nameField] != item[this.definition.keyField] &&
-                !this.definition.keyIsFixed
-            ) {
-                keysForDeploy.push(item[this.definition.keyField]);
-                Util.logger.info(
-                    ` - added ${this.definition.type} to fixKey queue: ${
-                        item[this.definition.keyField]
-                    }`
-                );
-            } else {
-                Util.logger.info(
-                    Util.getGrayMsg(
-                        ` ☇ skipping ${this.definition.type} ${
+                if (
+                    item[this.definition.nameField] != item[this.definition.keyField] &&
+                    !this.definition.keyIsFixed
+                ) {
+                    keysForDeploy.push(item[this.definition.keyField]);
+                    Util.logger.info(
+                        ` - added ${this.definition.type} to fixKey queue: ${
                             item[this.definition.keyField]
-                        }: key does not need to be updated`
-                    )
-                );
+                        }`
+                    );
+                } else {
+                    Util.logger.info(
+                        Util.getGrayMsg(
+                            ` ☇ skipping ${this.definition.type} ${
+                                item[this.definition.keyField]
+                            }: key does not need to be updated`
+                        )
+                    );
+                }
             }
+            Util.logger.info(`Found ${keysForDeploy.length} ${this.definition.type} keys to fix`);
         }
         return keysForDeploy;
     }
diff --git a/lib/util/cli.js b/lib/util/cli.js
index a1892779..d16c4fc6 100644
--- a/lib/util/cli.js
+++ b/lib/util/cli.js
@@ -53,6 +53,46 @@ const Cli = {
             return null;
         }
     },
+
+    /**
+     *
+     * @param {TYPE.SupportedMetadataTypes} type limit execution to given metadata type
+     * @param {TYPE.SupportedMetadataTypes[]} dependentTypes types that depent on type
+     * @returns {Promise.<boolean>} true if user wants to continue with retrieve
+     */
+    async postFixKeysReretrieve(type, dependentTypes) {
+        if (Util.isTrue(Util.skipInteraction?.fixKeysReretrieve)) {
+            return true;
+        } else if (Util.isFalse(Util.skipInteraction?.fixKeysReretrieve)) {
+            return false;
+        } else {
+            const now = await inquirer.prompt([
+                {
+                    type: 'confirm',
+                    name: 'fixKeysReretrieve',
+                    message: `Do you want to re-retrieve dependent types (${dependentTypes.join(
+                        ', '
+                    )}) now?`,
+                    default: true,
+                },
+            ]);
+            if (Util.OPTIONS._multiBuExecution) {
+                const remember = await inquirer.prompt([
+                    {
+                        type: 'confirm',
+                        name: 'rememberFixKeysReretrieve',
+                        message: `Remember answer for other BUs?`,
+                        default: true,
+                    },
+                ]);
+                if (remember.rememberFixKeysReretrieve) {
+                    Util.skipInteraction ||= {};
+                    Util.skipInteraction.fixKeysReretrieve = now.fixKeysReretrieve;
+                }
+            }
+            return now.fixKeysReretrieve;
+        }
+    },
     /**
      * helper that logs to cli which credentials are already existing in our config file
      *
diff --git a/lib/util/util.js b/lib/util/util.js
index 1e7f7550..5dc6cc1f 100644
--- a/lib/util/util.js
+++ b/lib/util/util.js
@@ -135,22 +135,24 @@ const Util = {
         process.exitCode = 1;
     },
     /**
-     * SFMC accepts multiple true values for Boolean attributes for which we are checking here
+     * SFMC accepts multiple true values for Boolean attributes for which we are checking here.
+     * The same problem occurs when evaluating boolean CLI flags
      *
      * @param {*} attrValue value
      * @returns {boolean} attribute value == true ? true : false
      */
     isTrue(attrValue) {
-        return ['true', 'TRUE', 'True', '1', 1, 'Y', true].includes(attrValue);
+        return ['true', 'TRUE', 'True', '1', 1, 'Y', 'y', true].includes(attrValue);
     },
     /**
-     * SFMC accepts multiple false values for Boolean attributes for which we are checking here
+     * SFMC accepts multiple false values for Boolean attributes for which we are checking here.
+     * The same problem occurs when evaluating boolean CLI flags
      *
      * @param {*} attrValue value
      * @returns {boolean} attribute value == false ? true : false
      */
     isFalse(attrValue) {
-        return ['false', 'FALSE', 'False', '0', 0, 'N', false].includes(attrValue);
+        return ['false', 'FALSE', 'False', '0', 0, 'N', 'n', false].includes(attrValue);
     },
 
     /**


Test patch:
diff --git a/test/type.automation.test.js b/test/type.automation.test.js
index 4057c62b..c36ba3b6 100644
--- a/test/type.automation.test.js
+++ b/test/type.automation.test.js
@@ -240,7 +240,6 @@ describe('type: automation', () => {
             );
             return;
         });
-        it('Should change the key during update via --changeKeyValue');
     });
     describe('Templating ================', () => {
         it('Should create a automation template via retrieveAsTemplate and build it', async () => {
diff --git a/test/type.dataExtension.test.js b/test/type.dataExtension.test.js
index d1a06837..e9a8a6d9 100644
--- a/test/type.dataExtension.test.js
+++ b/test/type.dataExtension.test.js
@@ -87,7 +87,6 @@ describe('type: dataExtension', () => {
             );
             return;
         });
-        it('Should change the key during update via --changeKeyValue');
         it('Should rename fields');
     });
     describe('Templating ================', () => {
diff --git a/test/type.fileTransfer.test.js b/test/type.fileTransfer.test.js
index 63102b96..2d33c05b 100644
--- a/test/type.fileTransfer.test.js
+++ b/test/type.fileTransfer.test.js
@@ -76,7 +76,6 @@ describe('type: fileTransfer', () => {
             );
             return;
         });
-        it('Should change the key during update via --changeKeyValue ');
     });
     describe('Templating ================', () => {
         it('Should create a fileTransfer template via retrieveAsTemplate and build it', async () => {
diff --git a/test/type.importFile.test.js b/test/type.importFile.test.js
index 40a62c1b..76e9c364 100644
--- a/test/type.importFile.test.js
+++ b/test/type.importFile.test.js
@@ -77,7 +77,6 @@ describe('type: importFile', () => {
             );
             return;
         });
-        it('Should change the key during update via --changeKeyValue ');
     });
     describe('Templating ================', () => {
         it('Should create a importFile template via retrieveAsTemplate and build it', async () => {
diff --git a/test/type.mobileKeyword.test.js b/test/type.mobileKeyword.test.js
index 9f37bffc..9f15adaf 100644
--- a/test/type.mobileKeyword.test.js
+++ b/test/type.mobileKeyword.test.js
@@ -120,7 +120,6 @@ describe('type: mobileKeyword', () => {
             );
             return;
         });
-        it('Should change the key during update via --changeKeyValue');
     });
     describe('Templating ================', () => {
         it('Should create a mobileKeyword template via retrieveAsTemplate and build it', async () => {
diff --git a/test/type.query.test.js b/test/type.query.test.js
index 5e8bcb4d..45ca9088 100644
--- a/test/type.query.test.js
+++ b/test/type.query.test.js
@@ -323,6 +323,7 @@ describe('type: query', () => {
         });
         it('Should NOT fixKeys and deploy', async () => {
             // WHEN
+            handler.setOptions({ skipInteraction: { fixKeysReretrieve: false } });
             const resultFixKeys = await handler.fixKeys('testInstance/testBU', 'query', [
                 'testExisting_query',
             ]);
@@ -354,8 +355,9 @@ describe('type: query', () => {
             );
             return;
         });
-        it('Should fixKeys and deploy by key', async () => {
+        it('Should fixKeys and deploy by key WITHOUT re-retrieving dependent types', async () => {
             // WHEN
+            handler.setOptions({ skipInteraction: { fixKeysReretrieve: false } });
             const resultFixKeys = await handler.fixKeys('testInstance/testBU', 'query', [
                 'testExisting_query_fixKeys',
                 'testExisting_query',
@@ -389,9 +391,48 @@ describe('type: query', () => {
             );
             return;
         });
-        it('Should fixKeys and deploy via --like', async () => {
+        it('Should fixKeys and deploy by key AND re-retrieve dependent types', async () => {
             // WHEN
-            handler.setOptions({ like: { key: 'testExisting_query_f%' } });
+            handler.setOptions({ skipInteraction: { fixKeysReretrieve: true } });
+            const resultFixKeys = await handler.fixKeys('testInstance/testBU', 'query', [
+                'testExisting_query_fixKeys',
+                'testExisting_query',
+            ]);
+            assert.equal(
+                resultFixKeys['testInstance/testBU'].length,
+                1,
+                'returned number of keys does not correspond to number of expected fixed keys'
+            );
+            assert.equal(
+                resultFixKeys['testInstance/testBU'][0],
+                'testExisting_query_fixedKeys',
+                'returned keys do not correspond to expected fixed keys'
+            );
+            // THEN
+            assert.equal(process.exitCode, false, 'fixKeys should not have thrown an error');
+            // confirm updated item
+            assert.deepEqual(
+                await testUtils.getActualJson('testExisting_query_fixedKeys', 'query'),
+                await testUtils.getExpectedJson('9999999', 'query', 'patch_fixKeys'),
+                'returned metadata was not equal expected for update query'
+            );
+            expect(
+                file(testUtils.getActualFile('testExisting_query_fixedKeys', 'query', 'sql'))
+            ).to.equal(file(testUtils.getExpectedFile('9999999', 'query', 'patch_fixKeys', 'sql')));
+            // check number of API calls
+            assert.equal(
+                testUtils.getAPIHistoryLength(),
+                31,
+                'Unexpected number of requests made. Run testUtils.logAPIHistoryDebug() to see the requests'
+            );
+            return;
+        });
+        it('Should fixKeys and deploy via --like WITHOUT re-retrieving dependent types', async () => {
+            // WHEN
+            handler.setOptions({
+                like: { key: 'testExisting_query_f%' },
+                skipInteraction: { fixKeysReretrieve: false },
+            });
             const resultFixKeys = await handler.fixKeys('testInstance/testBU', 'query');
             assert.equal(
                 resultFixKeys['testInstance/testBU'].length,
diff --git a/test/type.user.test.js b/test/type.user.test.js
index 18fc252b..34dda9f2 100644
--- a/test/type.user.test.js
+++ b/test/type.user.test.js
@@ -161,7 +161,6 @@ describe('type: user', () => {
             );
             return;
         });
-        it('Should change the key during update with --changeKeyValue');
     });
     describe('Templating ================', () => {
         // it('Should create a user template via retrieveAsTemplate and build it', async () => {});