From 24d2014a691f3fd18294672df361e6fe74049a67 Mon Sep 17 00:00:00 2001
From: josephjclark <jclark@openfn.org>
Date: Tue, 7 May 2024 13:51:53 +0100
Subject: [PATCH] Release:  cli-deploy: allow steps in workflows to have
 non-unique names (#679)

* Fixing:#667  (#675)

* Fixing:#667

* resolving: requested changes

* resolving: requested changes

* version: cli@1.2.4

---------

Co-authored-by: Satyam Mattoo <96661612+SatyamMattoo@users.noreply.github.com>
---
 packages/cli/CHANGELOG.md              |  6 +++++
 packages/cli/package.json              |  2 +-
 packages/deploy/CHANGELOG.md           |  6 +++++
 packages/deploy/package.json           |  2 +-
 packages/deploy/src/validator.ts       | 36 ++++++++++++++++----------
 packages/deploy/test/validator.test.ts | 26 ++++++++++++++++---
 6 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md
index 5f72ca82b..fd2dab493 100644
--- a/packages/cli/CHANGELOG.md
+++ b/packages/cli/CHANGELOG.md
@@ -1,5 +1,11 @@
 # @openfn/cli
 
+## 1.2.4
+
+### Patch Changes
+
+- deploy: Allow steps in different workflows to have the same name
+
 ## 1.2.3
 
 ### Patch Changes
diff --git a/packages/cli/package.json b/packages/cli/package.json
index 0c06a81cc..9c83ac47b 100644
--- a/packages/cli/package.json
+++ b/packages/cli/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/cli",
-  "version": "1.2.3",
+  "version": "1.2.4",
   "description": "CLI devtools for the openfn toolchain.",
   "engines": {
     "node": ">=18",
diff --git a/packages/deploy/CHANGELOG.md b/packages/deploy/CHANGELOG.md
index a1fd881f5..81f47d9b0 100644
--- a/packages/deploy/CHANGELOG.md
+++ b/packages/deploy/CHANGELOG.md
@@ -1,5 +1,11 @@
 # @openfn/deploy
 
+## 0.4.6
+
+### Patch Changes
+
+- Allow steps in different workflows to have the same name
+
 ## 0.4.5
 
 ### Patch Changes
diff --git a/packages/deploy/package.json b/packages/deploy/package.json
index 986e8572c..87cf338aa 100644
--- a/packages/deploy/package.json
+++ b/packages/deploy/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/deploy",
-  "version": "0.4.5",
+  "version": "0.4.6",
   "description": "Deploy projects to Lightning instances",
   "type": "module",
   "exports": {
diff --git a/packages/deploy/src/validator.ts b/packages/deploy/src/validator.ts
index 8f8d78158..a416f2407 100644
--- a/packages/deploy/src/validator.ts
+++ b/packages/deploy/src/validator.ts
@@ -13,28 +13,28 @@ export function parseAndValidate(input: string): {
   doc: ProjectSpec;
 } {
   let errors: Error[] = [];
-  let keys: string[] = [];
   const doc = YAML.parseDocument(input);
 
-  function pushUniqueKey(context: YAML.Pair, key: string) {
-    if (keys.includes(key)) {
+  function pushUniqueKey(context: YAML.Pair, key: string, arr: string[]) {
+    if (arr.includes(key)) {
       errors.push({
         context,
         message: `duplicate key: ${key}`,
       });
     } else {
-      keys.push(key);
+      arr.push(key);
     }
   }
 
-  function validateJobs(workflow: YAMLMap) {
+  function validateJobs(workflow: YAMLMap, jobKeys: string[]) {
     const jobs = workflow.getIn(['jobs']);
 
     if (jobs) {
       if (isMap(jobs)) {
         for (const job of jobs.items) {
           if (isPair(job)) {
-            pushUniqueKey(job, (job as any).key.value);
+            const jobName = (job as any).key.value;
+            pushUniqueKey(job, jobName, jobKeys);
           }
         }
       } else {
@@ -50,14 +50,24 @@ export function parseAndValidate(input: string): {
     if (typeof workflows === 'undefined') {
       // allow workflows to be unspecified, but ensure there is an empty
       // map to avoid errors downstream
-      doc.setIn(['workflows'], {})
-    }
-    else if (isMap(workflows)) {
+      doc.setIn(['workflows'], {});
+    } else if (isMap(workflows)) {
+      const workflowKeys: string[] = [];
       for (const workflow of workflows.items) {
         if (isPair(workflow)) {
-          pushUniqueKey(workflow, (workflow as any).key.value);
-
-          validateJobs((workflow as any).value);
+          const workflowName = (workflow as any).key.value;
+          const jobKeys: string[] = [];
+          pushUniqueKey(workflow, workflowName, workflowKeys);
+          const workflowValue = (workflow as any).value;
+          if (isMap(workflowValue)) {
+            validateJobs(workflowValue, jobKeys);
+          } else {
+            errors.push({
+              context: workflowValue,
+              message: `workflow '${workflowName}': must be a map`,
+              path: ['workflows', workflowName],
+            });
+          }
         }
       }
     } else {
@@ -114,4 +124,4 @@ export function parseAndValidate(input: string): {
   //      or put our own errors in the yamlDoc
 
   return { errors, doc: doc.toJSON() as ProjectSpec };
-}
+}
\ No newline at end of file
diff --git a/packages/deploy/test/validator.test.ts b/packages/deploy/test/validator.test.ts
index 67559c99b..71ff1c9f4 100644
--- a/packages/deploy/test/validator.test.ts
+++ b/packages/deploy/test/validator.test.ts
@@ -19,6 +19,7 @@ workflows:
     results.errors.find((e) => e.message === 'workflows: must be a map')
   );
 
+  // disallow two workflows with same name
   doc = `
 name: project-name
 workflows:
@@ -40,11 +41,30 @@ workflows:
 
   results = parseAndValidate(doc);
 
-  t.truthy(findError(results.errors, 'duplicate key: workflow-one'));
-
-  t.truthy(findError(results.errors, 'duplicate key: foo'));
+  t.truthy(findError(results.errors, 'duplicate key: workflow-one')); 
 
   t.truthy(findError(results.errors, 'jobs: must be a map'));
+  
+  // disallow two jobs of the same name in the same workflow
+  doc = `
+name: project-name
+workflows:
+  workflow-one:
+    name: workflow one
+  workflow-two:
+    name: workflow two
+    jobs:
+      foo:
+      foo:
+  workflow-three:
+    name: workflow three
+    jobs:
+      bar:
+  `;
+
+  results = parseAndValidate(doc);
+
+  t.truthy(findError(results.errors, 'duplicate key: foo'));
 
   doc = `
 name: project-name