Repository: Accenture/sfmc-devtools (js)

Issue:
[BUG] dataExtension fields not properly formatted after de-CREATE 
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current Behavior

after UPDATE we re-retrieve fields to ensure the field list is properly sorted. afterwards we also apply the field-postRetrieveTasks.

during CREATE however, we take what was deployed (including fields only required for the API call) and save that directly into the retrieve folder without further processing

### Expected Behavior

fields are always run through field-postRetrieveTasks

### Steps To Reproduce

_No response_

### Version

3.1.3

### Environment

- OS:
- Node:
- npm:


### Participation

- [X] I am willing to submit a pull request for this issue.

### Additional comments

_No response_

Pull request:
Bugfix/295 dataextension fields not properly formatted in retrieve-folder after de-create
# PR details

## What is the purpose of this pull request? (put an "X" next to an item)

- [x] Bugfix


## What changes did you make? (Give an overview)

- fixes #295 

## Is there anything you'd like reviewers to focus on?



## 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] ESLint & Prettier are not outputting errors or warnings
- [x] README.md updated (if applicable)



Patch:
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index f6c96c5a..56f78971 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -5,7 +5,7 @@
 _Please delete options that are not relevant._
 
 - [ ] Documentation update
-- [ ] Bug fix
+- [ ] Bugfix
 - [ ] New metadata support
 - [ ] Enhanced metadata
 - [ ] Add a CLI option
@@ -23,6 +23,7 @@ _Please delete options that are not relevant._
 ## Checklist
 
 - [ ] I have performed a self-review of my own code
+- [ ] ran `npm run test` to with 0 tests failing
 - [ ] I have commented my code, particularly in hard-to-understand areas
 - [ ] ESLint & Prettier are not outputting errors or warnings
 - [ ] README.md updated (if applicable)
