From 3ebb70dcadaaa01629387963f7c790a430498ffb Mon Sep 17 00:00:00 2001
From: jamesL92 <jamesL92@users.noreply.github.com>
Date: Thu, 28 Dec 2023 07:30:30 +0000
Subject: [PATCH] Python: Improve handling of previous overrides in eval
 command (#313)

---
 py/configu/commands/eval_command.py    | 29 ++++++++++++++++++++++----
 py/tests/commands/test_eval_command.py | 10 ++++-----
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/py/configu/commands/eval_command.py b/py/configu/commands/eval_command.py
index 9a4f4c21..238b0eb7 100644
--- a/py/configu/commands/eval_command.py
+++ b/py/configu/commands/eval_command.py
@@ -233,16 +233,16 @@ def reduce_prev(
             current: Tuple[str, EvalCommandReturnValue],
         ):
             key, value = current
-            if key not in merged or (
-                merged[key]["result"]["origin"] == EvaluatedConfigOrigin.EmptyValue
-                and value["result"]["origin"] != EvaluatedConfigOrigin.EmptyValue
+            if key not in merged or self._should_override_origin(
+                value["result"]["origin"],
+                merged[key]["result"]["origin"],
             ):
                 merged[key] = value
             return merged
 
         return reduce(
             reduce_prev,
-            reversed(list(previous_result.items()) + list(result.items())),
+            iter(list(previous_result.items()) + list(result.items())),
             {},
         )
 
@@ -292,6 +292,27 @@ def _eval_templates(result: EvalCommandReturn) -> EvalCommandReturn:
             should_render_templates = has_rendered_at_least_once
         return result
 
+    @staticmethod
+    def _should_override_origin(
+        next_origin: EvaluatedConfigOrigin, previous_origin: EvaluatedConfigOrigin
+    ) -> bool:
+        if previous_origin is None:
+            return True
+        if previous_origin == EvaluatedConfigOrigin.EmptyValue:
+            return next_origin != EvaluatedConfigOrigin.EmptyValue
+        if previous_origin == EvaluatedConfigOrigin.SchemaDefault:
+            return next_origin in (
+                EvaluatedConfigOrigin.SchemaDefault,
+                EvaluatedConfigOrigin.StoreSet,
+                EvaluatedConfigOrigin.ConfigsOverride,
+                EvaluatedConfigOrigin.SchemaTemplate,
+            )
+        return next_origin in (
+            EvaluatedConfigOrigin.StoreSet,
+            EvaluatedConfigOrigin.ConfigsOverride,
+            EvaluatedConfigOrigin.SchemaTemplate,
+        )
+
     def run(self):
         """
         Runs the eval command.
diff --git a/py/tests/commands/test_eval_command.py b/py/tests/commands/test_eval_command.py
index 5526a1b9..e404d61a 100644
--- a/py/tests/commands/test_eval_command.py
+++ b/py/tests/commands/test_eval_command.py
@@ -55,7 +55,7 @@ def test_configs_override_store():
     assert result["GREETING"]["result"]["value"] == "bonjour"
     assert result["GREETING"]["result"]["origin"] == EvaluatedConfigOrigin.ConfigsOverride
 
-def test_previous_overrides_configs():
+def test_previous_does_not_override_configs():
   store = InMemoryConfigStore()
   config_set = ConfigSet("")
   config_schema = ConfigSchema("tests/staticfiles/config_schemas/valid/complex.cfgu.json")
@@ -65,15 +65,15 @@ def test_previous_overrides_configs():
       schema=config_schema,
       configs={"GREETING": "bonjour", "SUBJECT": "foo"},
   ).run()
-
   result = EvalCommand(
     store=store,
     set=config_set,
     schema=config_schema,
     previous=previous
-  ).run()
-  assert result["GREETING"]["result"]["value"] == "hello"
-  assert result["GREETING"]["result"]["origin"] == EvaluatedConfigOrigin.SchemaDefault
+  )
+  result = result.run()
+  assert result["GREETING"]["result"]["value"] == "bonjour"
+  assert result["GREETING"]["result"]["origin"] == EvaluatedConfigOrigin.ConfigsOverride
 
 def test_latest_configs_override_previous():
   store = InMemoryConfigStore()