Merge pull request #942 from Accenture/bugfix/800-document-wrongly-ex…

…ecuted-during-retrieve-by-key Bugfix/800 document wrongly executed during retrieve by key
Accenture · May 30, 2023 · bea555f · bea555f
2 parents 6b120a9 + 1f3e0d3
commit bea555f
Show file tree

Hide file tree

Showing 12 changed files with 140 additions and 32 deletions.
diff --git a/docs/dist/documentation.md b/docs/dist/documentation.md
@@ -3167,6 +3167,7 @@ Provides default functionality that can be overwritten by child metadata type cl
     * [.getSOAPErrorMsg(ex)](#MetadataType.getSOAPErrorMsg) ⇒ <code>string</code>
     * [.retrieveSOAP(retrieveDir, [requestParams], [singleRetrieve], [additionalFields])](#MetadataType.retrieveSOAP) ⇒ <code>Promise.&lt;TYPE.MetadataTypeMapObj&gt;</code>
     * [.retrieveREST(retrieveDir, uri, [templateVariables], [singleRetrieve])](#MetadataType.retrieveREST) ⇒ <code>Promise.&lt;{metadata: (TYPE.MetadataTypeMap\|TYPE.MetadataTypeItem), type: string}&gt;</code>
+    * [.runDocumentOnRetrieve([singleRetrieve], metadataMap)](#MetadataType.runDocumentOnRetrieve) ⇒ <code>Promise.&lt;void&gt;</code>
     * [.parseResponseBody(body, [singleRetrieve])](#MetadataType.parseResponseBody) ⇒ <code>TYPE.MetadataTypeMap</code>
     * [.deleteFieldByDefinition(metadataEntry, fieldPath, definitionProperty, origin)](#MetadataType.deleteFieldByDefinition) ⇒ <code>void</code>
     * [.removeNotCreateableFields(metadataEntry)](#MetadataType.removeNotCreateableFields) ⇒ <code>void</code>
@@ -3632,6 +3633,19 @@ Retrieves Metadata for Rest Types
 | [templateVariables] | <code>TYPE.TemplateMap</code> | variables to be replaced in the metadata |
 | [singleRetrieve] | <code>string</code> \| <code>number</code> | key of single item to filter by |
 
+<a name="MetadataType.runDocumentOnRetrieve"></a>
+
+### MetadataType.runDocumentOnRetrieve([singleRetrieve], metadataMap) ⇒ <code>Promise.&lt;void&gt;</code>
+helper for [retrieveREST](retrieveREST) and [retrieveSOAP](retrieveSOAP)
+
+**Kind**: static method of [<code>MetadataType</code>](#MetadataType)  
+**Returns**: <code>Promise.&lt;void&gt;</code> - -  
+
+| Param | Type | Description |
+| --- | --- | --- |
+| [singleRetrieve] | <code>string</code> \| <code>number</code> | key of single item to filter by |
+| metadataMap | <code>TYPE.MetadataTypeMap</code> | saved metadata |
+
 <a name="MetadataType.parseResponseBody"></a>
 
 ### MetadataType.parseResponseBody(body, [singleRetrieve]) ⇒ <code>TYPE.MetadataTypeMap</code>

diff --git a/lib/metadataTypes/Automation.js b/lib/metadataTypes/Automation.js
@@ -78,9 +78,8 @@ class Automation extends MetadataType {
                 `Downloaded: ${this.definition.type} (${Object.keys(metadataMap).length})` +
                     Util.getKeysString(key)
             );
-            if (this.properties.metaDataTypes.documentOnRetrieve.includes(this.definition.type)) {
-                await this.document(metadataMap);
-            }
+
+            await this.runDocumentOnRetrieve(key, metadataMap);
         }
         return { metadata: metadataMap, type: this.definition.type };
     }

diff --git a/lib/metadataTypes/DataExtension.js b/lib/metadataTypes/DataExtension.js
@@ -464,9 +464,7 @@ class DataExtension extends MetadataType {
                 `Downloaded: ${this.definition.type} (${Object.keys(savedMetadata).length})` +
                     Util.getKeysString(key)
             );
-            if (this.properties.metaDataTypes.documentOnRetrieve.includes(this.definition.type)) {
-                await this.document(savedMetadata);
-            }
+            await this.runDocumentOnRetrieve(key, savedMetadata);
         }
         return { metadata: metadata, type: 'dataExtension' };
     }

diff --git a/lib/metadataTypes/MetadataType.js b/lib/metadataTypes/MetadataType.js
@@ -1016,12 +1016,7 @@ class MetadataType {
                 `Downloaded: ${this.definition.type} (${Object.keys(savedMetadata).length})` +
                     Util.getKeysString(singleRetrieve)
             );
-            if (
-                this.buObject &&
-                this.properties.metaDataTypes.documentOnRetrieve.includes(this.definition.type)
-            ) {
-                await this.document(savedMetadata);
-            }
+            await this.runDocumentOnRetrieve(singleRetrieve, savedMetadata);
         }
         return { metadata: metadata, type: this.definition.type };
     }
@@ -1063,6 +1058,7 @@ class MetadataType {
                 `Downloaded: ${this.definition.type} (${Object.keys(savedMetadata).length})` +
                     Util.getKeysString(singleRetrieve)
             );
+            await this.runDocumentOnRetrieve(singleRetrieve, savedMetadata);
         }
 
         return {
@@ -1071,6 +1067,34 @@ class MetadataType {
         };
     }
 
+    /**
+     * helper for {@link retrieveREST} and {@link retrieveSOAP}
+     *
+     * @param {string|number} [singleRetrieve] key of single item to filter by
+     * @param {TYPE.MetadataTypeMap} metadataMap saved metadata
+     * @returns {Promise.<void>} -
+     */
+    static async runDocumentOnRetrieve(singleRetrieve, metadataMap) {
+        if (
+            this.buObject &&
+            this.properties.metaDataTypes.documentOnRetrieve.includes(this.definition.type)
+        ) {
+            if (!singleRetrieve || (singleRetrieve && !this.definition.documentInOneFile)) {
+                const count = Object.keys(metadataMap).length;
+                Util.logger.debug(
+                    ` - Running document for ${count} record${count === 1 ? '' : 's'}`
+                );
+                await this.document(metadataMap);
+            } else {
+                Util.logger.info(
+                    Util.getGrayMsg(
+                        ` - Skipped running document because you supplied keys and ${this.definition.type} is documented in a single file for all.`
+                    )
+                );
+            }
+        }
+    }
+
     /**
      * Builds map of metadata entries mapped to their keyfields
      *

diff --git a/lib/metadataTypes/Role.js b/lib/metadataTypes/Role.js
@@ -102,9 +102,8 @@ class Role extends MetadataType {
                 `Downloaded: ${this.definition.type} (${Object.keys(savedMetadata).length})` +
                     Util.getKeysString(key)
             );
-            if (this.properties.metaDataTypes.documentOnRetrieve.includes(this.definition.type)) {
-                await this.document(savedMetadata);
-            }
+
+            await this.runDocumentOnRetrieve(key, savedMetadata);
         }
         return { metadata: parsed, type: this.definition.type };
     }

diff --git a/lib/metadataTypes/User.js b/lib/metadataTypes/User.js
@@ -731,13 +731,7 @@ class User extends MetadataType {
                     )
                 );
             }
-            if (
-                !singleRetrieve &&
-                this.buObject &&
-                this.properties.metaDataTypes.documentOnRetrieve.includes(this.definition.type)
-            ) {
-                await this.document(savedMetadata);
-            }
+            await this.runDocumentOnRetrieve(singleRetrieve, savedMetadata);
         }
         return { metadata: metadata, type: this.definition.type };
     }
@@ -944,14 +938,6 @@ class User extends MetadataType {
             return;
         }
 
-        // if ran as part of retrieve/deploy with key, exit here
-        if (metadata && Object.keys(metadata).length === 1) {
-            Util.logger.debug(
-                'Only 1 user found. Skipping documentation, assuming we ran retrieve-by-key.'
-            );
-            return;
-        }
-
         if (!metadata) {
             // load users from disk if document was called directly and not part of a retrieve
             try {

diff --git a/lib/metadataTypes/definitions/User.definition.js b/lib/metadataTypes/definitions/User.definition.js
@@ -17,6 +17,7 @@ module.exports = {
     typeDescription: 'Marketing Cloud users',
     typeName: 'User',
     typeRetrieveByDefault: false,
+    documentInOneFile: true,
     stringifyFieldsBeforeTemplate: ['DefaultBusinessUnit', 'c__AssociatedBusinessUnits'],
     fields: {
         AccountUserID: {

diff --git a/test/resources/1111111/user/retrieve-expected.md b/test/resources/1111111/user/retrieve-expected.md
@@ -0,0 +1,19 @@
+# User Overview - testInstance
+
+## Users (1)
+
+| Name | Last successful Login | Active | Access Locked out | API User | Must change PW | Default BU | BU Access | Roles | Login | ID | Key | E-Mail | Notification E-Mail | Timezone | SFMC Locale | Modified Date | Modified By | Created Date |
+| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
+| user test | 96 days | ✓ | - | - | - | <nobr>_ParentBU_ (1111111)</nobr> | <nobr>_ParentBU_ (1111111)</nobr>,<br> <nobr>testBU (9999999)</nobr> | <nobr>Administrator,</nobr><br> <nobr>Content Creator,</nobr><br> <nobr>Marketing Cloud Administrator</nobr> | user_test@accenture.asgr | 700301950 | testExisting_user | user_test@accenture.com | user_test@accenture.com | GMT+01:00 | en-GB | 2022-06-21 01:43:02.64 | 123456 | 2019-09-06 01:59:07.097 |
+
+
+## Inactivated Users (0)
+
+| Name | Last successful Login | Active | Access Locked out | API User | Must change PW | Default BU | BU Access | Roles | Login | ID | Key | E-Mail | Notification E-Mail | Timezone | SFMC Locale | Modified Date | Modified By | Created Date |
+| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
+
+
+## Installed Packages (0)
+
+| Name | Last successful Login | Active | Access Locked out | API User | Must change PW | Default BU | BU Access | Roles | Login | ID | Key | E-Mail | Notification E-Mail | Timezone | SFMC Locale | Modified Date | Modified By | Created Date |
+| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
diff --git a/test/resources/9999999/dataExtension/retrieve-expected.md b/test/resources/9999999/dataExtension/retrieve-expected.md
@@ -0,0 +1,18 @@
+## testExisting_dataExtension
+
+**Description:** bla bla
+
+**Folder:** Data Extensions/
+
+**Fields in table:** 4
+
+**Sendable:** Yes (`ContactKey` to `Subscriber Key`)
+
+**Testable:** Yes
+
+| Name | FieldType | MaxLength | IsPrimaryKey | IsNullable | DefaultValue |
+| --- | --- | --- | --- | --- | --- |
+| FirstName | Text | 50 | - | + |  |
+| LastName | Text | 50 | - | + |  |
+| EmailAddress | EmailAddress | 254 | - | - |  |
+| ContactKey | Text | 50 | + | - |  |
diff --git a/test/type.dataExtension.test.js b/test/type.dataExtension.test.js
@@ -1,4 +1,8 @@
-const assert = require('chai').assert;
+const chai = require('chai');
+const chaiFiles = require('chai-files');
+const assert = chai.assert;
+const expect = chai.expect;
+const file = chaiFiles.file;
 const cache = require('../lib/util/cache');
 const testUtils = require('./utils');
 const handler = require('../lib/index');
@@ -29,6 +33,13 @@ describe('type: dataExtension', () => {
 
                 'returned metadata was not equal expected'
             );
+            // check if MD file was created and equals expectations
+            expect(
+                file(testUtils.getActualDoc('testExisting_dataExtension', 'dataExtension'))
+            ).to.equal(
+                file(testUtils.getExpectedFile('9999999', 'dataExtension', 'retrieve', 'md'))
+            );
+
             assert.equal(
                 testUtils.getAPIHistoryLength(),
                 5,

diff --git a/test/type.user.test.js b/test/type.user.test.js
@@ -1,4 +1,9 @@
-const assert = require('chai').assert;
+const chai = require('chai');
+const chaiFiles = require('chai-files');
+const assert = chai.assert;
+chai.use(chaiFiles);
+const expect = chai.expect;
+const file = chaiFiles.file;
 const cache = require('../lib/util/cache');
 const testUtils = require('./utils');
 const handler = require('../lib/index');
@@ -29,13 +34,34 @@ describe('type: user', () => {
 
                 'returned metadata was not equal expected'
             );
+            // check if MD file was created and equals expectations
+            expect(file(`./docs/user/testInstance.users.md`)).to.equal(
+                file(testUtils.getExpectedFile('1111111', 'user', 'retrieve', 'md'))
+            );
+
             assert.equal(
                 testUtils.getAPIHistoryLength(),
                 6,
                 'Unexpected number of requests made. Run testUtils.logAPIHistoryDebug() to see the requests'
             );
             return;
         });
+        it('Should retrieve a specific user but not run document', async () => {
+            // WHEN
+            await handler.retrieve('testInstance/_ParentBU_', ['user'], ['testExisting_user']);
+            // THEN
+            assert.equal(process.exitCode, false, 'retrieve should not have thrown an error');
+
+            // because user is single-document-type we would not want to find an md file when we retrieve specific keys. only the generic retrieve updates it
+            expect(file(`./docs/user/testInstance.users.md`)).to.not.exist;
+
+            assert.equal(
+                testUtils.getAPIHistoryLength(),
+                4,
+                'Unexpected number of requests made. Run testUtils.logAPIHistoryDebug() to see the requests'
+            );
+            return;
+        });
     });
     describe('Deploy ================', () => {
         beforeEach(() => {
@@ -74,6 +100,9 @@ describe('type: user', () => {
                 await testUtils.getExpectedJson('1111111', 'user', 'update'),
                 'returned metadata was not equal expected for update'
             );
+            // because user is single-document-type we would not want to find an md file getting created by deploy. only retrieve updates it
+            expect(file(`./docs/user/testInstance.users.md`)).to.not.exist;
+
             assert.equal(
                 testUtils.getAPIHistoryLength(),
                 9,

diff --git a/test/utils.js b/test/utils.js
@@ -22,6 +22,16 @@ const resourceFactory = require('./resourceFactory');
  */
 exports.getActualJson = (customerKey, type, buName = 'testBU') =>
     File.readJSON(`./retrieve/testInstance/${buName}/${type}/${customerKey}.${type}-meta.json`);
+/**
+ * gets file from Retrieve folder
+ *
+ * @param {string} customerKey of metadata
+ * @param {string} type of metadata
+ * @param {string} [buName] used when we need to test on ParentBU
+ * @returns {Promise.<string>} file in string form
+ */
+exports.getActualDoc = (customerKey, type, buName = 'testBU') =>
+    `./retrieve/testInstance/${buName}/${type}/${customerKey}.${type}-doc.md`;
 /**
  * gets file from Retrieve folder
  *