Repository: SFDO-Tooling/CumulusCI
Pull request #3318 (2022-08-10 22:02:38)

Publish MetaDeploy plans in a deterministic order
Plans are published in a deterministic order: sorted by tier, and then by the order they appear in the `cumulusci.yml` file.
Clarified error message that appears when a user tries to include more than one primary or more than one secondary Plan in their `cumulusci.yml` file.


Patch:
diff --git a/cumulusci/tasks/metadeploy.py b/cumulusci/tasks/metadeploy.py
index b86da29e2..dcd7637df 100644
--- a/cumulusci/tasks/metadeploy.py
+++ b/cumulusci/tasks/metadeploy.py
@@ -131,7 +131,8 @@ class Publish(BaseMetaDeployTask):
             project_config.set_keychain(self.project_config.keychain)
 
             # Create each plan
-            for plan_name, plan_config in self.plan_configs.items():
+            for i, (plan_name, plan_config) in enumerate(self.plan_configs.items()):
+                plan_config["order_key"] = i
                 steps = self._freeze_steps(project_config, plan_config)
                 plan: MetaDeployPlan = MetaDeployPlan.parse_obj(
                     dict(plan_config, steps=steps)
@@ -175,7 +176,9 @@ class Publish(BaseMetaDeployTask):
             product, plan_name, plan_config
         )
 
-        parsed_plan = Plan.parse_obj(self.project_config.config["plans"][plan_name])
+        order_key = plan_config.pop("order_key")
+        parsed_plan = Plan.parse_obj(plan_config)
+        # TODO: clean this up to not use both Plan and MetaDeployPlan
         plan_json = MetaDeployPlan(
             plan_template=plan_template.url,
             is_listed=parsed_plan.is_listed,
@@ -189,6 +192,7 @@ class Publish(BaseMetaDeployTask):
             # Use same AllowedList as the product, if any
             visible_to=product.visible_to,
             preflight_checks=plan_config.get("checks"),
+            order_key=order_key,
         )
 
         # Create Plan
diff --git a/cumulusci/utils/yaml/cumulusci_yml.py b/cumulusci/utils/yaml/cumulusci_yml.py
index 1ff7cee48..20d431ade 100644
--- a/cumulusci/utils/yaml/cumulusci_yml.py
+++ b/cumulusci/utils/yaml/cumulusci_yml.py
@@ -20,6 +20,8 @@ from cumulusci.utils.yaml.safer_loader import load_yaml_data
 
 default_logger = getLogger(__name__)
 
+PLANS_VALIDATOR_ERROR_MESSAGE = "You can only define one 'primary' Plan and one 'secondary' Plan. All other Plans must be defined as 'additional'. The default tier is 'primary'."
+
 
 #  type aliases
 PythonClassPath = str
@@ -240,9 +242,19 @@ class CumulusCIRoot(CCIDictModel):
             existing_tiers.count(tier) > 1 for tier in ("primary", "secondary")
         )
         if has_duplicate_tiers:
-            raise ValueError("Only one plan can be defined as 'primary' or 'secondary'")
+            raise ValueError(PLANS_VALIDATOR_ERROR_MESSAGE)
         return plans
 
+    @validator("plans")
+    def sort_plans(cls, plans):
+        if not plans:
+            return {}
+        sorted_plans = sorted(
+            plans.items(),
+            key=lambda x: (["primary", "secondary", "additional"].index(x[1].tier),),
+        )
+        return dict(sorted_plans)
+
 
 class CumulusCIFile(CCIDictModel):
     __root__: Union[CumulusCIRoot, None]


Test patch:
diff --git a/cumulusci/tasks/tests/test_metadeploy.py b/cumulusci/tasks/tests/test_metadeploy.py
index d575164cd..0f3dfa77b 100644
--- a/cumulusci/tasks/tests/test_metadeploy.py
+++ b/cumulusci/tasks/tests/test_metadeploy.py
@@ -208,7 +208,21 @@ class TestPublish(GithubApiTestMixin):
                 },
                 "checks": [{"when": "False", "action": "error"}],
                 "allowed_org_providers": ["user", "devhub"],
-            }
+            },
+            "install2": {
+                "title": "Test Install2",
+                "slug": "install2",
+                "tier": "primary",
+                "steps": {
+                    1: {"flow": "install_prod"},
+                    2: {
+                        "task": "util_sleep",
+                        "checks": [{"when": "False", "action": "error"}],
+                    },
+                },
+                "checks": [{"when": "False", "action": "error"}],
+                "allowed_org_providers": ["user", "devhub"],
+            },
         }
         project_config.keychain.set_service(
             "metadeploy",
@@ -315,6 +329,11 @@ class TestPublish(GithubApiTestMixin):
             "https://metadeploy/plantemplates?product=abcdef&name=install",
             json={"data": [plantemplate_dict]},
         )
