diff --git a/.changeset/checkout-workflow-uuids.md b/.changeset/checkout-workflow-uuids.md new file mode 100644 index 000000000..af3c0d64c --- /dev/null +++ b/.changeset/checkout-workflow-uuids.md @@ -0,0 +1,5 @@ +--- +'@openfn/project': patch +--- + +Track workflow uuids in the checkout file, and load them onto workflows diff --git a/.changeset/deploy-checkout-local-ids.md b/.changeset/deploy-checkout-local-ids.md new file mode 100644 index 000000000..ac9d7f2a3 --- /dev/null +++ b/.changeset/deploy-checkout-local-ids.md @@ -0,0 +1,6 @@ +--- +'@openfn/cli': patch +'@openfn/project': patch +--- + +Fix deploying an edit to a renamed workflow deleting it from Lightning diff --git a/.changeset/diff-renamed-workflows.md b/.changeset/diff-renamed-workflows.md new file mode 100644 index 000000000..e8d5608f3 --- /dev/null +++ b/.changeset/diff-renamed-workflows.md @@ -0,0 +1,6 @@ +--- +'@openfn/cli': patch +'@openfn/project': patch +--- + +Show a renamed workflow as changed, not added and removed, in project diffs diff --git a/.changeset/merge-match-workflows.md b/.changeset/merge-match-workflows.md new file mode 100644 index 000000000..5d98302ee --- /dev/null +++ b/.changeset/merge-match-workflows.md @@ -0,0 +1,5 @@ +--- +'@openfn/project': patch +--- + +Match workflows by uuid, then id, then name when merging projects diff --git a/.changeset/shaggy-bears-clean.md b/.changeset/shaggy-bears-clean.md new file mode 100644 index 000000000..6dd9fff26 --- /dev/null +++ b/.changeset/shaggy-bears-clean.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Add OPENFN_NO_CONFIRM env var diff --git a/.changeset/workflow-yaml-no-id.md b/.changeset/workflow-yaml-no-id.md new file mode 100644 index 000000000..c5fbd37f7 --- /dev/null +++ b/.changeset/workflow-yaml-no-id.md @@ -0,0 +1,5 @@ +--- +'@openfn/project': patch +--- + +Workflow ids in workflow.yaml files are now ignored on load: the file name is used instead diff --git a/integration-tests/cli/src/run.ts b/integration-tests/cli/src/run.ts index 9735d1305..35f134ae7 100644 --- a/integration-tests/cli/src/run.ts +++ b/integration-tests/cli/src/run.ts @@ -3,9 +3,12 @@ import * as path from 'node:path'; const isProd = process.env.NODE_ENV === 'production'; -const getOptions = () => ({ +const getOptions = (env = {}) => ({ env: { + // there's nobody to answer a prompt (a test can override this) + OPENFN_NO_CONFIRM: 'true', ...process.env, + ...env, OPENFN_REPO_DIR: path.resolve('repo'), // workspaces live inside the kit git repo: don't pick up its branch OPENFN_BRANCH: 'false', @@ -23,10 +26,12 @@ const mapOpenFnPath = (cmd) => { }; const run = async ( - cmd: string + cmd: string, + // extra environment variables for this command only + env?: Record ): Promise<{ stdout: string; stderr?: string; err?: any }> => { return new Promise((resolve) => { - exec(mapOpenFnPath(cmd), getOptions(), (err, stdout, stderr) => { + exec(mapOpenFnPath(cmd), getOptions(env), (err, stdout, stderr) => { resolve({ err, stdout, stderr }); }); }); diff --git a/integration-tests/cli/test/deploy.test.ts b/integration-tests/cli/test/deploy.test.ts index 39c605f73..d2887ad04 100644 --- a/integration-tests/cli/test/deploy.test.ts +++ b/integration-tests/cli/test/deploy.test.ts @@ -1,3 +1,5 @@ +// Tests legacy `openfn deploy` and `openfn pull` commands against a mock Lightning + import test from 'ava'; import path from 'node:path'; import fs, { rm } from 'node:fs/promises'; @@ -157,7 +159,6 @@ test.serial('deploy a local project', async (t) => { `openfn deploy \ --project-path ${tmpDir}/project.yaml \ --state-path ${tmpDir}/.state.json \ - --no-confirm \ --log-json \ -l debug` ); @@ -188,7 +189,6 @@ test.serial('Update a project', async (t) => { const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; const first = await run(deployCmd); t.falsy(first.stderr); @@ -230,7 +230,6 @@ test.serial('pull a project', async (t) => { const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; await run(deployCmd); @@ -323,7 +322,7 @@ test.serial('redirect to v2 protocol if openfn.yaml is present', async (t) => { // v1 deploy -> should redirect to v2 const { stdout, stderr } = await run( - `openfn deploy --workspace ${tmpDir} --no-confirm --log-json -l debug` + `openfn deploy --workspace ${tmpDir} --log-json -l debug` ); t.falsy(stderr); assertLog(t, extractLogs(stdout), /Detected openfn.yaml file/i); @@ -364,7 +363,6 @@ workflows: `openfn deploy \ --project-path ${tmpDir}/project.yaml \ --state-path ${tmpDir}/.state.json \ - --no-confirm \ --log-json \ -l debug` ); @@ -397,7 +395,6 @@ test.serial('deploy then pull, changes one workflow, deploy', async (t) => { const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log debug \ --log-json`; await run(deployCmd); @@ -451,7 +448,6 @@ test.serial('deploy a v2 project.yaml', async (t) => { `openfn deploy \ --project-path ${tmpDir}/project.yaml \ --state-path ${tmpDir}/.state.json \ - --no-confirm \ --log-json \ -l debug` ); @@ -481,7 +477,6 @@ test.serial('deploy a new v2 project.yaml with credentials', async (t) => { const { stdout, stderr } = await run( `openfn deploy \ --project-path ${tmpDir}/project.yaml \ - --no-confirm \ --log-json \ -l debug` ); @@ -526,7 +521,6 @@ collections: const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; // first deploy - no collections yet @@ -573,7 +567,6 @@ ${collectionsWorkflow} const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; // first deploy - creates the collection diff --git a/integration-tests/cli/test/deploy.v2.test.ts b/integration-tests/cli/test/deploy.v2.test.ts index 7e5139dda..69c414849 100644 --- a/integration-tests/cli/test/deploy.v2.test.ts +++ b/integration-tests/cli/test/deploy.v2.test.ts @@ -1,3 +1,5 @@ +// Tests v2 `openfn project pull` and `openfn project deploy` loop against a mock Lightning + import test from 'ava'; import path from 'node:path'; import fs from 'node:fs/promises'; @@ -71,7 +73,7 @@ test.serial('pull, change and re-deploy', async (t) => { // deploy const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -99,9 +101,7 @@ test.serial('pull, change and re-deploy twice', async (t) => { // first deploy await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 1 }))'); - const first = await run( - `openfn project deploy --no-confirm --log-json -l debug` - ); + const first = await run(`openfn project deploy --log-json -l debug`); t.falsy(first.stderr); assertLog(t, extractLogs(first.stdout), /Updated project/); @@ -112,7 +112,7 @@ test.serial('pull, change and re-deploy twice', async (t) => { // second deploy after another update await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 2 }))'); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); const logs = extractLogs(stdout); @@ -140,9 +140,7 @@ test.serial('deploy and pull to check version history', async (t) => { await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 1 }))'); // deploy then pull - const { stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` - ); + const { stderr } = await run(`openfn project deploy --log-json -l debug`); t.falsy(stderr); await run(`openfn project pull ${projectId} --log-json -l debug`); @@ -174,7 +172,7 @@ test.serial('deploy then pull, change one workflow, deploy', async (t) => { // deploy const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); const logs = extractLogs(stdout); @@ -215,7 +213,7 @@ test.serial('Remove a whole workflow', async (t) => { // deploy const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -262,7 +260,7 @@ steps: await rimraf(path.join(tmpDir, 'workflows/my-workflow/my-job.js')); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -317,7 +315,7 @@ steps: ); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -372,9 +370,7 @@ test.serial.skip( await fs.writeFile(exprPath, "post('http://success.org')"); // deploy - const { stdout, stderr } = await run( - `openfn project deploy --no-confirm -l debug` - ); + const { stdout, stderr } = await run(`openfn project deploy -l debug`); console.log(stdout); t.falsy(stderr); const logs = extractLogs(stdout); @@ -400,9 +396,7 @@ test.serial('warn when local and remote workflows have diverged', async (t) => { // base await run(`openfn project pull ${projectId} --log-json -l debug`); await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 1 }))'); - const firstDeploy = await run( - `openfn project deploy --no-confirm --log-json -l debug` - ); + const firstDeploy = await run(`openfn project deploy --log-json -l debug`); assertLog(t, extractLogs(firstDeploy.stdout), /Updated project/); // remote changed from base @@ -424,7 +418,7 @@ test.serial('warn when local and remote workflows have diverged', async (t) => { // deploy with divergence const { stdout, err } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.truthy(err); const logs = extractLogs(stdout); @@ -448,7 +442,7 @@ test.serial('deploy a pulled v2 state file as a new project', async (t) => { // deploy that exact file back as a duplicate const { stdout, stderr } = await run( - `openfn project deploy ${pulledPath} --new --name my-duplicate --no-confirm --log-json -l debug` + `openfn project deploy ${pulledPath} --new --name my-duplicate --log-json -l debug` ); t.falsy(stderr); @@ -506,7 +500,7 @@ test.serial( // deploy main's pulled file, but target staging instead of main const { stdout, stderr } = await run( - `openfn project deploy ${mainPath} staging --no-confirm --log-json -l debug` + `openfn project deploy ${mainPath} staging --log-json -l debug` ); t.falsy(stderr); assertLog(t, extractLogs(stdout), /Updated project/); @@ -553,7 +547,7 @@ test.serial( await fs.writeFile(resourcesPath, jsonToYaml(before)); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -596,7 +590,7 @@ workflows: await fs.writeFile(exportedPath, specYaml); const { stdout, stderr } = await run( - `openfn project deploy ${exportedPath} --name my-duplicate --no-confirm --log-json -l debug` + `openfn project deploy ${exportedPath} --name my-duplicate --log-json -l debug` ); t.falsy(stderr); @@ -659,7 +653,7 @@ test.serial( await fs.writeFile(resourcesPath, jsonToYaml(before)); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); diff --git a/integration-tests/cli/test/project-v1.test.ts b/integration-tests/cli/test/project-v1.test.ts index 61aad365a..62ad835d8 100644 --- a/integration-tests/cli/test/project-v1.test.ts +++ b/integration-tests/cli/test/project-v1.test.ts @@ -1,3 +1,6 @@ +// Tests offline `openfn checkout`, `merge` and `projects` on static legacy v1 project yaml files +// (no Lightning); see project-v2 for the same commands on v2 files + import test from 'ava'; import { rm, mkdir, writeFile, readFile } from 'node:fs/promises'; import path from 'node:path'; diff --git a/integration-tests/cli/test/project-v2.test.ts b/integration-tests/cli/test/project-v2.test.ts index 5bd26a5d9..20285225c 100644 --- a/integration-tests/cli/test/project-v2.test.ts +++ b/integration-tests/cli/test/project-v2.test.ts @@ -1,3 +1,6 @@ +// Tests offline `openfn checkout`, `merge` and `projects` on static v2 project yaml files, +// plus executing a workflow from the checked out project; see project-v1 for the legacy format + import test from 'ava'; import { rm, mkdir, writeFile, readFile } from 'node:fs/promises'; import path from 'node:path'; diff --git a/integration-tests/cli/test/sync.test.ts b/integration-tests/cli/test/sync/commands.test.ts similarity index 86% rename from integration-tests/cli/test/sync.test.ts rename to integration-tests/cli/test/sync/commands.test.ts index 8291a6a64..a3415ad5f 100644 --- a/integration-tests/cli/test/sync.test.ts +++ b/integration-tests/cli/test/sync/commands.test.ts @@ -1,47 +1,15 @@ -import test from 'ava'; -import path from 'node:path'; -import fs from 'node:fs'; -import run from '../src/run'; -import { generateProject } from '@openfn/project'; -import createLightningServer from '@openfn/lightning-mock'; -import { rimraf } from 'rimraf'; - -let PORT = 5353; -let lightning; -const endpoint = `http://localhost:${PORT}/api/provision`; - -test.before(async () => { - await rimraf('tmp/sync'); - - lightning = createLightningServer({ - port: PORT, - }); -}); +// The v2 sync commands (fetch, pull and checkout) against a mock Lightning, +// asserting on what lands on disk (.projects state, checkout.yaml, workflows folder) -const initWorkspace = (t: any) => { - const id = t.title.replaceAll(' ', '_').toLowerCase(); - const p = path.resolve('tmp/sync', id); - - return { - workspace: p, - read: (filePath: string) => { - return fs.readFileSync(path.resolve(p, filePath), 'utf8'); - }, - }; -}; - -const gen = ( - name = 'patients', - workflows = ['trigger-job(expression="fn()")'] -) => { - // generate a project - const project = generateProject(name, workflows, { - openfnUuid: true, - }); - const state = project.serialize('state', { format: 'json' }); - lightning.addProject(state); - return project; -}; +import test from 'ava'; +import run from '../../src/run'; +import { setup } from './helpers'; + +const { lightning, endpoint, initWorkspace, gen } = setup( + test, + 'commands', + 5353 +); test('fetch a new project', async (t) => { const { workspace, read } = initWorkspace(t); diff --git a/integration-tests/cli/test/sync/conflict.test.ts b/integration-tests/cli/test/sync/conflict.test.ts new file mode 100644 index 000000000..3d6592670 --- /dev/null +++ b/integration-tests/cli/test/sync/conflict.test.ts @@ -0,0 +1,9 @@ +// Cases where the workspace and the remote project have both changed +// (not covered yet: these still need to be specified) + +import test from 'ava'; + +test.todo('rename in the app, then deploy a local edit without pulling'); +test.todo('rename a workflow differently in the app and locally'); +test.todo('delete a workflow in the app, then deploy a local edit to it'); +test.todo('add a workflow with the same name in the app and locally'); diff --git a/integration-tests/cli/test/sync/edit.test.ts b/integration-tests/cli/test/sync/edit.test.ts new file mode 100644 index 000000000..551f5c639 --- /dev/null +++ b/integration-tests/cli/test/sync/edit.test.ts @@ -0,0 +1,97 @@ +// Adding, removing and editing workflows locally, then deploying them to a mock Lightning, +// checking both the remote project and the checkout files + +import test from 'ava'; +import path from 'node:path'; +import fs from 'node:fs'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'edit', 5355); + +test('add a workflow and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [alphaBefore] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + + // add a workflow, using the existing one as a starting point + fs.mkdirSync(path.join(workspace, 'workflows/beta')); + fs.writeFileSync( + path.join(workspace, 'workflows/beta/beta.yaml'), + read('workflows/alpha/alpha.yaml') + .replace('id: alpha', 'id: beta') + .replace('name: Alpha', 'name: Beta') + ); + fs.writeFileSync(path.join(workspace, 'workflows/beta/job.js'), 'fn(beta)'); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Beta: added/); + + // the server has both workflows: the existing one is untouched, the new one has a uuid + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 2); + const alpha = workflows.find((w) => w.name === 'Alpha'); + const beta = workflows.find((w) => w.name === 'Beta'); + t.is(alpha.id, alphaBefore.id); + t.truthy(beta.id); + t.not(beta.id, alpha.id); + t.is(Object.values(beta.jobs)[0].body, 'fn(beta)'); + + // the checkout now tracks the new workflow, by the local id it was added with + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp(`workflow_uuids:\n alpha: ${alpha.id}\n beta: ${beta.id}`) + ); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('remove a workflow and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + '@name Beta trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const alphaBefore = ( + Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[] + ).find((w) => w.name === 'Alpha'); + + fs.rmSync(path.join(workspace, 'workflows/beta'), { recursive: true }); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Beta: deleted/); + + // the workflow is gone from the server, and the other is untouched + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, alphaBefore.id); + t.is(workflows[0].name, 'Alpha'); + + // and it's no longer tracked locally + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp(`workflow_uuids:\n alpha: ${alphaBefore.id}\n`) + ); + t.notRegex(checkout, /beta/); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); diff --git a/integration-tests/cli/test/sync/helpers.ts b/integration-tests/cli/test/sync/helpers.ts new file mode 100644 index 000000000..da9d74283 --- /dev/null +++ b/integration-tests/cli/test/sync/helpers.ts @@ -0,0 +1,62 @@ +// Shared setup for the sync tests +// +// ava runs test files in parallel, so each file gets its own mock Lightning +// (on its own port) and its own folder under tmp/sync + +import path from 'node:path'; +import fs from 'node:fs'; +import type { ExecutionContext, TestFn } from 'ava'; +import { generateProject } from '@openfn/project'; +import createLightningServer from '@openfn/lightning-mock'; +import { rimraf } from 'rimraf'; +import run from '../../src/run'; + +export const setup = (test: TestFn, name: string, port: number) => { + const root = path.resolve('tmp/sync', name); + const endpoint = `http://localhost:${port}/api/provision`; + + const lightning: any = createLightningServer({ port }); + + test.before(async () => { + await rimraf(root); + }); + + // Each test works in its own workspace, named after the test + const initWorkspace = (t: ExecutionContext) => { + const id = t.title.replaceAll(' ', '_').toLowerCase(); + const workspace = path.resolve(root, id); + + return { + workspace, + read: (filePath: string) => { + return fs.readFileSync(path.resolve(workspace, filePath), 'utf8'); + }, + // run a command against this workspace and server (and log it to the test output), without passing + // --workspace, --endpoint or --api-key (the env is set for this command only) + run: (cmd: string) => { + t.log(`$ ${cmd}`); + return run(cmd, { + OPENFN_WORKSPACE: workspace, + OPENFN_ENDPOINT: endpoint, + OPENFN_API_KEY: 'abc', + IGNORE_DOT_ENV: 'true', + }); + }, + }; + }; + + // generate a project and serve it from the mock Lightning + const gen = ( + name = 'patients', + workflows = ['trigger-job(expression="fn()")'] + ) => { + const project = generateProject(name, workflows, { + openfnUuid: true, + }); + const state = project.serialize('state', { format: 'json' }); + lightning.addProject(state); + return project; + }; + + return { lightning, endpoint, initWorkspace, gen }; +}; diff --git a/integration-tests/cli/test/sync/rename.test.ts b/integration-tests/cli/test/sync/rename.test.ts new file mode 100644 index 000000000..07fb4a164 --- /dev/null +++ b/integration-tests/cli/test/sync/rename.test.ts @@ -0,0 +1,570 @@ +// Renaming workflows locally, alone and combined with other edits, then deploying to a mock +// Lightning: the workflow must keep its uuid, and the checkout files must track its local id + +import test from 'ava'; +import path from 'node:path'; +import fs from 'node:fs'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'rename', 5356); + +test('rename a workflow and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobIds = Object.values(before.jobs).map((j) => j.id); + const triggerIds = Object.values(before.triggers).map((j) => j.id); + const edgeIds = Object.values(before.edges).map((j) => j.id); + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + + await run('openfn project deploy'); + + // the server has the same workflow and steps (by uuid), with the new name + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.deepEqual( + Object.values(workflows[0].jobs).map((j) => j.id), + jobIds + ); + t.deepEqual( + Object.values(workflows[0].triggers).map((j) => j.id), + triggerIds + ); + t.deepEqual( + Object.values(workflows[0].edges).map((j) => j.id), + edgeIds + ); + + // the workflow stays where it was, under the id it had locally + t.regex(read('workflows/workflow/workflow.yaml'), /name: Renamed/); + t.false(fs.existsSync(path.join(workspace, 'workflows/renamed'))); + + // and the checkout file tracks it by that local id + const checkout = read('.openfn/checkout.yaml'); + t.regex(checkout, /forked_from:\n workflow: /); + t.regex(checkout, new RegExp(`workflow_uuids:\n workflow: ${before.id}`)); + t.notRegex(checkout, /renamed/); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a workflow, deploy, then edit and deploy again', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + await run('openfn project deploy'); + + // now change the code in the renamed workflow and deploy again + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + const { stdout } = await run('openfn project deploy'); + + // it's an edit to the workflow, not a new workflow + t.regex(stdout, /Renamed: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + // the server workflow was updated in place + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.is(Object.values(workflows[0].jobs)[0].body, 'fn(x)'); + + // and the checkout is still tracking the local id + const checkout = read('.openfn/checkout.yaml'); + t.regex(checkout, /forked_from:\n workflow: /); + t.regex(checkout, new RegExp(`workflow_uuids:\n workflow: ${before.id}`)); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename and edit a workflow in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Renamed: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.is(Object.values(workflows[0].jobs)[0].body, 'fn(x)'); + + const checkout = read('.openfn/checkout.yaml'); + t.regex(checkout, new RegExp(`workflow_uuids:\n workflow: ${before.id}`)); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename two workflows, edit each, and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + '@name Beta trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const uuids: Record = {}; + for (const wf of Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]) { + uuids[wf.name] = wf.id; + } + + for (const [id, name] of [ + ['alpha', 'Alpha'], + ['beta', 'Beta'], + ]) { + const wfPath = path.join(workspace, `workflows/${id}/${id}.yaml`); + fs.writeFileSync( + wfPath, + read(`workflows/${id}/${id}.yaml`).replace( + `name: ${name}`, + `name: ${name} 2` + ) + ); + fs.writeFileSync( + path.join(workspace, `workflows/${id}/job.js`), + `fn(${id})` + ); + } + + await run('openfn project deploy'); + + // each workflow keeps its own uuid, with its own new name and code + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 2); + const alpha = workflows.find((w) => w.name === 'Alpha 2'); + const beta = workflows.find((w) => w.name === 'Beta 2'); + t.is(alpha.id, uuids.Alpha); + t.is(beta.id, uuids.Beta); + t.is(Object.values(alpha.jobs)[0].body, 'fn(alpha)'); + t.is(Object.values(beta.jobs)[0].body, 'fn(beta)'); + + // the checkout tracks both by their local ids + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp( + `workflow_uuids:\n alpha: ${uuids.Alpha}\n beta: ${uuids.Beta}` + ) + ); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('remove one workflow and rename another in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + '@name Beta trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const alphaBefore = ( + Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[] + ).find((w) => w.name === 'Alpha'); + + fs.rmSync(path.join(workspace, 'workflows/beta'), { recursive: true }); + fs.writeFileSync( + path.join(workspace, 'workflows/alpha/alpha.yaml'), + read('workflows/alpha/alpha.yaml').replace('name: Alpha', 'name: Alpha 2') + ); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Beta: deleted/); + t.regex(stdout, /Alpha 2: changed/); + + // one workflow is left, with the same uuid and its new name + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, alphaBefore.id); + t.is(workflows[0].name, 'Alpha 2'); + + // the checkout tracks only the survivor, by its local id + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp(`workflow_uuids:\n alpha: ${alphaBefore.id}\n`) + ); + t.notRegex(checkout, /beta/); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a step and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobIds = Object.values(before.jobs) + .map((j) => j.id) + .sort(); + const edgeIds = Object.values(before.edges).map((e) => e.id); + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + // rename the step (but not its id, or the file its code is in) + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + ' name: job\n', + ' name: Renamed job\n' + ) + ); + + await run('openfn project deploy'); + + // the server has the same steps and edge (by uuid), with the new name + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.deepEqual( + Object.values(workflows[0].jobs) + .map((j) => j.id) + .sort(), + jobIds + ); + t.is( + Object.values(workflows[0].jobs).find((j) => j.id === jobId).name, + 'Renamed job' + ); + t.deepEqual( + Object.values(workflows[0].edges).map((e) => e.id), + edgeIds + ); + + // the step stays where it was locally + t.regex(read('workflows/workflow/workflow.yaml'), /name: Renamed job/); + t.is(read('workflows/workflow/job.js'), 'fn()'); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +// Skipped: step ids drift after a step rename +// The first deploy refetches the project, which derives the step id from its new name +// (renamed-job), but the workspace still has job. The next deploy sees job removed and +// renamed-job added, so the step is replaced and gets a new uuid +test.skip('rename a step, deploy, then edit and deploy again', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + ' name: job\n', + ' name: Renamed job\n' + ) + ); + await run('openfn project deploy'); + + // now change the code in the renamed step and deploy again + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Workflow: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + // the step was updated in place, not replaced + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + const job = Object.values(workflows[0].jobs).find((j) => j.id === jobId); + t.is(job.name, 'Renamed job'); + t.is(job.body, 'fn(x)'); + t.is(Object.keys(workflows[0].jobs).length, 2); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename and edit a step in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + ' name: job\n', + ' name: Renamed job\n' + ) + ); + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Workflow: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + const job = Object.values(workflows[0].jobs).find((j) => j.id === jobId); + t.is(job.name, 'Renamed job'); + t.is(job.body, 'fn(x)'); + t.is(Object.keys(workflows[0].jobs).length, 2); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a workflow and one of its steps in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml') + .replace('name: Workflow', 'name: Renamed') + .replace(' name: job\n', ' name: Renamed job\n') + ); + + await run('openfn project deploy'); + + // the same workflow and step (by uuid), with new names + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.is( + Object.values(workflows[0].jobs).find((j) => j.id === jobId).name, + 'Renamed job' + ); + t.is(Object.keys(workflows[0].jobs).length, 2); + + // the checkout tracks the workflow by its local id + t.regex( + read('.openfn/checkout.yaml'), + new RegExp(`workflow_uuids:\n workflow: ${before.id}`) + ); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a workflow in the app, then pull', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // a file of the user's, next to the workflow + fs.writeFileSync( + path.join(workspace, 'workflows/workflow/notes.md'), + 'my notes' + ); + + // rename the workflow in the app + project.workflows[0].name = 'Renamed'; + project.workflows[0].workflow.name = 'Renamed'; + lightning.addProject(project.serialize('state', { format: 'json' })); + + // --force is needed because a rename also trips a false divergence warning + await run(`openfn project pull ${project.openfn.uuid} --force`); + + // there is one workflow, with its new name and the user's file alongside it + const folders = fs.readdirSync(path.join(workspace, 'workflows')); + const workflowFolders = folders.filter((folder) => + fs + .readdirSync(path.join(workspace, 'workflows', folder)) + .some((file) => file.endsWith('.yaml')) + ); + t.is(workflowFolders.length, 1); + const files = fs.readdirSync( + path.join(workspace, 'workflows', workflowFolders[0]) + ); + t.true(files.includes('notes.md')); + t.true(files.includes('job.js')); + t.regex( + read( + `workflows/${workflowFolders[0]}/${files.find((f) => + f.endsWith('.yaml') + )}` + ), + /name: Renamed/ + ); +}); + +test('rename a workflow locally, deploy, then pull', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // a file of the user's, next to the workflow + fs.writeFileSync( + path.join(workspace, 'workflows/workflow/notes.md'), + 'my notes' + ); + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + await run('openfn project deploy'); + + // --force is needed because a rename also trips a false divergence warning + await run(`openfn project pull ${project.openfn.uuid} --force`); + + // there is one workflow, with its new name and the user's file alongside it + const folders = fs.readdirSync(path.join(workspace, 'workflows')); + const workflowFolders = folders.filter((folder) => + fs + .readdirSync(path.join(workspace, 'workflows', folder)) + .some((file) => file.endsWith('.yaml')) + ); + t.is(workflowFolders.length, 1); + const files = fs.readdirSync( + path.join(workspace, 'workflows', workflowFolders[0]) + ); + t.true(files.includes('notes.md')); + t.true(files.includes('job.js')); + t.regex( + read( + `workflows/${workflowFolders[0]}/${files.find((f) => + f.endsWith('.yaml') + )}` + ), + /name: Renamed/ + ); +}); + +// (--force is needed because a rename also trips a false divergence warning: a separate known issue) +test('rename an aliased workflow in the app, then pull', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // alias the workflow, by renaming its folder + fs.renameSync( + path.join(workspace, 'workflows/workflow'), + path.join(workspace, 'workflows/wf') + ); + + // rename the workflow in the app + project.workflows[0].name = 'Renamed'; + project.workflows[0].workflow.name = 'Renamed'; + lightning.addProject(project.serialize('state', { format: 'json' })); + + await run(`openfn project pull ${project.openfn.uuid} --force`); + + // the rename is applied, and the alias is preserved + t.deepEqual(fs.readdirSync(path.join(workspace, 'workflows')), ['wf']); + t.regex(read('workflows/wf/workflow.yaml'), /name: Renamed/); + t.true(fs.existsSync(path.join(workspace, 'workflows/wf/job.js'))); + + // and the checkout still tracks the workflow by its local id + t.regex(read('.openfn/checkout.yaml'), /forked_from:\n workflow: /); +}); diff --git a/integration-tests/cli/test/sync/roundtrip.test.ts b/integration-tests/cli/test/sync/roundtrip.test.ts new file mode 100644 index 000000000..4259932d2 --- /dev/null +++ b/integration-tests/cli/test/sync/roundtrip.test.ts @@ -0,0 +1,70 @@ +// Sync round trips (pull, edit, deploy, pull again) against a mock Lightning, +// checking that the workspace and the remote project stay consistent + +import test from 'ava'; +import path from 'node:path'; +import fs from 'node:fs'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'roundtrip', 5354); + +test('pull, edit, deploy, then pull again', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + + await run('openfn project deploy'); + + // the server has the edit + const [workflow] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(Object.values(workflow.jobs)[0].body, 'fn(x)'); + + // the deploy updates the checkout, tracking the workflow as it is on disk + const checkoutAfterDeploy = read('.openfn/checkout.yaml'); + t.regex(checkoutAfterDeploy, /forked_from:\n workflow: /); + t.regex(checkoutAfterDeploy, /workflow_uuids:\n workflow: /); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // pulling what we just deployed changes nothing + t.is(read('workflows/workflow/job.js'), 'fn(x)'); + t.deepEqual(fs.readdirSync(path.join(workspace, 'workflows')), ['workflow']); + t.deepEqual( + fs.readdirSync(path.join(workspace, 'workflows/workflow')).sort(), + ['job.js', 'workflow.yaml'] + ); + t.is(read('.openfn/checkout.yaml'), checkoutAfterDeploy); +}); + +test('fetch only updates the tracked project', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + const checkoutBefore = read('.openfn/checkout.yaml'); + + // Make a local change + fs.writeFileSync( + path.join(workspace, 'workflows/workflow/job.js'), + 'fn(local)' + ); + + // And a contrasting remote change + project.workflows[0].steps[0].expression = 'fn(remote)'; + lightning.addProject(project.serialize('state', { format: 'json' })); + + // Now fetch + await run(`openfn project fetch ${project.openfn.uuid}`); + + // the tracked project has the remote change + t.regex(read('.projects/main@localhost.yaml'), /fn\(remote\)/); + + // but the workspace is untouched + t.is(read('workflows/workflow/job.js'), 'fn(local)'); + t.is(read('.openfn/checkout.yaml'), checkoutBefore); +}); diff --git a/packages/cli/README.md b/packages/cli/README.md index cc71f6a91..c67a3358b 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -266,7 +266,7 @@ openfn deploy - `-p, --project-path ` - path to the project file (defaults to `project.yaml`) - `-s, --state-path ` - path to the state file (defaults to `.state.json`) - `-c, --config, --config-path` - path to the config file (defaults to `.config.json`) -- `--no-confirm` - skip the confirmation prompt +- `--no-confirm` - skip the confirmation prompt (or set the `OPENFN_NO_CONFIRM=true` env var) ## Workflow Aliases diff --git a/packages/cli/src/options.ts b/packages/cli/src/options.ts index a22f1d4c6..6c9729c70 100644 --- a/packages/cli/src/options.ts +++ b/packages/cli/src/options.ts @@ -245,12 +245,20 @@ export const confirm: CLIOption = { yargs: { alias: ['y'], boolean: true, - description: "Skip confirmation prompts (e.g. 'Are you sure?')", + description: + "Skip confirmation prompts (e.g. 'Are you sure?'). Use --no-confirm or OPENFN_NO_CONFIRM=true to skip", }, ensure: (opts) => { if ((opts as any).y) { opts.confirm = false; } + // An explicit --confirm or --no-confirm always beats the env var + if ( + opts.confirm === undefined && + process.env.OPENFN_NO_CONFIRM === 'true' + ) { + opts.confirm = false; + } setDefaultValue(opts, 'confirm', true); }, }; diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index 710ee90a6..57dd45721 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -1,5 +1,9 @@ import yargs from 'yargs'; -import Project, { Workspace } from '@openfn/project'; +import Project, { + matchWorkflows, + relabelWorkflows, + Workspace, +} from '@openfn/project'; import type l from '@openfn/lexicon'; import path from 'path'; import fs from 'fs'; @@ -174,6 +178,21 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { } } + // The incoming project derives workflow ids from their names, so a workflow + // renamed elsewhere arrives with a new id. When checking out the project that is + // already checked out, keep the ids the workspace has (matched by uuid, id or + // name) so that a renamed workflow stays in its folder, with its files + if ( + localProject?.workflows.length && + tracked && + isSameProject(tracked, switchProject) + ) { + switchProject = relabelWorkflows( + switchProject, + matchWorkflows(switchProject.workflows, localProject.workflows) + ); + } + // Workflow aliases are just folder names, so carry them over from the // checked out project to keep renamed folders in place const incomingIds = new Set(switchProject.workflows.map((wf) => wf.id)); diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 80e3d3dcd..892b53b55 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -1,6 +1,8 @@ import yargs from 'yargs'; import Project, { + matchWorkflows, MergeProjectOptions, + relabelWorkflows, RESOURCES_FILE, toResourceChannels, versionsEqual, @@ -19,6 +21,8 @@ import { serialize, getSerializePath, writeCheckoutFile, + updateForkedFrom, + invertMatches, findLocallyChangedWorkflows, AuthOptions, } from './util'; @@ -129,6 +133,9 @@ export const hasRemoteDiverged = ( const refs = local.cli.forked_from ?? {}; + // The remote id of a workflow can differ from the local one (eg, after a rename) + const matches = matchWorkflows(local.workflows, remote.workflows ?? []); + const filteredWorkflows = workflows.length ? local.workflows.filter((w) => workflows.includes(w.id)) : local.workflows; @@ -137,7 +144,9 @@ export const hasRemoteDiverged = ( for (const wf of filteredWorkflows) { if (wf.id in refs) { const forkedVersion = refs[wf.id]; - const remoteVersion = remote.getWorkflow(wf.id)?.history.at(-1); + const remoteVersion = remote + .getWorkflow(matches[wf.id] ?? wf.id) + ?.history.at(-1); if (remoteVersion) { if (!versionsEqual(forkedVersion, remoteVersion!)) { diverged ??= []; @@ -646,7 +655,24 @@ export async function handler(options: DeployOptions, logger: Logger) { const configPath = path.resolve(workspacePath, configData.path); await mkdir(path.dirname(configPath), { recursive: true }); await writeFile(configPath, configData.content); - await writeCheckoutFile(workspacePath, finalProject, options.branch); + + // The fetched project derives its workflow ids from their names, which + // can differ from the ids we use locally (eg, after a rename). The + // checkout file tracks workflows by their local ids, so it matches the + // workflows on disk + const localIds = invertMatches( + matchWorkflows( + merged.workflows.filter((wf) => !wf.isRemoved()), + finalProject.workflows + ) + ); + // (the tracked project state keeps the ids it was fetched with) + updateForkedFrom(finalProject); + await writeCheckoutFile( + workspacePath, + relabelWorkflows(finalProject, localIds), + options.branch + ); // Sync collections into resources.yaml (anything else in there is left alone) const resourcesPath = path.resolve(workspacePath, RESOURCES_FILE); diff --git a/packages/cli/src/projects/diff.ts b/packages/cli/src/projects/diff.ts index af32f8d25..4dbb6d46a 100644 --- a/packages/cli/src/projects/diff.ts +++ b/packages/cli/src/projects/diff.ts @@ -131,7 +131,7 @@ export const printRichDiff = ( if (changed.length > 0) { for (const diff of changed) { - const localWf = local.getWorkflow(diff.id); + const localWf = local.getWorkflow(diff.bId ?? diff.id); const remoteWf = remote.getWorkflow(diff.id); const label = localWf?.name || diff.id; logger.always(c.yellow(`${label}: changed`)); diff --git a/packages/cli/src/projects/util.ts b/packages/cli/src/projects/util.ts index a34e8d7d2..932eb1152 100644 --- a/packages/cli/src/projects/util.ts +++ b/packages/cli/src/projects/util.ts @@ -260,6 +260,10 @@ export const updateForkedFrom = (proj: Project) => { return proj; }; +// Swap the keys and values of a workflow id mapping (eg, from matchWorkflows) +export const invertMatches = (matches: Record) => + Object.fromEntries(Object.entries(matches).map(([a, b]) => [b, a])); + // Set up the checkout file for a project without touching anything else // (use this when the workflows are already on disk, like after a git clone) export const writeCheckoutFile = async ( diff --git a/packages/cli/test/options/ensure/confirm.test.ts b/packages/cli/test/options/ensure/confirm.test.ts new file mode 100644 index 000000000..22f97a367 --- /dev/null +++ b/packages/cli/test/options/ensure/confirm.test.ts @@ -0,0 +1,72 @@ +import test from 'ava'; +import { confirm, Opts } from '../../../src/options'; + +const ENV = 'OPENFN_NO_CONFIRM'; +const original = process.env[ENV]; + +test.afterEach.always(() => { + if (original === undefined) { + delete process.env[ENV]; + } else { + process.env[ENV] = original; + } +}); + +test.serial('confirm defaults to true', (t) => { + delete process.env[ENV]; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); +}); + +test.serial('-y sets confirm to false', (t) => { + delete process.env[ENV]; + const opts = { y: true } as unknown as Opts; + confirm.ensure!(opts); + + t.false(opts.confirm); +}); + +test.serial('env=true with no flag sets confirm to false', (t) => { + process.env[ENV] = 'true'; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.false(opts.confirm); +}); + +test.serial('env=false with no flag leaves confirm true', (t) => { + process.env[ENV] = 'false'; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); +}); + +test.serial( + 'env=1 (truthy but not "true") with no flag leaves confirm true', + (t) => { + process.env[ENV] = '1'; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); + } +); + +test.serial('env=true with an explicit --confirm leaves confirm true', (t) => { + process.env[ENV] = 'true'; + const opts = { confirm: true } as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); +}); + +test.serial('env unset with --no-confirm sets confirm to false', (t) => { + delete process.env[ENV]; + const opts = { confirm: false } as Opts; + confirm.ensure!(opts); + + t.false(opts.confirm); +}); diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index 9bd5eb277..2e83ec576 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -256,6 +256,66 @@ test.serial( } ); +test.serial( + 'checkout: a workflow renamed in the tracked project keeps its folder and id', + async (t) => { + await checkoutHandler( + { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, + logger + ); + // a file of the user's, next to the workflow + fs.writeFileSync('/ws/workflows/simple-workflow-main/notes.md', 'my notes'); + + // the workflow is renamed (but has the same uuid) in the tracked project + const trackedPath = '/ws/.projects/project@app.openfn.org.yaml'; + const tracked = yamlToJson(fs.readFileSync(trackedPath, 'utf8')); + tracked.workflows[0].name = 'Renamed workflow'; + fs.writeFileSync(trackedPath, jsonToYaml(tracked)); + + // (force: a rename also trips a false divergence warning, which is a separate issue) + await checkoutHandler( + { + command: 'project-checkout', + project: 'my-project', + workspace: '/ws', + force: true, + }, + logger + ); + + // the workflow stays where it was, under the id it had, with the new name + t.deepEqual( + fs.readdirSync('/ws/workflows').sort(), + ['another-workflow-main', 'simple-workflow-main'].sort() + ); + t.deepEqual( + fs.readdirSync('/ws/workflows/simple-workflow-main').sort(), + [ + 'notes.md', + 'simple-workflow-main.yaml', + 'transform-data-to-fhir-standard.js', + ].sort() + ); + t.regex( + fs.readFileSync( + '/ws/workflows/simple-workflow-main/simple-workflow-main.yaml', + 'utf8' + ), + /name: Renamed workflow/ + ); + + // and the checkout file tracks it by that id + const checkout = yamlToJson( + fs.readFileSync('/ws/.openfn/checkout.yaml', 'utf8') + ); + t.deepEqual(Object.keys(checkout.forked_from).sort(), [ + 'another-workflow-main', + 'simple-workflow-main', + ]); + t.is(checkout.workflow_uuids['simple-workflow-main'], 'wf-id-main'); + } +); + test.serial( 'checkout: drops an alias that clashes with an incoming workflow id', async (t) => { @@ -511,6 +571,8 @@ formats: id: staging name: Staging uuid: +workflow_uuids: + simple-workflow: wf1 ` ); diff --git a/packages/cli/test/projects/deploy.test.ts b/packages/cli/test/projects/deploy.test.ts index 6abddc2a2..ebe3a2ae5 100644 --- a/packages/cli/test/projects/deploy.test.ts +++ b/packages/cli/test/projects/deploy.test.ts @@ -471,6 +471,60 @@ test.serial('deploy a change to a project', async (t) => { t.is(Object.keys(server.state.projects).length, 1); }); +// https://github.com/OpenFn/kit/issues/1594 +test.serial( + 'deploy an edit to a workflow after renaming it: the checkout tracks the local id', + async (t) => { + await setup(projectYaml); + const options = { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + confirm: false, + } as any; + + const wfPath = '/ws/workflows/my-workflow/my-workflow.yaml'; + await writeFile( + wfPath, + fs + .readFileSync(wfPath, 'utf8') + .replace('name: My Workflow', 'name: My Workflow (renamed)') + ); + await deploy(options, logger); + + // the files haven't moved, so the checkout file tracks the workflow by its + // local id, not the id derived from its new name + t.true(fs.existsSync(wfPath)); + const checkout = yamlToJson(readCheckout()!) as any; + t.deepEqual(Object.keys(checkout.forked_from), ['my-workflow']); + t.deepEqual(checkout.workflow_uuids, { + 'my-workflow': '72ca3eb0-042c-47a0-a2a1-a545ed4a8406', + }); + + await writeFile('/ws/workflows/my-workflow/transform-data.js', 'log()'); + logger._reset(); + await deploy(options, logger); + + // the summary shows an edit to the existing workflow + t.truthy(logger._find('always', /My Workflow \(renamed\): changed/)); + t.falsy(logger._find('always', /: (added|deleted)/)); + + // the workflow was updated, not deleted + const workflows = Object.values( + server.state.projects[UUID].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, '72ca3eb0-042c-47a0-a2a1-a545ed4a8406'); + t.is(workflows[0].name, 'My Workflow (renamed)'); + t.is( + Object.values(workflows[0].jobs).find( + (j) => j.name === 'Transform data' + ).body, + 'log()' + ); + } +); + test.serial( 'Error if the remote and local workflows have diverged', async (t) => { @@ -1033,8 +1087,8 @@ test('printRichDiff: should report mix of added, changed, and removed workflows' const wf1 = generateWorkflow('@id a trigger-x'); const wf2Remote = generateWorkflow('@id b trigger-y'); const wf2Local = generateWorkflow('@id b trigger-different'); - const wf3 = generateWorkflow('@id c trigger-z'); - const wf4 = generateWorkflow('@id d trigger-w'); + const wf3 = generateWorkflow('@id c @name wf-c trigger-z'); + const wf4 = generateWorkflow('@id d @name wf-d trigger-w'); const local = new Project({ name: 'local', @@ -1092,6 +1146,31 @@ test('hasRemoteDiverged: 1 workflow, no diverged', (t) => { t.falsy(diverged); }); +test('hasRemoteDiverged: finds a renamed workflow in the remote by its uuid', (t) => { + const local = new Project({ + id: 'p', + workflows: [ + { id: 'my-workflow', name: 'Wf', steps: [], openfn: { uuid: 'uuid-1' } }, + ], + cli: { forked_from: { 'my-workflow': 'a' } }, + } as any); + const remote = new Project({ + id: 'p', + workflows: [ + { + id: 'my-workflow-renamed', + name: 'Wf (renamed)', + steps: [], + history: ['a', 'b'], + openfn: { uuid: 'uuid-1' }, + }, + ], + } as any); + + // the remote has moved on since the local was forked + t.deepEqual(hasRemoteDiverged(local, remote), ['my-workflow']); +}); + test('hasRemoteDiverged: 1 workflow, 1 diverged', (t) => { const local = { workflows: [ diff --git a/packages/project/src/Project.ts b/packages/project/src/Project.ts index a4eb59943..e6040f5be 100644 --- a/packages/project/src/Project.ts +++ b/packages/project/src/Project.ts @@ -42,6 +42,8 @@ type CLIMeta = { version?: number; alias?: string | null; forked_from?: Record; + // uuid of each workflow, by local id. Tracked but not used + workflow_uuids?: Record; }; export class Project { diff --git a/packages/project/src/index.ts b/packages/project/src/index.ts index 924474356..81081a5ad 100644 --- a/packages/project/src/index.ts +++ b/packages/project/src/index.ts @@ -39,6 +39,9 @@ export { default as detectVersion } from './util/detect-version'; export type { MergeProjectOptions } from './merge/merge-project'; +export { default as matchWorkflows } from './merge/match-workflows'; +export { default as relabelWorkflows } from './util/relabel-workflows'; + export { mergeCollections } from './merge/merge-project'; export { toResourceChannels } from './util/resources'; diff --git a/packages/project/src/merge/match-workflows.ts b/packages/project/src/merge/match-workflows.ts new file mode 100644 index 000000000..050f707b0 --- /dev/null +++ b/packages/project/src/merge/match-workflows.ts @@ -0,0 +1,58 @@ +import Workflow from '../Workflow'; + +/** + * Pair up the workflows of a source project with those of a target + * + * A source workflow is matched to a target workflow by, in order: + * 1. uuid + * 2. id + * 3. name (only if it's unique among the workflows still unmatched on both sides) + * + * Matches are one-to-one. A match where the ids differ is a rename. + * + * Returns a map of source workflow id -> target workflow id. + * Unmatched workflows do not appear in it. + */ +export default function matchWorkflows( + source: Workflow[], + target: Workflow[] +): Record { + const matches: Record = {}; + const claimed = new Set(); // target ids + + const unmatched = () => source.filter((s) => !(s.id in matches)); + const available = () => target.filter((t) => !claimed.has(t.id)); + const claim = (s: Workflow, t: Workflow) => { + matches[s.id] = t.id; + claimed.add(t.id); + }; + + for (const s of unmatched()) { + const uuid = s.openfn?.uuid; + if (uuid) { + const t = available().find((t) => t.openfn?.uuid === uuid); + if (t) { + claim(s, t); + } + } + } + + for (const s of unmatched()) { + const t = available().find((t) => t.id === s.id); + if (t) { + claim(s, t); + } + } + + for (const s of unmatched()) { + if (s.name) { + const sources = unmatched().filter((x) => x.name === s.name); + const targets = available().filter((t) => t.name === s.name); + if (sources.length === 1 && targets.length === 1) { + claim(s, targets[0]); + } + } + } + + return matches; +} diff --git a/packages/project/src/merge/merge-project.ts b/packages/project/src/merge/merge-project.ts index af6b5f770..566049bf4 100644 --- a/packages/project/src/merge/merge-project.ts +++ b/packages/project/src/merge/merge-project.ts @@ -6,6 +6,7 @@ import { } from '@openfn/lexicon'; import { Project } from '../Project'; import { mergeWorkflows } from './merge-workflow'; +import matchWorkflows from './match-workflows'; import mapUuids from './map-uuids'; import baseMerge from '../util/base-merge'; import getDuplicates from '../util/get-duplicates'; @@ -94,11 +95,19 @@ export function merge( ); } + // Explicit mappings win. Otherwise, match source to target workflows + // by uuid, then id, then name + const workflowMappings = noMappings + ? matchWorkflows(sourceWorkflows, target.workflows) + : options.workflowMappings; + const getTargetWorkflow = (sourceWorkflow: Workflow) => { + const targetId = workflowMappings[sourceWorkflow.id]; + return targetId ? target.getWorkflow(targetId) : undefined; + }; + const potentialConflicts: Record = {}; for (const sourceWorkflow of sourceWorkflows) { - const targetId = - options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; - const targetWorkflow = target.getWorkflow(targetId); + const targetWorkflow = getTargetWorkflow(sourceWorkflow); if (targetWorkflow && !sourceWorkflow.canMergeInto(targetWorkflow)) { potentialConflicts[sourceWorkflow.id] = targetWorkflow?.id; } @@ -115,9 +124,7 @@ export function merge( } for (const sourceWorkflow of sourceWorkflows) { - const targetId = - options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; - const targetWorkflow = target.getWorkflow(targetId); + const targetWorkflow = getTargetWorkflow(sourceWorkflow); if (targetWorkflow) { usedTargetIds.add(targetWorkflow.id); diff --git a/packages/project/src/parse/from-fs.ts b/packages/project/src/parse/from-fs.ts index 4772f0577..0347fbf4e 100644 --- a/packages/project/src/parse/from-fs.ts +++ b/packages/project/src/parse/from-fs.ts @@ -12,7 +12,7 @@ import { findWorkspaceFile, loadCheckoutFile, } from '../util/config'; -import { countBy, omit } from 'lodash-es'; +import { countBy, isPlainObject, omit } from 'lodash-es'; import { Logger } from '@openfn/logger'; import omitNil from '../util/omit-nil'; import slugify from '../util/slugify'; @@ -33,8 +33,8 @@ export type FromFsConfig = { }; // Parse a single project from a root folder -// Note that this does NOT attempt to load UUIDS from the project file -// It just builds the project on disk +// Workflow uuids are not stored in the workflow files: if a workflow is tracked +// in the checkout file, its uuid is loaded from there. Other uuids are not loaded // I suppose we could take an option? export const parseProject = async (options: FromFsConfig) => { const { root, logger, alias } = options; @@ -45,10 +45,20 @@ export const parseProject = async (options: FromFsConfig) => { const checkout = loadCheckoutFile(root, options.branch, context.project) ?? {}; + const workflowUuids = isPlainObject(checkout.workflow_uuids) + ? (checkout.workflow_uuids as Record) + : undefined; + const proj: any = { id: options.name ? slugify(options.name) : checkout.id, name: options.name ? slugify(options.name) : checkout.name, - openfn: omit(checkout, ['id', 'alias', 'forked_from', 'collections']), + openfn: omit(checkout, [ + 'id', + 'alias', + 'forked_from', + 'workflow_uuids', + 'collections', + ]), // Legacy: collections used to be a list of names in openfn.yaml. A // collections key in resources.yaml takes precedence (see below) @@ -59,6 +69,7 @@ export const parseProject = async (options: FromFsConfig) => { workflows: [], cli: omitNil({ forked_from: checkout.forked_from, + workflow_uuids: workflowUuids, }), }; @@ -112,8 +123,9 @@ export const parseProject = async (options: FromFsConfig) => { wf = wf.workflow; } - if (wf.id && Array.isArray(wf.steps)) { - //logger?.log('Loading workflow at ', filePath); // TODO logger.debug + if (Array.isArray(wf.steps)) { + wf.id = path.basename(filePath, path.extname(filePath)); + for (const step of wf.steps) { // This is the saved, remote view of the step // TODO if the id has changed, how do we track? @@ -139,6 +151,11 @@ export const parseProject = async (options: FromFsConfig) => { } const workflow = new Workflow(wf); + // If the workflow is tracked, we know its uuid + const uuid = workflowUuids?.[workflow.id]; + if (uuid) { + workflow.openfn = { ...workflow.openfn, uuid }; + } // A workflow folder renamed away from the id is treated as an alias const dir = path.dirname(filePath); const folder = path.basename(dir); diff --git a/packages/project/src/serialize/to-fs.ts b/packages/project/src/serialize/to-fs.ts index 536472ef5..a73140852 100644 --- a/packages/project/src/serialize/to-fs.ts +++ b/packages/project/src/serialize/to-fs.ts @@ -69,6 +69,9 @@ export const extractWorkflow = ( const wf = Object.assign( { + // DEPRECATED: the id is a local artefact and is ignored on load (the file name is used) + // It's still written so that older CLI versions can read these files + // TODO remove this in the next major id: workflow.id, name: workflow.name, start: workflow.start, diff --git a/packages/project/src/util/config.ts b/packages/project/src/util/config.ts index b4801e834..d2a9f1c97 100644 --- a/packages/project/src/util/config.ts +++ b/packages/project/src/util/config.ts @@ -71,6 +71,18 @@ export const extractCheckout = ( project.forked_from = source.cli.forked_from; } + // Track the uuid of each workflow, by local id + // This is recorded for future use and isn't read by anything yet + const workflowUuids = source.workflows.reduce((obj: any, wf) => { + if (wf.openfn?.uuid) { + obj[wf.id] = wf.openfn.uuid; + } + return obj; + }, {}); + if (Object.keys(workflowUuids).length) { + project.workflow_uuids = workflowUuids; + } + return { path: getCheckoutPath(branch), content: jsonToYaml(sortKeys(project)), diff --git a/packages/project/src/util/project-diff.ts b/packages/project/src/util/project-diff.ts index f942bfad3..2b71b524a 100644 --- a/packages/project/src/util/project-diff.ts +++ b/packages/project/src/util/project-diff.ts @@ -1,12 +1,16 @@ import { isEqual } from 'lodash-es'; import { Project } from '../Project'; import { toResourceChannels } from './resources'; +import matchWorkflows from '../merge/match-workflows'; export type DiffType = 'added' | 'changed' | 'removed'; export type WorkflowDiff = { + // The workflow id in A (or in B, if the workflow was added) id: string; type: DiffType; + // The workflow id in B, if it differs from the id in A (ie, it was renamed) + bId?: string; }; export type ChannelDiff = { @@ -34,7 +38,9 @@ export type ProjectDiff = { * Compare two projects and return the changes showing how * project B has diverged from project A. * - * Workflows are identified by their ID and compared using version hashes. + * Workflows are paired up by uuid, then id, then name (see matchWorkflows) and + * compared using version hashes. A workflow with a different id in each project + * is reported with its id in A, and its id in B as `bId`. * * @param a - The baseline project (e.g., main branch) * @param b - The comparison project (e.g., staging branch) @@ -48,6 +54,9 @@ export type ProjectDiff = { * - 'removed': workflow exists in A but not in B * - 'changed': workflow exists in both but has different version hashes * + * If a workflow list is passed, a workflow is only considered if the list + * includes its id in A or in B. + * * @example * ```typescript * const main = await Project.from('fs', { root: '.' }); @@ -64,30 +73,39 @@ export function diff( ): ProjectDiff { const diffs: WorkflowDiff[] = []; + const inScope = (...ids: Array) => + !workflows?.length || ids.some((id) => id && workflows.includes(id)); + + // B's workflow id -> A's workflow id + const matches = matchWorkflows(b.workflows, a.workflows); + const matchedInB = Object.fromEntries( + Object.entries(matches).map(([idB, idA]) => [idA, idB]) + ); + // Check all of project A's workflows for (const workflowA of a.workflows) { - if (workflows?.length && !workflows.includes(workflowA.id)) { + const workflowB = b.getWorkflow(matchedInB[workflowA.id]); + + if (!inScope(workflowA.id, workflowB?.id)) { continue; } - const workflowB = b.getWorkflow(workflowA.id); - if (!workflowB) { // workflow exists in A but not in B = removed diffs.push({ id: workflowA.id, type: 'removed' }); } else if (workflowA.getVersionHash() !== workflowB.getVersionHash()) { // workflow exists in both but with different content = changed - diffs.push({ id: workflowA.id, type: 'changed' }); + diffs.push({ + id: workflowA.id, + type: 'changed', + ...(workflowB.id !== workflowA.id && { bId: workflowB.id }), + }); } } // Check for workflows that were added in B for (const workflowB of b.workflows) { - if (workflows?.length && !workflows.includes(workflowB.id)) { - continue; - } - - if (!a.getWorkflow(workflowB.id)) { + if (!(workflowB.id in matches) && inScope(workflowB.id)) { // workflow exists in B but not in A = added diffs.push({ id: workflowB.id, type: 'added' }); } diff --git a/packages/project/src/util/relabel-workflows.ts b/packages/project/src/util/relabel-workflows.ts new file mode 100644 index 000000000..630e46286 --- /dev/null +++ b/packages/project/src/util/relabel-workflows.ts @@ -0,0 +1,24 @@ +import Project from '../Project'; +import Workflow from '../Workflow'; + +/** + * Returns a copy of the project with some workflows given new ids + * + * `ids` maps current workflow id -> new id. Other workflows are untouched. + * + * The copy is shallow: only the workflow list and the cli meta are new + */ +export default function relabelWorkflows( + project: Project, + ids: Record +): Project { + const workflows = project.workflows.map((wf) => + ids[wf.id] && ids[wf.id] !== wf.id + ? new Workflow({ ...(wf.toJSON() as any), id: ids[wf.id] }) + : wf + ); + return Object.assign(Object.create(Object.getPrototypeOf(project)), project, { + workflows, + cli: { ...project.cli }, + }); +} diff --git a/packages/project/test/merge/match-workflows.test.ts b/packages/project/test/merge/match-workflows.test.ts new file mode 100644 index 000000000..b00ef7c53 --- /dev/null +++ b/packages/project/test/merge/match-workflows.test.ts @@ -0,0 +1,88 @@ +import test from 'ava'; +import Workflow from '../../src/Workflow'; +import matchWorkflows from '../../src/merge/match-workflows'; + +const wf = (id: string, name?: string, uuid?: string) => + new Workflow({ + id, + name, + steps: [], + ...(uuid && { openfn: { uuid } }), + } as any); + +test('match by id', (t) => { + const result = matchWorkflows([wf('a'), wf('b')], [wf('b'), wf('c')]); + t.deepEqual(result, { b: 'b' }); +}); + +test('match by uuid when the ids differ (a rename)', (t) => { + const result = matchWorkflows( + [wf('new-id', 'Wf', 'uuid-1')], + [wf('old-id', 'Wf', 'uuid-1')] + ); + t.deepEqual(result, { 'new-id': 'old-id' }); +}); + +test('match by name when there is no uuid and the ids differ', (t) => { + const result = matchWorkflows( + [wf('a', 'My Workflow')], + [wf('b', 'My Workflow')] + ); + t.deepEqual(result, { a: 'b' }); +}); + +test('uuid beats id', (t) => { + // each source shares an id with one target, and a uuid with the other + const result = matchWorkflows( + [wf('a', 'A', 'uuid-1'), wf('b', 'B', 'uuid-2')], + [wf('a', 'A', 'uuid-2'), wf('b', 'B', 'uuid-1')] + ); + t.deepEqual(result, { a: 'b', b: 'a' }); +}); + +test('id beats name', (t) => { + const result = matchWorkflows( + [wf('a', 'Same'), wf('b', 'Other')], + [wf('b', 'Same'), wf('c', 'Other')] + ); + // a has the name of target b, but b is already claimed by id + t.deepEqual(result, { b: 'b' }); +}); + +test('ids match even if the uuids differ (eg, merging a sandbox)', (t) => { + const result = matchWorkflows( + [wf('a', 'A', 'uuid-1')], + [wf('a', 'A', 'uuid-2')] + ); + t.deepEqual(result, { a: 'a' }); +}); + +test('matches are one-to-one', (t) => { + // both sources want the same target + const result = matchWorkflows( + [wf('a', 'A', 'uuid-1'), wf('b', 'B', 'uuid-1')], + [wf('x', 'X', 'uuid-1')] + ); + t.deepEqual(result, { a: 'x' }); +}); + +test('do not match by name if the name is not unique in the source', (t) => { + const result = matchWorkflows( + [wf('a', 'Same'), wf('b', 'Same')], + [wf('x', 'Same')] + ); + t.deepEqual(result, {}); +}); + +test('do not match by name if the name is not unique in the target', (t) => { + const result = matchWorkflows( + [wf('a', 'Same')], + [wf('x', 'Same'), wf('y', 'Same')] + ); + t.deepEqual(result, {}); +}); + +test('workflows with no name are never matched by name', (t) => { + const result = matchWorkflows([wf('a')], [wf('b')]); + t.deepEqual(result, {}); +}); diff --git a/packages/project/test/merge/merge-project.test.ts b/packages/project/test/merge/merge-project.test.ts index 1a92f3ce0..8671afeb1 100644 --- a/packages/project/test/merge/merge-project.test.ts +++ b/packages/project/test/merge/merge-project.test.ts @@ -1097,8 +1097,8 @@ test('id match: same workflow in source and target project', (t) => { }); test('no id match: union of both workflow', (t) => { - const source = generateWorkflow('@id workflow a-b'); - const target = generateWorkflow('@id another_workflow a-b'); + const source = generateWorkflow('@id workflow @name one a-b'); + const target = generateWorkflow('@id another_workflow @name two a-b'); const source_project = createProject(source); const target_project = createProject(target); @@ -1167,16 +1167,67 @@ test('id match: preserve target uuid', (t) => { t.is(result.workflows[0].openfn.uuid, target.openfn.uuid); }); +test('uuid match: a renamed workflow keeps the source id and the target uuid', (t) => { + const source = generateWorkflow('@id local-id @name Wf a-new', { + openfnUuid: true, + }); + const target = generateWorkflow('@id remote-id @name Wf a-old', { + openfnUuid: true, + }); + // the workflows are linked by uuid + source.openfn = { uuid: 'wf-uuid' }; + target.openfn = { uuid: 'wf-uuid' }; + + const result = merge(createProject(source), createProject(target)); + + t.is(result.workflows.length, 1); + t.is(result.workflows[0].id, 'local-id'); + t.is(result.workflows[0].openfn?.uuid, 'wf-uuid'); + t.truthy(result.workflows[0].get('new')); +}); + +test('name match: a workflow with a different id but the same name is merged', (t) => { + const source = generateWorkflow('@id local-id @name Wf a-new'); + const target = generateWorkflow('@id remote-id @name Wf a-old'); + + const result = merge(createProject(source), createProject(target)); + + t.is(result.workflows.length, 1); + t.is(result.workflows[0].id, 'local-id'); + t.truthy(result.workflows[0].get('new')); +}); + +test('explicit workflow mappings are not overridden by uuid or name matching', (t) => { + const source = generateWorkflow('@id x @name Same a-new'); + const target1 = generateWorkflow('@id y @name Same a-old'); + const target2 = generateWorkflow('@id z @name Other a-other'); + source.openfn = { uuid: 'uuid-1' }; + target1.openfn = { uuid: 'uuid-1' }; + + const result = merge( + createProject(source), + createProject([target1, target2]), + { + workflowMappings: { x: 'z' }, + } + ); + + // x was merged into z, and y was left alone + t.truthy(result.getWorkflow('x')?.get('new')); + t.truthy(result.getWorkflow('y')?.get('old')); + t.falsy(result.getWorkflow('z')); +}); + test('options: no mappings & removeUnmapped=false', (t) => { const source_project = createProject([ generateWorkflow('@id a a-new'), generateWorkflow('@id b b-new'), - generateWorkflow('@id c c-new'), + generateWorkflow('@id c @name unique-c c-new'), ]); const target_project = createProject([ generateWorkflow('@id a a-old'), generateWorkflow('@id b b-old'), - generateWorkflow('@id d d-old'), + generateWorkflow('@id d @name unique-d d-old'), ]); const result = merge(source_project, target_project); // should be the new version of a,b,c diff --git a/packages/project/test/parse/from-fs.test.ts b/packages/project/test/parse/from-fs.test.ts index 22dc5d4a2..565ef75a2 100644 --- a/packages/project/test/parse/from-fs.test.ts +++ b/packages/project/test/parse/from-fs.test.ts @@ -221,6 +221,80 @@ test.serial('should track forked_from', async (t) => { t.falsy(project.openfn!.forked_from); }); +const checkoutWithWorkflow = (checkout: object) => { + mockFile('/ws/openfn.yaml', buildConfig()); + mockFile('/ws/.openfn/checkout.yaml', { id: 'my-project', ...checkout }); + mockFile('/ws/workflows/workflow-1/workflow-1.yaml', { + name: 'Workflow 1', + steps: [{ id: 'a' }], + }); +}; + +test.serial('should track workflow_uuids from the checkout file', async (t) => { + checkoutWithWorkflow({ workflow_uuids: { 'workflow-1': 'uuid-1' } }); + + const project = await parseProject({ root: '/ws' }); + + t.deepEqual(project.cli.workflow_uuids, { 'workflow-1': 'uuid-1' }); + // it's tracking metadata, not project config + t.falsy(project.openfn!.workflow_uuids); +}); + +test.serial('should set the uuid on a tracked workflow', async (t) => { + checkoutWithWorkflow({ workflow_uuids: { 'workflow-1': 'uuid-1' } }); + mockFile('/ws/workflows/workflow-2/workflow-2.yaml', { + name: 'Workflow 2', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.getWorkflow('workflow-1')!.openfn?.uuid, 'uuid-1'); + // untracked workflows have no uuid + t.falsy(project.getWorkflow('workflow-2')!.openfn?.uuid); +}); + +test.serial( + 'should find the uuid of a tracked workflow in a renamed folder', + async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + mockFile('/ws/.openfn/checkout.yaml', { + id: 'my-project', + workflow_uuids: { 'workflow-1': 'uuid-1' }, + }); + mockFile('/ws/workflows/my-folder/workflow-1.yaml', { + name: 'Workflow 1', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + const wf = project.getWorkflow('workflow-1')!; + t.is(wf.alias, 'my-folder'); + t.is(wf.openfn?.uuid, 'uuid-1'); + } +); + +test.serial('should load a checkout file with no workflow_uuids', async (t) => { + checkoutWithWorkflow({ forked_from: { 'workflow-1': 'abcd' } }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + t.is(project.cli.workflow_uuids, undefined); + t.deepEqual(project.cli.forked_from, { 'workflow-1': 'abcd' }); +}); + +test.serial('should ignore a malformed workflow_uuids', async (t) => { + checkoutWithWorkflow({ workflow_uuids: 'nope' }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + t.is(project.cli.workflow_uuids, undefined); + t.falsy(project.openfn!.workflow_uuids); +}); + test.serial('should load the name from config', async (t) => { mockFile('/ws/openfn.yaml', { workspace: buildConfig(), @@ -436,3 +510,35 @@ test.serial('should override the name and id from options', async (t) => { t.is(project.name, 'hello-world'); t.is(project.id, 'hello-world'); }); + +test.serial('should take the workflow id from the file name', async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + // no id in the file, and a name that would slugify to something else + mockFile('/ws/workflows/my-workflow/my-workflow.yaml', { + name: 'My Workflow (renamed)', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + const wf = project.getWorkflow('my-workflow')!; + t.is(wf.name, 'My Workflow (renamed)'); + t.is(wf.alias, undefined); +}); + +test.serial('should prefer the file name to an id in the file', async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + mockFile('/ws/workflows/my-workflow/my-workflow.yaml', { + id: 'something-else', + name: 'My Workflow', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + t.is(project.workflows[0].id, 'my-workflow'); +}); diff --git a/packages/project/test/util/config.test.ts b/packages/project/test/util/config.test.ts index 0cd128af3..63434d79e 100644 --- a/packages/project/test/util/config.test.ts +++ b/packages/project/test/util/config.test.ts @@ -8,6 +8,7 @@ import { loadWorkspaceFile, } from '../../src/util/config'; import mock from 'mock-fs'; +import { yamlToJson } from '../../src/util/yaml'; import Project from '../../src'; test.afterEach(() => { @@ -330,3 +331,41 @@ test('hasCheckoutMeta: looks for the checkout file for the given branch', (t) => t.false(hasCheckoutMeta('/ws', 'main')); t.false(hasCheckoutMeta('/ws')); }); + +test('generate checkout file with workflow_uuids, keyed by workflow id', (t) => { + const proj = new Project({ + id: 'my-project', + workflows: [ + { id: 'wf-a', name: 'A', steps: [], openfn: { uuid: 'uuid-a' } }, + { id: 'wf-b', name: 'B', steps: [], openfn: { uuid: 'uuid-b' } }, + ], + }); + const result = extractCheckout(proj); + t.deepEqual(yamlToJson(result.content).workflow_uuids, { + 'wf-a': 'uuid-a', + 'wf-b': 'uuid-b', + }); +}); + +test('checkout file workflow_uuids skips workflows with no uuid', (t) => { + const proj = new Project({ + id: 'my-project', + workflows: [ + { id: 'wf-a', name: 'A', steps: [], openfn: { uuid: 'uuid-a' } }, + { id: 'wf-b', name: 'B', steps: [] }, + ], + }); + const result = extractCheckout(proj); + t.deepEqual(yamlToJson(result.content).workflow_uuids, { + 'wf-a': 'uuid-a', + }); +}); + +test("checkout file excludes workflow_uuids if there aren't any", (t) => { + const proj = new Project({ + id: 'my-project', + workflows: [{ id: 'wf-b', name: 'B', steps: [] }], + }); + const result = extractCheckout(proj); + t.false(result.content.includes('workflow_uuids')); +}); diff --git a/packages/project/test/util/project-diff.test.ts b/packages/project/test/util/project-diff.test.ts index 509527761..171157bb9 100644 --- a/packages/project/test/util/project-diff.test.ts +++ b/packages/project/test/util/project-diff.test.ts @@ -150,8 +150,8 @@ test('diff: should only consider removed workflows from a filter list', (t) => { test('diff: should detect multiple changes at once', (t) => { const wf1 = generateWorkflow('@id a trigger-x'); const wf2 = generateWorkflow('@id b trigger-y'); - const wf3 = generateWorkflow('@id c trigger-z'); - const wf4 = generateWorkflow('@id d trigger-w'); + const wf3 = generateWorkflow('@id c @name wf-c trigger-z'); + const wf4 = generateWorkflow('@id d @name wf-d trigger-w'); // wf2 will be changed in projectB const wf2Changed = generateWorkflow('@id b trigger-different'); @@ -242,6 +242,74 @@ const channel = (name: string, extra = {}) => ({ const withChannels = (channels?: any[]) => new Project({ name: 'p', workflows: [], channels }); +const withUuid = (wf: any, uuid: string) => { + wf.openfn = { uuid }; + return wf; +}; + +test('diff: a workflow with a different id but the same uuid is changed, not added and removed', (t) => { + const wfA = withUuid(generateWorkflow('@id old-id @name Wf trigger-x'), 'u1'); + const wfB = withUuid(generateWorkflow('@id new-id @name Wf trigger-y'), 'u1'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.deepEqual(diffs, [{ id: 'old-id', type: 'changed', bId: 'new-id' }]); +}); + +test('diff: a workflow with a different id but the same name is changed', (t) => { + const wfA = generateWorkflow('@id old-id @name Wf trigger-x'); + const wfB = generateWorkflow('@id new-id @name Wf trigger-y'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.deepEqual(diffs, [{ id: 'old-id', type: 'changed', bId: 'new-id' }]); +}); + +test('diff: a workflow with only a different id and identical content has no diff', (t) => { + const wfA = withUuid(generateWorkflow('@id old-id @name Wf trigger-x'), 'u1'); + const wfB = withUuid(generateWorkflow('@id new-id @name Wf trigger-x'), 'u1'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.is(diffs.length, 0); +}); + +test('diff: a renamed workflow is considered if the filter list has its id in either project', (t) => { + const wfA = withUuid(generateWorkflow('@id old-id @name Wf trigger-x'), 'u1'); + const wfB = withUuid(generateWorkflow('@id new-id @name Wf trigger-y'), 'u1'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + t.is(diff(projectA, projectB, ['new-id']).workflows.length, 1); + t.is(diff(projectA, projectB, ['old-id']).workflows.length, 1); + t.is(diff(projectA, projectB, ['other']).workflows.length, 0); +}); + +test('diff: workflows with no uuid, id or name in common are still added and removed', (t) => { + const wfA = generateWorkflow('@id a @name wf-a trigger-x'); + const wfB = generateWorkflow('@id b @name wf-b trigger-x'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.deepEqual(diffs, [ + { id: 'a', type: 'removed' }, + { id: 'b', type: 'added' }, + ]); +}); + test('diff: resources.channels is empty for identical channels', (t) => { const a = withChannels([channel('one')]); const b = withChannels([channel('one')]); diff --git a/packages/project/test/util/relabel-workflows.test.ts b/packages/project/test/util/relabel-workflows.test.ts new file mode 100644 index 000000000..9ac6c355c --- /dev/null +++ b/packages/project/test/util/relabel-workflows.test.ts @@ -0,0 +1,56 @@ +import test from 'ava'; +import Project from '../../src'; +import relabelWorkflows from '../../src/util/relabel-workflows'; + +const createProject = () => + new Project({ + id: 'p', + workflows: [ + { + id: 'wf-a', + name: 'A', + steps: [{ id: 'step', expression: 'fn(s => s)' }], + history: ['abcd'], + openfn: { uuid: 'uuid-a' }, + }, + { id: 'wf-b', name: 'B', steps: [], openfn: { uuid: 'uuid-b' } }, + ], + } as any); + +test('give a workflow a new id, keeping everything else', (t) => { + const result = relabelWorkflows(createProject(), { 'wf-a': 'renamed' }); + + t.deepEqual( + result.workflows.map((w) => w.id), + ['renamed', 'wf-b'] + ); + const wf = result.getWorkflow('renamed')!; + t.is(wf.name, 'A'); + t.is(wf.openfn?.uuid, 'uuid-a'); + t.deepEqual(wf.history, ['abcd']); + t.is(wf.steps[0].expression, 'fn(s => s)'); +}); + +test('does not mutate the original project', (t) => { + const project = createProject(); + relabelWorkflows(project, { 'wf-a': 'renamed' }); + + t.truthy(project.getWorkflow('wf-a')); + t.falsy(project.getWorkflow('renamed')); +}); + +test('ids that are not mapped are left alone', (t) => { + const result = relabelWorkflows(createProject(), { nope: 'x' }); + t.deepEqual( + result.workflows.map((w) => w.id), + ['wf-a', 'wf-b'] + ); +}); + +test('does not share cli meta with the original project', (t) => { + const project = createProject(); + const result = relabelWorkflows(project, { 'wf-a': 'renamed' }); + result.cli.forked_from = { renamed: 'abcd' }; + + t.is(project.cli.forked_from, undefined); +});