diff --git a/lib/metadataTypes/DataExtension.js b/lib/metadataTypes/DataExtension.js
index 1c030cf1..963cdf41 100644
--- a/lib/metadataTypes/DataExtension.js
+++ b/lib/metadataTypes/DataExtension.js
@@ -228,20 +228,20 @@ class DataExtension extends MetadataType {
      * @param {TYPE.DataExtensionMap} upsertedMetadata metadata mapped by their keyField
      * @returns {void}
      */
-    static postDeployTasks(upsertedMetadata) {
-        if (!DataExtension.oldFields) {
-            // only run postDeploy if we are in update mode
-            return;
+    static async postDeployTasks(upsertedMetadata) {
+        if (DataExtension.oldFields) {
+            // at least some dataExtensions were updated
+            Util.logger.info(
+                'Re-retrieving updated dataExtension fields from server to ensure correct info in local JSONs and Docs.'
+            );
         }
-        Util.logger.info(
-            'Re-retrieving updated dataExtension fields from server to ensure correct info in local JSONs and Docs.'
-        );
 
         // somewhat of a workardoun but it ensures we get the field list from the server rather than whatever we might have in cache got returned during update/add. This ensures a complete and correctly ordered field list
         for (const key in upsertedMetadata) {
             const item = upsertedMetadata[key];
             const cachedVersion = cache.getByKey('dataExtension', item.CustomerKey);
             if (cachedVersion) {
+                // UPDATE
                 // restore retention values that are typically not returned by the update call
                 item.RowBasedRetention = cachedVersion.RowBasedRetention;
                 item.ResetRetentionPeriodOnImport = cachedVersion.ResetRetentionPeriodOnImport;
@@ -252,13 +252,19 @@ class DataExtension extends MetadataType {
                 const existingFields = DataExtension.oldFields[item[this.definition.nameField]];
                 if (item.Fields !== '' && existingFields) {
                     // TODO should be replaced by a manual sort using existingFields
-                    // ! this is inefficient because it triggers a new download of the fields during the saveResults() step
+                    // ! this is inefficient because it triggers a new download of the fields
                     item.Fields.length = 0;
+                    await this._retrieveFieldsForSingleDe(upsertedMetadata, key);
                 }
                 // sort Fields entry to the end of the object for saving in .json
                 const fieldsBackup = item.Fields;
                 delete item.Fields;
                 item.Fields = fieldsBackup;
+            } else {
+                // CREATE
+                item.Fields.forEach((field) => {
+                    DataExtensionField.postRetrieveTasks(field, true);
+                });
             }
         }
     }
@@ -396,12 +402,6 @@ class DataExtension extends MetadataType {
      * @returns {TYPE.DataExtensionItem} metadata
      */
     static async postRetrieveTasks(metadata, _, isTemplating) {
-        if (!metadata.Fields || !metadata.Fields.length) {
-            // assume we were in deploy mode. retrieve fields.
-            const tempList = {};
-            tempList[metadata[this.definition.keyField]] = metadata;
-            await this._retrieveFieldsForSingleDe(tempList, metadata[this.definition.keyField]);
-        }
         // if retrieving template, replace the name with customer key if that wasn't already the case
         if (isTemplating) {
             const warningMsg =
diff --git a/lib/util/util.js b/lib/util/util.js
index 50eca756..550862f8 100644
--- a/lib/util/util.js
+++ b/lib/util/util.js
@@ -41,7 +41,7 @@ const Util = {
      * @returns {boolean} attribute value == true ? true : false
      */
     isTrue(attrValue) {
-        return ['true', 'TRUE', 'True', '1', 1, 'Y'].includes(attrValue);
+        return ['true', 'TRUE', 'True', '1', 1, 'Y', true].includes(attrValue);
     },
     /**
      * SFMC accepts multiple false values for Boolean attributes for which we are checking here
@@ -50,7 +50,7 @@ const Util = {
      * @returns {boolean} attribute value == false ? true : false
      */
     isFalse(attrValue) {
-        return ['false', 'FALSE', 'False', '0', 0, 'N'].includes(attrValue);
+        return ['false', 'FALSE', 'False', '0', 0, 'N', false].includes(attrValue);
     },
     /**
      * helper to check if given value is a number


Test patch:
diff --git a/test/dataExtension.test.js b/test/dataExtension.test.js
index a87867f5..31aebe10 100644
--- a/test/dataExtension.test.js
+++ b/test/dataExtension.test.js
@@ -54,7 +54,7 @@ describe('retrieve', () => {
         );
         assert.equal(
             Object.values(testUtils.getAPIHistory()).flat().length,
-            15,
+            16,
             'Unexpected number of requests made'
         );
         return;
diff --git a/test/resources/9999999/dataExtension/create-expected.json b/test/resources/9999999/dataExtension/create-expected.json
index db3ed098..961b7c68 100644
--- a/test/resources/9999999/dataExtension/create-expected.json
+++ b/test/resources/9999999/dataExtension/create-expected.json
@@ -10,8 +10,6 @@
     "RetainUntil": "",
     "Fields": [
         {
-            "PartnerKey": "",
-            "ObjectID": "",
             "Name": "testField",
             "DefaultValue": "",
             "MaxLength": 254,
diff --git a/test/resources/9999999/dataExtension/update-expected.json b/test/resources/9999999/dataExtension/update-expected.json
index c8dff813..4e536209 100644
--- a/test/resources/9999999/dataExtension/update-expected.json
+++ b/test/resources/9999999/dataExtension/update-expected.json
@@ -10,13 +10,35 @@
     "RetainUntil": "",
     "Fields": [
         {
-            "PartnerKey": "",
-            "ObjectID": "",
-            "Name": "testField",
+            "Name": "FirstName",
+            "DefaultValue": "",
+            "MaxLength": 50,
+            "IsRequired": false,
+            "IsPrimaryKey": false,
+            "FieldType": "Text"
+        },
+        {
+            "Name": "LastName",
+            "DefaultValue": "",
+            "MaxLength": 50,
+            "IsRequired": false,
+            "IsPrimaryKey": false,
+            "FieldType": "Text"
+        },
+        {
+            "Name": "EmailAddress",
             "DefaultValue": "",
             "MaxLength": 254,
             "IsRequired": true,
             "IsPrimaryKey": false,
+            "FieldType": "EmailAddress"
+        },
+        {
+            "Name": "ContactKey",
+            "DefaultValue": "",
+            "MaxLength": 50,
+            "IsRequired": true,
+            "IsPrimaryKey": true,
             "FieldType": "Text"
         }
     ],
diff --git a/test/utils.js b/test/utils.js
index 3f51129e..d273ee1a 100644
--- a/test/utils.js
+++ b/test/utils.js
@@ -2,6 +2,7 @@ const File = require('../lib/util/file');
 const path = require('path');
 const axios = require('axios');
 const MockAdapter = require('axios-mock-adapter');
+const Util = require('../lib/util/util');
 
 // for some reason doesnt realize below reference
 // eslint-disable-next-line no-unused-vars
@@ -54,6 +55,7 @@ exports.getExpectedFile = (mid, type, action) =>
  * @returns {void}
  */
 exports.mockSetup = () => {
+    Util.setLoggingLevel({ debug: true });
     apimock = new MockAdapter(axios, { onNoMatch: 'throwException' });
     // set access_token to mid to allow for autorouting of mock to correct resources
     apimock.onPost(authResources.success.url).reply((config) => {