From 610094d8af855f89c0a2135c180cb6e840e060d9 Mon Sep 17 00:00:00 2001
From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com>
Date: Mon, 16 Sep 2024 19:31:00 -0400
Subject: [PATCH] Decyjphr/pr 682 (#685)

* update comments from #249

I found the logic behind some of what happened here hard to follow and updated the comments to try to make it easier to follow.

* fix environments updating global array

The environments plugin was changing `MergeDeep.NAME_FIELDS`, which is a global object. The reason was to avoid environments being filtered out from the change list if they only have a name field.
However, the environments plugin has it's own overriden sync method, and thus we can simply drop the whole filtering from that method.

Fixes #108

* remove repeating line

This has to be a "typo" from 9a74e05 calling the same assignment twice. Removing to clean up.

---------

Co-authored-by: Torgeir S. hos Sykehuspartner <93591857+torgst@users.noreply.github.com>
---
 lib/mergeDeep.js            | 1 -
 lib/plugins/diffable.js     | 3 ++-
 lib/plugins/environments.js | 8 ++------
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js
index a685a048..054a5b34 100644
--- a/lib/mergeDeep.js
+++ b/lib/mergeDeep.js
@@ -378,5 +378,4 @@ class MergeDeep {
   }
 }
 MergeDeep.NAME_FIELDS = NAME_FIELDS
-MergeDeep.NAME_FIELDS = NAME_FIELDS
 module.exports = MergeDeep
diff --git a/lib/plugins/diffable.js b/lib/plugins/diffable.js
index 74871691..8db6a0ef 100644
--- a/lib/plugins/diffable.js
+++ b/lib/plugins/diffable.js
@@ -93,7 +93,7 @@ module.exports = class Diffable extends ErrorStash {
           }
         }
 
-        // Filter out all empty entries (usually from repo override)
+        // Remove any null or undefined values from the diffables (usually comes from repo override)
         for (const entry of filteredEntries) {
           for (const key of Object.keys(entry)) {
             if (entry[key] === null || entry[key] === undefined) {
@@ -101,6 +101,7 @@ module.exports = class Diffable extends ErrorStash {
             }
           }
         }
+        // Delete any diffable that now only has name and no other attributes
         filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !MergeDeep.NAME_FIELDS.includes(key)).length !== 0)
 
         const changes = []
diff --git a/lib/plugins/environments.js b/lib/plugins/environments.js
index 6d52b409..4ceaa9ff 100644
--- a/lib/plugins/environments.js
+++ b/lib/plugins/environments.js
@@ -17,10 +17,6 @@ module.exports = class Environments extends Diffable {
               }
             })
         }
-
-        // Remove 'name' from filtering list so Environments with only a name defined are processed.
-        MergeDeep.NAME_FIELDS.splice(MergeDeep.NAME_FIELDS.indexOf('name'), 1)
-
     }
 
     async find() {
@@ -296,7 +292,7 @@ module.exports = class Environments extends Diffable {
         let filteredEntries = this.filterEntries()
         return this.find().then(existingRecords => {
 
-          // Filter out all empty entries (usually from repo override)
+          // Remove any null or undefined values from the diffables (usually comes from repo override)
           for (const entry of filteredEntries) {
             for (const key of Object.keys(entry)) {
               if (entry[key] === null || entry[key] === undefined) {
@@ -304,7 +300,7 @@ module.exports = class Environments extends Diffable {
               }
             }
           }
-          filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !MergeDeep.NAME_FIELDS.includes(key)).length !== 0)
+          // For environments, we want to keep the entries with only name defined.
 
           const changes = []