+        responses.add(
+            "GET",
+            "https://metadeploy/plantemplates?product=abcdef&name=install2",
+            json={"data": [plantemplate_dict]},
+        )
         responses.add(
             "POST",
             "https://metadeploy/plans",
@@ -345,8 +364,15 @@ class TestPublish(GithubApiTestMixin):
         task = Publish(project_config, task_config)
         task()
 
-        body = json.loads(responses.calls[-2].request.body)
+        body = json.loads(responses.calls[-8].request.body)
+
         assert body["supported_orgs"] == "Both"
+        assert body["order_key"] == 0
+        assert body["title"] == "Test Install"
+
+        body2 = json.loads(responses.calls[-2].request.body)
+        assert body2["order_key"] == 1
+        assert body2["title"] == "Test Install2"
 
         steps = body["steps"]
         self.maxDiff = None
@@ -416,6 +442,20 @@ class TestPublish(GithubApiTestMixin):
                     "description": "title of installation plan",
                 },
             },
+            "plan:install2": {
+                "post_install_message": {
+                    "message": "",
+                    "description": "shown after successful installation (markdown)",
+                },
+                "preflight_message": {
+                    "message": "",
+                    "description": "shown before user starts installation (markdown)",
+                },
+                "title": {
+                    "message": "Test Install2",
+                    "description": "title of installation plan",
+                },
+            },
             "steps": {
                 "Install {product} {version}": {
                     "message": "Install {product} {version}",
diff --git a/cumulusci/utils/yaml/tests/test_cumulusci_yml.py b/cumulusci/utils/yaml/tests/test_cumulusci_yml.py
index e8dda297f..e87383183 100644
--- a/cumulusci/utils/yaml/tests/test_cumulusci_yml.py
+++ b/cumulusci/utils/yaml/tests/test_cumulusci_yml.py
@@ -1,6 +1,7 @@
 import os
 from io import StringIO
 from pathlib import Path
+from typing import OrderedDict
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
@@ -8,7 +9,9 @@ from pydantic import ValidationError
 
 from cumulusci.utils import temporary_dir
 from cumulusci.utils.yaml.cumulusci_yml import (
+    PLANS_VALIDATOR_ERROR_MESSAGE,
     GitHubSourceModel,
+    Plan,
     _validate_files,
     _validate_url,
     cci_safe_load,
@@ -236,7 +239,7 @@ plans:
     tier: primary
     """
     cci_safe_load(StringIO(yaml))
-    assert "Only one plan can be defined as 'primary' or 'secondary'" in caplog.text
+    assert PLANS_VALIDATOR_ERROR_MESSAGE in caplog.text
 
 
 def test_single_primary_plan_implicit(caplog):
@@ -248,7 +251,7 @@ plans:
     slug: implicit_primary
     """
     cci_safe_load(StringIO(yaml))
-    assert "Only one plan can be defined as 'primary' or 'secondary'" in caplog.text
+    assert PLANS_VALIDATOR_ERROR_MESSAGE in caplog.text
 
 
 def test_single_secondary_plan(caplog):
@@ -260,27 +263,76 @@ plans:
     tier: secondary
     """
     cci_safe_load(StringIO(yaml))
-    assert "Only one plan can be defined as 'primary' or 'secondary'" in caplog.text
+    assert PLANS_VALIDATOR_ERROR_MESSAGE in caplog.text
 
 
-def test_multiple_additional_plan(caplog):
+def test_multiple_additional_plans_out_of_order(caplog):
     yaml = """
 plans:
-  first:
-    slug: implicit_primary
-  second:
-    tier: secondary
   third:
     tier: additional
+  second:
+    tier: secondary
   fourth:
     tier: additional
+  first:
+    slug: implicit_primary
     """
-    parsed_yaml: dict = cci_safe_load(StringIO(yaml))
-    expected: dict = {
-        "first": {"slug": "implicit_primary"},
-        "second": {"tier": "secondary"},
-        "third": {"tier": "additional"},
-        "fourth": {"tier": "additional"},
+    result = parse_from_yaml(StringIO(yaml))
+
+    expected: OrderedDict = {
+        "first": Plan(
+            title=None,
+            description=None,
+            tier="primary",
+            slug="implicit_primary",
+            is_listed=True,
+            steps=None,
+            checks=[],
+            error_message=None,
+            post_install_message=None,
+            preflight_message=None,
+            allowed_org_providers=["user"],
+        ),
+        "second": Plan(
+            title=None,
+            description=None,
+            tier="secondary",
+            slug=None,
+            is_listed=True,
+            steps=None,
+            checks=[],
+            error_message=None,
+            post_install_message=None,
+            preflight_message=None,
+            allowed_org_providers=["user"],
+        ),
+        "third": Plan(
+            title=None,
+            description=None,
+            tier="additional",
+            slug=None,
+            is_listed=True,
+            steps=None,
+            checks=[],
+            error_message=None,
+            post_install_message=None,
+            preflight_message=None,
+            allowed_org_providers=["user"],
+        ),
+        "fourth": Plan(
+            title=None,
+            description=None,
+            tier="additional",
+            slug=None,
+            is_listed=True,
+            steps=None,
+            checks=[],
+            error_message=None,
+            post_install_message=None,
+            preflight_message=None,
+            allowed_org_providers=["user"],
+        ),
     }
     assert "" == caplog.text
-    assert expected == parsed_yaml["plans"]
+    assert OrderedDict(expected) == OrderedDict(result["plans"])